Generalize WebContentsObserver::SecurityStyleChanged.

This will put the onus on individual observers to extract the security
information they require, after a change has occurred.  The intent is to
allow chrome/browser-residing observers to monitor security level
changes, independent of style.

BUG=641508

Review-Url: https://codereview.chromium.org/2562503002
Cr-Commit-Position: refs/heads/master@{#437659}

chrome/browser/android/vr_shell/vr_web_contents_observer.cc

@@ -22,7 +22,7 @@ VrWebContentsObserver::VrWebContentsObserver(content::WebContents* web_contents,
       ui_interface_(ui_interface),
       vr_shell_(vr_shell) {
   ui_interface_->SetURL(web_contents->GetVisibleURL());
-  SetSecurityLevel();
+  DidChangeVisibleSecurityState();
 }
 
 VrWebContentsObserver::~VrWebContentsObserver() {}
@@ -60,16 +60,7 @@ void VrWebContentsObserver::DidFinishNavigation(
   }
 }
 
-// TODO(cjgrant): We care about security level, not style. Refactor
-// WebContentsObserver (or equivalent) to expose more general security
-// information, so we can stop abusing this callback.
-void VrWebContentsObserver::SecurityStyleChanged(
-    blink::WebSecurityStyle security_style,
-    const content::SecurityStyleExplanations& explanations) {
-  SetSecurityLevel();
-}
-
-void VrWebContentsObserver::SetSecurityLevel() {
+void VrWebContentsObserver::DidChangeVisibleSecurityState() {
   const auto* helper = SecurityStateTabHelper::FromWebContents(web_contents());
   DCHECK(helper);
   security_state::SecurityInfo security_info;

chrome/browser/android/vr_shell/vr_web_contents_observer.h

@@ -39,9 +39,7 @@ class CONTENT_EXPORT VrWebContentsObserver
       content::NavigationHandle* navigation_handle) override;
   void DidToggleFullscreenModeForTab(
       bool entered_fullscreen, bool will_cause_resize) override;
-  void SecurityStyleChanged(
-      blink::WebSecurityStyle security_style,
-      const content::SecurityStyleExplanations& explanations) override;
+  void DidChangeVisibleSecurityState() override;
   void WebContentsDestroyed() override;
   void WasHidden() override;
   void RenderViewHostChanged(content::RenderViewHost* old_host,

chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc

@@ -49,6 +49,7 @@
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/browser/security_style_explanations.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_browser_thread.h"
@@ -1123,10 +1124,10 @@ class SecurityStyleTestObserver : public content::WebContentsObserver {
   }
 
   // WebContentsObserver:
-  void SecurityStyleChanged(blink::WebSecurityStyle security_style,
-                            const content::SecurityStyleExplanations&
-                                security_style_explanations) override {
-    latest_security_style_ = security_style;
+  void DidChangeVisibleSecurityState() override {
+    content::SecurityStyleExplanations security_style_explanations;
+    latest_security_style_ = web_contents()->GetDelegate()->GetSecurityStyle(
+        web_contents(), &security_style_explanations);
   }
 
  private:

chrome/browser/ssl/security_state_tab_helper_browser_tests.cc

@@ -63,9 +63,9 @@ enum CertificateStatus { VALID_CERTIFICATE, INVALID_CERTIFICATE };
 const base::FilePath::CharType kDocRoot[] =
     FILE_PATH_LITERAL("chrome/test/data");
 
-// A WebContentsObserver useful for testing the SecurityStyleChanged()
-// method: it keeps track of the latest security style and explanation
-// that was fired.
+// A WebContentsObserver useful for testing the DidChangeVisibleSecurityState()
+// method: it keeps track of the latest security style and explanation that was
+// fired.
 class SecurityStyleTestObserver : public content::WebContentsObserver {
  public:
   explicit SecurityStyleTestObserver(content::WebContents* web_contents)
@@ -73,11 +73,11 @@ class SecurityStyleTestObserver : public content::WebContentsObserver {
         latest_security_style_(blink::WebSecurityStyleUnknown) {}
   ~SecurityStyleTestObserver() override {}
 
-  void SecurityStyleChanged(blink::WebSecurityStyle security_style,
-                            const content::SecurityStyleExplanations&
-                                security_style_explanations) override {
-    latest_security_style_ = security_style;
-    latest_explanations_ = security_style_explanations;
+  void DidChangeVisibleSecurityState() override {
+    content::SecurityStyleExplanations explanations;
+    latest_security_style_ = web_contents()->GetDelegate()->GetSecurityStyle(
+        web_contents(), &explanations);
+    latest_explanations_ = explanations;
   }
 
   blink::WebSecurityStyle latest_security_style() const {
@@ -343,9 +343,9 @@ class SecurityStateTabHelperTestWithPasswordCcSwitch
   DISALLOW_COPY_AND_ASSIGN(SecurityStateTabHelperTestWithPasswordCcSwitch);
 };
 
-class SecurityStyleChangedTest : public InProcessBrowserTest {
+class DidChangeVisibleSecurityStateTest : public InProcessBrowserTest {
  public:
-  SecurityStyleChangedTest()
+  DidChangeVisibleSecurityStateTest()
       : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {
     https_server_.ServeFilesFromSourceDirectory(base::FilePath(kDocRoot));
   }
@@ -359,7 +359,7 @@ class SecurityStyleChangedTest : public InProcessBrowserTest {
   net::EmbeddedTestServer https_server_;
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(SecurityStyleChangedTest);
+  DISALLOW_COPY_AND_ASSIGN(DidChangeVisibleSecurityStateTest);
 };
 
 IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest, HttpPage) {
@@ -1512,9 +1512,10 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest, AddedTab) {
                              false /* expect cert status error */);
 }
 
-// Tests that the WebContentsObserver::SecurityStyleChanged event fires
+// Tests that the WebContentsObserver::DidChangeVisibleSecurityState event fires
 // with the current style on HTTP, broken HTTPS, and valid HTTPS pages.
-IN_PROC_BROWSER_TEST_F(SecurityStyleChangedTest, SecurityStyleChangedObserver) {
+IN_PROC_BROWSER_TEST_F(DidChangeVisibleSecurityStateTest,
+                       DidChangeVisibleSecurityStateObserver) {
   ASSERT_TRUE(https_server_.Start());
   ASSERT_TRUE(embedded_test_server()->Start());
 
@@ -1650,14 +1651,14 @@ IN_PROC_BROWSER_TEST_F(SecurityStyleChangedTest, SecurityStyleChangedObserver) {
 // and test that the observed security style matches.
 #if defined(OS_CHROMEOS)
 // Flaky on Chrome OS. See https://crbug.com/638576.
-#define MAYBE_SecurityStyleChangedObserverGoBack \
-  DISABLED_SecurityStyleChangedObserverGoBack
+#define MAYBE_DidChangeVisibleSecurityStateObserverGoBack \
+  DISABLED_DidChangeVisibleSecurityStateObserverGoBack
 #else
-#define MAYBE_SecurityStyleChangedObserverGoBack \
-  SecurityStyleChangedObserverGoBack
+#define MAYBE_DidChangeVisibleSecurityStateObserverGoBack \
+  DidChangeVisibleSecurityStateObserverGoBack
 #endif
-IN_PROC_BROWSER_TEST_F(SecurityStyleChangedTest,
-                       MAYBE_SecurityStyleChangedObserverGoBack) {
+IN_PROC_BROWSER_TEST_F(DidChangeVisibleSecurityStateTest,
+                       MAYBE_DidChangeVisibleSecurityStateObserverGoBack) {
   ASSERT_TRUE(https_server_.Start());
 
   net::EmbeddedTestServer https_test_server_expired(
@@ -1856,7 +1857,7 @@ class BrowserTestNonsecureURLRequest : public InProcessBrowserTest {
 // Tests that a connection with obsolete TLS settings does not get a
 // secure connection explanation.
 IN_PROC_BROWSER_TEST_F(BrowserTestNonsecureURLRequest,
-                       SecurityStyleChangedObserverNonsecureConnection) {
+                       DidChangeVisibleSecurityStateObserverNonsecureConnection) {
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   SecurityStyleTestObserver observer(web_contents);

content/browser/devtools/protocol/security_handler.cc

@@ -75,13 +75,9 @@ void SecurityHandler::AttachToRenderFrameHost() {
   WebContents* web_contents = WebContents::FromRenderFrameHost(host_);
   WebContentsObserver::Observe(web_contents);
 
-  // Send an initial SecurityStyleChanged event.
+  // Send an initial DidChangeVisibleSecurityState event.
   DCHECK(enabled_);
-  SecurityStyleExplanations security_style_explanations;
-  blink::WebSecurityStyle security_style =
-      web_contents->GetDelegate()->GetSecurityStyle(
-          web_contents, &security_style_explanations);
-  SecurityStyleChanged(security_style, security_style_explanations);
+  DidChangeVisibleSecurityState();
 }
 
 void SecurityHandler::SetRenderFrameHost(RenderFrameHost* host) {
@@ -90,11 +86,14 @@ void SecurityHandler::SetRenderFrameHost(RenderFrameHost* host) {
     AttachToRenderFrameHost();
 }
 
-void SecurityHandler::SecurityStyleChanged(
-    blink::WebSecurityStyle security_style,
-    const SecurityStyleExplanations& security_style_explanations) {
+void SecurityHandler::DidChangeVisibleSecurityState() {
   DCHECK(enabled_);
 
+  SecurityStyleExplanations security_style_explanations;
+  blink::WebSecurityStyle security_style =
+      web_contents()->GetDelegate()->GetSecurityStyle(
+          web_contents(), &security_style_explanations);
+
   const std::string security_state =
       SecurityStyleToProtocolSecurityState(security_style);
 

content/browser/devtools/protocol/security_handler.h

@@ -32,9 +32,7 @@ class SecurityHandler : public WebContentsObserver {
   void AttachToRenderFrameHost();
 
   // WebContentsObserver overrides
-  void SecurityStyleChanged(
-      blink::WebSecurityStyle security_style,
-      const SecurityStyleExplanations& security_style_explanations) override;
+  void DidChangeVisibleSecurityState() override;
 
   std::unique_ptr<Client> client_;
   bool enabled_;

content/browser/web_contents/web_contents_impl.cc

@@ -1471,14 +1471,8 @@ void WebContentsImpl::AttachToOuterWebContentsFrame(
 void WebContentsImpl::DidChangeVisibleSecurityState() {
   if (delegate_) {
     delegate_->VisibleSecurityStateChanged(this);
-
-    SecurityStyleExplanations security_style_explanations;
-    blink::WebSecurityStyle security_style =
-        delegate_->GetSecurityStyle(this, &security_style_explanations);
-    for (auto& observer : observers_) {
-      observer.SecurityStyleChanged(security_style,
-                                    security_style_explanations);
-    }
+    for (auto& observer : observers_)
+      observer.DidChangeVisibleSecurityState();
   }
 }
 

content/public/browser/web_contents_observer.h

@@ -19,7 +19,6 @@
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_sender.h"
 #include "third_party/WebKit/public/platform/WebInputEvent.h"
-#include "third_party/WebKit/public/platform/WebSecurityStyle.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/base/page_transition_types.h"
 #include "ui/base/window_open_disposition.h"
@@ -41,7 +40,6 @@ struct LoadCommittedDetails;
 struct Referrer;
 struct ResourceRedirectDetails;
 struct ResourceRequestDetails;
-struct SecurityStyleExplanations;
 
 // An observer API implemented by classes which are interested in various page
 // load events from WebContents.  They also get a chance to filter IPC messages.
@@ -297,13 +295,8 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener,
                            const base::string16& error_description,
                            bool was_ignored_by_handler) {}
 
-  // This method is invoked when the SecurityStyle of the WebContents changes.
-  // |security_style| is the new SecurityStyle. |security_style_explanations|
-  // contains human-readable strings explaining why the SecurityStyle of the
-  // page has been downgraded.
-  virtual void SecurityStyleChanged(
-      blink::WebSecurityStyle security_style,
-      const SecurityStyleExplanations& security_style_explanations) {}
+  // This method is invoked when the visible security state of the page changes.
+  virtual void DidChangeVisibleSecurityState() {}
 
   // This method is invoked when content was loaded from an in-memory cache.
   virtual void DidLoadResourceFromMemoryCache(