Commit 80fbc26d authored by Samuel Huang's avatar Samuel Huang Committed by Commit Bot

Revert "[Bundles] Add module_installer_backend.cc with native OnInstallModule handler."

This reverts commit 461a68da.

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/+/1756154Reviewed-by: default avatarSamuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#687309}
parent 78c0ba60
...@@ -2866,7 +2866,6 @@ jumbo_split_static_library("browser") { ...@@ -2866,7 +2866,6 @@ jumbo_split_static_library("browser") {
"//components/feed:feature_list", "//components/feed:feature_list",
"//components/invalidation/impl:feature_list", "//components/invalidation/impl:feature_list",
"//components/language/android:language_bridge", "//components/language/android:language_bridge",
"//components/module_installer/android:module_installer",
"//components/omnibox/browser", "//components/omnibox/browser",
"//components/payments/content/android", "//components/payments/content/android",
"//components/resources:components_resources", "//components/resources:components_resources",
......
...@@ -58,22 +58,6 @@ android_library("module_installer_impl_java") { ...@@ -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") { android_library("module_installer_test_java") {
testonly = true testonly = true
java_files = [ "javatests/src/org/chromium/components/module_installer/ModuleInstallerRule.java" ] java_files = [ "javatests/src/org/chromium/components/module_installer/ModuleInstallerRule.java" ]
......
include_rules = [ include_rules = [
"+base",
"+components/crash/android", "+components/crash/android",
] ]
...@@ -5,13 +5,10 @@ ...@@ -5,13 +5,10 @@
package org.chromium.components.module_installer; package org.chromium.components.module_installer;
import org.chromium.base.ThreadUtils; import org.chromium.base.ThreadUtils;
import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.task.AsyncTask;
import java.util.List; import java.util.List;
/** A backend for installing dynamic feature modules that contain the actual install logic. */ /** A backend for installing dynamic feature modules that contain the actual install logic. */
@JNINamespace("module_installer")
/* package */ abstract class ModuleInstallerBackend { /* package */ abstract class ModuleInstallerBackend {
private final OnFinishedListener mListener; private final OnFinishedListener mListener;
...@@ -51,28 +48,6 @@ import java.util.List; ...@@ -51,28 +48,6 @@ import java.util.List;
/** To be called when module install has finished. */ /** To be called when module install has finished. */
protected void onFinished(boolean success, List<String> moduleNames) { protected void onFinished(boolean success, List<String> moduleNames) {
ThreadUtils.assertOnUiThread(); mListener.onFinished(success, moduleNames);
// 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);
} }
private static native void nativeOnInstallModule(boolean success, String moduleName);
} }
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/android/jni_android.h"
#include "components/module_installer/android/module_installer_jni_headers/ModuleInstallerBackend_jni.h"
namespace module_installer {
// Handles native specific initialization in after module is installed. This may
// be blocking, so the call should be done asynchronously.
void JNI_ModuleInstallerBackend_OnInstallModule(
JNIEnv* env,
jboolean j_success,
const base::android::JavaParamRef<jstring>& j_module_name) {
// TODO(crbug.com/927131): Add code to detect DevUI DFM installation, and load
// the associated resource PAK file. This will require the following:
// bool success = j_success;
// (Needs #include "base/android/jni_string.h").
// std::string module_name(ConvertJavaStringToUTF8(j_module_name));
// TODO(crbug.com/986960): Specialize this function to perform resource
// loading only. Required parameters can be injected to support data-driven
// flow. This will require massive refactoring first, however.
}
} // namespace module_installer
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment