Create a mechanism define declarative rules via the extension manifest.

extensions/browser/api/declarative/rules_registry_unittest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "extensions/browser/api/declarative/rules_registry.h"
 
 #include <algorithm>
 
 #include "base/message_loop/message_loop.h"
+#include "base/test/values_test_util.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/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 {
 const char kExtensionId[] = "foobar";
 const char kRuleId[] = "foo";
 const int key = extensions::RulesRegistryService::kDefaultRulesRegistryID;
 }  // namespace
 
 namespace extensions {
 
-TEST(RulesRegistryTest, FillOptionalIdentifiers) {
+using base::test::ParseJson;
+
+class RulesRegistryTest : public ExtensionsTest {
+ public:
+  RulesRegistryTest() {}

[not at google - send to devlin, 2015/06/02 22:44:50]

Empty constructor isn't necessary, but this needs a destructor.

However, you can also do "using RulesRegistryTest = ExtensionsTest" to have the
same namespacing advantages, I think.

[danduong, 2015/06/03 00:18:06]

Done.

+};
+
+TEST_F(RulesRegistryTest, FillOptionalIdentifiers) {
   base::MessageLoopForUI message_loop;
   content::TestBrowserThread thread(content::BrowserThread::UI, &message_loop);
 
   std::string error;
   scoped_refptr<RulesRegistry> registry =
       new TestRulesRegistry(content::BrowserThread::UI, "" /*event_name*/, key);
 
   // Add rules and check that their identifiers are filled and unique.
 
   std::vector<linked_ptr<RulesRegistry::Rule> > add_rules;
@@ skipping 88 matching lines @@
 
   registry->OnExtensionUninstalled(kExtensionId);
   EXPECT_EQ(0u /*extensions*/ + 0u /*rules*/,
             registry->GetNumberOfUsedRuleIdentifiersForTesting());
 
   // Make sure that deletion traits of registry are executed.
   registry = NULL;
   message_loop.RunUntilIdle();
 }
 
-TEST(RulesRegistryTest, FillOptionalPriority) {
+TEST_F(RulesRegistryTest, FillOptionalPriority) {
   base::MessageLoopForUI message_loop;
   content::TestBrowserThread thread(content::BrowserThread::UI, &message_loop);
 
   std::string error;
   scoped_refptr<RulesRegistry> registry =
       new TestRulesRegistry(content::BrowserThread::UI, "" /*event_name*/, key);
 
   // Add rules and check that their priorities are filled if they are empty.
 
   std::vector<linked_ptr<RulesRegistry::Rule> > add_rules;
@@ skipping 15 matching lines @@
   EXPECT_GT(RulesRegistry::DEFAULT_PRIORITY, 2);
   EXPECT_EQ(2, std::min(*get_rules[0]->priority, *get_rules[1]->priority));
   EXPECT_EQ(RulesRegistry::DEFAULT_PRIORITY,
             std::max(*get_rules[0]->priority, *get_rules[1]->priority));
 
   // Make sure that deletion traits of registry are executed.
   registry = NULL;
   message_loop.RunUntilIdle();
 }
 
+TEST_F(RulesRegistryTest, ExtensionWithRulesInManifest) {

[Mike Wittman, 2015/06/02 23:11:59]

Can you add tests for handling and reporting of errors in the rules specified in
the manifest as well?

+  base::MessageLoopForUI message_loop;
+  content::TestBrowserThread thread(content::BrowserThread::UI, &message_loop);
+
+  // Create extension
+  scoped_ptr<base::DictionaryValue> manifest(
+      (base::DictionaryValue*)ParseJson(

[not at google - send to devlin, 2015/06/02 22:44:50]

Use C++ style static_cast<>, not C style cast.

However in this case you can just used ParseDictionary from api_test_utils. I
also thought there was a version somewhere which let you use single quotes, in
the //extensions directory perhaps, maybe you can find it.

[danduong, 2015/06/03 00:18:06]

Done.

+          "{"
+          "  \"name\" : \"Test\","
+          "  \"version\" : \"1\","
+          "  \"declarativeContent\" : {"
+          "    \"onPageChanged\": ["
+          "      {"
+          "        \"actions\": ["
+          "          {"
+          "            \"instanceType\": \"declarativeContent.ShowPageAction\""

[not at google - send to devlin, 2015/06/02 22:44:50]

This is a bit of an awkward API. Better to try to make it match the
public-facing API rather than the internal API that we use.

For example, I'd expect this to be:

"declarative_content_rules": [{
  "conditions": [
    "declarativeContent.PageStateMatcher": {
      "css": ["video"]
    }
  ],
  "actions": ["declarativeContent.ShowPageAction"]
}]

[danduong, 2015/06/03 00:18:06]

Where should we specify onPageChanged? How should we support
declarativeWebRequest as well?

[not at google - send to devlin, 2015/06/03 16:21:44]

Oh right, I suppose the event declaration syntax should specify that. I'll try
again:

"declarative_events": [{
  "name": "declarativeContent.OnPageChanged",
  "conditions": [
    "declarativeContent.PageStateMatcher": {
      "css": ["video"]
    }
  ],
  "actions": ["declarativeContent.ShowPageAction"]
}]

[danduong, 2015/06/03 16:45:53]

I don't think that's valid JSON. Do you mean:

"conditions": [
  {
    "declarativeContent.PageStateMatcher": {
      "css": ["video"]
    }
  }
],

If this is the case, the object nesting seems a little awkward. It is saying
that an individual condition could have a PageStateMatcher and potentially
another kind of matcher, but not 2 page state matchers.

instanceType flattens this out, but we can switch to instance_type if we prefer
underscores in manifest. Do we expect this transformation universally? 

e.g. does
 pageUrl: { hostEquals: 'www.google.com', schemes: ['https'] },
become
 page_url: { 'host_equals': 'www.google.com', 'schemes: ['https']},

+          "          }"
+          "        ],"
+          "        \"conditions\" : ["
+          "          {"
+          "            \"css\": [\"video\"],"
+          "            \"instanceType\" : "
+          "                \"declarativeContent.PageStateMatcher\""
+          "          }"
+          "        ]"
+          "      }"
+          "    ]"
+          "  }"
+          "}").release());
+  scoped_refptr<Extension> extension = ExtensionBuilder()
+                                           .SetManifest(manifest.Pass())
+                                           .SetID(kExtensionId)
+                                           .Build();
+  ExtensionRegistry::Get(browser_context())->AddEnabled(extension);
+
+  scoped_refptr<RulesRegistry> registry = new TestRulesRegistry(

[Mike Wittman, 2015/06/02 23:11:59]

I'm not sure why this stored in a scoped_refptr in the other tests. I think this
can be allocated on the stack, or stored in a plain scoped_ptr if you want to
ensure it's only accessed via the RulesRegistry interface.

[not at google - send to devlin, 2015/06/02 23:14:40]

TestRulesRegistry --> RulesRegistry --> RefCountedThreadSafe

:-(

[Mike Wittman, 2015/06/02 23:16:00]

Ah, should have known.

+      browser_context(), "declarativeContent.onPageChanged",
+      content::BrowserThread::UI, NULL, key);

[not at google - send to devlin, 2015/06/02 22:44:50]

nullptr not NULL - though I think that if you remove the BrowserContext
dependency from RulesRegistry then you could use the simplified
TestRulesRegistry constructor anyway.

[danduong, 2015/06/03 00:18:06]

Done.

+  // 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());

[not at google - send to devlin, 2015/06/02 22:44:50]

Could you assert that this rule matches the one constructed?

[danduong, 2015/06/03 00:18:06]

Done.

+}
+
 }  // namespace extensions