Fixes for Safe Browsing with unrelated pending navigations.

chrome/browser/safe_browsing/safe_browsing_blocking_page.cc

 // Copyright (c) 2012 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.
 //
 // Implementation of the SafeBrowsingBlockingPage class.
 
 #include "chrome/browser/safe_browsing/safe_browsing_blocking_page.h"
 
 #include <string>
 
@@ skipping 19 matching lines @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/locale_settings.h"
 #include "components/google/core/browser/google_util.h"
 #include "components/security_interstitials/core/controller_client.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/interstitial_page.h"
 #include "content/public/browser/navigation_controller.h"
+#include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/renderer_preferences.h"
 #include "grit/browser_resources.h"
 #include "net/base/escape.h"
 #include "ui/base/l10n/l10n_util.h"
 
 using base::UserMetricsAction;
 using content::BrowserThread;
 using content::InterstitialPage;
@@ skipping 52 matching lines @@
 SafeBrowsingBlockingPageFactory* SafeBrowsingBlockingPage::factory_ = NULL;
 
 // The default SafeBrowsingBlockingPageFactory.  Global, made a singleton so we
 // don't leak it.
 class SafeBrowsingBlockingPageFactoryImpl
     : public SafeBrowsingBlockingPageFactory {
  public:
   SafeBrowsingBlockingPage* CreateSafeBrowsingPage(
       SafeBrowsingUIManager* ui_manager,
       WebContents* web_contents,
-      const SafeBrowsingBlockingPage::UnsafeResourceList& unsafe_resources)
-      override {
+      const SafeBrowsingBlockingPage::UnsafeResourceList& unsafe_resources,
+      const GURL& main_frame_url) override {
     return new SafeBrowsingBlockingPage(ui_manager, web_contents,
-        unsafe_resources);
+                                        unsafe_resources, main_frame_url);
   }
 
  private:
   friend struct base::DefaultLazyInstanceTraits<
       SafeBrowsingBlockingPageFactoryImpl>;
 
   SafeBrowsingBlockingPageFactoryImpl() { }
 
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingBlockingPageFactoryImpl);
 };
 
 static base::LazyInstance<SafeBrowsingBlockingPageFactoryImpl>
     g_safe_browsing_blocking_page_factory_impl = LAZY_INSTANCE_INITIALIZER;
 
 // static
 content::InterstitialPageDelegate::TypeID
     SafeBrowsingBlockingPage::kTypeForTesting =
         &SafeBrowsingBlockingPage::kTypeForTesting;
 
 SafeBrowsingBlockingPage::SafeBrowsingBlockingPage(
     SafeBrowsingUIManager* ui_manager,
     WebContents* web_contents,
-    const UnsafeResourceList& unsafe_resources)
+    const UnsafeResourceList& unsafe_resources,
+    const GURL& main_frame_url)
     : SecurityInterstitialPage(web_contents, unsafe_resources[0].url),
       malware_details_proceed_delay_ms_(
           kMalwareDetailsProceedDelayMilliSeconds),
       ui_manager_(ui_manager),
       is_main_frame_load_blocked_(IsMainPageLoadBlocked(unsafe_resources)),
+      main_frame_url_(main_frame_url),

[Charlie Reis, 2015/12/11 05:39:24]

Any reason main_frame_url_ is listed before unsafe_resources_ in the members but
in the other order in the constructor parameters?

[mattm, 2015/12/15 01:42:25]

