More implementation details of extension content verification

chrome/browser/extensions/extension_service.h

Index: chrome/browser/extensions/extension_service.h
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index ca80400f6adf930451067bf2b8471645f19fe134..83cfa88dd625159d37f4615cc5788f75f556991f 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -25,6 +25,7 @@
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "extensions/browser/content_verifier.h"
+#include "extensions/browser/content_verifier_delegate.h"
 #include "extensions/browser/extension_function_histogram_value.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/external_provider_interface.h"
@@ -128,7 +129,7 @@ class ExtensionService
       public extensions::ExternalProviderInterface::VisitorInterface,
       public content::NotificationObserver,
       public extensions::Blacklist::Observer,
-      public extensions::ContentVerifierObserver {
+      public extensions::ContentVerifierDelegate {

[Ken Rockot(use gerrit already), 2014/05/09 23:51:16]

If I could have one wish, just one wish, it would be that nobody ever added any
more responsibility to ExtensionService.

Any chance this delegation could be done by a new object? Like some kind of
ContentVerificationDelegateThatDisablesExtensionsWhichFailVerification that
ExtensionSystemImpl owns? Maybe with a slightly shorter name than that.

[asargent_no_longer_on_chrome, 2014/05/10 00:12:43]

=)

Funny story: I originally didn't have ExtensionService involved with this code
at all because I was afraid of the dumping ground effect, and *yoz* suggested I
move it here in this comment on my earlier CL:

https://codereview.chromium.org/266963003/diff/40001/chrome/browser/extension...


Anyhow, when it was just that one function it seemed ok, but now that the
interface has grown I think you're right. I'll try moving this into an object
inside ExtensionSystemImpl.

  public:
   // Attempts to uninstall an extension from a given ExtensionService. Returns
   // true iff the target extension exists.
@@ -473,8 +474,13 @@ class ExtensionService
     external_updates_finished_callback_ = callback;
   }
 
-  // ContentVerifierObserver implementation.
-  virtual void ContentVerifyFailed(const std::string& extension_id) OVERRIDE;
+  // ContentVerifierDelegate implementation.
+  virtual bool ShouldBeVerified(
+      const extensions::Extension& extension) OVERRIDE;
+  virtual const extensions::ContentVerifierKey& PublicKey() OVERRIDE;
+  virtual GURL GetSignatureFetchUrl(const std::string& extension_id,
+                                    const base::Version& version) OVERRIDE;
+  virtual void VerifyFailed(const std::string& extension_id) OVERRIDE;
 
   // Adds/Removes update observers.
   void AddUpdateObserver(extensions::UpdateObserver* observer);