Add the about_page key to shared_module manifest

chrome/common/extensions/manifest_url_handler.cc

Index: chrome/common/extensions/manifest_url_handler.cc
diff --git a/chrome/common/extensions/manifest_url_handler.cc b/chrome/common/extensions/manifest_url_handler.cc
index 557c1c2296abe3ff459c856c952b7a1ed2f8fc70..d09f6f9fcc0d87bfa1d4bcca1440193572271251 100644
--- a/chrome/common/extensions/manifest_url_handler.cc
+++ b/chrome/common/extensions/manifest_url_handler.cc
@@ -85,6 +85,11 @@ const GURL& ManifestURL::GetOptionsPage(const Extension* extension) {
 }
 
 // static
+const GURL& ManifestURL::GetAboutPage(const Extension* extension) {
+  return GetManifestURL(extension, keys::kOptionsPage);
+}
+
+// static
 const GURL ManifestURL::GetDetailsURL(const Extension* extension) {
   return extension->from_webstore() ?
       GURL(extension_urls::GetWebstoreItemDetailURLPrefix() + extension->id()) :
@@ -264,6 +269,57 @@ const std::vector<std::string> OptionsPageHandler::Keys() const {
   return SingleKey(keys::kOptionsPage);
 }
 
+AboutPageHandler::AboutPageHandler() {

[benwells, 2014/06/05 03:59:17]

This class is so similar to the options page class. I wonder if we could pull
out the commonality into a superclass. What do you think that would look like?

It would mean if we ever found / fixed a bug (e.g. in how we detect if a URL is
absolute) we wouldn't need to do it twice.

[sashab, 2014/06/10 05:44:13]

Not sure if they're _that_ similar. The options_page key allows for an absolute
URL for extensions, but a relative URL for packaged apps. But yes, I agree that
they are very similar.

A lot of it is already method calls, so I'm not sure how much could really be
abstracted out. Also, the error messages are different constants.

If you have any ideas though, let me know. I can also add a TODO.

[benwells, 2014/06/11 00:47:29]

I could imagine a URLManifestHandler, which when overridden you just provided
some fields like whether it is absolute or relative and error strings. But,
whatevs.

+}
+
+AboutPageHandler::~AboutPageHandler() {
+}
+
+bool AboutPageHandler::Parse(Extension* extension, base::string16* error) {
+  scoped_ptr<ManifestURL> manifest_url(new ManifestURL);
+  std::string options_str;

[benwells, 2014/06/05 03:59:17]

Nit: options_str -> about_str

[sashab, 2014/06/10 05:44:13]

Done.

+  if (!extension->manifest()->GetString(keys::kAboutPage, &options_str)) {
+    *error = base::ASCIIToUTF16(errors::kInvalidAboutPage);
+    return false;
+  }
+
+  GURL absolute(options_str);
+  if (absolute.is_valid()) {
+    *error = base::ASCIIToUTF16(errors::kInvalidAboutPageExpectRelativePath);
+    return false;
+  }
+  manifest_url->url_ = extension->GetResourceURL(options_str);
+  if (!manifest_url->url_.is_valid()) {
+    *error = base::ASCIIToUTF16(errors::kInvalidAboutPage);
+    return false;
+  }
+  extension->SetManifestData(keys::kAboutPage, manifest_url.release());
+  return true;
+}
+
+bool AboutPageHandler::Validate(const Extension* extension,
+                                std::string* error,
+                                std::vector<InstallWarning>* warnings) const {
+  // Validate path to the options page.
+  if (!extensions::ManifestURL::GetAboutPage(extension).is_empty()) {
+    const base::FilePath about_path =
+        extensions::file_util::ExtensionURLToRelativeFilePath(
+            extensions::ManifestURL::GetAboutPage(extension));
+    const base::FilePath path =
+        extension->GetResource(about_path).GetFilePath();
+    if (path.empty() || !base::PathExists(path)) {
+      *error = l10n_util::GetStringFUTF8(IDS_EXTENSION_LOAD_ABOUT_PAGE_FAILED,
+                                         about_path.LossyDisplayName());
+      return false;
+    }
+  }
+  return true;
+}
+
+const std::vector<std::string> AboutPageHandler::Keys() const {
+  return SingleKey(keys::kAboutPage);
+}
+
 URLOverridesHandler::URLOverridesHandler() {
 }