Add a non-blocking "OneShotEvent" class

chrome/browser/extensions/extension_system.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_SYSTEM_H_
 #define CHROME_BROWSER_EXTENSIONS_EXTENSION_SYSTEM_H_
 
 #include <string>
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/extensions/api/api_resource_manager.h"
 #include "chrome/browser/extensions/api/serial/serial_connection.h"
 #include "chrome/browser/extensions/api/socket/socket.h"
 #include "chrome/browser/extensions/api/usb/usb_device_resource.h"
 #include "chrome/browser/profiles/profile_keyed_service.h"
 #include "chrome/common/extensions/extension_constants.h"
+#include "extensions/common/one_shot_event.h"
 
 class ExtensionInfoMap;
 class ExtensionProcessManager;
 class ExtensionService;
 class Profile;
 
 namespace extensions {
 // Unfortunately, for the ApiResourceManager<> template classes, we don't seem
 // to be able to forward-declare because of compilation errors on Windows.
 class AlarmManager;
@@ skipping 105 matching lines @@
   // extensions before anything else needs them to know.
   virtual void RegisterExtensionWithRequestContexts(
       const Extension* extension) {}
 
   // Called by the ExtensionService that lives in this system. Lets the
   // info map clean up its RequestContexts once all the listeners to the
   // EXTENSION_UNLOADED notification have finished running.
   virtual void UnregisterExtensionWithRequestContexts(
       const std::string& extension_id,
       const extension_misc::UnloadedExtensionReason reason) {}
+
+  // Whether the extension system has completed its startup tasks.

[not at google - send to devlin, 2013/05/16 15:51:08]

The comment (and in a way, method name) implies that this method returns a bool
when it doesn't. It's not clear from this file how OneShotEvent state -->
readiness.

[Jeffrey Yasskin, 2013/05/16 19:05:36]

Good point; re-commented.

+  virtual const OneShotEvent& ready() const = 0;
+
+ protected:
+  friend class ::ExtensionService;
+  // Called by the ExtensionService when it finishes loading extensions on
+  // startup.
+  virtual void ExtensionServiceReady() = 0;

[not at google - send to devlin, 2013/05/16 15:51:08]

I can see why ExtensionService owns this event, but calling back into the
ExtensionSystem from ExtensionService for this seems like a violation of
demeter. Can ExtensionSystem pass the event into ExtensionService instead?

[Jeffrey Yasskin, 2013/05/16 19:05:36]

I agree that it's icky, and I tried your suggestion, but it looks like it would
cause fragile tests. Several tests call ExtensionService::Init(), which I'd have
to change to ExtensionService::Init(OneShotEvent*). However, then the tests
would be passing in an Event that doesn't affect ExtensionService::is_ready() or
ExtensionSystem::ready(). I could store the Event* inside the ExtensionService
to make is_ready() consistent, but then we'd still have the state Matt was
worried about
(https://codereview.chromium.org/14757022/diff/19001/chrome/browser/extensions...)
where there are two different ready-states for different parts of the extensions
system.

[not at google - send to devlin, 2013/05/16 21:38:17]

A constructor parameter to ExtensionService won't work?

[Jeffrey Yasskin, 2013/05/16 22:19:21]

That seems to work and be cleaner that this callback. Thanks!

 };
 
 // The ExtensionSystem for ProfileImpl and OffTheRecordProfileImpl.
 // Implementation details: non-shared services are owned by
 // ExtensionSystemImpl, a ProfileKeyedService with separate incognito
 // instances. A private Shared class (also a ProfileKeyedService,
 // but with a shared instance for incognito) keeps the common services.
 class ExtensionSystemImpl : public ExtensionSystem {
  public:
   explicit ExtensionSystemImpl(Profile* profile);
@@ skipping 29 matching lines @@
   virtual ExtensionWarningService* warning_service() OVERRIDE;
   virtual Blacklist* blacklist() OVERRIDE;  // shared
 
   virtual void RegisterExtensionWithRequestContexts(
       const Extension* extension) OVERRIDE;
 
   virtual void UnregisterExtensionWithRequestContexts(
       const std::string& extension_id,
       const extension_misc::UnloadedExtensionReason reason) OVERRIDE;
 
+  virtual const OneShotEvent& ready() const OVERRIDE;
+
  private:
   friend class ExtensionSystemSharedFactory;
 
   // Owns the Extension-related systems that have a single instance
   // shared between normal and incognito profiles.
   class Shared : public ProfileKeyedService {
    public:
     explicit Shared(Profile* profile);
     virtual ~Shared();
 
@@ skipping 11 matching lines @@
     ExtensionPrefs* extension_prefs();
     ShellWindowGeometryCache* shell_window_geometry_cache();
     ExtensionService* extension_service();
     ManagementPolicy* management_policy();
     UserScriptMaster* user_script_master();
     Blacklist* blacklist();
     ExtensionInfoMap* info_map();
     LazyBackgroundTaskQueue* lazy_background_task_queue();
     EventRouter* event_router();
     ExtensionWarningService* warning_service();
+    OneShotEvent* ready() { return &ready_; }
 
    private:
     Profile* profile_;
 
     // The services that are shared between normal and incognito profiles.
 
     scoped_ptr<StateStore> state_store_;
     scoped_ptr<StateStore> rules_store_;
     scoped_ptr<ExtensionPrefs> extension_prefs_;
     // ShellWindowGeometryCache depends on ExtensionPrefs.
     scoped_ptr<ShellWindowGeometryCache> shell_window_geometry_cache_;
     // LazyBackgroundTaskQueue is a dependency of
     // MessageService and EventRouter.
     scoped_ptr<LazyBackgroundTaskQueue> lazy_background_task_queue_;
     scoped_ptr<EventRouter> event_router_;
     scoped_ptr<NavigationObserver> navigation_observer_;
     scoped_refptr<UserScriptMaster> user_script_master_;
     // Blacklist depends on ExtensionPrefs.
     scoped_ptr<Blacklist> blacklist_;
     // StandardManagementPolicyProvider depends on ExtensionPrefs and Blacklist.
     scoped_ptr<StandardManagementPolicyProvider>
         standard_management_policy_provider_;
     // ExtensionService depends on ExtensionPrefs, StateStore, and Blacklist.
     scoped_ptr<ExtensionService> extension_service_;
     scoped_ptr<ManagementPolicy> management_policy_;
     // extension_info_map_ needs to outlive extension_process_manager_.
     scoped_refptr<ExtensionInfoMap> extension_info_map_;
     scoped_ptr<ExtensionWarningService> extension_warning_service_;
     scoped_ptr<ExtensionWarningBadgeService> extension_warning_badge_service_;
+
+    OneShotEvent ready_;
   };
 
+  virtual void ExtensionServiceReady() OVERRIDE;
+
   Profile* profile_;
 
   Shared* shared_;
 
   // |extension_process_manager_| must be destroyed before the Profile's
   // |io_data_|. While |extension_process_manager_| still lives, we handle
   // incoming resource requests from extension processes and those require
   // access to the ResourceContext owned by |io_data_|.
   scoped_ptr<ExtensionProcessManager> extension_process_manager_;
   scoped_ptr<LocationManager> location_manager_;
   scoped_ptr<ApiResourceManager<SerialConnection> > serial_connection_manager_;
   scoped_ptr<ApiResourceManager<Socket> > socket_manager_;
   scoped_ptr<ApiResourceManager<
                UsbDeviceResource> > usb_device_resource_manager_;
   scoped_ptr<RulesRegistryService> rules_registry_service_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionSystemImpl);
 };
 
 }  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_SYSTEM_H_