Support extension precedences in declarative WebRequest API

This CL introduces precedences of extensions in the same way as they are handled by the old WebRequest API: If two extensions conflict, the most recently installed extension wins.

BUG=112155
TEST=no


Review URL: https://chromiumcodereview.appspot.com/10406013

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

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

@@ -29,7 +29,8 @@ void RegisterToExtensionWebRequestEventRouterOnIO(
 
 }  // namespace
 
-RulesRegistryService::RulesRegistryService(Profile* profile) {
+RulesRegistryService::RulesRegistryService(Profile* profile)
+    : profile_(profile) {
   if (profile) {
     registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
                    content::Source<Profile>(profile));
@@ -40,7 +41,7 @@ RulesRegistryService::~RulesRegistryService() {}
 
 void RulesRegistryService::RegisterDefaultRulesRegistries() {
   scoped_refptr<WebRequestRulesRegistry> web_request_rules_registry(
-      new WebRequestRulesRegistry);
+      new WebRequestRulesRegistry(profile_));
   RegisterRulesRegistry(declarative_webrequest_constants::kOnRequest,
                         web_request_rules_registry);
   content::BrowserThread::PostTask(

chrome/browser/extensions/api/declarative/rules_registry_service.h

@@ -70,6 +70,8 @@ class RulesRegistryService : public content::NotificationObserver  {
 
   content::NotificationRegistrar registrar_;
 
+  Profile* profile_;
+
   DISALLOW_COPY_AND_ASSIGN(RulesRegistryService);
 };
 

chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry.cc

@@ -6,11 +6,15 @@
 
 #include "chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.h"
 #include "chrome/browser/extensions/api/web_request/web_request_api_helpers.h"
+#include "chrome/browser/extensions/extension_system.h"
 #include "net/url_request/url_request.h"
 
 namespace extensions {
 
-WebRequestRulesRegistry::WebRequestRulesRegistry() {}
+WebRequestRulesRegistry::WebRequestRulesRegistry(Profile* profile) {
+  if (profile)
+    extension_info_map_ = ExtensionSystem::Get(profile)->info_map();
+}
 
 std::set<WebRequestRule::GlobalRuleId>
 WebRequestRulesRegistry::GetMatches(net::URLRequest* request,
@@ -59,8 +63,8 @@ std::list<LinkedPtrEventResponseDelta> WebRequestRulesRegistry::CreateDeltas(
 std::string WebRequestRulesRegistry::AddRulesImpl(
     const std::string& extension_id,
     const std::vector<linked_ptr<RulesRegistry::Rule> >& rules) {
-  // TODO(battre): Retrieve this from somewhere
-  base::Time extension_installation_time;
+  base::Time extension_installation_time =
+      GetExtensionInstallationTime(extension_id);
 
   std::string error;
   RulesMap new_webrequest_rules;
@@ -175,4 +179,12 @@ bool WebRequestRulesRegistry::IsEmpty() const {
 
 WebRequestRulesRegistry::~WebRequestRulesRegistry() {}
 
+base::Time WebRequestRulesRegistry::GetExtensionInstallationTime(
+    const std::string& extension_id) const {
+  if (!extension_info_map_.get())  // May be NULL during testing.
+    return base::Time();
+
+  return extension_info_map_->GetInstallTime(extension_id);
+}
+
 }  // namespace extensions

chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry.h

@@ -11,12 +11,17 @@
 #include <set>
 #include <vector>
 
+#include "base/time.h"
 #include "base/memory/linked_ptr.h"
+#include "base/memory/ref_counted.h"
 #include "chrome/browser/extensions/api/declarative/rules_registry_with_cache.h"
 #include "chrome/browser/extensions/api/declarative_webrequest/request_stages.h"
 #include "chrome/browser/extensions/api/declarative_webrequest/webrequest_rule.h"
+#include "chrome/browser/extensions/extension_info_map.h"
 #include "chrome/common/extensions/matcher/url_matcher.h"
 
+class Profile;
+
 namespace extension_web_request_api_helpers {
 struct EventResponseDelta;
 }
@@ -60,7 +65,7 @@ class WebRequestRule;
 // example 'scheme': 'http') are fulfilled.
 class WebRequestRulesRegistry : public RulesRegistryWithCache {
  public:
-  WebRequestRulesRegistry();
+  explicit WebRequestRulesRegistry(Profile* profile);
 
   // TODO(battre): This will become an implementation detail, because we need
   // a way to also execute the actions of the rules.
@@ -87,13 +92,16 @@ class WebRequestRulesRegistry : public RulesRegistryWithCache {
   // Returns true if this object retains no allocated data. Only for debugging.
   bool IsEmpty() const;
 
+ protected:
+  virtual ~WebRequestRulesRegistry();
+  virtual base::Time GetExtensionInstallationTime(
+      const std::string& extension_id) const;
+
  private:
   typedef std::map<URLMatcherConditionSet::ID, WebRequestRule*> RuleTriggers;
   typedef std::map<WebRequestRule::GlobalRuleId, linked_ptr<WebRequestRule> >
       RulesMap;
 
-  virtual ~WebRequestRulesRegistry();
-
   // Map that tells us which WebRequestRule may match under the condition that
   // the URLMatcherConditionSet::ID was returned by the |url_matcher_|.
   RuleTriggers rule_triggers_;
@@ -101,6 +109,8 @@ class WebRequestRulesRegistry : public RulesRegistryWithCache {
   RulesMap webrequest_rules_;
 
   URLMatcher url_matcher_;
+
+  scoped_refptr<ExtensionInfoMap> extension_info_map_;
 };
 
 }  // namespace extensions

chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/values.h"
 #include "chrome/common/extensions/matcher/url_matcher_constants.h"
 #include "chrome/browser/extensions/api/declarative_webrequest/webrequest_constants.h"
+#include "chrome/browser/extensions/api/web_request/web_request_api_helpers.h"
 #include "content/test/test_browser_thread.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -20,13 +21,32 @@ const char kExtensionId[] = "ext1";
 const char kExtensionId2[] = "ext2";
 const char kRuleId1[] = "rule1";
 const char kRuleId2[] = "rule2";
+const char kRuleId3[] = "rule3";
 }  // namespace
 
 namespace extensions {
 
+namespace helpers = extension_web_request_api_helpers;
 namespace keys = declarative_webrequest_constants;
 namespace keys2 = url_matcher_constants;
 
+class TestWebRequestRulesRegistry : public WebRequestRulesRegistry {
+ public:
+  TestWebRequestRulesRegistry() : WebRequestRulesRegistry(NULL) {}
+  virtual ~TestWebRequestRulesRegistry() {}
+
+ protected:
+  virtual base::Time GetExtensionInstallationTime(
+      const std::string& extension_id) const {
+    if (extension_id == kExtensionId)
+      return base::Time() + base::TimeDelta::FromDays(1);
+    else if (extension_id == kExtensionId2)
+      return base::Time() + base::TimeDelta::FromDays(2);
+    else
+      return base::Time();
+  }
+};
+
 class WebRequestRulesRegistryTest : public testing::Test {
  public:
  public:
@@ -116,6 +136,32 @@ class WebRequestRulesRegistryTest : public testing::Test {
     return rule;
   }
 
+  linked_ptr<RulesRegistry::Rule> CreateRedirectRule(
+      const std::string& destination) {
+    DictionaryValue condition_dict;
+    condition_dict.SetString(keys::kInstanceTypeKey, keys::kRequestMatcherType);
+
+    linked_ptr<json_schema_compiler::any::Any> condition = make_linked_ptr(
+        new json_schema_compiler::any::Any);
+    condition->Init(condition_dict);
+
+    DictionaryValue action_dict;
+    action_dict.SetString(keys::kInstanceTypeKey, keys::kRedirectRequestType);
+    action_dict.SetString(keys::kRedirectUrlKey, destination);
+
+    linked_ptr<json_schema_compiler::any::Any> action = make_linked_ptr(
+        new json_schema_compiler::any::Any);
+    action->Init(action_dict);
+
+    linked_ptr<RulesRegistry::Rule> rule =
+        make_linked_ptr(new RulesRegistry::Rule);
+    rule->id.reset(new std::string(kRuleId3));
+    rule->priority.reset(new int(100));
+    rule->actions.push_back(action);
+    rule->conditions.push_back(condition);
+    return rule;
+  }
+
  protected:
   MessageLoop message_loop;
   content::TestBrowserThread ui;
@@ -123,7 +169,8 @@ class WebRequestRulesRegistryTest : public testing::Test {
 };
 
 TEST_F(WebRequestRulesRegistryTest, AddRulesImpl) {
-  scoped_refptr<WebRequestRulesRegistry> registry(new WebRequestRulesRegistry);
+  scoped_refptr<WebRequestRulesRegistry> registry(
+      new TestWebRequestRulesRegistry());
   std::string error;
 
   std::vector<linked_ptr<RulesRegistry::Rule> > rules;
@@ -153,7 +200,8 @@ TEST_F(WebRequestRulesRegistryTest, AddRulesImpl) {
 }
 
 TEST_F(WebRequestRulesRegistryTest, RemoveRulesImpl) {
-  scoped_refptr<WebRequestRulesRegistry> registry(new WebRequestRulesRegistry);
+  scoped_refptr<WebRequestRulesRegistry> registry(
+      new TestWebRequestRulesRegistry());
   std::string error;
 
   // Setup RulesRegistry to contain two rules.
@@ -194,7 +242,8 @@ TEST_F(WebRequestRulesRegistryTest, RemoveRulesImpl) {
 }
 
 TEST_F(WebRequestRulesRegistryTest, RemoveAllRulesImpl) {
-  scoped_refptr<WebRequestRulesRegistry> registry(new WebRequestRulesRegistry);
+  scoped_refptr<WebRequestRulesRegistry> registry(
+      new TestWebRequestRulesRegistry());
   std::string error;
 
   // Setup RulesRegistry to contain two rules, one for each extension.
@@ -238,4 +287,44 @@ TEST_F(WebRequestRulesRegistryTest, RemoveAllRulesImpl) {
   EXPECT_TRUE(registry->IsEmpty());
 }
 
+TEST_F(WebRequestRulesRegistryTest, Precedences) {
+  scoped_refptr<WebRequestRulesRegistry> registry(
+      new TestWebRequestRulesRegistry());
+  std::string error;
+
+  std::vector<linked_ptr<RulesRegistry::Rule> > rules_to_add_1(1);
+  rules_to_add_1[0] = CreateRedirectRule("http://www.foo.com");
+  error = registry->AddRules(kExtensionId, rules_to_add_1);
+  EXPECT_EQ("", error);
+
+  std::vector<linked_ptr<RulesRegistry::Rule> > rules_to_add_2(1);
+  rules_to_add_2[0] = CreateRedirectRule("http://www.bar.com");
+  error = registry->AddRules(kExtensionId2, rules_to_add_2);
+  EXPECT_EQ("", error);
+
+  GURL url("http://www.google.com");
+  TestURLRequest request(url, NULL);
+  std::list<LinkedPtrEventResponseDelta> deltas =
+      registry->CreateDeltas(&request, ON_BEFORE_REQUEST);
+
+  // The second extension is installed later and will win for this reason
+  // in conflict resolution.
+  ASSERT_EQ(2u, deltas.size());
+  deltas.sort(&helpers::InDecreasingExtensionInstallationTimeOrder);
+
+  std::list<LinkedPtrEventResponseDelta>::iterator i = deltas.begin();
+  LinkedPtrEventResponseDelta winner = *i++;
+  LinkedPtrEventResponseDelta loser = *i;
+
+  EXPECT_EQ(kExtensionId2, winner->extension_id);
+  EXPECT_EQ(base::Time() + base::TimeDelta::FromDays(2),
+            winner->extension_install_time);
+  EXPECT_EQ(GURL("http://www.bar.com"), winner->new_url);
+
+  EXPECT_EQ(kExtensionId, loser->extension_id);
+  EXPECT_EQ(base::Time() + base::TimeDelta::FromDays(1),
+            loser->extension_install_time);
+  EXPECT_EQ(GURL("http://www.foo.com"), loser->new_url);
+}
+
 }  // namespace extensions