Fix chrome_url_overides to work with packed extensions.

The InitFromValue code was modifying the JSON with some local data which
was then being re-used by the unpacker and sent up to the browser which
caused it to use this temporary local data as if it was the original
manifest source.

BUG=24398
TEST=pack and install override_igoogle extension

Review URL: http://codereview.chromium.org/276074

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

6 files changed, 47 insertions, 38 deletions

diff --git a/chrome/browser/extensions/extension_dom_ui.cc b/chrome/browser/extensions/extension_dom_ui.cc
index a3e9ad2..3661d6d 100644
--- a/chrome/browser/extensions/extension_dom_ui.cc
+++ b/chrome/browser/extensions/extension_dom_ui.cc
@@ -138,8 +138,8 @@ bool ExtensionDOMUI::HandleChromeURLOverride(GURL* url, Profile* profile) {
 
 // static
 void ExtensionDOMUI::RegisterChromeURLOverrides(
-    Profile* profile, const DictionaryValue* new_overrides) {
-  if (!new_overrides)
+    Profile* profile, const Extension::URLOverrideMap& overrides) {
+  if (overrides.empty())
     return;
 
   PrefService* prefs = profile->GetPrefs();
@@ -148,19 +148,13 @@ void ExtensionDOMUI::RegisterChromeURLOverrides(
 
   // For each override provided by the extension, add it to the front of
   // the override list if it's not already in the list.
-  DictionaryValue::key_iterator iter = new_overrides->begin_keys();
-  for (;iter != new_overrides->end_keys(); ++iter) {
-    Value* val;
-    new_overrides->Get(*iter, &val);
-    std::string string_val;
-    if (!val->GetAsString(&string_val)) {
-      NOTREACHED();
-      continue;
-    }
+  Extension::URLOverrideMap::const_iterator iter = overrides.begin();
+  for (;iter != overrides.end(); ++iter) {
+    const std::wstring key = UTF8ToWide((*iter).first);
     ListValue* page_overrides;
-    if (!all_overrides->GetList(*iter, &page_overrides)) {
+    if (!all_overrides->GetList(key, &page_overrides)) {
       page_overrides = new ListValue();
-      all_overrides->Set(*iter, page_overrides);
+      all_overrides->Set(key, page_overrides);
     } else {
       // Verify that the override isn't already in the list.
       ListValue::iterator i = page_overrides->begin();
@@ -170,7 +164,7 @@ void ExtensionDOMUI::RegisterChromeURLOverrides(
           NOTREACHED();
           continue;
         }
-        if (override_val == string_val)
+        if (override_val == (*iter).first)
           break;
       }
       // This value is already in the list, leave it alone.
@@ -179,7 +173,7 @@ void ExtensionDOMUI::RegisterChromeURLOverrides(
     }
     // Insert the override at the front of the list.  Last registered override
     // wins.
-    page_overrides->Insert(0, val->DeepCopy());
+    page_overrides->Insert(0, new StringValue((*iter).second.spec()));
   }
 }
 
@@ -226,27 +220,24 @@ void ExtensionDOMUI::UnregisterChromeURLOverride(const std::string& page,
 
 // static
 void ExtensionDOMUI::UnregisterChromeURLOverrides(
-    Profile* profile, const DictionaryValue* new_overrides) {
-  if (!new_overrides)
+    Profile* profile, const Extension::URLOverrideMap& overrides) {
+  if (overrides.empty())
     return;
   PrefService* prefs = profile->GetPrefs();
   DictionaryValue* all_overrides =
       prefs->GetMutableDictionary(kExtensionURLOverrides);
-  DictionaryValue::key_iterator iter = new_overrides->begin_keys();
-  for (; iter != new_overrides->end_keys(); ++iter) {
-    Value* val;
-    if (!new_overrides->Get(*iter, &val)) {
-      NOTREACHED();
-      return;
-    }
+  Extension::URLOverrideMap::const_iterator iter = overrides.begin();
+  for (;iter != overrides.end(); ++iter) {
+    std::wstring page = UTF8ToWide((*iter).first);
     ListValue* page_overrides;
-    if (!all_overrides->GetList(*iter, &page_overrides)) {
+    if (!all_overrides->GetList(page, &page_overrides)) {
       // If it's being unregistered, it should already be in the list.
       NOTREACHED();
       continue;
     } else {
-      UnregisterAndReplaceOverride(WideToUTF8(*iter), profile, page_overrides,
-                                   val);
+      StringValue override((*iter).second.spec());
+      UnregisterAndReplaceOverride((*iter).first, profile,
+                                   page_overrides, &override);
     }
   }
 }
diff --git a/chrome/browser/extensions/extension_dom_ui.h b/chrome/browser/extensions/extension_dom_ui.h
index a729167..20549ac 100644
--- a/chrome/browser/extensions/extension_dom_ui.h
+++ b/chrome/browser/extensions/extension_dom_ui.h
@@ -8,6 +8,7 @@
 #include "base/scoped_ptr.h"
 #include "chrome/browser/dom_ui/dom_ui.h"
 #include "chrome/browser/extensions/extension_function_dispatcher.h"
+#include "chrome/common/extensions/extension.h"
 
 class ListValue;
 class PrefService;
@@ -15,7 +16,7 @@ class TabContents;
 
 // This class implements DOMUI for extensions and allows extensions to put UI in
 // the main tab contents area.
-class ExtensionDOMUI 
+class ExtensionDOMUI
     : public DOMUI,
       public ExtensionFunctionDispatcher::Delegate {
  public:
@@ -43,9 +44,9 @@ class ExtensionDOMUI
   // Page names are the keys, and chrome-extension: URLs are the values.
   // (e.g. { "newtab": "chrome-extension://<id>/my_new_tab.html" }
   static void RegisterChromeURLOverrides(Profile* profile,
-                                         const DictionaryValue* overrides);
+    const Extension::URLOverrideMap& overrides);
   static void UnregisterChromeURLOverrides(Profile* profile,
-                                           const DictionaryValue* overrides);
+    const Extension::URLOverrideMap& overrides);
   static void UnregisterChromeURLOverride(const std::string& page,
                                           Profile* profile,
                                           Value* override);
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 4ead417..a7354b1 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -1036,13 +1036,15 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
       *error = errors::kInvalidChromeURLOverrides;
       return false;
     }
+
     // Validate that the overrides are all strings
     DictionaryValue::key_iterator iter = overrides->begin_keys();
     while (iter != overrides->end_keys()) {
+      std::string page = WideToUTF8(*iter);
       // For now, only allow the new tab page.  Others will work when we remove
       // this check, but let's keep it simple for now.
       // TODO(erikkay) enable other pages as well
-      if (WideToUTF8(*iter) != chrome::kChromeUINewTabHost) {
+      if (page != chrome::kChromeUINewTabHost) {
         *error = errors::kInvalidChromeURLOverrides;
         return false;
       }
@@ -1052,14 +1054,18 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
         return false;
       }
       // Replace the entry with a fully qualified chrome-extension:// URL.
-      GURL url = GetResourceURL(val);
-      overrides->SetString(*iter, url.spec());
+      chrome_url_overrides_[page] = GetResourceURL(val);
       ++iter;
     }
-    chrome_url_overrides_.reset(
-        static_cast<DictionaryValue*>(overrides->DeepCopy()));
   }
 
+  // Although |source| is passed in as a const, it's still possible to modify
+  // it.  This is dangerous since the utility process re-uses |source| after
+  // it calls InitFromValue, passing it up to the browser process which calls
+  // InitFromValue again.  As a result, we need to make sure that nobody
+  // accidentally modifies it.
+  DCHECK(source.Equals(manifest_value_.get()));
+
   return true;
 }
 
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index f506ffd..eac617f 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -26,6 +26,7 @@
 class Extension {
  public:
   typedef std::vector<URLPattern> HostPermissions;
+  typedef std::map<const std::string, GURL> URLOverrideMap;
 
   // What an extension was loaded from.
   enum Location {
@@ -259,8 +260,8 @@ class Extension {
   }
 
   // Chrome URL overrides (see ExtensionOverrideUI).
-  DictionaryValue* GetChromeURLOverrides() const {
-    return chrome_url_overrides_.get();
+  const URLOverrideMap& GetChromeURLOverrides() const {
+    return chrome_url_overrides_;
   }
 
   // Runtime data:
@@ -384,7 +385,7 @@ class Extension {
 
   // A map of chrome:// hostnames (newtab, downloads, etc.) to Extension URLs
   // which override the handling of those URLs.
-  scoped_ptr<DictionaryValue> chrome_url_overrides_;
+  URLOverrideMap chrome_url_overrides_;
 
   // Runtime data:
 
diff --git a/chrome/test/data/extensions/samples/override_igoogle/manifest.json b/chrome/test/data/extensions/samples/override_igoogle/manifest.json
new file mode 100644
index 0000000..461da24
--- /dev/null
+++ b/chrome/test/data/extensions/samples/override_igoogle/manifest.json
@@ -0,0 +1,7 @@
+{
+  "name": "iGoogle new tab page",
+  "version": "0.1",
+  "chrome_url_overrides": {
+    "newtab": "redirect.html"
+  }
+}
\ No newline at end of file
diff --git a/chrome/test/data/extensions/samples/override_igoogle/redirect.html b/chrome/test/data/extensions/samples/override_igoogle/redirect.html
new file mode 100755
index 0000000..35117f4
--- /dev/null
+++ b/chrome/test/data/extensions/samples/override_igoogle/redirect.html
@@ -0,0 +1,3 @@
+<head>
+<meta http-equiv="refresh"content="0;URL=http://www.google.com/ig">
+</head>