Skip to content

Commit 6c698c0

Browse files
kaibolaylfkellogg
andauthored
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 fc63ab4 commit 6c698c0

File tree

5 files changed

+55
-23
lines changed

5 files changed

+55
-23
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: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,22 +62,28 @@ public class FirebaseAppDistributionRegistrar implements ComponentRegistrar {
6262
Component.builder(FeedbackSender.class)
6363
.add(Dependency.required(FirebaseApp.class))
6464
.add(Dependency.requiredProvider(FirebaseInstallationsApi.class))
65+
.add(Dependency.required(blockingExecutor))
6566
.factory(c -> buildFeedbackSender(c, c.get(blockingExecutor)))
6667
.build(),
6768
LibraryVersionComponent.create(LIBRARY_NAME, BuildConfig.VERSION_NAME));
6869
}
6970

70-
private FeedbackSender buildFeedbackSender(ComponentContainer container, Executor blockingExecutor) {
71+
private FeedbackSender buildFeedbackSender(
72+
ComponentContainer container, Executor blockingExecutor) {
7173
FirebaseApp firebaseApp = container.get(FirebaseApp.class);
7274
Provider<FirebaseInstallationsApi> firebaseInstallationsApiProvider =
7375
container.getProvider(FirebaseInstallationsApi.class);
7476
FirebaseAppDistributionTesterApiClient testerApiClient =
7577
new FirebaseAppDistributionTesterApiClient(
76-
firebaseApp, firebaseInstallationsApiProvider, new TesterApiHttpClient(firebaseApp), blockingExecutor);
77-
return new FeedbackSender(testerApiClient);
78+
firebaseApp,
79+
firebaseInstallationsApiProvider,
80+
new TesterApiHttpClient(firebaseApp),
81+
blockingExecutor);
82+
return new FeedbackSender(testerApiClient, blockingExecutor);
7883
}
7984

80-
private FirebaseAppDistribution buildFirebaseAppDistribution(ComponentContainer container, Executor blockingExecutor) {
85+
private FirebaseAppDistribution buildFirebaseAppDistribution(
86+
ComponentContainer container, Executor blockingExecutor) {
8187
FirebaseApp firebaseApp = container.get(FirebaseApp.class);
8288
Context context = firebaseApp.getApplicationContext();
8389
Provider<FirebaseInstallationsApi> firebaseInstallationsApiProvider =

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)