Trigger Dangerous indicator for unsafe subresources

chrome/browser/safe_browsing/ui_manager_unittest.cc

Index: chrome/browser/safe_browsing/ui_manager_unittest.cc
diff --git a/chrome/browser/safe_browsing/ui_manager_unittest.cc b/chrome/browser/safe_browsing/ui_manager_unittest.cc
index fb6ba19cb7a71325e367fe388bd15aef168cd0bb..0807da78315376dfd74f8a4744fffb7607c12ec6 100644
--- a/chrome/browser/safe_browsing/ui_manager_unittest.cc
+++ b/chrome/browser/safe_browsing/ui_manager_unittest.cc
@@ -5,14 +5,17 @@
 #include "chrome/browser/safe_browsing/ui_manager.h"
 
 #include "base/run_loop.h"
+#include "chrome/browser/safe_browsing/safe_browsing_blocking_page.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/safe_browsing/ui_manager.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
+#include "components/safe_browsing_db/safe_browsing_prefs.h"
 #include "components/safe_browsing_db/util.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_delegate.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/web_contents_tester.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -113,6 +116,9 @@ class SafeBrowsingUIManagerTest : public ChromeRenderViewHostTestHarness {
     ui_manager_->OnBlockingPageDone(resources, proceed);
   }
 
+ protected:
+  SafeBrowsingUIManager* ui_manager() { return ui_manager_.get(); }
+
  private:
   scoped_refptr<SafeBrowsingUIManager> ui_manager_;
 };
@@ -271,4 +277,110 @@ TEST_F(SafeBrowsingUIManagerTest, IOCallbackDontProceed) {
   EXPECT_FALSE(waiter.proceed());
 }
 
+namespace {
+
+// A WebContentsDelegate that records whether
+// VisibleSecurityStateChanged() was called.
+class SecurityStateWebContentsDelegate : public content::WebContentsDelegate {
+ public:
+  SecurityStateWebContentsDelegate() {}
+  ~SecurityStateWebContentsDelegate() override {}
+
+  bool visible_security_state_changed() const {
+    return visible_security_state_changed_;
+  }
+
+  void ClearVisibleSecurityStateChanged() {
+    visible_security_state_changed_ = false;
+  }
+
+  // WebContentsDelegate:
+  void VisibleSecurityStateChanged(content::WebContents* source) override {
+    visible_security_state_changed_ = true;
+  }
+
+ private:
+  bool visible_security_state_changed_ = false;
+  DISALLOW_COPY_AND_ASSIGN(SecurityStateWebContentsDelegate);
+};
+
+// A test blocking page that does not create windows.
+class TestSafeBrowsingBlockingPage : public SafeBrowsingBlockingPage {
+ public:
+  TestSafeBrowsingBlockingPage(SafeBrowsingUIManager* manager,
+                               content::WebContents* web_contents,
+                               const GURL& main_frame_url,
+                               const UnsafeResourceList& unsafe_resources)
+      : SafeBrowsingBlockingPage(manager,
+                                 web_contents,
+                                 main_frame_url,
+                                 unsafe_resources) {
+    // Don't delay details at all for the unittest.
+    threat_details_proceed_delay_ms_ = 0;
+    DontCreateViewForTesting();
+  }
+};
+
+// A factory that creates TestSafeBrowsingBlockingPages.
+class TestSafeBrowsingBlockingPageFactory
+    : public SafeBrowsingBlockingPageFactory {
+ public:
+  TestSafeBrowsingBlockingPageFactory() {}
+  ~TestSafeBrowsingBlockingPageFactory() override {}
+
+  SafeBrowsingBlockingPage* CreateSafeBrowsingPage(
+      SafeBrowsingUIManager* delegate,
+      content::WebContents* web_contents,
+      const GURL& main_frame_url,
+      const SafeBrowsingBlockingPage::UnsafeResourceList& unsafe_resources)
+      override {
+    return new TestSafeBrowsingBlockingPage(delegate, web_contents,
+                                            main_frame_url, unsafe_resources);
+  }
+};
+
+}  // namespace
+
+// Tests that the WebContentsDelegate is notified of a visible security
+// state change when a blocking page is shown for a subresource.
+TEST_F(SafeBrowsingUIManagerTest,
+       VisibleSecurityStateChangedForUnsafeSubresource) {
+  TestSafeBrowsingBlockingPageFactory factory;
+  SafeBrowsingBlockingPage::RegisterFactory(&factory);
+  SecurityStateWebContentsDelegate delegate;
+  web_contents()->SetDelegate(&delegate);
+
+  // Simulate a blocking page showing for an unsafe subresource.
+  SafeBrowsingUIManager::UnsafeResource resource =
+      MakeUnsafeResource(kBadURL, true /* is_subresource */);

[Lei Zhang, 2016/10/28 23:03:46]

I think the simple cause of the MSAN error is resource.is_subframe not being
initialized. Given the simple error, I recommended we do (3) - revert the
revert, and just init is_subframe and call it a day.

+  // Needed for showing the blocking page.
+  resource.threat_source = safe_browsing::ThreatSource::REMOTE;
+  NavigateAndCommit(GURL("http://example.test"));
+
+  delegate.ClearVisibleSecurityStateChanged();
+  EXPECT_FALSE(delegate.visible_security_state_changed());
+  ui_manager()->DisplayBlockingPage(resource);
+  EXPECT_TRUE(delegate.visible_security_state_changed());
+
+  // Simulate proceeding through the blocking page.
+  SafeBrowsingCallbackWaiter waiter;
+  resource.callback =
+      base::Bind(&SafeBrowsingCallbackWaiter::OnBlockingPageDoneOnIO,
+                 base::Unretained(&waiter));
+  resource.callback_thread =
+      BrowserThread::GetTaskRunnerForThread(BrowserThread::IO);
+  std::vector<SafeBrowsingUIManager::UnsafeResource> resources;
+  resources.push_back(resource);
+
+  delegate.ClearVisibleSecurityStateChanged();
+  EXPECT_FALSE(delegate.visible_security_state_changed());
+  SimulateBlockingPageDone(resources, true);
+  EXPECT_TRUE(delegate.visible_security_state_changed());
+
+  waiter.WaitForCallback();
+  EXPECT_TRUE(waiter.callback_called());
+  EXPECT_TRUE(waiter.proceed());
+  EXPECT_TRUE(IsWhitelisted(resource));
+}
+
 }  // namespace safe_browsing