Skip to content

Commit

Permalink
[AW][ComponentUpdater] Post service connection to UI thread
Browse files Browse the repository at this point in the history
onServiceConnected is called on the app's main thread which might be
different from chromium UI thread
https://chromium.googlesource.com/chromium/src/+/HEAD/android_webview/docs/threading.md
So remove that assertion and post the result to chromium's UI thread
where the rest of the code assumes it will be running.

Bug: 1234415
Test: patch the CL with finch config to enable allowlist component and run CQ tests
Change-Id: Id4718c7b812cad0c4fc748fd8470eff5065f1b8f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3059495
Reviewed-by: Bo <[email protected]>
Reviewed-by: Sorin Jianu <[email protected]>
Reviewed-by: Richard Coles <[email protected]>
Commit-Queue: Hazem Ashmawy <[email protected]>
Cr-Commit-Position: refs/heads/master@{#907187}
  • Loading branch information
HazemSamir authored and Chromium LUCI CQ committed Jul 30, 2021
1 parent 951fd86 commit 8987841
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 20 deletions.
1 change: 1 addition & 0 deletions components/component_updater/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ android_library("embedded_component_loader_java") {
":component_provider_service_aidl_java",
":embedded_component_loader_jni_headers",
"//base:base_java",
"//content/public/android:content_main_dex_java",
"//third_party/androidx:androidx_annotation_annotation_java",
]
srcjar_deps = [ ":component_loader_policy_enum_srcjar" ]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
include_rules = [
"+content/public/android/java/src/org/chromium/content_public/browser/UiThreadTaskTraits.java",
]
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import org.chromium.base.ContextUtils;
import org.chromium.base.Log;
import org.chromium.base.ThreadUtils;
import org.chromium.base.task.PostTask;
import org.chromium.content_public.browser.UiThreadTaskTraits;

import java.util.Collection;
import java.util.HashSet;
Expand Down Expand Up @@ -92,29 +94,29 @@ public ComponentLoaderPolicyBridge getComponentLoaderPolicy() {

@Override
public void onServiceConnected(ComponentName className, IBinder service) {
ThreadUtils.assertOnUiThread();

try {
IComponentsProviderService providerService =
IComponentsProviderService.Stub.asInterface(service);
for (ComponentResultReceiver receiver : mComponentsResultReceivers) {
String componentId = receiver.getComponentLoaderPolicy().getComponentId();
providerService.getFilesForComponent(componentId, receiver);
}
} catch (RemoteException e) {
Log.d(TAG, "Remote Exception calling ComponentProviderService", e);
if (!mComponentsResultReceivers.isEmpty()) {
// Clearing up receivers here to avoid unbinding multiple times in the future.
// This means if some receivers get their result after this step, their results
// will be ignored.
PostTask.runOrPostTask(UiThreadTaskTraits.DEFAULT, () -> {
try {
IComponentsProviderService providerService =
IComponentsProviderService.Stub.asInterface(service);
for (ComponentResultReceiver receiver : mComponentsResultReceivers) {
receiver.getComponentLoaderPolicy().componentLoadFailed(
ComponentLoadResult.REMOTE_EXCEPTION);
String componentId = receiver.getComponentLoaderPolicy().getComponentId();
providerService.getFilesForComponent(componentId, receiver);
}
} catch (RemoteException e) {
Log.d(TAG, "Remote Exception calling ComponentProviderService", e);
if (!mComponentsResultReceivers.isEmpty()) {
// Clearing up receivers here to avoid unbinding multiple times in the future.
// This means if some receivers get their result after this step, their results
// will be ignored.
for (ComponentResultReceiver receiver : mComponentsResultReceivers) {
receiver.getComponentLoaderPolicy().componentLoadFailed(
ComponentLoadResult.REMOTE_EXCEPTION);
}
mComponentsResultReceivers.clear();
ContextUtils.getApplicationContext().unbindService(this);
}
mComponentsResultReceivers.clear();
ContextUtils.getApplicationContext().unbindService(this);
}
}
});
}

@Override
Expand Down

0 comments on commit 8987841

Please sign in to comment.