More implementation details of extension content verification

chrome/browser/extensions/extension_service.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 CHROME_BROWSER_EXTENSIONS_EXTENSION_SERVICE_H_
 #define CHROME_BROWSER_EXTENSIONS_EXTENSION_SERVICE_H_
 
 #include <list>
 #include <map>
 #include <set>
 #include <string>
 #include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/prefs/pref_change_registrar.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/extensions/blacklist.h"
 #include "chrome/browser/extensions/extension_sync_service.h"
 #include "chrome/browser/extensions/pending_extension_manager.h"
 #include "content/public/browser/devtools_agent_host.h"
 #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"
 #include "extensions/browser/management_policy.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/manifest.h"
 #include "extensions/common/one_shot_event.h"
 
@@ skipping 83 matching lines @@
   virtual base::SequencedTaskRunner* GetFileTaskRunner() = 0;
 };
 
 // Manages installed and running Chromium extensions. An instance is shared
 // between normal and incognito profiles.
 class ExtensionService
     : public ExtensionServiceInterface,
       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.
   static bool UninstallExtensionHelper(ExtensionService* extensions_service,
                                        const std::string& extension_id);
 
   // Constructor stores pointers to |profile| and |extension_prefs| but
   // ownership remains at caller.
   ExtensionService(Profile* profile,
                    const base::CommandLine* command_line,
@@ skipping 324 matching lines @@
     install_updates_when_idle_ = value;
   }
 
   // Set a callback to be called when all external providers are ready and their
   // extensions have been installed.
   void set_external_updates_finished_callback_for_test(
       const base::Closure& callback) {
     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);
   void RemoveUpdateObserver(extensions::UpdateObserver* observer);
 
  private:
   // Populates greylist_.
   void LoadGreylistFromPrefs();
 
   // Signals *ready_ and sends a notification to the listeners.
@@ skipping 224 matching lines @@
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
                            GreylistedExtensionDisabled);
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
                            GreylistDontEnableManuallyDisabled);
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
                            GreylistUnknownDontChange);
   DISALLOW_COPY_AND_ASSIGN(ExtensionService);
 };
 
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_SERVICE_H_