Skip to content

Commit

Permalink
Revert "[Bundles] Add module_installer_backend.cc with native OnInsta…
Browse files Browse the repository at this point in the history
…llModule handler."

This reverts commit 461a68d.

Reason for revert: This is no longer needed: We've decided to let each DFM load native resources (on install) in its own way, instead of relying on common infrastructure.

Reverting this also fixes crbug.com/994035

Original change's description:
> [Bundles] Add module_installer_backend.cc with native OnInstallModule handler.
>
> To prepare for handling native resource loading after DFM installation,
> this CL adds a new hook in ModuleInstallerBackend.onFinished() to call
> (via JNI) a new native handler for each newly installed module:
>   JNI_ModuleInstallerBackend_OnInstallModule()
>
> AsyncTask is used to make the JNI call asynchronous, taking place
> outside the UI thread. This allows file access (e.g., to open native
> resources) to take place in the native code, thereby preventing
> assert failure in Debug builds.
>
> After the JNI call, |mListener.onFinished()| is called on UI thread,
> just like before.
>
> Bug: 927131
> Change-Id: I36880cc7739a48534052b51d3d655d1ae0beb29a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1702905
> Commit-Queue: Samuel Huang <huangs@chromium.org>
> Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
> Reviewed-by: danakj <danakj@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#680452}

TBR=danakj@chromium.org,huangs@chromium.org,agrieve@chromium.org,tiborg@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 927131,994035
Change-Id: If9fdbffb35d226615135c9db596ad50960bc2958
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1756154
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#687309}
  • Loading branch information
samuelhuang authored and Commit Bot committed Aug 15, 2019
1 parent 78c0ba6 commit 80fbc26
Show file tree
Hide file tree
Showing 5 changed files with 1 addition and 71 deletions.
1 change: 0 additions & 1 deletion chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2866,7 +2866,6 @@ jumbo_split_static_library("browser") {
"//components/feed:feature_list",
"//components/invalidation/impl:feature_list",
"//components/language/android:language_bridge",
"//components/module_installer/android:module_installer",
"//components/omnibox/browser",
"//components/payments/content/android",
"//components/resources:components_resources",
Expand Down
16 changes: 0 additions & 16 deletions components/module_installer/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,6 @@ android_library("module_installer_impl_java") {
]
}

generate_jni("module_installer_jni_headers") {
sources = [
"java/src-impl/org/chromium/components/module_installer/ModuleInstallerBackend.java",
]
}

static_library("module_installer") {
sources = [
"module_installer_backend.cc",
]
deps = [
":module_installer_jni_headers",
"//base",
]
}

android_library("module_installer_test_java") {
testonly = true
java_files = [ "javatests/src/org/chromium/components/module_installer/ModuleInstallerRule.java" ]
Expand Down
1 change: 0 additions & 1 deletion components/module_installer/android/DEPS
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
include_rules = [
"+base",
"+components/crash/android",
]
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,10 @@
package org.chromium.components.module_installer;

import org.chromium.base.ThreadUtils;
import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.task.AsyncTask;

import java.util.List;

/** A backend for installing dynamic feature modules that contain the actual install logic. */
@JNINamespace("module_installer")
/* package */ abstract class ModuleInstallerBackend {
private final OnFinishedListener mListener;

Expand Down Expand Up @@ -51,28 +48,6 @@ public ModuleInstallerBackend(OnFinishedListener listener) {

/** To be called when module install has finished. */
protected void onFinished(boolean success, List<String> moduleNames) {
ThreadUtils.assertOnUiThread();

// Call native to perform additional tasks (e.g., load resources). These
// tasks can be blocking, and are done asynchronously (outside the UI
// thread). Once done, return to call |mListender.onFinished()| on the
// UI thread.
// TODO(crbug.com/987252): Add timing metrics.
new AsyncTask<Void>() {
@Override
protected Void doInBackground() {
for (String moduleName : moduleNames) {
nativeOnInstallModule(success, moduleName);
}
return null;
}

@Override
protected void onPostExecute(Void result) {
mListener.onFinished(success, moduleNames);
}
}.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
mListener.onFinished(success, moduleNames);
}

private static native void nativeOnInstallModule(boolean success, String moduleName);
}
27 changes: 0 additions & 27 deletions components/module_installer/android/module_installer_backend.cc

This file was deleted.

0 comments on commit 80fbc26

Please sign in to comment.