Redo how AppBannerInfoBars are created and stored

chrome/browser/android/banners/app_banner_manager.cc

 // Copyright 2014 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 "chrome/browser/android/banners/app_banner_manager.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 17 matching lines @@
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/frame_navigate_params.h"
 #include "content/public/common/manifest.h"
 #include "jni/AppBannerManager_jni.h"
 #include "net/base/load_flags.h"
 #include "ui/gfx/android/java_bitmap.h"
 #include "ui/gfx/screen.h"
 
 using base::android::ConvertJavaStringToUTF8;
+using base::android::ConvertJavaStringToUTF16;
 using base::android::ConvertUTF8ToJavaString;
 using base::android::ConvertUTF16ToJavaString;
 
 namespace {
 const char kBannerTag[] = "google-play-id";
 }
 
 namespace banners {
 
 AppBannerManager::AppBannerManager(JNIEnv* env, jobject obj)
@@ skipping 12 matching lines @@
                                    jstring jpackage) {
   if (!web_contents())
     return;
 
   GURL url(ConvertJavaStringToUTF8(env, jurl));
   std::string package_name = ConvertJavaStringToUTF8(env, jpackage);
   AppBannerSettingsHelper::Block(web_contents(), url, package_name);
 }
 
 void AppBannerManager::Block() const {
-  if (!web_contents() || manifest_.IsEmpty())
-    return;
-
-  AppBannerSettingsHelper::Block(web_contents(),
-                                 web_contents()->GetURL(),
-                                 manifest_.start_url.spec());
-}
-
-void AppBannerManager::Install() const {
   if (!web_contents())
     return;
 
-  if (!manifest_.IsEmpty()) {
-    InstallManifestApp(manifest_, *app_icon_.get());
+  if (!web_app_data_.IsEmpty()) {
+    AppBannerSettingsHelper::Block(web_contents(),
+                                   web_contents()->GetURL(),
+                                   web_app_data_.start_url.spec());
   }
 }
 
+void AppBannerManager::OnInfoBarDestroyed() {
+  weak_infobar_ptr_ = nullptr;
+}
+
+bool AppBannerManager::OnButtonClicked() const {

[newt (away), 2015/02/05 19:21:09]

Why does this always return true? Is this return value even needed then?

[gone, 2015/02/05 19:24:19]

Follow up CL returns false when installing native apps to prevent the infobar
from going away.

+  if (!web_contents())
+    return true;
+
+  if (!web_app_data_.IsEmpty()) {
+    InstallManifestApp(web_app_data_, *app_icon_.get());
+    return true;
+  }
+
+  return true;
+}
+
+base::string16 AppBannerManager::GetTitle() const {
+  return app_title_;
+}
+
 gfx::Image AppBannerManager::GetIcon() const {
   return gfx::Image::CreateFrom1xBitmap(*app_icon_.get());
 }
 
 void AppBannerManager::ReplaceWebContents(JNIEnv* env,
                                           jobject obj,
                                           jobject jweb_contents) {
   content::WebContents* web_contents =
       content::WebContents::FromJavaWebContents(jweb_contents);
   Observe(web_contents);
 }
 
 void AppBannerManager::DidNavigateMainFrame(
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
   // Clear current state.
   fetcher_.reset();
-  manifest_ = content::Manifest();
+  app_title_ = base::string16();
   app_icon_.reset();
-
-  // Get rid of the current banner.
-  JNIEnv* env = base::android::AttachCurrentThread();
-  ScopedJavaLocalRef<jobject> jobj = weak_java_banner_view_manager_.get(env);
-  if (jobj.is_null())
-    return;
-  Java_AppBannerManager_dismissCurrentBanner(env, jobj.obj(), DISMISS_NAVIGATE);
+  web_app_data_ = content::Manifest();
 }
 
 void AppBannerManager::DidFinishLoad(
     content::RenderFrameHost* render_frame_host,
     const GURL& validated_url) {
   if (render_frame_host->GetParent())
     return;
   validated_url_ = validated_url;
 
   // See if the page has a manifest. Using Unretained(this) here is safe as the
@@ skipping 14 matching lines @@
     Send(new ChromeViewMsg_RetrieveMetaTagContent(routing_id(),
                                                   validated_url_,
                                                   kBannerTag));
     return;
   }
 
   // TODO(benwells): Check triggering parameters here and if there is a meta
   // tag.
 
   // Create an infobar to promote the manifest's app.
-  manifest_ = manifest;
-
+  web_app_data_ = manifest;
+  app_title_ = web_app_data_.name.string();
   GURL icon_url =
       ManifestIconSelector::FindBestMatchingIcon(
           manifest.icons,
           GetPreferredIconSize(),
           gfx::Screen::GetScreenFor(web_contents()->GetNativeView()));
   if (icon_url.is_empty())
     return;
 
   FetchIcon(icon_url);
 }
 
 bool AppBannerManager::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(AppBannerManager, message)
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_DidRetrieveMetaTagContent,
                         OnDidRetrieveMetaTagContent)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
 void AppBannerManager::OnFetchComplete(const GURL url, const SkBitmap* bitmap) {
-  if (bitmap) {
-    JNIEnv* env = base::android::AttachCurrentThread();
-
-    ScopedJavaLocalRef<jobject> jobj = weak_java_banner_view_manager_.get(env);
-    if (jobj.is_null())
-      return;
-
-    bool displayed;
-    if (manifest_.IsEmpty()) {
-      ScopedJavaLocalRef<jobject> jimage = gfx::ConvertToJavaBitmap(bitmap);
-      ScopedJavaLocalRef<jstring> jimage_url(
-          ConvertUTF8ToJavaString(env, url.spec()));
-
-      displayed = Java_AppBannerManager_createBanner(env,
-                                                     jobj.obj(),
-                                                     jimage_url.obj(),
-                                                     jimage.obj());
-    } else {
-      app_icon_.reset(new SkBitmap(*bitmap));
-      InfoBarService* service = InfoBarService::FromWebContents(web_contents());
-      displayed = AppBannerInfoBarDelegate::CreateForWebApp(
-          service,
-          this,
-          manifest_.name.string(),
-          manifest_.start_url) != NULL;
-    }
-
-    if (displayed)
-      banners::TrackDisplayEvent(DISPLAY_CREATED);
-  } else {
+  fetcher_.reset();
+  if (!bitmap || url != app_icon_url_) {
     DVLOG(1) << "Failed to retrieve image: " << url;
+    return;
   }
 
-  fetcher_.reset();
+  JNIEnv* env = base::android::AttachCurrentThread();
+  ScopedJavaLocalRef<jobject> jobj = weak_java_banner_view_manager_.get(env);
+  if (jobj.is_null())
+    return;
+
+  app_icon_.reset(new SkBitmap(*bitmap));
+  InfoBarService* service = InfoBarService::FromWebContents(web_contents());
+
+  weak_infobar_ptr_ = nullptr;
+  if (!web_app_data_.IsEmpty()){
+    weak_infobar_ptr_ = AppBannerInfoBarDelegate::CreateForWebApp(
+        service,
+        this,
+        web_app_data_.start_url);
+  }
+
+  if (weak_infobar_ptr_ != nullptr)
+    banners::TrackDisplayEvent(DISPLAY_CREATED);
 }
 
 void AppBannerManager::OnDidRetrieveMetaTagContent(
     bool success,
     const std::string& tag_name,
     const std::string& tag_content,
     const GURL& expected_url) {
   DCHECK(web_contents());
   if (!success || tag_name != kBannerTag || validated_url_ != expected_url ||
       tag_content.size() >= chrome::kMaxMetaTagAttributeLength) {
@@ skipping 37 matching lines @@
 
   // Begin asynchronously fetching the app icon.
   Profile* profile =
       Profile::FromBrowserContext(web_contents()->GetBrowserContext());
   fetcher_.reset(new chrome::BitmapFetcher(image_url, this));
   fetcher_.get()->Start(
       profile->GetRequestContext(),
       std::string(),
       net::URLRequest::CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE,
       net::LOAD_NORMAL);
+  app_icon_url_ = 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());
@@ skipping 30 matching lines @@
   return base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableAppInstallAlerts);
 }
 
 // Register native methods
 bool RegisterAppBannerManager(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 }  // namespace banners