[App banners] Stop passing JNI barrier to get icon size

Also deletes a bunch of comments that no longer apply and need to
be rewritten.

BUG=457413

Review URL: https://codereview.chromium.org/932263002

Cr-Commit-Position: refs/heads/master@{#316960}

chrome/android/java/src/org/chromium/chrome/browser/Tab.java

@@ -1260,7 +1260,7 @@ public class Tab implements ViewGroup.OnHierarchyChangeListener,
         initializeNative();
 
         if (AppBannerManager.isEnabled()) {
-            mAppBannerManager = new AppBannerManager(this);
+            mAppBannerManager = new AppBannerManager(this, mContext);
             addObserver(mAppBannerManager);
         }
     }

chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java

@@ -4,9 +4,9 @@
 
 package org.chromium.chrome.browser.banners;
 
+import android.content.Context;
 import android.text.TextUtils;
 
-import org.chromium.base.ApplicationStatus;
 import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
 import org.chromium.base.VisibleForTesting;
@@ -64,8 +64,9 @@ public class AppBannerManager extends EmptyTabObserver {
      * Constructs an AppBannerManager for the given tab.
      * @param tab Tab that the AppBannerManager will be attached to.
      */
-    public AppBannerManager(Tab tab) {
-        mNativePointer = nativeInit();
+    public AppBannerManager(Tab tab, Context context) {
+        int iconSize = context.getResources().getDimensionPixelSize(R.dimen.app_banner_icon_size);
+        mNativePointer = nativeInit(iconSize);
         mTab = tab;
         updatePointers();
     }
@@ -95,21 +96,14 @@ public class AppBannerManager extends EmptyTabObserver {
         nativeReplaceWebContents(mNativePointer, mTab.getWebContents());
     }
 
-    @CalledByNative
-    private int getPreferredIconSize() {
-        return ApplicationStatus.getApplicationContext().getResources().getDimensionPixelSize(
-                R.dimen.app_banner_icon_size);
-    }
-
     /**
      * 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.
      */
     @CalledByNative
-    private void fetchAppDetails(String url, String packageName) {
+    private void fetchAppDetails(String url, String packageName, int iconSize) {
         if (sAppDetailsDelegate == null) return;
-        int iconSize = getPreferredIconSize();
         sAppDetailsDelegate.getAppDetailsAsynchronously(
                 createAppDetailsObserver(), url, packageName, iconSize);
     }
@@ -153,7 +147,7 @@ public class AppBannerManager extends EmptyTabObserver {
     }
 
     private static native boolean nativeIsEnabled();
-    private native long nativeInit();
+    private native long nativeInit(int iconSize);
     private native void nativeDestroy(long nativeAppBannerManager);
     private native void nativeReplaceWebContents(long nativeAppBannerManager,
             WebContents webContents);

chrome/browser/android/banners/app_banner_manager.cc

@@ -91,8 +91,9 @@ void AppBannerManager::BannerBitmapFetcher::OnFetchComplete(
   delete this;
 }
 
-AppBannerManager::AppBannerManager(JNIEnv* env, jobject obj)
-    : fetcher_(nullptr),
+AppBannerManager::AppBannerManager(JNIEnv* env, jobject obj, int icon_size)
+    : preferred_icon_size_(icon_size),
+      fetcher_(nullptr),
       weak_java_banner_view_manager_(env, obj),
       weak_factory_(this) {
 }
@@ -179,7 +180,7 @@ void AppBannerManager::OnDidCheckHasServiceWorker(bool has_service_worker) {
     GURL icon_url =
         ManifestIconSelector::FindBestMatchingIcon(
             web_app_data_.icons,
-            GetPreferredIconSize(),
+            preferred_icon_size_,
             gfx::Screen::GetScreenFor(web_contents()->GetNativeView()));
     if (icon_url.is_empty())
       return;
@@ -300,7 +301,8 @@ void AppBannerManager::OnDidRetrieveMetaTagContent(
   Java_AppBannerManager_fetchAppDetails(env,
                                         jobj.obj(),
                                         jurl.obj(),
-                                        jpackage.obj());
+                                        jpackage.obj(),
+                                        preferred_icon_size_);
 }
 
 bool AppBannerManager::OnAppDetailsRetrieved(JNIEnv* env,
@@ -342,22 +344,13 @@ bool AppBannerManager::FetchIcon(const GURL& image_url) {
   return true;
 }
 
-int AppBannerManager::GetPreferredIconSize() {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  ScopedJavaLocalRef<jobject> jobj = weak_java_banner_view_manager_.get(env);
-  if (jobj.is_null())
-    return 0;
-
-  return Java_AppBannerManager_getPreferredIconSize(env, jobj.obj());
-}
-
 // static
 base::Time AppBannerManager::GetCurrentTime() {
   return base::Time::Now() + gTimeDeltaForTesting;
 }
 
-jlong Init(JNIEnv* env, jobject obj) {
-  AppBannerManager* manager = new AppBannerManager(env, obj);
+jlong Init(JNIEnv* env, jobject obj, jint icon_size) {
+  AppBannerManager* manager = new AppBannerManager(env, obj, icon_size);
   return reinterpret_cast<intptr_t>(manager);
 }
 

chrome/browser/android/banners/app_banner_manager.h

@@ -31,38 +31,10 @@ class InfoBar;
  * Manages when an app banner is created or dismissed.
  *
  * Hooks the wiring together for getting the data for a particular app.
- * Monitors at most one package at a time, and tracks the info for the
- * most recent app that was requested.  Any work in progress for other apps is
- * discarded.
+ * Monitors at most one app at a time, tracking the info for the most recently
+ * requested app. Any work in progress for other apps is discarded.
  *
- * The procedure for creating a banner spans multiple asynchronous calls across
- * the JNI boundary, as well as querying a Service to get info about the app.
- *
- * 0) A navigation of the main frame is triggered.  Upon completion of the load,
- *    the page is parsed for the correct meta tag.  If it doesn't exist, abort.
- *
- * 1) The AppBannerManager is alerted about the tag's contents, which should
- *    be the Play Store package name.  This is sent to the Java side
- *    AppBannerManager.
- *
- * 2) The AppBannerManager's ServiceDelegate is asynchronously queried about the
- *    package name.
- *
- * 3) At some point, the Java-side AppBannerManager is alerted of the completed
- *    query and is given back data about the requested package, which includes a
- *    URL for the app's icon.  This URL is sent to native code for retrieval.
- *
- * 4) The process of fetching the icon begins by invoking the BitmapFetcher,
- *    which works asynchonously.
- *
- * 5) Once the icon has been downloaded, the icon is sent to the Java-side
- *    AppBannerManager to (finally) create a AppBannerView, assuming that the
- *    app we retrieved the details for is still for the page that requested it.
- *
- * Because of the asynchronous nature of this pipeline, it's entirely possible
- * that a request to show a banner is interrupted by another request.  The
- * Java side manages what happens in these situations, which will usually result
- * in dropping the old banner request on the floor.
+ * TODO(dfalcantara): Update this when the pipeline requirements resolidify.
  */
 
 namespace banners {
@@ -71,7 +43,7 @@ class AppBannerManager : public content::WebContentsObserver {
  public:
   class BannerBitmapFetcher;
 
-  AppBannerManager(JNIEnv* env, jobject obj);
+  AppBannerManager(JNIEnv* env, jobject obj, int icon_size);
   ~AppBannerManager() override;
 
   // Destroys the AppBannerManager.
@@ -115,9 +87,6 @@ class AppBannerManager : public content::WebContentsObserver {
   bool OnMessageReceived(const IPC::Message& message) override;
 
  private:
-  // Gets the preferred icon size for the banner icons.
-  int GetPreferredIconSize();
-
   // Called when the manifest has been retrieved, or if there is no manifest to
   // retrieve.
   void OnDidGetManifest(const content::Manifest& manifest);
@@ -144,6 +113,9 @@ class AppBannerManager : public content::WebContentsObserver {
   // Cancels an active BitmapFetcher, stopping its banner from appearing.
   void CancelActiveFetcher();
 
+  // Icon size that we want to be use for the launcher.
+  const int preferred_icon_size_;
+
   // Fetches the icon for an app.  Weakly held because they delete themselves.
   BannerBitmapFetcher* fetcher_;