Add a non-blocking "OneShotEvent" class

chrome/browser/extensions/extension_system.h

Index: chrome/browser/extensions/extension_system.h
diff --git a/chrome/browser/extensions/extension_system.h b/chrome/browser/extensions/extension_system.h
index c2bdc609f84d2d17dc27648ae4ca335c980aac9a..d84376c40ec217a65a07e7bc299c33586fb37cd5 100644
--- a/chrome/browser/extensions/extension_system.h
+++ b/chrome/browser/extensions/extension_system.h
@@ -15,6 +15,7 @@
 #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;
@@ -140,6 +141,15 @@ class ExtensionSystem : public ProfileKeyedService {
   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.
@@ -189,6 +199,8 @@ class ExtensionSystemImpl : public ExtensionSystem {
       const std::string& extension_id,
       const extension_misc::UnloadedExtensionReason reason) OVERRIDE;
 
+  virtual const OneShotEvent& ready() const OVERRIDE;
+
  private:
   friend class ExtensionSystemSharedFactory;
 
@@ -220,6 +232,7 @@ class ExtensionSystemImpl : public ExtensionSystem {
     LazyBackgroundTaskQueue* lazy_background_task_queue();
     EventRouter* event_router();
     ExtensionWarningService* warning_service();
+    OneShotEvent* ready() { return &ready_; }
 
    private:
     Profile* profile_;
@@ -249,8 +262,12 @@ class ExtensionSystemImpl : public ExtensionSystem {
     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_;