Move the validation of icons for page actions to load time.

This is in line with how we validate content scripts and
enables installing page actions as .crx filed.

BUG=None
TEST=Install a Page Action as a .crx file. It should install.

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

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

3 files changed, 22 insertions, 16 deletions

diff --git a/chrome/browser/extensions/extension.cc b/chrome/browser/extensions/extension.cc
index da1e841..51d7983 100644
--- a/chrome/browser/extensions/extension.cc
+++ b/chrome/browser/extensions/extension.cc
@@ -112,8 +112,6 @@ const char* Extension::kInvalidZipHashError =
     "Required key 'zip_hash' is missing or invalid.";
 const char* Extension::kMissingFileError =
     "At least one js or css file is required for 'content_scripts[*]'.";
-const char* Extension::kMissingPageActionIcon =
-    "Unable to find 'page_actions[*].icon'";
 const char* Extension::kInvalidThemeError =
     "Invalid value for 'theme'.";
 const char* Extension::kInvalidThemeImagesError =
@@ -363,11 +361,6 @@ PageAction* Extension::LoadPageActionHelper(
     return NULL;
   }
   FilePath icon_path = path_.AppendASCII(icon);
-  if (!file_util::PathExists(icon_path)) {
-    *error = ExtensionErrorUtils::FormatErrorMessage(kMissingPageActionIcon,
-        IntToString(definition_index));
-    return NULL;
-  }
   result->set_icon_path(icon_path);
 
   // Read the page action |id|.
diff --git a/chrome/browser/extensions/extension.h b/chrome/browser/extensions/extension.h
index f944142..ece0578 100644
--- a/chrome/browser/extensions/extension.h
+++ b/chrome/browser/extensions/extension.h
@@ -26,10 +26,10 @@ class Extension {
   // What an extension was loaded from.
   enum Location {
     INVALID,
-    INTERNAL,  // A crx file from the internal Extensions directory
+    INTERNAL,  // A crx file from the internal Extensions directory.
     EXTERNAL,  // A crx file from an external directory (via eg the registry
-               // on Windows)
-    LOAD  // --load-extension 
+               // on Windows).
+    LOAD  // --load-extension.
   };
 
   // The name of the manifest inside an extension.
@@ -101,7 +101,6 @@ class Extension {
   static const char* kInvalidThemeColorsError;
   static const char* kInvalidThemeTintsError;
   static const char* kMissingFileError;
-  static const char* kMissingPageActionIcon;
 
   // The number of bytes in a legal id.
   static const size_t kIdSize;
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 23c37fe..aa9145f 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -490,7 +490,7 @@ Extension* ExtensionsServiceBackend::LoadExtension(
     extension->set_location(Extension::INTERNAL);
 
   // TODO(glen): Add theme resource validation here. http://crbug.com/11678
-  // Validate that claimed script resources actually exist. 
+  // Validate that claimed script resources actually exist.
   for (size_t i = 0; i < extension->content_scripts().size(); ++i) {
     const UserScript& script = extension->content_scripts()[i];
 
@@ -498,8 +498,8 @@ Extension* ExtensionsServiceBackend::LoadExtension(
       const FilePath& path = script.js_scripts()[j].path();
       if (!file_util::PathExists(path)) {
         ReportExtensionLoadError(extension_path,
-          StringPrintf("Could not load '%s' for content script.",
-          WideToUTF8(path.ToWStringHack()).c_str()));
+            StringPrintf("Could not load '%s' for content script.",
+            WideToUTF8(path.ToWStringHack()).c_str()));
         return NULL;
       }
     }
@@ -508,13 +508,27 @@ Extension* ExtensionsServiceBackend::LoadExtension(
       const FilePath& path = script.css_scripts()[j].path();
       if (!file_util::PathExists(path)) {
         ReportExtensionLoadError(extension_path,
-          StringPrintf("Could not load '%s' for content script.",
-          WideToUTF8(path.ToWStringHack()).c_str()));
+            StringPrintf("Could not load '%s' for content script.",
+            WideToUTF8(path.ToWStringHack()).c_str()));
         return NULL;
       }
     }
   }
 
+  // Validate icon location for page actions.
+  const PageActionMap& page_actions = extension->page_actions();
+  for (PageActionMap::const_iterator i(page_actions.begin());
+       i != page_actions.end(); ++i) {
+    PageAction* page_action = i->second;
+    FilePath path = page_action->icon_path();
+    if (!file_util::PathExists(path)) {
+      ReportExtensionLoadError(extension_path,
+          StringPrintf("Could not load icon '%s' for page action.",
+          WideToUTF8(path.ToWStringHack()).c_str()));
+      return NULL;
+    }
+  }
+
   return extension.release();
 }