Don't post tasks from RulesRegistry c-tor

chrome/browser/extensions/api/declarative/rules_registry.cc

Index: chrome/browser/extensions/api/declarative/rules_registry.cc
diff --git a/chrome/browser/extensions/api/declarative/rules_registry.cc b/chrome/browser/extensions/api/declarative/rules_registry.cc
index 306266e3e219e97f5f7759bd1ff264cd12f51161..ca4a440a32b682691ac6deffd95a926142c888f0 100644
--- a/chrome/browser/extensions/api/declarative/rules_registry.cc
+++ b/chrome/browser/extensions/api/declarative/rules_registry.cc
@@ -86,14 +86,13 @@ RulesRegistry::RulesRegistry(
       weak_ptr_factory_(profile ? this : NULL),
       last_generated_rule_identifier_id_(0) {
   if (cache_delegate) {
+    ready_.reset(new OneShotEvent);
     cache_delegate_ = cache_delegate->GetWeakPtr();
     cache_delegate->Init(this);
-  } else {
-    content::BrowserThread::PostTask(
-        owner_thread,
-        FROM_HERE,
-        base::Bind(&RulesRegistry::MarkReady, this, base::Time::Now()));

[Jeffrey Yasskin, 2013/12/17 22:55:05]

Would this CL be simpler if you just called ready_.Signal() right here, instead
of Posting a call to MarkReady()?
I'm not particularly happy about needing to check ready_ for NULL instead of
just posting tasks to it. Removing the double-check is why OneShotEvent accepts
Post()s even after it's signaled.

[vabr (Chromium), 2013/12/18 12:01:51]

I'm afraid calling ready_.Signal() here would lead to a DCHECK failing
CalledOnValidThread as soon as ready_.Post() is used. This is because the
constructor runs on UI thread, but the rest runs on owner_thread (IO for
WebRequestRulesRegistry).

Currently, OneShotEvent detaches from the thread it's constructed on just to
allow constructing it on another thread than it's used to.

What I can do is:
1) Allow to construct it as already signalled, or
2) provide a OneShotEvent method to detach on demand.

Does anything from the above sound good to you? Any other suggestions?

   }
+  // Note that if |cache_delegate| is NULL, the Rulesregistry never signals its

[Jeffrey Yasskin, 2013/12/17 22:55:05]

capitalization: RulesRegistry

[vabr (Chromium), 2013/12/18 12:01:51]

Done.

+  // |ready_| event. That's OK, because that event is only ever needed by the
+  // cache delegate.
 }
 
 std::string RulesRegistry::AddRulesNoFill(
@@ -272,7 +271,7 @@ void RulesRegistry::MarkReady(base::Time storage_init_time) {
                         base::Time::Now() - storage_init_time);
   }
 
-  ready_.Signal();
+  ready_->Signal();
 }
 
 void RulesRegistry::ProcessChangedRules(const std::string& extension_id) {
@@ -296,18 +295,19 @@ void RulesRegistry::MaybeProcessChangedRules(const std::string& extension_id) {
   // Read and initialize |process_changed_rules_requested_[extension_id]| if
   // necessary. (Note that the insertion below will not overwrite
   // |process_changed_rules_requested_[extension_id]| if that already exists.
+  bool processing_needed = profile_ && ready_;
   std::pair<ProcessStateMap::iterator, bool> insertion =
       process_changed_rules_requested_.insert(std::make_pair(
           extension_id,
-          profile_ ? NOT_SCHEDULED_FOR_PROCESSING : NEVER_PROCESS));
+          processing_needed ? NOT_SCHEDULED_FOR_PROCESSING : NEVER_PROCESS));
   if (insertion.first->second != NOT_SCHEDULED_FOR_PROCESSING)
     return;
 
   process_changed_rules_requested_[extension_id] = SCHEDULED_FOR_PROCESSING;
-  ready_.Post(FROM_HERE,
-              base::Bind(&RulesRegistry::ProcessChangedRules,
-                         weak_ptr_factory_.GetWeakPtr(),
-                         extension_id));
+  ready_->Post(FROM_HERE,
+               base::Bind(&RulesRegistry::ProcessChangedRules,
+                          weak_ptr_factory_.GetWeakPtr(),
+                          extension_id));
 }
 
 bool RulesRegistry::IsUniqueId(const std::string& extension_id,