[App banners] Start addressing race conditions

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 27 matching lines @@
 #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";
 base::TimeDelta gTimeDeltaForTesting;
-}
+}  // namespace
 
 namespace banners {
 
+// Fetches a bitmap and deletes itself when completed.
+class AppBannerManager::BannerBitmapFetcher
+    : public chrome::BitmapFetcher,
+      public chrome::BitmapFetcherDelegate {
+ public:
+  BannerBitmapFetcher(const GURL& image_url,
+                      banners::AppBannerManager* manager);
+
+  // Prevents informing the AppBannerManager that the fetch has completed.
+  void Cancel();
+
+  // chrome::BitmapFetcherDelegate overrides
+  void OnFetchComplete(const GURL url, const SkBitmap* icon) override;
+
+ private:
+  banners::AppBannerManager* manager_;
+  bool is_cancelled_;
+};
+
+AppBannerManager::BannerBitmapFetcher::BannerBitmapFetcher(
+    const GURL& image_url,
+    banners::AppBannerManager* manager)
+    : chrome::BitmapFetcher(image_url, this),
+      manager_(manager),
+      is_cancelled_(false) {
+}
+
+void AppBannerManager::BannerBitmapFetcher::Cancel() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  is_cancelled_ = true;
+}
+
+void AppBannerManager::BannerBitmapFetcher::OnFetchComplete(
+    const GURL url,
+    const SkBitmap* icon) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  if (!is_cancelled_)
+    manager_->OnFetchComplete(this, url, icon);
+
+  delete this;
+}
+
 AppBannerManager::AppBannerManager(JNIEnv* env, jobject obj)
-    : weak_java_banner_view_manager_(env, obj), weak_factory_(this) {
+    : fetcher_(nullptr),
+      weak_java_banner_view_manager_(env, obj),
+      weak_factory_(this) {
 }
 
 AppBannerManager::~AppBannerManager() {
+  CancelActiveFetcher();
 }
 
 void AppBannerManager::Destroy(JNIEnv* env, jobject obj) {
   delete this;
 }
 
 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();
+  CancelActiveFetcher();
   app_title_ = base::string16();
   web_app_data_ = content::Manifest();
   native_app_data_.Reset();
   native_app_package_ = std::string();
 }
 
 void AppBannerManager::DidFinishLoad(
     content::RenderFrameHost* render_frame_host,
     const GURL& validated_url) {
   if (render_frame_host->GetParent())
@@ skipping 69 matching lines @@
                                                  GetCurrentTime())) {
     return false;
   }
 
   AppBannerSettingsHelper::RecordBannerEvent(
       web_contents(), validated_url_, package_or_start_url,
       AppBannerSettingsHelper::APP_BANNER_EVENT_DID_SHOW, GetCurrentTime());
   return true;
 }
 
+void AppBannerManager::CancelActiveFetcher() {
+  // Fetchers delete themselves.
+  if (fetcher_ != nullptr) {
+    fetcher_->Cancel();
+    fetcher_ = nullptr;
+  }
+}
+
 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) {
-  fetcher_.reset();
-  if (!bitmap || url != app_icon_url_) {
-    DVLOG(1) << "Failed to retrieve image: " << url;
+void AppBannerManager::OnFetchComplete(BannerBitmapFetcher* fetcher,
+                                       const GURL url,
+                                       const SkBitmap* bitmap) {
+  if (fetcher_ != fetcher)
     return;
-  }
+  fetcher_ = nullptr;
+
+  if (!bitmap || url != app_icon_url_)
+    return;
 
   JNIEnv* env = base::android::AttachCurrentThread();
   ScopedJavaLocalRef<jobject> jobj = weak_java_banner_view_manager_.get(env);
   if (jobj.is_null())
     return;
 
   InfoBarService* service = InfoBarService::FromWebContents(web_contents());
 
   AppBannerInfoBar* weak_infobar_ptr = nullptr;
   if (!native_app_data_.is_null()) {
@@ skipping 61 matching lines @@
   if (validated_url_ != web_contents()->GetURL())
     return false;
 
   std::string image_url = ConvertJavaStringToUTF8(env, jicon_url);
   app_title_ = ConvertJavaStringToUTF16(env, japp_title);
   native_app_package_ = ConvertJavaStringToUTF8(env, japp_package);
   native_app_data_.Reset(env, japp_data);
   return FetchIcon(GURL(image_url));
 }
 
-int AppBannerManager::GetNumActiveFetchers(JNIEnv* env, jobject obj) {
-  return fetcher_.get() ? 1 : 0;
+bool AppBannerManager::IsFetcherActive(JNIEnv* env, jobject obj) {
+  return fetcher_ != nullptr;
 }
 
 bool AppBannerManager::FetchIcon(const GURL& image_url) {
   if (!web_contents())
     return false;
 
   // Begin asynchronously fetching the app icon.
   Profile* profile =
       Profile::FromBrowserContext(web_contents()->GetBrowserContext());
-  fetcher_.reset(new chrome::BitmapFetcher(image_url, this));
-  fetcher_.get()->Start(
+
+  CancelActiveFetcher();
+  fetcher_ = new BannerBitmapFetcher(image_url, this);
+  fetcher_->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();
@@ skipping 30 matching lines @@
 void SetTimeDeltaForTesting(JNIEnv* env, jclass clazz, jint days) {
   gTimeDeltaForTesting = base::TimeDelta::FromDays(days);
 }
 
 // Register native methods
 bool RegisterAppBannerManager(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 }  // namespace banners