Generalize WebContentsObserver::SecurityStyleChanged

content/public/browser/web_contents_observer.h

Index: content/public/browser/web_contents_observer.h
diff --git a/content/public/browser/web_contents_observer.h b/content/public/browser/web_contents_observer.h
index 5db620927adf80a521d2bf4ab737bfda6e779248..0031b3445baad2ab25d2c16fa137cef019a21046 100644
--- a/content/public/browser/web_contents_observer.h
+++ b/content/public/browser/web_contents_observer.h
@@ -307,6 +307,9 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener,
       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,