Make RulesRegistry::process_changed_rules_requested_ a map

process_changed_rules_requested_ stores the state of progress on writing a set of rules to the rules store.
For each extension there are separate write tasks. Currently, process_changed_rules_requested_ is share among
all such writes, which is wrong.

This CL makes process_changed_rules_requested_ a map from extension IDs to the state flags.

BUG=320645

Review URL: https://codereview.chromium.org/79433002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@237555 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 43 insertions, 10 deletions

diff --git a/chrome/browser/extensions/api/declarative/rules_registry.cc b/chrome/browser/extensions/api/declarative/rules_registry.cc
index e01b6f0..c9db12e 100644
--- a/chrome/browser/extensions/api/declarative/rules_registry.cc
+++ b/chrome/browser/extensions/api/declarative/rules_registry.cc
@@ -4,10 +4,13 @@
 
 #include "chrome/browser/extensions/api/declarative/rules_registry.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
+#include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
 #include "base/values.h"
@@ -81,8 +84,6 @@ RulesRegistry::RulesRegistry(
       event_name_(event_name),
       webview_key_(webview_key),
       weak_ptr_factory_(profile ? this : NULL),
-      process_changed_rules_requested_(profile ? NOT_SCHEDULED_FOR_PROCESSING
-                                               : NEVER_PROCESS),
       last_generated_rule_identifier_id_(0) {
   if (cache_delegate) {
     cache_delegate_ = cache_delegate->GetWeakPtr();
@@ -271,7 +272,8 @@ void RulesRegistry::MarkReady(base::Time storage_init_time) {
 void RulesRegistry::ProcessChangedRules(const std::string& extension_id) {
   DCHECK(content::BrowserThread::CurrentlyOn(owner_thread()));
 
-  process_changed_rules_requested_ = NOT_SCHEDULED_FOR_PROCESSING;
+  DCHECK(ContainsKey(process_changed_rules_requested_, extension_id));
+  process_changed_rules_requested_[extension_id] = NOT_SCHEDULED_FOR_PROCESSING;
 
   std::vector<linked_ptr<Rule> > new_rules;
   GetAllRules(extension_id, &new_rules);
@@ -285,10 +287,17 @@ void RulesRegistry::ProcessChangedRules(const std::string& extension_id) {
 }
 
 void RulesRegistry::MaybeProcessChangedRules(const std::string& extension_id) {
-  if (process_changed_rules_requested_ != NOT_SCHEDULED_FOR_PROCESSING)
+  // 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.
+  std::pair<ProcessStateMap::iterator, bool> insertion =
+      process_changed_rules_requested_.insert(std::make_pair(
+          extension_id,
+          profile_ ? NOT_SCHEDULED_FOR_PROCESSING : NEVER_PROCESS));
+  if (insertion.first->second != NOT_SCHEDULED_FOR_PROCESSING)
     return;
 
-  process_changed_rules_requested_ = SCHEDULED_FOR_PROCESSING;
+  process_changed_rules_requested_[extension_id] = SCHEDULED_FOR_PROCESSING;
   ready_.Post(FROM_HERE,
               base::Bind(&RulesRegistry::ProcessChangedRules,
                          weak_ptr_factory_.GetWeakPtr(),
diff --git a/chrome/browser/extensions/api/declarative/rules_registry.h b/chrome/browser/extensions/api/declarative/rules_registry.h
index 59000b2..7871c50 100644
--- a/chrome/browser/extensions/api/declarative/rules_registry.h
+++ b/chrome/browser/extensions/api/declarative/rules_registry.h
@@ -195,6 +195,7 @@ class RulesRegistry : public base::RefCountedThreadSafe<RulesRegistry> {
     // to schedule one.
     NOT_SCHEDULED_FOR_PROCESSING
   };
+  typedef std::map<ExtensionId, ProcessChangedRulesState> ProcessStateMap;
 
   base::WeakPtr<RulesRegistry> GetWeakPtr() {
     DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
@@ -204,7 +205,8 @@ class RulesRegistry : public base::RefCountedThreadSafe<RulesRegistry> {
   // Common processing after extension's rules have changed.
   void ProcessChangedRules(const std::string& extension_id);
 
-  // Calls ProcessChangedRules if |process_changed_rules_requested_| ==
+  // Calls ProcessChangedRules if
+  // |process_changed_rules_requested_(extension_id)| ==
   // NOT_SCHEDULED_FOR_PROCESSING.
   void MaybeProcessChangedRules(const std::string& extension_id);
 
@@ -246,7 +248,7 @@ class RulesRegistry : public base::RefCountedThreadSafe<RulesRegistry> {
   // instance.
   base::WeakPtr<RulesCacheDelegate> cache_delegate_;
 
-  ProcessChangedRulesState process_changed_rules_requested_;
+  ProcessStateMap process_changed_rules_requested_;
 
   // Returns whether any existing rule is registered with identifier |rule_id|
   // for extension |extension_id|.
diff --git a/chrome/browser/extensions/api/declarative/rules_registry_with_cache_unittest.cc b/chrome/browser/extensions/api/declarative/rules_registry_with_cache_unittest.cc
index f0c392c..53a2604 100644
--- a/chrome/browser/extensions/api/declarative/rules_registry_with_cache_unittest.cc
+++ b/chrome/browser/extensions/api/declarative/rules_registry_with_cache_unittest.cc
@@ -10,6 +10,7 @@
 
 #include "base/command_line.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "chrome/browser/extensions/api/declarative/rules_cache_delegate.h"
 #include "chrome/browser/extensions/api/declarative/test_rules_registry.h"
 #include "chrome/browser/extensions/extension_prefs.h"
@@ -45,8 +46,11 @@ class RulesRegistryWithCacheTest : public testing::Test {
   RulesRegistryWithCacheTest()
       : ui_thread_(content::BrowserThread::UI, &message_loop_),
         file_thread_(content::BrowserThread::FILE, &message_loop_),
-        registry_(new TestRulesRegistry(content::BrowserThread::UI,
+        cache_delegate_(new RulesCacheDelegate(false)),
+        registry_(new TestRulesRegistry(&profile_,
                                         "" /*event_name*/,
+                                        content::BrowserThread::UI,
+                                        cache_delegate_.get(),
                                         RulesRegistry::WebViewKey(0, 0))) {}
 
   virtual ~RulesRegistryWithCacheTest() {}
@@ -93,6 +97,8 @@ class RulesRegistryWithCacheTest : public testing::Test {
   base::MessageLoop message_loop_;
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread file_thread_;
+  TestingProfile profile_;
+  scoped_ptr<RulesCacheDelegate> cache_delegate_;
   scoped_refptr<TestRulesRegistry> registry_;
 #if defined OS_CHROMEOS
   chromeos::ScopedTestDeviceSettingsService test_device_settings_service_;
@@ -229,7 +235,6 @@ TEST_F(RulesRegistryWithCacheTest, DeclarativeRulesStored) {
   TestingValueStore* store = system->value_store();
 
   const std::string event_name("testEvent");
-  const RulesRegistry::WebViewKey key(0, 0);
   const std::string rules_stored_key(
       RulesCacheDelegate::GetRulesStoredKey(
           event_name, profile.IsOffTheRecord()));
@@ -304,7 +309,6 @@ TEST_F(RulesRegistryWithCacheTest, RulesStoredFlagMultipleRegistries) {
 
   const std::string event_name1("testEvent1");
   const std::string event_name2("testEvent2");
-  const RulesRegistry::WebViewKey key(0, 0);
   const std::string rules_stored_key1(
       RulesCacheDelegate::GetRulesStoredKey(
           event_name1, profile.IsOffTheRecord()));
@@ -384,4 +388,22 @@ TEST_F(RulesRegistryWithCacheTest, RulesPreservedAcrossRestart) {
   EXPECT_EQ(1, GetNumberOfRules(kExtensionId, registry.get()));
 }
 
+TEST_F(RulesRegistryWithCacheTest, ConcurrentStoringOfRules) {
+  extensions::TestExtensionSystem* system =
+      static_cast<extensions::TestExtensionSystem*>(
+          extensions::ExtensionSystem::Get(&profile_));
+  system->CreateExtensionPrefs(CommandLine::ForCurrentProcess(),
+                               base::FilePath());
+  system->CreateExtensionService(
+      CommandLine::ForCurrentProcess(), base::FilePath(), false);
+  TestingValueStore* store = system->value_store();
+
+  int write_count = store->write_count();
+  EXPECT_EQ("", AddRule(kExtensionId, kRuleId));
+  EXPECT_EQ("", AddRule(kExtension2Id, kRule2Id));
+  system->SetReady();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(write_count + 2, store->write_count());
+}
+
 }  //  namespace extensions