Componentize subresource SB CI logic

Android Webview Safe Browsing interstitials will require the same logic
desktop and android use for showing interstitials triggered by a bad
subresource. This CL moves that logic to //components.

Bug: 1016581
Change-Id: Ia1f5fd52cdff3c7bf7ea7d38b6329b8adf316718
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1884811
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Auto-Submit: Carlos IL <carlosil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710070}

chrome/browser/safe_browsing/ui_manager.cc

@@ -160,28 +160,6 @@ void SafeBrowsingUIManager::RemoveObserver(Observer* observer) {
   observer_list_.RemoveObserver(observer);
 }
 
-void SafeBrowsingUIManager::DisplayBlockingPage(
-    const UnsafeResource& resource) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  BaseUIManager::DisplayBlockingPage(resource);
-  if (!resource.IsMainPageLoadBlocked() && !IsWhitelisted(resource) &&
-      SafeBrowsingInterstitialsAreCommittedNavigations()) {
-    content::WebContents* contents = resource.web_contents_getter.Run();
-    content::NavigationEntry* entry = resource.GetNavigationEntryForResource();
-    // entry can be null if we are on a brand new tab, and a resource is added
-    // via javascript without a navigation.
-    GURL blocked_url = entry ? entry->GetURL() : resource.url;
-    SafeBrowsingBlockingPage* blocking_page =
-        SafeBrowsingBlockingPage::CreateBlockingPage(this, contents,
-                                                     blocked_url, resource);
-    SafeBrowsingSubresourceTabHelper::CreateForWebContents(contents);
-    contents->GetController().LoadPostCommitErrorPage(
-        contents->GetMainFrame(), blocked_url, blocking_page->GetHTMLContents(),
-        net::ERR_BLOCKED_BY_CLIENT);
-    delete blocking_page;
-  }
-}
-
 const std::string SafeBrowsingUIManager::app_locale() const {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   return g_browser_process->GetApplicationLocale();
@@ -237,4 +215,15 @@ GURL SafeBrowsingUIManager::GetMainFrameWhitelistUrlForResourceForTesting(
   return GetMainFrameWhitelistUrlForResource(resource);
 }
 
+BaseBlockingPage* SafeBrowsingUIManager::CreateBlockingPageForSubresource(
+    content::WebContents* contents,
+    const GURL& blocked_url,
+    const UnsafeResource& unsafe_resource) {
+  SafeBrowsingSubresourceTabHelper::CreateForWebContents(contents);
+  SafeBrowsingBlockingPage* blocking_page =
+      SafeBrowsingBlockingPage::CreateBlockingPage(this, contents, blocked_url,
+                                                   unsafe_resource);
+  return blocking_page;
+}
+
 }  // namespace safe_browsing

chrome/browser/safe_browsing/ui_manager.h

@@ -31,6 +31,8 @@ class HistoryService;
 
 namespace safe_browsing {
 
+class BaseBlockingPage;
+
 struct HitReport;
 
 // Construction needs to happen on the main thread.
@@ -89,8 +91,6 @@ class SafeBrowsingUIManager : public BaseUIManager {
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* remove);
 
-  void DisplayBlockingPage(const UnsafeResource& resource) override;
-
   const std::string app_locale() const override;
   history::HistoryService* history_service(
       content::WebContents* web_contents) override;
@@ -122,6 +122,13 @@ class SafeBrowsingUIManager : public BaseUIManager {
   static GURL GetMainFrameWhitelistUrlForResourceForTesting(
       const safe_browsing::SafeBrowsingUIManager::UnsafeResource& resource);
 
+  // Creates a blocking page, used for interstitials triggered by subresources.
+  // Override is using a different blocking page.
+  BaseBlockingPage* CreateBlockingPageForSubresource(
+      content::WebContents* contents,
+      const GURL& blocked_url,
+      const UnsafeResource& unsafe_resource) override;
+
   // Safebrowsing service.
   scoped_refptr<SafeBrowsingService> sb_service_;
 

components/safe_browsing/base_ui_manager.cc

@@ -251,6 +251,25 @@ void BaseUIManager::DisplayBlockingPage(
     // SafeBrowsingNavigationThrottle.
     resource.callback_thread->PostTask(
         FROM_HERE, base::BindOnce(resource.callback, false));
+    if (!resource.IsMainPageLoadBlocked() && !IsWhitelisted(resource)) {
+      // For subresource triggered interstitials, we trigger the error page
+      // navigation from here since there will be no navigation to intercept
+      // in the throttle.
+      content::WebContents* contents = resource.web_contents_getter.Run();
+      content::NavigationEntry* entry =
+          resource.GetNavigationEntryForResource();
+      // entry can be null if we are on a brand new tab, and a resource is added
+      // via javascript without a navigation.
+      GURL blocked_url = entry ? entry->GetURL() : resource.url;
+
+      BaseBlockingPage* blocking_page =
+          CreateBlockingPageForSubresource(contents, blocked_url, resource);
+
+      contents->GetController().LoadPostCommitErrorPage(
+          contents->GetMainFrame(), blocked_url,
+          blocking_page->GetHTMLContents(), net::ERR_BLOCKED_BY_CLIENT);
+      delete blocking_page;
+    }
     return;
   }
   ShowBlockingPageForResource(resource);
@@ -272,6 +291,18 @@ bool BaseUIManager::SafeBrowsingInterstitialsAreCommittedNavigations() {
   return false;
 }
 
+BaseBlockingPage* BaseUIManager::CreateBlockingPageForSubresource(
+    content::WebContents* contents,
+    const GURL& blocked_url,
+    const UnsafeResource& unsafe_resource) {
+  // TODO(carlosil): This can be removed once all implementations of SB use
+  // committed interstitials. In the meantime, there is no create method for the
+  // non-committed implementations, and this code won't be called if committed
+  // interstitials are disabled.
+  NOTREACHED();
+  return nullptr;
+}
+
 // A SafeBrowsing hit is sent after a blocking page for malware/phishing
 // or after the warning dialog for download urls, only for extended_reporting
 // users who are not in incognito mode.

components/safe_browsing/base_ui_manager.h

@@ -27,6 +27,8 @@ class HistoryService;
 
 namespace safe_browsing {
 
+class BaseBlockingPage;
+
 // Construction needs to happen on the main thread.
 class BaseUIManager
     : public base::RefCountedThreadSafe<BaseUIManager> {
@@ -149,6 +151,13 @@ class BaseUIManager
 
   friend class base::RefCountedThreadSafe<BaseUIManager>;
 
+  // Creates a blocking page, used for interstitials triggered by subresources.
+  // Should be overridden with a blocking page implementation.
+  virtual BaseBlockingPage* CreateBlockingPageForSubresource(
+      content::WebContents* contents,
+      const GURL& blocked_url,
+      const UnsafeResource& unsafe_resource);
+
   // Stores unsafe resources so they can be fetched from a navigation throttle
   // in the committed interstitials flow. Implemented as a pair vector since
   // most of the time it will be empty or contain a single element.