Fix for ERROR:url_pattern_set.cc(240)] Invalid url pattern: chrome://print/*

chrome/browser/extensions/extension_prefs_unittest.cc

Index: chrome/browser/extensions/extension_prefs_unittest.cc
diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc
index d9067d3c9daaea0510bb1f46e465f4a605a9b0aa..ed8c68d2885c55b17f325b2f5dd9294cce753be1 100644
--- a/chrome/browser/extensions/extension_prefs_unittest.cc
+++ b/chrome/browser/extensions/extension_prefs_unittest.cc
@@ -913,4 +913,46 @@ class ExtensionPrefsClearLastLaunched : public ExtensionPrefsTest {
 };
 TEST_F(ExtensionPrefsClearLastLaunched, ExtensionPrefsClearLastLaunched) {}
 
+class ExtensionPrefsChromePattern : public ExtensionPrefsTest {
+ public:
+  ~ExtensionPrefsChromePattern() override {}
+  void Initialize() override {
+    base::DictionaryValue dictionary;
+    dictionary.SetString(manifest_keys::kName, "test");
+    dictionary.SetString(manifest_keys::kVersion, "0.1");
+    webstore_extension_ = prefs_.AddExtensionWithManifestAndFlags(
+        dictionary, Manifest::COMPONENT, Extension::NO_FLAGS);
+
+    APIPermissionSet api_perms;
+    api_perms.insert(APIPermission::kTab);
+    api_perms.insert(APIPermission::kBookmark);
+    api_perms.insert(APIPermission::kHistory);
+
+    ManifestPermissionSet empty_manifest_permissions;
+
+    URLPatternSet ehosts, shosts;
+    AddPattern(&shosts, "chrome://print/*");

[not at google - send to devlin, 2015/04/09 14:56:07]

Thanks for writing this test, getting there, but I think you could do this in a
more principled way. Perhaps the other tests should be more like this too, but,
eh:

webstore_extension_ = ExtensionBuilder()
  .SetManifest(DictionaryBuilder()
    .Set(manifest_keys::kName, "test")
    .Set(manifest_keys::kVersion, "0.1"))
  .SetLocation(Manifest::COMPONENT).Build();

// (btw, a utility in TestExtensionPrefs to add an Extension* would be good),
// it could let you write prefs_->AddExtension(extension.get()).
prefs_->OnExtensionInstalled(extension.get(),
                             Extension::ENABLED,
                             syncer::StringOrdinal::CreateInitialOrdinal(),
                             std::string());

and then in the Verify() method, read a new Extension* back out and assert that
it can access "chrome://print/*".

and I'd feel more comfortable if you tested here that a non-Component extension
*can't* access chrome://print.

and instead of "webstore_extension_" I'd call it just "component_extension_".

[Deepak, 2015/04/10 05:16:34]

1) I have added a new function in test_extension_prefs.cc so that we can use
existing infrastructure with Manifest::COMPONENT location.
2) Checking "chrome://print/*" in Verify().
3) Code for When we have non-Component extension then we will get false as it
can not access "chrome://print/*".
4) other nit's addressed.

+
+    active_perms_ = new PermissionSet(api_perms, empty_manifest_permissions,
+                                      ehosts, shosts);
+    // Set the active permissions.
+    prefs()->SetActivePermissions(webstore_extension_->id(),
+                                  active_perms_.get());

[not at google - send to devlin, 2015/04/09 14:56:07]

I don't know exactly why that GetInstalledExtensionInfo is failing without
digging into the code. However, I think the new code you have in
extension_prefs.cc now (i.e. deleting what you've commented out) is fine.

[Deepak, 2015/04/10 05:16:34]

Done.

+  }
+
+  void Verify() override {
+    URLPatternSet scriptable_hosts;
+    std::string pref_key = "active_permissions.scriptable_host";
+    prefs()->SetAllowFileAccess(webstore_extension_->id(), true);

[not at google - send to devlin, 2015/04/09 14:56:07]

Why is file access relevant?

[Deepak, 2015/04/10 05:16:34]

Done.

+    EXPECT_TRUE(prefs()->ReadPrefAsURLPatternSet(
+        webstore_extension_->id(), pref_key, &scriptable_hosts, 15));
+  }
+
+ private:
+  scoped_refptr<PermissionSet> active_perms_;
+  scoped_refptr<Extension> webstore_extension_;
+};
+TEST_F(ExtensionPrefsChromePattern, ExtensionPrefsChromePattern) {
+}
+
 }  // namespace extensions