Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1882)

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java

Issue 2124243002: Refactor the Java AppBannerManager to be owned by native code. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Addressing reviewer comments Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java b/chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java
index 19f4531ee843bf071aeeb98c7d70f283938dc009..b3e0dea497d509976be38c1d550bda98792b6fab 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java
@@ -12,44 +12,50 @@ import org.chromium.base.VisibleForTesting;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace;
import org.chromium.chrome.browser.ShortcutHelper;
-import org.chromium.chrome.browser.tab.EmptyTabObserver;
-import org.chromium.chrome.browser.tab.Tab;
import org.chromium.content_public.browser.WebContents;
/**
* Manages an AppBannerInfoBar for a Tab.
*
- * The AppBannerManager manages a single AppBannerInfoBar, creating a new one when it detects that
- * the current webpage is requesting a banner to be built. The actual observation of the WebContents
- * (which triggers the automatic creation and removal of banners, among other things) is done by the
- * native-side AppBannerManager.
- *
- * This Java-side class owns its native-side counterpart, which is basically used to grab resources
- * from the network.
+ * The AppBannerManager is responsible for fetching details about native apps to display in the
+ * banner. The actual observation of the WebContents (which triggers the automatic creation and
+ * removal of banners, among other things) is done by the native-side AppBannerManagerAndroid.
*/
@JNINamespace("banners")
-public class AppBannerManager extends EmptyTabObserver {
+public class AppBannerManager {
private static final String TAG = "AppBannerManager";
/** Retrieves information about a given package. */
private static AppDetailsDelegate sAppDetailsDelegate;
- /** Whether the banners are enabled. */
- private static Boolean sIsEnabled;
+ /** Whether add to home screen is permitted by the system. */
+ private static Boolean sIsSupported;
+
+ /** Whether the tab to which this manager is attached to is permitted to show banners. */
+ private boolean mIsEnabledForTab;
/** Pointer to the native side AppBannerManager. */
private long mNativePointer;
/**
- * Checks if app banners are enabled.
- * @return True if banners are enabled, false otherwise.
+ * Checks if the add to home screen intent is supported.
+ * @return true if add to home screen is supported, false otherwise.
*/
- public static boolean isEnabled() {
- if (sIsEnabled == null) {
+ public static boolean isSupported() {
+ if (sIsSupported == null) {
Context context = ContextUtils.getApplicationContext();
- sIsEnabled = ShortcutHelper.isAddToHomeIntentSupported(context);
+ sIsSupported = ShortcutHelper.isAddToHomeIntentSupported(context);
}
- return sIsEnabled;
+ return sIsSupported;
+ }
+
+ /**
+ * Checks if app banners are enabled for the tab which this manager is attached to.
+ * @return true if app banners can be shown for this tab, false otherwise.
+ */
+ @CalledByNative
+ private boolean isEnabledForTab() {
+ return isSupported() && mIsEnabledForTab;
}
/**
@@ -62,41 +68,25 @@ public class AppBannerManager extends EmptyTabObserver {
}
/**
- * Constructs an AppBannerManager for the given tab.
- * @param tab Tab that the AppBannerManager will be attached to.
+ * Constructs an AppBannerManager.
+ * @param nativePointer the native-side object that owns this AppBannerManager.
*/
- public AppBannerManager(Tab tab, Context context) {
- mNativePointer = nativeInit();
- updatePointers(tab);
+ private AppBannerManager(long nativePointer) {
+ mNativePointer = nativePointer;
+ mIsEnabledForTab = isSupported();
}
- @Override
- public void onWebContentsSwapped(Tab tab, boolean didStartLoad,
- boolean didFinishLoad) {
- updatePointers(tab);
- }
-
- @Override
- public void onContentChanged(Tab tab) {
- updatePointers(tab);
+ @CalledByNative
+ private static AppBannerManager create(long nativePointer) {
+ return new AppBannerManager(nativePointer);
}
- /**
- * Destroys the native AppBannerManager.
- */
- public void destroy() {
- nativeDestroy(mNativePointer);
+ @CalledByNative
+ private void destroy() {
mNativePointer = 0;
}
/**
- * Updates which WebContents the native AppBannerManager is monitoring.
- */
- private void updatePointers(Tab tab) {
- nativeReplaceWebContents(mNativePointer, tab.getWebContents());
- }
-
- /**
* Grabs package information for the banner asynchronously.
* @param url URL for the page that is triggering the banner.
* @param packageName Name of the package that is being advertised.
@@ -133,15 +123,15 @@ public class AppBannerManager extends EmptyTabObserver {
};
}
- /** Requests the app banner. This method is called from the DevTools. */
- public void requestAppBanner() {
- nativeRequestAppBanner(mNativePointer);
+ /** Enables or disables app banners. */
+ public void setIsEnabledForTab(boolean state) {
+ mIsEnabledForTab = state;
}
- /** Enables or disables the app banners for testing. */
+ /** Overrides whether the system supports add to home screen. Used in testing. */
@VisibleForTesting
- static void setIsEnabledForTesting(boolean state) {
- sIsEnabled = state;
+ public static void setIsSupported(boolean state) {
+ sIsSupported = state;
}
/** Sets a constant (in days) that gets added to the time when the current time is requested. */
@@ -162,19 +152,21 @@ public class AppBannerManager extends EmptyTabObserver {
nativeSetEngagementWeights(directEngagement, indirectEngagement);
}
- /** Returns whether a AppBannerDataFetcher is actively retrieving data. */
+ /** Returns whether an AppBannerDataFetcher is actively retrieving data. */
@VisibleForTesting
public boolean isFetcherActiveForTesting() {
return nativeIsFetcherActive(mNativePointer);
}
- private native long nativeInit();
- private native void nativeDestroy(long nativeAppBannerManagerAndroid);
- private native void nativeReplaceWebContents(long nativeAppBannerManagerAndroid,
+ /** Returns the AppBannerManager object. This is owned by the C++ banner manager. */
+ public static AppBannerManager getAppBannerManagerForWebContents(WebContents webContents) {
+ return nativeGetJavaBannerManagerForWebContents(webContents);
+ }
+
+ private static native AppBannerManager nativeGetJavaBannerManagerForWebContents(
WebContents webContents);
private native boolean nativeOnAppDetailsRetrieved(long nativeAppBannerManagerAndroid,
AppData data, String title, String packageName, String imageUrl);
- private native void nativeRequestAppBanner(long nativeAppBannerManagerAndroid);
// Testing methods.
private static native void nativeSetTimeDeltaForTesting(int days);

Powered by Google App Engine
This is Rietveld 408576698