Commit 53b4682e authored by Alexander Cooper's avatar Alexander Cooper Committed by Commit Bot

Reduce Xr Install Helper dependency on //chrome

While not fully moving the InstallHelpers to a component yet, this
reduces their dependency on //chrome specific code, and gets them a step
closer to being moved to //components. The main dependencies that needed
to be broken were on Tab/TabUtils. Tab was typically used to get the
WebContents and the Activity. WebContents are now passed directly
in, rather than gotten from the Tab (which in C++ code was gotten from
the WebContents anyway...), and are then used to get the top level
window and thus the appropriate corresponding activity.

In addition to removing these dependencies on TabUtils, the ArCore's
install helper *also* now uses ApplicationStatus to track when it is
resumed, thus reducing it's dependency on ChromeActivity. This is
similar to what Vr does for it's DFM install. VR's install path is a bit
different in that it launches an activity for a result, and thus that is
not changed quite yet.

Bug: 960542
Change-Id: Ia42a73ecd1c491ee9899f8113a2905860b66366a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438639
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Auto-Submit: Alexander Cooper <alcooper@chromium.org>
Reviewed-by: default avatarMichael Thiessen <mthiesse@chromium.org>
Reviewed-by: default avatarPiotr Bialecki <bialpio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813350}
parent 157f4d88
...@@ -18,9 +18,9 @@ import org.chromium.base.annotations.CalledByNative; ...@@ -18,9 +18,9 @@ import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace; import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods; import org.chromium.base.annotations.NativeMethods;
import org.chromium.chrome.browser.infobar.InfoBarIdentifier; import org.chromium.chrome.browser.infobar.InfoBarIdentifier;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabUtils;
import org.chromium.chrome.browser.ui.messages.infobar.SimpleConfirmInfoBarBuilder; import org.chromium.chrome.browser.ui.messages.infobar.SimpleConfirmInfoBarBuilder;
import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.WindowAndroid;
/** /**
* Manages logic around VrCore Installation and Versioning * Manages logic around VrCore Installation and Versioning
...@@ -143,13 +143,26 @@ public class VrCoreInstallUtils { ...@@ -143,13 +143,26 @@ public class VrCoreInstallUtils {
mNativeVrCoreInstallUtils = 0; mNativeVrCoreInstallUtils = 0;
} }
private Activity getActivity(final WebContents webContents) {
if (webContents == null) return null;
WindowAndroid window = webContents.getTopLevelNativeWindow();
if (window == null) return null;
return window.getActivity().get();
}
/** /**
* Prompts the user to install or update VRSupport if needed. * Prompts the user to install or update VRSupport if needed.
*/ */
@CalledByNative @CalledByNative
@VisibleForTesting @VisibleForTesting
protected void requestInstallVrCore(final Tab tab) { protected void requestInstallVrCore(final WebContents webContents) {
if (tab == null) { if (webContents == null) {
maybeNotifyNativeOnInstallResult(false);
return;
}
final Activity activity = getActivity(webContents);
if (activity == null) {
maybeNotifyNativeOnInstallResult(false); maybeNotifyNativeOnInstallResult(false);
return; return;
} }
...@@ -184,7 +197,6 @@ public class VrCoreInstallUtils { ...@@ -184,7 +197,6 @@ public class VrCoreInstallUtils {
return; return;
} }
final Activity activity = TabUtils.getActivity(tab);
SimpleConfirmInfoBarBuilder.Listener listener = new SimpleConfirmInfoBarBuilder.Listener() { SimpleConfirmInfoBarBuilder.Listener listener = new SimpleConfirmInfoBarBuilder.Listener() {
@Override @Override
public void onInfoBarDismissed() { public void onInfoBarDismissed() {
...@@ -206,7 +218,7 @@ public class VrCoreInstallUtils { ...@@ -206,7 +218,7 @@ public class VrCoreInstallUtils {
return false; return false;
} }
}; };
SimpleConfirmInfoBarBuilder.create(tab.getWebContents(), listener, SimpleConfirmInfoBarBuilder.create(webContents, listener,
InfoBarIdentifier.VR_SERVICES_UPGRADE_ANDROID, activity, InfoBarIdentifier.VR_SERVICES_UPGRADE_ANDROID, activity,
org.chromium.chrome.vr.R.drawable.vr_services, infobarText, buttonText, null, null, org.chromium.chrome.vr.R.drawable.vr_services, infobarText, buttonText, null, null,
true); true);
......
...@@ -938,10 +938,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent> ...@@ -938,10 +938,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
mPictureInPictureController.cleanup(this); mPictureInPictureController.cleanup(this);
} }
VrModuleProvider.getDelegate().maybeRegisterVrEntryHook(this); VrModuleProvider.getDelegate().maybeRegisterVrEntryHook(this);
ArDelegate arDelegate = ArDelegateProvider.getDelegate();
if (arDelegate != null) {
arDelegate.registerOnResumeActivity(this);
}
getManualFillingComponent().onResume(); getManualFillingComponent().onResume();
} }
......
...@@ -6,6 +6,8 @@ package org.chromium.chrome.browser.vr; ...@@ -6,6 +6,8 @@ package org.chromium.chrome.browser.vr;
import android.app.Activity; import android.app.Activity;
import org.chromium.base.ActivityState;
import org.chromium.base.ApplicationStatus;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.Log; import org.chromium.base.Log;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
...@@ -13,15 +15,15 @@ import org.chromium.base.annotations.JNINamespace; ...@@ -13,15 +15,15 @@ import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods; import org.chromium.base.annotations.NativeMethods;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.infobar.InfoBarIdentifier; import org.chromium.chrome.browser.infobar.InfoBarIdentifier;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabUtils;
import org.chromium.chrome.browser.ui.messages.infobar.SimpleConfirmInfoBarBuilder; import org.chromium.chrome.browser.ui.messages.infobar.SimpleConfirmInfoBarBuilder;
import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.WindowAndroid;
/** /**
* Installs AR DFM and ArCore runtimes. * Installs AR DFM and ArCore runtimes.
*/ */
@JNINamespace("vr") @JNINamespace("vr")
public class ArCoreInstallUtils { public class ArCoreInstallUtils implements ApplicationStatus.ActivityStateListener {
private static final String TAG = "ArCoreInstallUtils"; private static final String TAG = "ArCoreInstallUtils";
private long mNativeArCoreInstallUtils; private long mNativeArCoreInstallUtils;
...@@ -84,13 +86,40 @@ public class ArCoreInstallUtils { ...@@ -84,13 +86,40 @@ public class ArCoreInstallUtils {
return availability != ArCoreShim.Availability.SUPPORTED_INSTALLED; return availability != ArCoreShim.Availability.SUPPORTED_INSTALLED;
} }
@Override
public void onActivityStateChange(Activity activity, int newState) {
// We only care about activity state changes if we're the ones that have requested an
// install, and then only if it's a resume.
if (!(sRequestInstallInstance == this && newState == ActivityState.RESUMED)) return;
onArCoreRequestInstallReturned(activity);
// After we've gotten resumed, we no longer need to track activity state.
sRequestInstallInstance = null;
ApplicationStatus.unregisterActivityStateListener(this);
}
private Activity getActivity(final WebContents webContents) {
if (webContents == null) return null;
WindowAndroid window = webContents.getTopLevelNativeWindow();
if (window == null) return null;
return window.getActivity().get();
}
@CalledByNative @CalledByNative
private void requestInstallSupportedArCore(final Tab tab) { private void requestInstallSupportedArCore(final WebContents webContents) {
assert shouldRequestInstallSupportedArCore(); assert shouldRequestInstallSupportedArCore();
final Activity activity = getActivity(webContents);
if (activity == null) {
Log.w(TAG, "Could not get Activity");
maybeNotifyNativeOnRequestInstallSupportedArCoreResult(false);
return;
}
@ArCoreShim.Availability @ArCoreShim.Availability
int arCoreAvailability = getArCoreInstallStatus(); int arCoreAvailability = getArCoreInstallStatus();
final Activity activity = TabUtils.getActivity(tab);
String infobarText = null; String infobarText = null;
String buttonText = null; String buttonText = null;
switch (arCoreAvailability) { switch (arCoreAvailability) {
...@@ -162,9 +191,11 @@ public class ArCoreInstallUtils { ...@@ -162,9 +191,11 @@ public class ArCoreInstallUtils {
return false; return false;
} }
}; };
ApplicationStatus.registerStateListenerForActivity(this, activity);
// TODO(ijamardo, https://crbug.com/838833): Add icon for AR info bar. // TODO(ijamardo, https://crbug.com/838833): Add icon for AR info bar.
SimpleConfirmInfoBarBuilder.create(tab.getWebContents(), listener, SimpleConfirmInfoBarBuilder.create(webContents, listener,
InfoBarIdentifier.AR_CORE_UPGRADE_ANDROID, tab.getContext(), InfoBarIdentifier.AR_CORE_UPGRADE_ANDROID, activity,
R.drawable.ic_error_outline_googblue_24dp, infobarText, buttonText, null, null, R.drawable.ic_error_outline_googblue_24dp, infobarText, buttonText, null, null,
true); true);
} }
...@@ -194,18 +225,6 @@ public class ArCoreInstallUtils { ...@@ -194,18 +225,6 @@ public class ArCoreInstallUtils {
} }
} }
/**
* This method should be called by the Activity that gets resumed.
* We are only interested in the cases where our current Activity got paused
* as a result of a call to ArCoreApk.requestInstall() method.
*/
public static void onResumeActivityWithNative(Activity activity) {
if (sRequestInstallInstance != null) {
sRequestInstallInstance.onArCoreRequestInstallReturned(activity);
sRequestInstallInstance = null;
}
}
public static void installArCoreDeviceProviderFactory() { public static void installArCoreDeviceProviderFactory() {
ArCoreInstallUtilsJni.get().installArCoreDeviceProviderFactory(); ArCoreInstallUtilsJni.get().installArCoreDeviceProviderFactory();
} }
......
...@@ -16,9 +16,7 @@ import org.chromium.base.ThreadUtils; ...@@ -16,9 +16,7 @@ import org.chromium.base.ThreadUtils;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace; import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods; import org.chromium.base.annotations.NativeMethods;
import org.chromium.chrome.browser.app.ChromeActivity; import org.chromium.content_public.browser.WebContents;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabUtils;
/** /**
* Provides ARCore classes access to java-related app functionality. * Provides ARCore classes access to java-related app functionality.
...@@ -76,11 +74,11 @@ public class ArCoreJavaUtils { ...@@ -76,11 +74,11 @@ public class ArCoreJavaUtils {
} }
@CalledByNative @CalledByNative
private void startSession(final Tab tab, boolean useOverlay) { private void startSession(final WebContents webContents, boolean useOverlay) {
if (DEBUG_LOGS) Log.i(TAG, "startSession"); if (DEBUG_LOGS) Log.i(TAG, "startSession");
mArImmersiveOverlay = new ArImmersiveOverlay(); mArImmersiveOverlay = new ArImmersiveOverlay();
sActiveSessionInstance = this; sActiveSessionInstance = this;
mArImmersiveOverlay.show((ChromeActivity) TabUtils.getActivity(tab), this, useOverlay); mArImmersiveOverlay.show(webContents, this, useOverlay);
} }
@CalledByNative @CalledByNative
......
...@@ -4,8 +4,6 @@ ...@@ -4,8 +4,6 @@
package org.chromium.chrome.browser.vr; package org.chromium.chrome.browser.vr;
import android.app.Activity;
/** /**
* Interface used by ChromeActivity to communicate with AR code that is only * Interface used by ChromeActivity to communicate with AR code that is only
* available if |enable_arcore| is set to true at build time. * available if |enable_arcore| is set to true at build time.
...@@ -16,12 +14,6 @@ public interface ArDelegate { ...@@ -16,12 +14,6 @@ public interface ArDelegate {
**/ **/
public void init(); public void init();
/**
* Needs to be called in Activity's onResumeWithNative() method in order
* to notify AR that the activity was resumed.
**/
public void registerOnResumeActivity(Activity activity);
/** /**
* Used to let AR immersive mode intercept the Back button to exit immersive mode. * Used to let AR immersive mode intercept the Back button to exit immersive mode.
*/ */
......
...@@ -4,8 +4,6 @@ ...@@ -4,8 +4,6 @@
package org.chromium.chrome.browser.vr; package org.chromium.chrome.browser.vr;
import android.app.Activity;
import org.chromium.base.annotations.UsedByReflection; import org.chromium.base.annotations.UsedByReflection;
/** /**
...@@ -22,11 +20,6 @@ public class ArDelegateImpl implements ArDelegate { ...@@ -22,11 +20,6 @@ public class ArDelegateImpl implements ArDelegate {
ArCoreInstallUtils.installArCoreDeviceProviderFactory(); ArCoreInstallUtils.installArCoreDeviceProviderFactory();
} }
@Override
public void registerOnResumeActivity(Activity activity) {
ArCoreInstallUtils.onResumeActivityWithNative(activity);
}
@Override @Override
public boolean onBackPressed() { public boolean onBackPressed() {
return ArCoreJavaUtils.onBackPressed(); return ArCoreJavaUtils.onBackPressed();
......
...@@ -30,6 +30,7 @@ import org.chromium.chrome.browser.fullscreen.FullscreenManager; ...@@ -30,6 +30,7 @@ import org.chromium.chrome.browser.fullscreen.FullscreenManager;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.content_public.browser.ScreenOrientationDelegate; import org.chromium.content_public.browser.ScreenOrientationDelegate;
import org.chromium.content_public.browser.ScreenOrientationProvider; import org.chromium.content_public.browser.ScreenOrientationProvider;
import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.display.DisplayAndroidManager; import org.chromium.ui.display.DisplayAndroidManager;
import org.chromium.ui.widget.Toast; import org.chromium.ui.widget.Toast;
...@@ -57,10 +58,10 @@ public class ArImmersiveOverlay ...@@ -57,10 +58,10 @@ public class ArImmersiveOverlay
private Integer mPrimaryPointerId; private Integer mPrimaryPointerId;
public void show( public void show(
@NonNull ChromeActivity activity, @NonNull ArCoreJavaUtils caller, boolean useOverlay) { @NonNull WebContents webContents, @NonNull ArCoreJavaUtils caller, boolean useOverlay) {
if (DEBUG_LOGS) Log.i(TAG, "constructor"); if (DEBUG_LOGS) Log.i(TAG, "constructor");
mArCoreJavaUtils = caller; mArCoreJavaUtils = caller;
mActivity = activity; mActivity = ChromeActivity.fromWebContents(webContents);
mPointerIdToData = new HashMap<Integer, PointerData>(); mPointerIdToData = new HashMap<Integer, PointerData>();
mPrimaryPointerId = null; mPrimaryPointerId = null;
......
...@@ -90,7 +90,8 @@ public class VrInstallUpdateInfoBarTest { ...@@ -90,7 +90,8 @@ public class VrInstallUpdateInfoBarTest {
VrCoreInstallUtils vrCoreInstallUtils = VrCoreInstallUtils.create(0); VrCoreInstallUtils vrCoreInstallUtils = VrCoreInstallUtils.create(0);
setVrCoreCompatibility(checkerReturnCompatibility); setVrCoreCompatibility(checkerReturnCompatibility);
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
vrCoreInstallUtils.requestInstallVrCore(mVrTestRule.getActivity().getActivityTab()); vrCoreInstallUtils.requestInstallVrCore(
mVrTestRule.getActivity().getCurrentWebContents());
}); });
View decorView = mVrTestRule.getActivity().getWindow().getDecorView(); View decorView = mVrTestRule.getActivity().getWindow().getDecorView();
if (checkerReturnCompatibility == VrCoreVersionChecker.VrCoreCompatibility.VR_READY) { if (checkerReturnCompatibility == VrCoreVersionChecker.VrCoreCompatibility.VR_READY) {
......
...@@ -4,13 +4,12 @@ ...@@ -4,13 +4,12 @@
#include "chrome/browser/android/vr/android_vr_utils.h" #include "chrome/browser/android/vr/android_vr_utils.h"
#include "chrome/browser/android/tab_android.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
namespace vr { namespace vr {
base::android::ScopedJavaLocalRef<jobject> GetTabFromRenderer( base::android::ScopedJavaLocalRef<jobject> GetJavaWebContents(
int render_process_id, int render_process_id,
int render_frame_id) { int render_frame_id) {
content::RenderFrameHost* render_frame_host = content::RenderFrameHost* render_frame_host =
...@@ -21,14 +20,7 @@ base::android::ScopedJavaLocalRef<jobject> GetTabFromRenderer( ...@@ -21,14 +20,7 @@ base::android::ScopedJavaLocalRef<jobject> GetTabFromRenderer(
content::WebContents::FromRenderFrameHost(render_frame_host); content::WebContents::FromRenderFrameHost(render_frame_host);
DCHECK(web_contents); DCHECK(web_contents);
TabAndroid* tab_android = TabAndroid::FromWebContents(web_contents); return web_contents->GetJavaWebContents();
DCHECK(tab_android);
base::android::ScopedJavaLocalRef<jobject> j_tab_android =
tab_android->GetJavaObject();
DCHECK(!j_tab_android.is_null());
return j_tab_android;
} }
} // namespace vr } // namespace vr
...@@ -12,7 +12,7 @@ ...@@ -12,7 +12,7 @@
// chrome/browser/vr/*util.cc|h // chrome/browser/vr/*util.cc|h
namespace vr { namespace vr {
base::android::ScopedJavaLocalRef<jobject> GetTabFromRenderer( base::android::ScopedJavaLocalRef<jobject> GetJavaWebContents(
int render_process_id, int render_process_id,
int render_frame_id); int render_frame_id);
......
...@@ -79,7 +79,7 @@ void ArCoreInstallHelper::EnsureInstalled( ...@@ -79,7 +79,7 @@ void ArCoreInstallHelper::EnsureInstalled(
// When completed, java will call: OnRequestInstallSupportedArCoreResult // When completed, java will call: OnRequestInstallSupportedArCoreResult
Java_ArCoreInstallUtils_requestInstallSupportedArCore( Java_ArCoreInstallUtils_requestInstallSupportedArCore(
env, java_install_utils_, env, java_install_utils_,
GetTabFromRenderer(render_process_id, render_frame_id)); GetJavaWebContents(render_process_id, render_frame_id));
return; return;
} }
......
...@@ -49,7 +49,7 @@ void ArCoreJavaUtils::RequestArSession( ...@@ -49,7 +49,7 @@ void ArCoreJavaUtils::RequestArSession(
Java_ArCoreJavaUtils_startSession( Java_ArCoreJavaUtils_startSession(
env, j_arcore_java_utils_, env, j_arcore_java_utils_,
GetTabFromRenderer(render_process_id, render_frame_id), use_overlay); GetJavaWebContents(render_process_id, render_frame_id), use_overlay);
} }
void ArCoreJavaUtils::EndSession() { void ArCoreJavaUtils::EndSession() {
......
...@@ -65,7 +65,7 @@ void VrCoreInstallHelper::EnsureInstalled( ...@@ -65,7 +65,7 @@ void VrCoreInstallHelper::EnsureInstalled(
// When completed, java will call: OnInstallResult // When completed, java will call: OnInstallResult
Java_VrCoreInstallUtils_requestInstallVrCore( Java_VrCoreInstallUtils_requestInstallVrCore(
env, java_install_utils_, env, java_install_utils_,
GetTabFromRenderer(render_process_id, render_frame_id)); GetJavaWebContents(render_process_id, render_frame_id));
return; return;
} }
......
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