Allow declarative webrequest and webrequest in extensions.

extensions/browser/api/declarative/rules_registry_service.cc

Index: extensions/browser/api/declarative/rules_registry_service.cc
diff --git a/extensions/browser/api/declarative/rules_registry_service.cc b/extensions/browser/api/declarative/rules_registry_service.cc
index 12df0135a3122468affc88ae27c61b99954f91e9..02fb49defdcbf5788c8b7abd95b718826fcdb00b 100644
--- a/extensions/browser/api/declarative/rules_registry_service.cc
+++ b/extensions/browser/api/declarative/rules_registry_service.cc
@@ -95,8 +95,10 @@ void RulesRegistryService::EnsureDefaultRulesRegistriesRegistered(
     scoped_refptr<ContentRulesRegistry> content_rules_registry =
         ExtensionsAPIClient::Get()->CreateContentRulesRegistry(
             browser_context_, content_rules_cache_delegate);
-    RegisterRulesRegistry(content_rules_registry);
-    content_rules_registry_ = content_rules_registry.get();
+    if (content_rules_registry.get()) {

[Ken Rockot(use gerrit already), 2014/09/29 17:22:53]

I realize you're only preserving previous behavior, but I would feel more
comfortable if this just held onto a copy of the content registry's key
(content_rules_registry->webview_key()) rather than a direct weak pointer.

Then content_rules_registry() can perform the lookup in rule_registries_ and
safely return either a valid raw pointer or NULL

(or nullptr since that's now allowed in Chrome. if you're feeling adventurous):P

[Xi Han, 2014/09/29 18:16:21]

Thank you Ken. I added this NULL check since I found in app_shell the function
CreateContentRulesRegistry(...) will return NULL. So using nullptr check might
be better here. The (content_rules_registry->webview_key()) will return an
object and it cannot pass compile:(. 


On 2014/09/29 17:22:53, Ken Rockot wrote:

[Ken Rockot(use gerrit already), 2014/09/29 19:31:42]

This isn't what I meant; the condition was fine before. What I meant was that
content_rules_registry_ should not exist as a raw pointer, but rather maybe it
should be a string content_rules_registry_key_ (which may be empty); so then
this code would change to:

if (content_rules_registry.get()) {
  RegisterRulesRegistry(content_rules_registry)
  content_rules_registry_key_ = content_rules_registry->webview_key();
}


And then you replace the content_rules_registry() accessor with a
GetContentRulesRegistry() method, which has this implementation:

RulesRegistryService::GetContentRulesRegistry() {
  RulesRegistryMap::const_iterator i =
rule_registry_.find(content_rules_registry_key_);
  if (i != rule_registry_.end()) {
    return i->second.get();
  }
  return nullptr;
}


Does that make more sense? This is safer because it avoids holding onto a raw
pointer to the content registry.

[Fady Samuel, 2014/09/29 22:07:11]

Hi Ken,

In AppShell, this code path will never execute. WebViewKey will always be
non-zero. 

EnsureDefaultRulesRegistriesRegistered(WebViewKey(0, 0)); on line 54 will create
a bunch of RulesRegistries that we will never use in AppShell because only the
Chrome Extension Declarative WebRequest API has a WebViewKey of 0,0.

I would like to suggest that we leave the code as is in this CL, and try to find
ways to clean up the code in a subsequent CL. WDYT? Thanks.

+      RegisterRulesRegistry(content_rules_registry);
+      content_rules_registry_ = content_rules_registry.get();
+    }
   }
 }