Fixes for Safe Browsing with unrelated pending navigations.

chrome/browser/safe_browsing/ui_manager.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.
 
 #include "chrome/browser/safe_browsing/ui_manager.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/debug/leak_tracker.h"
@@ skipping 70 matching lines @@
   // Client-side phishing detection interstitials never block the main frame
   // load, since they happen after the page is finished loading.
   if (threat_type == SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL ||
       threat_type == SB_THREAT_TYPE_CLIENT_SIDE_MALWARE_URL) {
     return false;
   }
 
   return true;
 }
 
+content::NavigationEntry*
+SafeBrowsingUIManager::UnsafeResource::GetNavigationEntryForResource() const {
+  WebContents* contents =
+      tab_util::GetWebContentsByID(render_process_host_id, render_view_id);
+  if (!contents)
+    return NULL;

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

nit: nullptr

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

Done.

+  if (IsMainPageLoadBlocked())
+    return contents->GetController().GetPendingEntry();
+  return contents->GetController().GetLastCommittedEntry();

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

This should have a comment explaining why we return the pending entry for the
main frame and the last committed for the subframe.

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

Done.

+}
+
 // SafeBrowsingUIManager -------------------------------------------------------
 
 SafeBrowsingUIManager::SafeBrowsingUIManager(
     const scoped_refptr<SafeBrowsingService>& service)
     : sb_service_(service) {}
 
 SafeBrowsingUIManager::~SafeBrowsingUIManager() {}
 
 void SafeBrowsingUIManager::StopOnIOThread(bool shutdown) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ skipping 66 matching lines @@
       DCHECK(resource.callback_thread);
       resource.callback_thread->PostTask(FROM_HERE,
                                          base::Bind(resource.callback, true));
     }
     return;
   }
 
   if (resource.threat_type != SB_THREAT_TYPE_SAFE) {
     HitReport hit_report;
     hit_report.malicious_url = resource.url;
-    hit_report.page_url = web_contents->GetURL();
     hit_report.is_subresource = resource.is_subresource;
     hit_report.threat_type = resource.threat_type;
     hit_report.threat_source = resource.threat_source;
 
-    NavigationEntry* entry = web_contents->GetController().GetActiveEntry();
-    if (entry)
-      hit_report.referrer_url = entry->GetReferrer().url;
+    NavigationEntry* entry = resource.GetNavigationEntryForResource();
+    hit_report.page_url = entry->GetURL();
+    hit_report.referrer_url = entry->GetReferrer().url;
 
     // When the malicious url is on the main frame, and resource.original_url
     // is not the same as the resource.url, that means we have a redirect from
     // resource.original_url to resource.url.
     // Also, at this point, page_url points to the _previous_ page that we
     // were on. We replace page_url with resource.original_url and referrer
     // with page_url.
     if (!resource.is_subresource &&
         !resource.original_url.is_empty() &&
         resource.original_url != resource.url) {
@@ skipping 109 matching lines @@
   WebContents* web_contents = tab_util::GetWebContentsByID(
       resource.render_process_host_id, resource.render_view_id);
 
   WhitelistUrlSet* site_list =
       static_cast<WhitelistUrlSet*>(web_contents->GetUserData(kWhitelistKey));
   if (!site_list) {
     site_list = new WhitelistUrlSet;
     web_contents->SetUserData(kWhitelistKey, site_list);
   }
 
-  GURL whitelisted_url(resource.is_subresource ? web_contents->GetVisibleURL()
+  NavigationEntry* entry = resource.GetNavigationEntryForResource();
+  GURL whitelisted_url(resource.is_subresource ? entry->GetURL()
                                                : resource.url);
   site_list->Insert(whitelisted_url);
 }
 
 // Check if the user has already ignored a SB warning for this WebContents and
 // top-level domain.
 bool SafeBrowsingUIManager::IsWhitelisted(const UnsafeResource& resource) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   WebContents* web_contents = tab_util::GetWebContentsByID(
       resource.render_process_host_id, resource.render_view_id);
 
-  GURL maybe_whitelisted_url(
-      resource.is_subresource ? web_contents->GetVisibleURL() : resource.url);
+  NavigationEntry* entry = resource.GetNavigationEntryForResource();
+  GURL maybe_whitelisted_url(resource.is_subresource ? entry->GetURL()
+                                                     : resource.url);
   WhitelistUrlSet* site_list =
       static_cast<WhitelistUrlSet*>(web_contents->GetUserData(kWhitelistKey));
   if (!site_list)
     return false;
   return site_list->Contains(maybe_whitelisted_url);
 }
 
 }  // namespace safe_browsing