Skip to content

Commit d8829db

Browse files
authored
feat(feedback): Support runtime enable/disable of shake-to-report (#5827)
1 parent 43777a5 commit d8829db

16 files changed

Lines changed: 862 additions & 105 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
- Allow child spans to use explicit start timestamps through `ISpan` ([#5929](https://github.com/getsentry/sentry-java/pull/5929))
88
- Make `ISpan.startChild` overloads with `SpanOptions` public ([#5927](https://github.com/getsentry/sentry-java/pull/5927))
9+
- Add `Sentry.feedback().enableOnShake()`, `Sentry.feedback().disableOnShake()`, and `Sentry.feedback().isOnShakeEnabled()` to toggle and query shake-to-report at runtime ([#5827](https://github.com/getsentry/sentry-java/pull/5827))
910

1011
### Improvements
1112

‎sentry-android-core/api/sentry-android-core.api‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,9 +293,12 @@ public abstract class io/sentry/android/core/EnvelopeFileObserverIntegration : i
293293
public final fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V
294294
}
295295

296-
public final class io/sentry/android/core/FeedbackShakeIntegration : android/app/Application$ActivityLifecycleCallbacks, io/sentry/Integration, java/io/Closeable {
296+
public final class io/sentry/android/core/FeedbackShakeIntegration : android/app/Application$ActivityLifecycleCallbacks, io/sentry/Integration, io/sentry/SentryFeedbackOptions$IShakeController, java/io/Closeable {
297297
public fun <init> (Landroid/app/Application;)V
298298
public fun close ()V
299+
public fun disableOnShake ()V
300+
public fun enableOnShake ()V
301+
public fun isOnShakeEnabled ()Z
299302
public fun onActivityCreated (Landroid/app/Activity;Landroid/os/Bundle;)V
300303
public fun onActivityDestroyed (Landroid/app/Activity;)V
301304
public fun onActivityPaused (Landroid/app/Activity;)V
@@ -573,7 +576,9 @@ public abstract interface class io/sentry/android/core/SentryUserFeedbackDialog$
573576

574577
public class io/sentry/android/core/SentryUserFeedbackForm : android/app/AlertDialog {
575578
protected fun onCreate (Landroid/os/Bundle;)V
579+
public fun onDetachedFromWindow ()V
576580
protected fun onStart ()V
581+
protected fun onStop ()V
577582
public fun setCancelable (Z)V
578583
public fun setOnDismissListener (Landroid/content/DialogInterface$OnDismissListener;)V
579584
public fun show ()V

‎sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java‎

Lines changed: 183 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,54 @@
44

55
import android.app.Activity;
66
import android.app.Application;
7+
import android.app.Dialog;
78
import android.os.Bundle;
89
import io.sentry.IScopes;
910
import io.sentry.Integration;
11+
import io.sentry.SentryFeedbackOptions;
1012
import io.sentry.SentryLevel;
1113
import io.sentry.SentryOptions;
1214
import io.sentry.util.Objects;
1315
import java.io.Closeable;
1416
import java.io.IOException;
1517
import java.lang.ref.WeakReference;
18+
import java.util.concurrent.CopyOnWriteArrayList;
1619
import org.jetbrains.annotations.NotNull;
1720
import org.jetbrains.annotations.Nullable;
21+
import org.jetbrains.annotations.TestOnly;
1822

1923
/**
20-
* Detects shake gestures and shows the user feedback dialog when a shake is detected. Only active
21-
* when {@link io.sentry.SentryFeedbackOptions#isUseShakeGesture()} returns {@code true}.
24+
* Detects shake gestures and shows the user feedback dialog when a shake is detected. {@link
25+
* io.sentry.SentryFeedbackOptions#isUseShakeGesture()} determines the initial state; it can be
26+
* toggled at runtime via {@code Sentry.feedback().enableOnShake()} and {@code
27+
* Sentry.feedback().disableOnShake()}.
28+
*
29+
* <p>Shake detection is scoped to the resumed activity: a dialog belongs to the window of the
30+
* activity that created it, so it can only ever be visible while that activity is resumed. Dialogs
31+
* report themselves via {@link #onDialogVisible(Activity, Dialog)} / {@link #onDialogGone(Dialog)}
32+
* and detection is then suppressed for the activity hosting them, which keeps a shake from stacking
33+
* a second dialog on top of a visible one without letting a dialog on a backgrounded activity
34+
* suppress detection elsewhere.
2235
*/
2336
public final class FeedbackShakeIntegration
24-
implements Integration, Closeable, Application.ActivityLifecycleCallbacks {
37+
implements Integration,
38+
Closeable,
39+
Application.ActivityLifecycleCallbacks,
40+
SentryFeedbackOptions.IShakeController {
2541

2642
private final @NotNull Application application;
2743
private final @NotNull SentryShakeDetector shakeDetector;
2844
private @Nullable SentryAndroidOptions options;
45+
private volatile boolean enabled = false;
2946
private volatile @Nullable WeakReference<Activity> currentActivityRef;
30-
private volatile boolean isDialogShowing = false;
31-
private volatile @Nullable Runnable previousOnFormClose;
47+
48+
/**
49+
* The feedback dialogs that are currently visible, together with the activity hosting them. More
50+
* than one can be visible at a time, e.g. when the app calls {@code Sentry.feedback().show()}
51+
* while another dialog is already showing.
52+
*/
53+
private final @NotNull CopyOnWriteArrayList<VisibleDialog> visibleDialogs =
54+
new CopyOnWriteArrayList<>();
3255

3356
public FeedbackShakeIntegration(final @NotNull Application application) {
3457
this.application = Objects.requireNonNull(application, "Application is required");
@@ -46,13 +69,25 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions
4669

4770
final @NotNull SentryAndroidOptions options = this.options;
4871

49-
if (!options.getFeedbackOptions().isUseShakeGesture()) {
72+
// Always expose the runtime toggle, even when the option starts out disabled.
73+
options.getFeedbackOptions().setShakeController(this);
74+
75+
if (options.getFeedbackOptions().isUseShakeGesture()) {
76+
enableOnShake();
77+
}
78+
}
79+
80+
@Override
81+
public synchronized void enableOnShake() {
82+
final @Nullable SentryAndroidOptions options = this.options;
83+
if (enabled || options == null) {
5084
return;
5185
}
86+
enabled = true;
5287

53-
// Re-arm the detector in case this integration is being re-registered after a previous close()
54-
// (e.g. a second Sentry.init reusing the same options), otherwise the closed latch would keep
55-
// shake detection off permanently.
88+
// Re-arm the detector in case it was closed before, either by disableOnShake() or by a previous
89+
// close() (e.g. a second Sentry.init reusing the same options), otherwise the closed latch
90+
// would keep shake detection off permanently.
5691
shakeDetector.reopen();
5792

5893
// Resolving the accelerometer is the most expensive part of init (the first SensorManager
@@ -72,7 +107,7 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions
72107
application.registerActivityLifecycleCallbacks(this);
73108
options.getLogger().log(SentryLevel.DEBUG, "FeedbackShakeIntegration installed.");
74109

75-
// In case of a deferred init, hook into any already-resumed activity
110+
// In case of a deferred init or runtime enable, hook into any already-resumed activity
76111
final @Nullable Activity activity = CurrentActivityHolder.getInstance().getActivity();
77112
if (activity != null) {
78113
currentActivityRef = new WeakReference<>(activity);
@@ -81,34 +116,111 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions
81116
}
82117

83118
@Override
84-
public void close() throws IOException {
119+
public synchronized void disableOnShake() {
120+
if (!enabled) {
121+
return;
122+
}
123+
enabled = false;
124+
85125
application.unregisterActivityLifecycleCallbacks(this);
86126
shakeDetector.close();
87-
// Restore onFormClose if a dialog is still showing, since lifecycle callbacks
88-
// are now unregistered and onActivityDestroyed cleanup won't fire.
89-
if (isDialogShowing) {
90-
isDialogShowing = false;
91-
if (options != null) {
92-
options.getFeedbackOptions().setOnFormClose(previousOnFormClose);
93-
}
94-
previousOnFormClose = null;
95-
}
96127
currentActivityRef = null;
97128
}
98129

99130
@Override
100-
public void onActivityResumed(final @NotNull Activity activity) {
101-
// If a dialog is showing on a different activity (e.g. user navigated via notification),
102-
// clean up since the dialog's host activity is going away and onActivityDestroyed
103-
// won't match currentActivity anymore.
104-
final @Nullable Activity current = currentActivityRef != null ? currentActivityRef.get() : null;
105-
if (isDialogShowing && current != null && current != activity) {
106-
isDialogShowing = false;
107-
if (options != null) {
108-
options.getFeedbackOptions().setOnFormClose(previousOnFormClose);
131+
public boolean isOnShakeEnabled() {
132+
return enabled;
133+
}
134+
135+
/**
136+
* Reports a feedback dialog as visible on {@code host}. Shake detection is suppressed for that
137+
* activity until the dialog reports back via {@link #onDialogGone(Dialog)}, so a shake can never
138+
* stack a second dialog on top of a visible one — no matter how the visible one was opened.
139+
*/
140+
void onDialogVisible(final @NotNull Activity host, final @NotNull Dialog dialog) {
141+
visibleDialogs.add(new VisibleDialog(host, dialog));
142+
stopShakeDetection();
143+
}
144+
145+
/** Reports a feedback dialog as no longer visible. Safe to call more than once per dialog. */
146+
void onDialogGone(final @NotNull Dialog dialog) {
147+
if (!removeDialog(dialog)) {
148+
return;
149+
}
150+
final @Nullable WeakReference<Activity> currentRef = currentActivityRef;
151+
final @Nullable Activity current = currentRef == null ? null : currentRef.get();
152+
if (enabled && current != null) {
153+
startShakeDetection(current);
154+
}
155+
}
156+
157+
private boolean removeDialog(final @NotNull Dialog dialog) {
158+
boolean removed = false;
159+
for (final @NotNull VisibleDialog visibleDialog : visibleDialogs) {
160+
// Drop entries whose dialog was collected without reporting back, so they can't suppress
161+
// detection forever.
162+
final @Nullable Dialog trackedDialog = visibleDialog.dialogRef.get();
163+
if (trackedDialog == dialog) {
164+
removed = visibleDialogs.remove(visibleDialog) || removed;
165+
} else if (trackedDialog == null) {
166+
visibleDialogs.remove(visibleDialog);
109167
}
110-
previousOnFormClose = null;
111168
}
169+
return removed;
170+
}
171+
172+
private boolean hasDialogOn(final @NotNull Activity activity) {
173+
for (final @NotNull VisibleDialog visibleDialog : visibleDialogs) {
174+
if (visibleDialog.dialogRef.get() != null && visibleDialog.activityRef.get() == activity) {
175+
return true;
176+
}
177+
}
178+
return false;
179+
}
180+
181+
@TestOnly
182+
@Nullable
183+
Activity getDialogActivity() {
184+
for (final @NotNull VisibleDialog visibleDialog : visibleDialogs) {
185+
if (visibleDialog.dialogRef.get() != null) {
186+
return visibleDialog.activityRef.get();
187+
}
188+
}
189+
return null;
190+
}
191+
192+
/** Creates the dialog shown on shake. Replaceable in tests to simulate a failing show(). */
193+
interface DialogFactory {
194+
@NotNull
195+
Dialog create(final @NotNull Activity activity);
196+
}
197+
198+
private @NotNull DialogFactory dialogFactory =
199+
activity -> new SentryUserFeedbackForm.Builder(activity).create();
200+
201+
@TestOnly
202+
void setDialogFactory(final @NotNull DialogFactory dialogFactory) {
203+
this.dialogFactory = dialogFactory;
204+
}
205+
206+
private static final class VisibleDialog {
207+
private final @NotNull WeakReference<Activity> activityRef;
208+
private final @NotNull WeakReference<Dialog> dialogRef;
209+
210+
VisibleDialog(final @NotNull Activity activity, final @NotNull Dialog dialog) {
211+
this.activityRef = new WeakReference<>(activity);
212+
this.dialogRef = new WeakReference<>(dialog);
213+
}
214+
}
215+
216+
@Override
217+
public void close() throws IOException {
218+
disableOnShake();
219+
visibleDialogs.clear();
220+
}
221+
222+
@Override
223+
public void onActivityResumed(final @NotNull Activity activity) {
112224
currentActivityRef = new WeakReference<>(activity);
113225
startShakeDetection(activity);
114226
}
@@ -118,16 +230,11 @@ public void onActivityPaused(final @NotNull Activity activity) {
118230
// Only stop if this is the activity we're tracking. When transitioning between
119231
// activities, B.onResume may fire before A.onPause — stopping unconditionally
120232
// would kill shake detection for the new activity.
121-
final @Nullable Activity current = currentActivityRef != null ? currentActivityRef.get() : null;
233+
final @Nullable WeakReference<Activity> currentRef = currentActivityRef;
234+
final @Nullable Activity current = currentRef != null ? currentRef.get() : null;
122235
if (activity == current) {
123236
stopShakeDetection();
124-
// Keep currentActivityRef set when a dialog is showing so onActivityDestroyed
125-
// can still match and clean up. Otherwise the cleanup condition
126-
// (activity == current) would always be false since onPause fires
127-
// before onDestroy.
128-
if (!isDialogShowing) {
129-
currentActivityRef = null;
130-
}
237+
currentActivityRef = null;
131238
}
132239
}
133240

@@ -146,67 +253,58 @@ public void onActivitySaveInstanceState(
146253
final @NotNull Activity activity, final @NotNull Bundle outState) {}
147254

148255
@Override
149-
public void onActivityDestroyed(final @NotNull Activity activity) {
150-
// Only reset if this is the activity that hosts the dialog — the dialog cannot
151-
// outlive its host activity being destroyed.
152-
final @Nullable Activity current = currentActivityRef != null ? currentActivityRef.get() : null;
153-
if (isDialogShowing && activity == current) {
154-
isDialogShowing = false;
155-
currentActivityRef = null;
156-
if (options != null) {
157-
options.getFeedbackOptions().setOnFormClose(previousOnFormClose);
158-
}
159-
previousOnFormClose = null;
160-
}
161-
}
256+
public void onActivityDestroyed(final @NotNull Activity activity) {}
162257

163258
private void startShakeDetection(final @NotNull Activity activity) {
164259
if (options == null) {
165260
return;
166261
}
167262
// Stop any existing detection (e.g. when transitioning between activities)
168263
stopShakeDetection();
264+
// A dialog is already visible here, so a shake could only stack a second one on top of it.
265+
// The dialog has no detector of its own in this case: SentryUserFeedbackForm only starts one
266+
// while shake-to-report is globally disabled, which is exactly when this integration is not
267+
// detecting either.
268+
if (hasDialogOn(activity)) {
269+
return;
270+
}
169271
shakeDetector.start(
170272
activity,
171273
() -> {
172274
final @Nullable WeakReference<Activity> ref = currentActivityRef;
173275
final Activity active = ref != null ? ref.get() : null;
174276
final Boolean inBackground = AppState.getInstance().isInBackground();
175-
if (active != null
176-
&& options != null
177-
&& !isDialogShowing
178-
&& !Boolean.TRUE.equals(inBackground)) {
179-
active.runOnUiThread(
180-
() -> {
181-
if (isDialogShowing || active.isFinishing() || active.isDestroyed()) {
182-
return;
183-
}
184-
try {
185-
isDialogShowing = true;
186-
final Runnable captured = options.getFeedbackOptions().getOnFormClose();
187-
previousOnFormClose = captured;
188-
options
189-
.getFeedbackOptions()
190-
.setOnFormClose(
191-
() -> {
192-
isDialogShowing = false;
193-
options.getFeedbackOptions().setOnFormClose(captured);
194-
if (captured != null) {
195-
captured.run();
196-
}
197-
previousOnFormClose = null;
198-
});
199-
new SentryUserFeedbackForm.Builder(active).create().show();
200-
} catch (Throwable e) {
201-
isDialogShowing = false;
202-
options.getFeedbackOptions().setOnFormClose(previousOnFormClose);
203-
previousOnFormClose = null;
204-
options
205-
.getLogger()
206-
.log(SentryLevel.ERROR, "Failed to show feedback dialog on shake.", e);
207-
}
208-
});
277+
if (active == null
278+
|| options == null
279+
|| !enabled
280+
|| hasDialogOn(active)
281+
|| Boolean.TRUE.equals(inBackground)) {
282+
return;
209283
}
284+
active.runOnUiThread(
285+
() -> {
286+
// Re-check on the main thread: shake-to-report may have been disabled, or an
287+
// earlier queued shake may have shown a dialog in the meantime (the dialog reports
288+
// itself synchronously in onStart).
289+
if (!enabled
290+
|| hasDialogOn(active)
291+
|| active.isFinishing()
292+
|| active.isDestroyed()) {
293+
return;
294+
}
295+
@Nullable Dialog dialog = null;
296+
try {
297+
dialog = dialogFactory.create(active);
298+
dialog.show();
299+
} catch (Throwable e) {
300+
if (dialog != null) {
301+
onDialogGone(dialog);
302+
}
303+
options
304+
.getLogger()
305+
.log(SentryLevel.ERROR, "Failed to show feedback dialog on shake.", e);
306+
}
307+
});
210308
});
211309
}
212310

0 commit comments

Comments
 (0)