Not particularly. I wanted to keep main_frame_url_ next to
is_main_frame_load_blocked_ in the members, but just stuck it on the end of the
parameter list. Changed.

       unsafe_resources_(unsafe_resources),
       proceeded_(false) {
   bool malware = false;
   bool harmful = false;
   bool phishing = false;
   for (UnsafeResourceList::const_iterator iter = unsafe_resources_.begin();
        iter != unsafe_resources_.end(); ++iter) {
     const UnsafeResource& resource = *iter;
     SBThreatType threat_type = resource.threat_type;
     if (threat_type == SB_THREAT_TYPE_URL_MALWARE ||
@@ skipping 59 matching lines @@
     SBThreatType threat_type) {
   return threat_type == SB_THREAT_TYPE_URL_PHISHING ||
          threat_type == SB_THREAT_TYPE_URL_MALWARE ||
          threat_type == SB_THREAT_TYPE_URL_UNWANTED ||
          threat_type == SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL ||
          threat_type == SB_THREAT_TYPE_CLIENT_SIDE_MALWARE_URL;
 }
 
 bool SafeBrowsingBlockingPage::CanShowThreatDetailsOption() {
   return (!web_contents()->GetBrowserContext()->IsOffTheRecord() &&
-          web_contents()->GetURL().SchemeIs(url::kHttpScheme) &&
+          main_frame_url_.SchemeIs(url::kHttpScheme) &&
           IsPrefEnabled(prefs::kSafeBrowsingExtendedReportingOptInAllowed));
 }
 
 SafeBrowsingBlockingPage::~SafeBrowsingBlockingPage() {
 }
 
 void SafeBrowsingBlockingPage::CommandReceived(const std::string& page_cmd) {
   if (page_cmd == "\"pageLoadComplete\"") {
     // content::WaitForRenderFrameReady sends this message when the page
     // load completes. Ignore it.
@@ skipping 134 matching lines @@
 
   // Check to see if some new notifications of unsafe resources have been
   // received while we were showing the interstitial.
   UnsafeResourceMap* unsafe_resource_map = GetUnsafeResourcesMap();
   UnsafeResourceMap::iterator iter = unsafe_resource_map->find(web_contents());
   SafeBrowsingBlockingPage* blocking_page = NULL;
   if (iter != unsafe_resource_map->end() && !iter->second.empty()) {
     // Build an interstitial for all the unsafe resources notifications.
     // Don't show it now as showing an interstitial while an interstitial is
     // already showing would cause DontProceed() to be invoked.
-    blocking_page = factory_->CreateSafeBrowsingPage(ui_manager_,
-                                                     web_contents(),
-                                                     iter->second);
+    blocking_page = factory_->CreateSafeBrowsingPage(
+        ui_manager_, web_contents(), iter->second,
+        // All queued unsafe resources should be for the same page:
+        iter->second[0].GetNavigationEntryForResource()->GetURL());
     unsafe_resource_map->erase(iter);
   }
 
   // Now that this interstitial is gone, we can show the new one.
   if (blocking_page)
     blocking_page->Show();
 }
 
 content::InterstitialPageDelegate::TypeID
 SafeBrowsingBlockingPage::GetTypeForTesting() const {
@@ skipping 71 matching lines @@
 // static
 SafeBrowsingBlockingPage::UnsafeResourceMap*
     SafeBrowsingBlockingPage::GetUnsafeResourcesMap() {
   return g_unsafe_resource_map.Pointer();
 }
 
 // static
 SafeBrowsingBlockingPage* SafeBrowsingBlockingPage::CreateBlockingPage(
     SafeBrowsingUIManager* ui_manager,
     WebContents* web_contents,
-    const UnsafeResource& unsafe_resource) {
+    const UnsafeResource& unsafe_resource,
+    const GURL& main_frame_url) {
   std::vector<UnsafeResource> resources;
   resources.push_back(unsafe_resource);
   // Set up the factory if this has not been done already (tests do that
   // before this method is called).
   if (!factory_)
     factory_ = g_safe_browsing_blocking_page_factory_impl.Pointer();
-  return factory_->CreateSafeBrowsingPage(ui_manager, web_contents, resources);
+  return factory_->CreateSafeBrowsingPage(ui_manager, web_contents, resources,
+                                          main_frame_url);
 }
 
 // static
 void SafeBrowsingBlockingPage::ShowBlockingPage(
     SafeBrowsingUIManager* ui_manager,
     const UnsafeResource& unsafe_resource) {
   DVLOG(1) << __FUNCTION__ << " " << unsafe_resource.url.spec();
   WebContents* web_contents = tab_util::GetWebContentsByID(
       unsafe_resource.render_process_host_id, unsafe_resource.render_view_id);
 
   InterstitialPage* interstitial =
       InterstitialPage::GetInterstitialPage(web_contents);
   if (interstitial && !unsafe_resource.is_subresource) {
     // There is already an interstitial showing and we are about to display a
     // new one for the main frame. Just hide the current one, it is now
     // irrelevent
     interstitial->DontProceed();
     interstitial = NULL;
   }
 
   if (!interstitial) {
     // There are no interstitial currently showing in that tab, go ahead and
     // show this interstitial.
-    SafeBrowsingBlockingPage* blocking_page =
-        CreateBlockingPage(ui_manager, web_contents, unsafe_resource);
+    SafeBrowsingBlockingPage* blocking_page = CreateBlockingPage(
+        ui_manager, web_contents, unsafe_resource,
+        unsafe_resource.GetNavigationEntryForResource()->GetURL());
     blocking_page->Show();
     return;
   }
 
   // This is an interstitial for a page's resource, let's queue it.
   UnsafeResourceMap* unsafe_resource_map = GetUnsafeResourcesMap();
   (*unsafe_resource_map)[web_contents].push_back(unsafe_resource);
 }
 
 // static
@@ skipping 130 matching lines @@
           IDS_MALWARE_V3_PRIMARY_PARAGRAPH,
           GetFormattedHostName()));
   load_time_data->SetString(
       "explanationParagraph",
       is_main_frame_load_blocked_ ?
           l10n_util::GetStringFUTF16(
               IDS_MALWARE_V3_EXPLANATION_PARAGRAPH,
               GetFormattedHostName()) :
           l10n_util::GetStringFUTF16(
               IDS_MALWARE_V3_EXPLANATION_PARAGRAPH_SUBRESOURCE,
-              base::UTF8ToUTF16(web_contents()->GetURL().host()),
+              base::UTF8ToUTF16(main_frame_url_.host()),
               GetFormattedHostName()));
   load_time_data->SetString(
       "finalParagraph",
       l10n_util::GetStringUTF16(IDS_MALWARE_V3_PROCEED_PARAGRAPH));
 
   PopulateExtendedReportingOption(load_time_data);
 }
 
 void SafeBrowsingBlockingPage::PopulateHarmfulLoadTimeData(
     base::DictionaryValue* load_time_data) {
@@ skipping 46 matching lines @@
   } else {
     load_time_data->SetString(
         "finalParagraph",
         l10n_util::GetStringUTF16(IDS_PHISHING_V3_PROCEED_PARAGRAPH));
   }
 
   PopulateExtendedReportingOption(load_time_data);
 }
 
 }  // namespace safe_browsing