Generalize WebContentsObserver::SecurityStyleChanged

content/public/browser/web_contents_observer.h

 // 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.
 
 #ifndef CONTENT_PUBLIC_BROWSER_WEB_CONTENTS_OBSERVER_H_
 #define CONTENT_PUBLIC_BROWSER_WEB_CONTENTS_OBSERVER_H_
 
 #include <stdint.h>
 
 #include "base/macros.h"
@@ skipping 289 matching lines @@
                            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() {}

[boliu, 2016/12/07 18:58:03]

I don't understand why this is needed when SecurityStyleChanged already exists

[cjgrant, 2016/12/07 19:15:53]

See my initial comments on the CL.  "Security style" looks to be a digest of the
more detailed security state.  If the callback invocation was "fixed" to
actually only be called when style changes, we'd potentially miss events.  I
think that's why Emily suggested reworking the callback for clarity.

And while reworking the single callback is one option, it's also an option to
have a second callback - one tailored to observers of security style, and
another for Chrome-internal observers who know how to pull more detail out of
the delegate.

[boliu, 2016/12/07 19:53:19]

I prefer the just one callback. Having two very close but subtly different
callbacks just invites bugs. At least calling things more frequently could only
lead to wasting work, but not incorrect code.

But if estark@ prefers two, then that's fine too. Definitely write a long
comment explaining the difference between these two though.

+
   // This method is invoked when content was loaded from an in-memory cache.
   virtual void DidLoadResourceFromMemoryCache(
       const GURL& url,
       const std::string& mime_type,
       ResourceType resource_type) {}
 
   // This method is invoked when a response has been received for a resource
   // request.
   virtual void DidGetResourceResponseStart(
       const ResourceRequestDetails& details) {}
@@ skipping 201 matching lines @@
   void ResetWebContents();
 
   WebContentsImpl* web_contents_;
 
   DISALLOW_COPY_AND_ASSIGN(WebContentsObserver);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_BROWSER_WEB_CONTENTS_OBSERVER_H_