Create a mechanism define declarative rules via the extension manifest.

extensions/browser/api/declarative/rules_registry_unittest.cc

Index: extensions/browser/api/declarative/rules_registry_unittest.cc
diff --git a/extensions/browser/api/declarative/rules_registry_unittest.cc b/extensions/browser/api/declarative/rules_registry_unittest.cc
index 33da872a1667d3d6a2db96dacefff7f5c57ea830..8e77b2107dbc4f5ec37ab017946b700afd09cf44 100644
--- a/extensions/browser/api/declarative/rules_registry_unittest.cc
+++ b/extensions/browser/api/declarative/rules_registry_unittest.cc
@@ -7,9 +7,15 @@
 #include <algorithm>
 
 #include "base/message_loop/message_loop.h"
+#include "base/values.h"
 #include "content/public/test/test_browser_thread.h"
 #include "extensions/browser/api/declarative/rules_registry_service.h"
 #include "extensions/browser/api/declarative/test_rules_registry.h"
+#include "extensions/browser/api_test_utils.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/extensions_test.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/extension_builder.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -20,7 +26,10 @@ const int key = extensions::RulesRegistryService::kDefaultRulesRegistryID;
 
 namespace extensions {
 
-TEST(RulesRegistryTest, FillOptionalIdentifiers) {
+using api_test_utils::ParseDictionary;
+using RulesRegistryTest = ExtensionsTest;
+
+TEST_F(RulesRegistryTest, FillOptionalIdentifiers) {
   base::MessageLoopForUI message_loop;
   content::TestBrowserThread thread(content::BrowserThread::UI, &message_loop);
 
@@ -129,7 +138,7 @@ TEST(RulesRegistryTest, FillOptionalIdentifiers) {
   message_loop.RunUntilIdle();
 }
 
-TEST(RulesRegistryTest, FillOptionalPriority) {
+TEST_F(RulesRegistryTest, FillOptionalPriority) {
   base::MessageLoopForUI message_loop;
   content::TestBrowserThread thread(content::BrowserThread::UI, &message_loop);
 
@@ -165,4 +174,249 @@ TEST(RulesRegistryTest, FillOptionalPriority) {
   message_loop.RunUntilIdle();
 }
 
+TEST_F(RulesRegistryTest, TwoRulesInManifest) {

[Mike Wittman, 2015/06/04 18:40:13]

Can you add a comment before each test describing what it's testing?

[danduong, 2015/06/04 19:56:07]

Done.

[Mike Wittman, 2015/06/04 22:15:15]

convention is to put the comments above the TEST declaration

[danduong, 2015/06/05 06:41:57]

Done.

+  base::MessageLoopForUI message_loop;
+  content::TestBrowserThread thread(content::BrowserThread::UI, &message_loop);
+
+  // Create extension
+  scoped_ptr<base::DictionaryValue> manifest = ParseDictionary(
+      "{"
+      "  \"name\": \"Test\","
+      "  \"version\": \"1\","
+      "  \"event_rules\": ["
+      "    {"
+      "      \"event\": \"declarativeContent.onPageChanged\","
+      "      \"actions\": [{"
+      "        \"type\": \"declarativeContent.ShowPageAction\""
+      "      }],"
+      "      \"conditions\" : [{"
+      "        \"css\": [\"video\"],"
+      "        \"type\" : \"declarativeContent.PageStateMatcher\""
+      "      }]"
+      "    },"
+      "    {"
+      "      \"event\": \"declarativeContent.onPageChanged\","
+      "      \"actions\": [{"
+      "        \"type\": \"declarativeContent.ShowPageAction\""
+      "      }],"
+      "      \"conditions\" : [{"
+      "        \"css\": [\"input[type='password']\"],"
+      "        \"type\" : \"declarativeContent.PageStateMatcher\""
+      "      }]"
+      "    }"
+      "  ]"
+      "}");
+  scoped_refptr<Extension> extension = ExtensionBuilder()
+                                           .SetManifest(manifest.Pass())
+                                           .SetID(kExtensionId)
+                                           .Build();
+  ExtensionRegistry::Get(browser_context())->AddEnabled(extension);
+
+  scoped_refptr<RulesRegistry> registry = new TestRulesRegistry(
+      browser_context(), "declarativeContent.onPageChanged",
+      content::BrowserThread::UI, nullptr, key);
+  // Simulate what RulesRegistryService would do on extension load.
+  registry->OnExtensionLoaded(kExtensionId);
+
+  std::vector<linked_ptr<RulesRegistry::Rule>> get_rules;
+  registry->GetAllRules(kExtensionId, &get_rules);
+
+  ASSERT_EQ(2u, get_rules.size());
+  ASSERT_EQ(1u, get_rules[0]->actions.size());
+  const base::DictionaryValue* action;

[not at google - send to devlin, 2015/06/04 17:58:49]

it's nice to initialise pointers to null.

[danduong, 2015/06/04 19:10:59]

Done.

+  ASSERT_EQ(true, get_rules[0]->actions[0]->GetAsDictionary(&action));

[not at google - send to devlin, 2015/06/04 17:58:49]

ASSERT_TRUE

[danduong, 2015/06/04 19:10:59]

Done.

+  std::string instance_type;
+  ASSERT_EQ(true, action->GetStringASCII("instanceType", &instance_type));
+  ASSERT_EQ("declarativeContent.ShowPageAction", instance_type);
+  ASSERT_EQ(1u, get_rules[0]->conditions.size());
+  const base::DictionaryValue* condition;
+  ASSERT_EQ(true, get_rules[0]->conditions[0]->GetAsDictionary(&condition));
+  ASSERT_EQ(true, condition->GetStringASCII("instanceType", &instance_type));
+  ASSERT_EQ("declarativeContent.PageStateMatcher", instance_type);

[Mike Wittman, 2015/06/04 18:40:14]

Checks that can fail without causing the test to crash should use EXPECT rather
than ASSERT.

[danduong, 2015/06/04 19:56:07]

Done.

+  const base::ListValue* css;
+  ASSERT_EQ(true, condition->GetList("css", &css));
+  ASSERT_EQ(1u, css->GetSize());
+  std::string css_value;
+  ASSERT_EQ(true, css->GetString(0, &css_value));
+  ASSERT_EQ("video", css_value);
+
+  ASSERT_EQ(true, get_rules[1]->actions[0]->GetAsDictionary(&action));
+  ASSERT_EQ(true, action->GetStringASCII("instanceType", &instance_type));
+  ASSERT_EQ("declarativeContent.ShowPageAction", instance_type);
+  ASSERT_EQ(1u, get_rules[1]->conditions.size());
+  ASSERT_EQ(true, get_rules[1]->conditions[0]->GetAsDictionary(&condition));
+  ASSERT_EQ(true, condition->GetStringASCII("instanceType", &instance_type));
+  ASSERT_EQ("declarativeContent.PageStateMatcher", instance_type);
+  ASSERT_EQ(true, condition->GetList("css", &css));
+  ASSERT_EQ(1u, css->GetSize());
+  ASSERT_EQ(true, css->GetString(0, &css_value));
+  ASSERT_EQ("input[type='password']", css_value);

[not at google - send to devlin, 2015/06/04 17:58:49]

You can have a much easier time here defining the expected structure like

scoped_ptr<base::Dictionary> expected_first_rule = ParseDictionary(
    "{\"actions\": ...},
     {\"conditions\": ...})");
scoped_ptr<base::Dictionary> expected_second_rule = ...;

then pulling out each individually and using Value::Equals or whatever.

[danduong, 2015/06/04 19:56:07]

Done.

+}
+
+TEST_F(RulesRegistryTest, DeleteRuleInManifest) {
+  base::MessageLoopForUI message_loop;
+  content::TestBrowserThread thread(content::BrowserThread::UI, &message_loop);
+
+  // Create extension
+  scoped_ptr<base::DictionaryValue> manifest = ParseDictionary(
+      "{"
+      "  \"name\": \"Test\","
+      "  \"version\": \"1\","
+      "  \"event_rules\": [{"
+      "    \"event\": \"declarativeContent.onPageChanged\","
+      "    \"actions\": [{"
+      "      \"type\": \"declarativeContent.ShowPageAction\""
+      "    }],"
+      "    \"conditions\" : [{"
+      "      \"css\": [\"video\"],"
+      "      \"type\" : \"declarativeContent.PageStateMatcher\""
+      "    }]"
+      "  }]"
+      "}");
+  scoped_refptr<Extension> extension = ExtensionBuilder()
+                                           .SetManifest(manifest.Pass())
+                                           .SetID(kExtensionId)
+                                           .Build();
+  ExtensionRegistry::Get(browser_context())->AddEnabled(extension);
+
+  scoped_refptr<RulesRegistry> registry = new TestRulesRegistry(
+      browser_context(), "declarativeContent.onPageChanged",
+      content::BrowserThread::UI, nullptr, key);
+  // Simulate what RulesRegistryService would do on extension load.
+  registry->OnExtensionLoaded(kExtensionId);
+  // Add some extra rules outside of the manifest.
+  std::vector<linked_ptr<RulesRegistry::Rule>> add_rules;
+  add_rules.push_back(make_linked_ptr(new RulesRegistry::Rule));
+  add_rules.push_back(make_linked_ptr(new RulesRegistry::Rule));
+  registry->AddRules(kExtensionId, add_rules);
+
+  std::vector<linked_ptr<RulesRegistry::Rule>> get_rules;
+  registry->GetAllRules(kExtensionId, &get_rules);
+  ASSERT_EQ(3u, get_rules.size());
+
+  // Remove a rule from outside the manifest.
+  std::vector<std::string> remove_ids;
+  remove_ids.push_back(*(get_rules[1]->id));
+  ASSERT_EQ(true, registry->RemoveRules(kExtensionId, remove_ids).empty());
+  get_rules.clear();
+  registry->GetAllRules(kExtensionId, &get_rules);
+  ASSERT_EQ(2u, get_rules.size());
+
+  // Attempt to remove rule in manifest.
+  remove_ids.clear();
+  remove_ids.push_back(*(get_rules[0]->id));
+  ASSERT_EQ(false, registry->RemoveRules(kExtensionId, remove_ids).empty());
+  get_rules.clear();
+  registry->GetAllRules(kExtensionId, &get_rules);
+  ASSERT_EQ(2u, get_rules.size());
+
+  // Remove all rules.
+  registry->RemoveAllRules(kExtensionId);
+  get_rules.clear();
+  registry->GetAllRules(kExtensionId, &get_rules);
+  ASSERT_EQ(0u, get_rules.size());
+}
+

[not at google - send to devlin, 2015/06/04 17:58:49]

Test somewhere around here what happens if you try to add rules programmatically
with some rules declared; it looks to me like that's what you're indirectly
testing (?) but it also looks like you've written code to prevent that.

[danduong, 2015/06/04 19:10:59]

I believe that is tested unless you mean something else.

297: removes a rule that was added programmatically.
305: tries to remove a rule that was added in the manifest and expectedly
failes.

+TEST_F(RulesRegistryTest, TwoBadRulesOneGoodInManifest) {
+  base::MessageLoopForUI message_loop;
+  content::TestBrowserThread thread(content::BrowserThread::UI, &message_loop);
+
+  // Create extension
+  scoped_ptr<base::DictionaryValue> manifest = ParseDictionary(
+      "{"
+      "  \"name\": \"Test\","
+      "  \"version\": \"1\","
+      "  \"event_rules\": ["
+      "    {"
+      "      \"event\": \"declarativeContent.onPageChanged\","
+      "      \"actions\": [{"
+      "        \"type\": \"declarativeContent.ShowPageAction\""
+      "      }],"
+      "      \"conditions\" : [{"
+      "        \"css\": [\"video\"],"
+      "        \"type\" : \"declarativeContent.PageStateMatcher\""
+      "      }]"
+      "    },"
+      "    {"
+      "      \"actions\": [{"
+      "        \"type\": \"declarativeContent.ShowPageAction\""
+      "      }],"
+      "      \"conditions\" : [{"
+      "        \"css\": [\"video\"],"
+      "        \"type\" : \"declarativeContent.PageStateMatcher\""
+      "      }]"
+      "    },"
+      "    {"
+      "      \"event\": \"declarativeContent.onPageChanged\","
+      "      \"actions\": [{"
+      "        \"type\": \"declarativeContent.ShowPageAction\""
+      "      }],"
+      "      \"conditions\" : [{}]"
+      "    }"
+      "  ]"
+      "}");
+  scoped_refptr<Extension> extension = ExtensionBuilder()
+                                           .SetManifest(manifest.Pass())
+                                           .SetID(kExtensionId)
+                                           .Build();
+  ExtensionRegistry::Get(browser_context())->AddEnabled(extension);
+
+  scoped_refptr<RulesRegistry> registry = new TestRulesRegistry(
+      browser_context(), "declarativeContent.onPageChanged",
+      content::BrowserThread::UI, nullptr, key);
+  // Simulate what RulesRegistryService would do on extension load.
+  registry->OnExtensionLoaded(kExtensionId);
+
+  std::vector<linked_ptr<RulesRegistry::Rule>> get_rules;
+  registry->GetAllRules(kExtensionId, &get_rules);
+
+  ASSERT_EQ(1u, get_rules.size());
+  ASSERT_EQ(1u, get_rules[0]->actions.size());
+  const base::DictionaryValue* action;
+  ASSERT_EQ(true, get_rules[0]->actions[0]->GetAsDictionary(&action));
+  std::string instance_type;
+  ASSERT_EQ(true, action->GetStringASCII("instanceType", &instance_type));
+  ASSERT_EQ("declarativeContent.ShowPageAction", instance_type);
+  ASSERT_EQ(1u, get_rules[0]->conditions.size());
+  const base::DictionaryValue* condition;
+  ASSERT_EQ(true, get_rules[0]->conditions[0]->GetAsDictionary(&condition));
+  ASSERT_EQ(true, condition->GetStringASCII("instanceType", &instance_type));
+  ASSERT_EQ("declarativeContent.PageStateMatcher", instance_type);
+  const base::ListValue* css;
+  ASSERT_EQ(true, condition->GetList("css", &css));
+  ASSERT_EQ(1u, css->GetSize());
+  std::string css_value;
+  ASSERT_EQ(true, css->GetString(0, &css_value));
+  ASSERT_EQ("video", css_value);
+}
+
+TEST_F(RulesRegistryTest, NoValidRulesInManifest) {
+  base::MessageLoopForUI message_loop;
+  content::TestBrowserThread thread(content::BrowserThread::UI, &message_loop);
+
+  // Create extension
+  scoped_ptr<base::DictionaryValue> manifest = ParseDictionary(
+      "{"
+      "  \"name\": \"Test\","
+      "  \"version\": \"1\","
+      "  \"event_rules\": {}"
+      "}");
+  scoped_refptr<Extension> extension = ExtensionBuilder()
+                                           .SetManifest(manifest.Pass())
+                                           .SetID(kExtensionId)
+                                           .Build();
+  ExtensionRegistry::Get(browser_context())->AddEnabled(extension);
+
+  scoped_refptr<RulesRegistry> registry = new TestRulesRegistry(
+      browser_context(), "declarativeContent.onPageChanged",
+      content::BrowserThread::UI, nullptr, key);
+  // Simulate what RulesRegistryService would do on extension load.
+  registry->OnExtensionLoaded(kExtensionId);
+
+  std::vector<linked_ptr<RulesRegistry::Rule>> get_rules;
+  registry->GetAllRules(kExtensionId, &get_rules);
+
+  ASSERT_EQ(0u, get_rules.size());
+}
+
 }  // namespace extensions