Skip to content

Commit 2e8813e

Browse files
kaibolaylfkellogg
andcommitted
Do I/O (reading the screenshot) on separate thread. (#4347)
* Do I/O (reading the screenshot) on separate thread. * Use executor * Make FeedbackSender depend on Blocking executor * `./gradlew :firebase-appdistribution:googleJavaFormat` Co-authored-by: Lee Kellogg <[email protected]>
1 parent 1610e52 commit 2e8813e

File tree

5 files changed

+53
-22
lines changed

5 files changed

+53
-22
lines changed

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FeedbackActivity.java

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -92,21 +92,37 @@ private void setupView() {
9292
findViewById(R.id.backButton).setOnClickListener(v -> finish());
9393
findViewById(R.id.sendButton).setOnClickListener(this::submitFeedback);
9494

95-
Bitmap screenshot = screenshotUri == null ? null : readScreenshot();
96-
if (screenshot != null) {
97-
ImageView screenshotImageView = this.findViewById(R.id.screenshotImageView);
98-
screenshotImageView.setImageBitmap(screenshot);
99-
CheckBox checkBox = findViewById(R.id.screenshotCheckBox);
100-
checkBox.setChecked(true);
101-
checkBox.setOnClickListener(
102-
v -> screenshotImageView.setVisibility(checkBox.isChecked() ? VISIBLE : GONE));
103-
} else {
104-
LogWrapper.getInstance().e(TAG, "No screenshot available");
105-
CheckBox checkBox = findViewById(R.id.screenshotCheckBox);
106-
checkBox.setText(R.string.no_screenshot);
107-
checkBox.setClickable(false);
108-
checkBox.setChecked(false);
109-
}
95+
setupScreenshot();
96+
}
97+
98+
private void setupScreenshot() {
99+
feedbackSender
100+
.getBlockingExecutor()
101+
.execute(
102+
() -> {
103+
// do I/O on separate thread in order to not block the UI
104+
Bitmap screenshot = screenshotUri == null ? null : readScreenshot();
105+
if (screenshot != null) {
106+
runOnUiThread(
107+
() -> {
108+
ImageView imageView = findViewById(R.id.screenshotImageView);
109+
imageView.setImageBitmap(screenshot);
110+
CheckBox checkBox = findViewById(R.id.screenshotCheckBox);
111+
checkBox.setChecked(true);
112+
checkBox.setOnClickListener(
113+
v -> imageView.setVisibility(checkBox.isChecked() ? VISIBLE : GONE));
114+
});
115+
} else {
116+
LogWrapper.getInstance().e(TAG, "No screenshot available");
117+
runOnUiThread(
118+
() -> {
119+
CheckBox checkBox = findViewById(R.id.screenshotCheckBox);
120+
checkBox.setText(R.string.no_screenshot);
121+
checkBox.setClickable(false);
122+
checkBox.setChecked(false);
123+
});
124+
}
125+
});
110126
}
111127

112128
@Nullable

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FeedbackSender.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,17 @@
1919
import com.google.android.gms.tasks.Task;
2020
import com.google.android.gms.tasks.Tasks;
2121
import com.google.firebase.FirebaseApp;
22+
import java.util.concurrent.Executor;
2223

2324
/** Sends tester feedback to the Tester API. */
2425
class FeedbackSender {
25-
2626
private final FirebaseAppDistributionTesterApiClient testerApiClient;
27+
private final Executor blockingExecutor;
2728

28-
FeedbackSender(FirebaseAppDistributionTesterApiClient testerApiClient) {
29+
FeedbackSender(
30+
FirebaseAppDistributionTesterApiClient testerApiClient, Executor blockingExecutor) {
2931
this.testerApiClient = testerApiClient;
32+
this.blockingExecutor = blockingExecutor;
3033
}
3134

3235
/** Get an instance of FeedbackSender. */
@@ -42,6 +45,12 @@ Task<Void> sendFeedback(String releaseName, String feedbackText, @Nullable Uri s
4245
.onSuccessTask(testerApiClient::commitFeedback);
4346
}
4447

48+
// TODO(kbolay): Remove this hack to make the executor available in FeedbackAction and use a more
49+
// sophisticated dependency injection solution.
50+
Executor getBlockingExecutor() {
51+
return blockingExecutor;
52+
}
53+
4554
private Task<String> attachScreenshot(String feedbackName, @Nullable Uri screenshotUri) {
4655
if (screenshotUri == null) {
4756
return Tasks.forResult(feedbackName);

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionRegistrar.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,19 +67,24 @@ public class FirebaseAppDistributionRegistrar implements ComponentRegistrar {
6767
Component.builder(FeedbackSender.class)
6868
.add(Dependency.required(FirebaseApp.class))
6969
.add(Dependency.requiredProvider(FirebaseInstallationsApi.class))
70+
.add(Dependency.required(blockingExecutor))
7071
.factory(c -> buildFeedbackSender(c, c.get(blockingExecutor)))
7172
.build(),
7273
LibraryVersionComponent.create(LIBRARY_NAME, BuildConfig.VERSION_NAME));
7374
}
7475

75-
private FeedbackSender buildFeedbackSender(ComponentContainer container, Executor blockingExecutor) {
76+
private FeedbackSender buildFeedbackSender(
77+
ComponentContainer container, Executor blockingExecutor) {
7678
FirebaseApp firebaseApp = container.get(FirebaseApp.class);
7779
Provider<FirebaseInstallationsApi> firebaseInstallationsApiProvider =
7880
container.getProvider(FirebaseInstallationsApi.class);
7981
FirebaseAppDistributionTesterApiClient testerApiClient =
8082
new FirebaseAppDistributionTesterApiClient(
81-
firebaseApp, firebaseInstallationsApiProvider, new TesterApiHttpClient(firebaseApp), blockingExecutor);
82-
return new FeedbackSender(testerApiClient);
83+
firebaseApp,
84+
firebaseInstallationsApiProvider,
85+
new TesterApiHttpClient(firebaseApp),
86+
blockingExecutor);
87+
return new FeedbackSender(testerApiClient, blockingExecutor);
8388
}
8489

8590
// TODO(b/258264924): Migrate to go/firebase-android-executors

firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FeedbackSenderTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.google.android.gms.tasks.Tasks;
2626
import com.google.firebase.appdistribution.FirebaseAppDistributionException;
2727
import com.google.firebase.appdistribution.FirebaseAppDistributionException.Status;
28+
import java.util.concurrent.Executors;
2829
import org.junit.Before;
2930
import org.junit.Test;
3031
import org.junit.runner.RunWith;
@@ -47,7 +48,7 @@ public class FeedbackSenderTest {
4748
@Before
4849
public void setup() {
4950
MockitoAnnotations.initMocks(this);
50-
feedbackSender = new FeedbackSender(mockTesterApiClient);
51+
feedbackSender = new FeedbackSender(mockTesterApiClient, Executors.newSingleThreadExecutor());
5152
}
5253

5354
@Test

firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionTesterApiClientTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@
3535
import com.google.firebase.inject.Provider;
3636
import com.google.firebase.installations.FirebaseInstallationsApi;
3737
import com.google.firebase.installations.InstallationTokenResult;
38-
import java.util.concurrent.Executors;
3938
import java.io.FileNotFoundException;
39+
import java.util.concurrent.Executors;
4040
import org.json.JSONException;
4141
import org.json.JSONObject;
4242
import org.junit.Before;

0 commit comments

Comments
 (0)