Added proper extension manifest parser for launcher_page.

Previously, AppListViewDelegate manually parsed the manifest. We now
have a proper parser class that generates appropriate errors for invalid
launcher_page fields in a manifest, with full valid and invalid unit
tests.

BUG=399131

Review URL: https://codereview.chromium.org/489753002

Cr-Commit-Position: refs/heads/master@{#291856}

chrome/browser/ui/app_list/app_list_view_delegate.cc

@@ -41,8 +41,8 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension_set.h"
-#include "extensions/common/manifest.h"
 #include "extensions/common/manifest_constants.h"
+#include "extensions/common/manifest_handlers/launcher_page_info.h"
 #include "grit/theme_resources.h"
 #include "ui/app_list/app_list_switches.h"
 #include "ui/app_list/app_list_view_delegate_observer.h"
@@ -135,20 +135,12 @@ void GetCustomLauncherPageUrls(content::BrowserContext* browser_context,
        it != enabled_extensions.end();
        ++it) {
     const extensions::Extension* extension = it->get();
-    const extensions::Manifest* manifest = extension->manifest();
-    if (!manifest->HasKey(extensions::manifest_keys::kLauncherPage))
+    extensions::LauncherPageInfo* info =
+        extensions::LauncherPageHandler::GetInfo(extension);
+    if (!info)
       continue;
-    std::string launcher_page_page;
-    if (!manifest->GetString(extensions::manifest_keys::kLauncherPagePage,
-                             &launcher_page_page)) {
-      // TODO(mgiuca): Add a proper manifest parser to catch this error properly
-      // and display it on the extensions page.
-      LOG(ERROR) << "Extension " << extension->id() << ": "
-                 << extensions::manifest_keys::kLauncherPage
-                 << " has no 'page' attribute; will be ignored.";
-      continue;
-    }
-    urls->push_back(extension->GetResourceURL(launcher_page_page));
+
+    urls->push_back(extension->GetResourceURL(info->page));
   }
 }
 

chrome/chrome_tests_unit.gypi

@@ -1839,6 +1839,7 @@
         'common/extensions/manifest_tests/extension_manifests_icons_unittest.cc',
         'common/extensions/manifest_tests/extension_manifests_initvalue_unittest.cc',
         'common/extensions/manifest_tests/extension_manifests_isolatedapp_unittest.cc',
+        'common/extensions/manifest_tests/extension_manifests_launcher_page_unittest.cc',
         'common/extensions/manifest_tests/extension_manifests_kiosk_unittest.cc',
         'common/extensions/manifest_tests/extension_manifests_launch_unittest.cc',
         'common/extensions/manifest_tests/extension_manifests_manifest_version_unittest.cc',

chrome/common/extensions/manifest_tests/extension_manifests_initvalue_unittest.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/command_line.h"
 #include "base/i18n/rtl.h"
 #include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
@@ -11,11 +12,19 @@
 #include "components/crx_file/id_util.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest_constants.h"
+#include "extensions/common/switches.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace extensions {
 
+namespace {
+
+// The ID of test manifests requiring whitelisting.
+const char kWhitelistID[] = "lmadimbbgapmngbiclpjjngmdickadpl";
+
+}  // namespace
+
 namespace errors = manifest_errors;
 namespace keys = manifest_keys;
 
@@ -23,6 +32,8 @@ class InitValueManifestTest : public ExtensionManifestTest {
 };
 
 TEST_F(InitValueManifestTest, InitFromValueInvalid) {
+  CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      extensions::switches::kWhitelistedExtensionID, kWhitelistID);
   Testcase testcases[] = {
     Testcase("init_invalid_version_missing.json", errors::kInvalidVersion),
     Testcase("init_invalid_version_invalid.json", errors::kInvalidVersion),
@@ -32,6 +43,12 @@ TEST_F(InitValueManifestTest, InitFromValueInvalid) {
              errors::kInvalidDescription),
     Testcase("init_invalid_icons_invalid.json", errors::kInvalidIcons),
     Testcase("init_invalid_icons_path_invalid.json", errors::kInvalidIconPath),
+    Testcase("init_invalid_launcher_page_invalid.json",
+             errors::kInvalidLauncherPage),
+    Testcase("init_invalid_launcher_page_page_missing.json",
+             errors::kLauncherPagePageRequired),
+    Testcase("init_invalid_launcher_page_page_invalid.json",
+             errors::kInvalidLauncherPagePage),
     Testcase("init_invalid_script_invalid.json",
              errors::kInvalidContentScriptsList),
     Testcase("init_invalid_script_item_invalid.json",

chrome/common/extensions/manifest_tests/extension_manifests_launcher_page_unittest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/command_line.h"
+#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
+#include "extensions/common/constants.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/manifest_handlers/launcher_page_info.h"
+#include "extensions/common/switches.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace extensions {
+
+namespace {
+
+// The ID of test manifests requiring whitelisting.
+const char kWhitelistID[] = "lmadimbbgapmngbiclpjjngmdickadpl";
+
+}  // namespace
+
+typedef ExtensionManifestTest LauncherPageManifestTest;
+
+TEST_F(LauncherPageManifestTest, ValidLauncherPage) {
+  CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      extensions::switches::kWhitelistedExtensionID, kWhitelistID);
+  scoped_refptr<extensions::Extension> extension(
+      LoadAndExpectSuccess("init_valid_launcher_page.json"));
+  ASSERT_TRUE(extension.get());
+  extensions::LauncherPageInfo* info =
+      extensions::LauncherPageHandler::GetInfo(extension.get());
+  ASSERT_TRUE(info);
+  EXPECT_EQ("test.html", info->page);
+}
+
+}  // namespace extensions

chrome/test/data/extensions/manifest_tests/init_invalid_launcher_page_invalid.json

+{
+   "name": "my extension",
+   // chrome-extension://lmadimbbgapmngbiclpjjngmdickadpl
+   "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAv7Qi8BghDr2b2Wdg5vv7vGWQxWpo6dD4Jt3okhb3oOF0zmnhr1G/e16J8WxtygaF2mshjVP11/j/yu8n7AsrFw5hwi3ROwmsB8T1vB+rHGh9NfF/iX8w1z2rLkXlPemHof7nzC67Y3TRrl0ONqIO4ef9z4NEnnzQ0EeIX51924G5pj9YjTderWIso9+8mehelDwMgBZu66T1jTuxq4SOEvuDe9IKXwJfVPfhTf0f8YAH+NUdleKY+2zR7u8BDK42OkhhKs4XB3ZHDTr+n7KObXYXJukpr/eNqbXyU4lsUlJobFDzygZxjEOw87HhP9fK3V0v+eYrQ4+9JctqxorT5wIDAQAB",
+   "version": "1.0.0.0",
+   "app": {
+     "background": {
+       "scripts": ["dummy.js"]
+     }
+   },
+   "launcher_page": 42
+}

chrome/test/data/extensions/manifest_tests/init_invalid_launcher_page_page_invalid.json

+{
+   "name": "my extension",
+   // chrome-extension://lmadimbbgapmngbiclpjjngmdickadpl
+   "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAv7Qi8BghDr2b2Wdg5vv7vGWQxWpo6dD4Jt3okhb3oOF0zmnhr1G/e16J8WxtygaF2mshjVP11/j/yu8n7AsrFw5hwi3ROwmsB8T1vB+rHGh9NfF/iX8w1z2rLkXlPemHof7nzC67Y3TRrl0ONqIO4ef9z4NEnnzQ0EeIX51924G5pj9YjTderWIso9+8mehelDwMgBZu66T1jTuxq4SOEvuDe9IKXwJfVPfhTf0f8YAH+NUdleKY+2zR7u8BDK42OkhhKs4XB3ZHDTr+n7KObXYXJukpr/eNqbXyU4lsUlJobFDzygZxjEOw87HhP9fK3V0v+eYrQ4+9JctqxorT5wIDAQAB",
+   "version": "1.0.0.0",
+   "app": {
+     "background": {
+       "scripts": ["dummy.js"]
+     }
+   },
+   "launcher_page": {
+     "page": 42
+   }
+}

chrome/test/data/extensions/manifest_tests/init_invalid_launcher_page_page_missing.json

+{
+   "name": "my extension",
+   // chrome-extension://lmadimbbgapmngbiclpjjngmdickadpl
+   "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAv7Qi8BghDr2b2Wdg5vv7vGWQxWpo6dD4Jt3okhb3oOF0zmnhr1G/e16J8WxtygaF2mshjVP11/j/yu8n7AsrFw5hwi3ROwmsB8T1vB+rHGh9NfF/iX8w1z2rLkXlPemHof7nzC67Y3TRrl0ONqIO4ef9z4NEnnzQ0EeIX51924G5pj9YjTderWIso9+8mehelDwMgBZu66T1jTuxq4SOEvuDe9IKXwJfVPfhTf0f8YAH+NUdleKY+2zR7u8BDK42OkhhKs4XB3ZHDTr+n7KObXYXJukpr/eNqbXyU4lsUlJobFDzygZxjEOw87HhP9fK3V0v+eYrQ4+9JctqxorT5wIDAQAB",
+   "version": "1.0.0.0",
+   "app": {
+     "background": {
+       "scripts": ["dummy.js"]
+     }
+   },
+   "launcher_page": {}
+}

chrome/test/data/extensions/manifest_tests/init_valid_launcher_page.json

+{
+   "name": "my extension",
+   // chrome-extension://lmadimbbgapmngbiclpjjngmdickadpl
+   "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAv7Qi8BghDr2b2Wdg5vv7vGWQxWpo6dD4Jt3okhb3oOF0zmnhr1G/e16J8WxtygaF2mshjVP11/j/yu8n7AsrFw5hwi3ROwmsB8T1vB+rHGh9NfF/iX8w1z2rLkXlPemHof7nzC67Y3TRrl0ONqIO4ef9z4NEnnzQ0EeIX51924G5pj9YjTderWIso9+8mehelDwMgBZu66T1jTuxq4SOEvuDe9IKXwJfVPfhTf0f8YAH+NUdleKY+2zR7u8BDK42OkhhKs4XB3ZHDTr+n7KObXYXJukpr/eNqbXyU4lsUlJobFDzygZxjEOw87HhP9fK3V0v+eYrQ4+9JctqxorT5wIDAQAB",
+   "version": "1.0.0.0",
+   "app": {
+     "background": {
+       "scripts": ["dummy.js"]
+     }
+   },
+   "launcher_page": {
+     "page": "test.html"
+   }
+}

extensions/common/common_manifest_handlers.cc

@@ -11,6 +11,7 @@
 #include "extensions/common/manifest_handlers/icons_handler.h"
 #include "extensions/common/manifest_handlers/incognito_info.h"
 #include "extensions/common/manifest_handlers/kiosk_mode_info.h"
+#include "extensions/common/manifest_handlers/launcher_page_info.h"
 #include "extensions/common/manifest_handlers/nacl_modules_handler.h"
 #include "extensions/common/manifest_handlers/offline_enabled_info.h"
 #include "extensions/common/manifest_handlers/sandboxed_page_info.h"
@@ -29,6 +30,7 @@ void RegisterCommonManifestHandlers() {
   (new IconsHandler)->Register();
   (new IncognitoHandler)->Register();
   (new KioskModeHandler)->Register();
+  (new LauncherPageHandler)->Register();
 #if !defined(DISABLE_NACL)
   (new NaClModulesHandler)->Register();
 #endif

extensions/common/manifest_constants.cc

@@ -465,6 +465,9 @@ const char kInvalidKioskOnly[] =
     "Invalid value for 'kiosk_only'.";
 const char kInvalidKioskOnlyButNotEnabled[] =
     "The 'kiosk_only' key is set, but 'kiosk_enabled' is not set.";
+const char kInvalidLauncherPage[] = "Invalid value for 'launcher_page'.";
+const char kInvalidLauncherPagePage[] =
+    "Invalid value for 'launcher_page.page'.";
 const char kInvalidLaunchContainer[] =
     "Invalid value for 'app.launch.container'.";
 const char kInvalidLaunchValue[] =
@@ -666,6 +669,8 @@ const char kInsecureContentSecurityPolicy[] =
     " see http://developer.chrome.com/extensions/contentSecurityPolicy.html";
 const char kKeyIsDeprecatedWithReplacement[] =
     "Key \"*\" is deprecated.  Key \"*\" should be used instead.";
+const char kLauncherPagePageRequired[] =
+    "The 'launcher_page.page' key is required.";
 const char kLaunchPathAndExtentAreExclusive[] =
     "The 'app.launch.local_path' and 'app.urls' keys cannot both be set.";
 const char kLaunchPathAndURLAreExclusive[] =

extensions/common/manifest_constants.h

@@ -355,6 +355,8 @@ extern const char kInvalidKeyBindingUnknownPlatform[];
 extern const char kInvalidKioskEnabled[];
 extern const char kInvalidKioskOnly[];
 extern const char kInvalidKioskOnlyButNotEnabled[];
+extern const char kInvalidLauncherPage[];
+extern const char kInvalidLauncherPagePage[];
 extern const char kInvalidLaunchContainer[];
 extern const char kInvalidLaunchValue[];
 extern const char kInvalidLaunchValueContainer[];
@@ -450,6 +452,7 @@ extern const char kInvalidWebURLs[];
 extern const char kInvalidZipHash[];
 extern const char kInsecureContentSecurityPolicy[];
 extern const char kKeyIsDeprecatedWithReplacement[];
+extern const char kLauncherPagePageRequired[];
 extern const char kLaunchPathAndExtentAreExclusive[];
 extern const char kLaunchPathAndURLAreExclusive[];
 extern const char kLaunchURLRequired[];

extensions/common/manifest_handlers/launcher_page_info.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "extensions/common/manifest_handlers/launcher_page_info.h"
+
+#include "base/files/file_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "extensions/common/manifest.h"
+#include "extensions/common/manifest_constants.h"
+#include "grit/extensions_strings.h"
+#include "ui/base/l10n/l10n_util.h"
+
+namespace extensions {
+
+LauncherPageHandler::LauncherPageHandler() {
+}
+
+LauncherPageHandler::~LauncherPageHandler() {
+}
+
+// static
+LauncherPageInfo* LauncherPageHandler::GetInfo(const Extension* extension) {
+  return static_cast<LauncherPageInfo*>(
+      extension->GetManifestData(manifest_keys::kLauncherPage));
+}
+
+bool LauncherPageHandler::Parse(Extension* extension, base::string16* error) {
+  const extensions::Manifest* manifest = extension->manifest();
+  scoped_ptr<LauncherPageInfo> launcher_page_info(new LauncherPageInfo);
+  const base::DictionaryValue* launcher_page_dict = NULL;
+  if (!manifest->GetDictionary(manifest_keys::kLauncherPage,
+                               &launcher_page_dict)) {
+    *error = base::ASCIIToUTF16(manifest_errors::kInvalidLauncherPage);
+    return false;
+  }
+
+  if (!manifest->HasPath(extensions::manifest_keys::kLauncherPagePage)) {
+    *error = base::ASCIIToUTF16(manifest_errors::kLauncherPagePageRequired);
+    return false;
+  }
+
+  std::string launcher_page_page;
+  if (!manifest->GetString(extensions::manifest_keys::kLauncherPagePage,
+                           &launcher_page_page)) {
+    *error = base::ASCIIToUTF16(manifest_errors::kInvalidLauncherPagePage);
+    return false;
+  }
+
+  launcher_page_info->page = launcher_page_page;
+
+  extension->SetManifestData(manifest_keys::kLauncherPage,
+                             launcher_page_info.release());
+  return true;
+}
+
+bool LauncherPageHandler::Validate(
+    const Extension* extension,
+    std::string* error,
+    std::vector<InstallWarning>* warnings) const {
+  LauncherPageInfo* info = GetInfo(extension);
+  const base::FilePath path = extension->GetResource(info->page).GetFilePath();
+  if (!base::PathExists(path)) {
+    *error = l10n_util::GetStringFUTF8(IDS_EXTENSION_LOAD_LAUNCHER_PAGE_FAILED,
+                                       base::UTF8ToUTF16(info->page));
+    return false;
+  }
+
+  return true;
+}
+
+const std::vector<std::string> LauncherPageHandler::Keys() const {
+  return SingleKey(manifest_keys::kLauncherPage);
+}
+
+}  // namespace extensions

extensions/common/manifest_handlers/launcher_page_info.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef EXTENSIONS_COMMON_MANIFEST_HANDLERS_LAUNCHER_PAGE_INFO_H_
+#define EXTENSIONS_COMMON_MANIFEST_HANDLERS_LAUNCHER_PAGE_INFO_H_
+
+#include <string>
+#include <vector>
+
+#include "base/strings/string16.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/manifest_handler.h"
+
+namespace extensions {
+
+struct LauncherPageInfo : public Extension::ManifestData {
+  // The page URL.
+  std::string page;
+};
+
+// Parses the "launcher_page" manifest key.
+class LauncherPageHandler : public ManifestHandler {
+ public:
+  LauncherPageHandler();
+  virtual ~LauncherPageHandler();
+
+  // Gets the LauncherPageInfo for a given |extension|.
+  static LauncherPageInfo* GetInfo(const Extension* extension);
+
+  // ManifestHandler overrides:
+  virtual bool Parse(Extension* extension, base::string16* error) OVERRIDE;
+  virtual bool Validate(const Extension* extension,
+                        std::string* error,
+                        std::vector<InstallWarning>* warnings) const OVERRIDE;
+
+ private:
+  virtual const std::vector<std::string> Keys() const OVERRIDE;
+
+  DISALLOW_COPY_AND_ASSIGN(LauncherPageHandler);
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_COMMON_MANIFEST_HANDLERS_LAUNCHER_PAGE_INFO_H_

extensions/extensions.gyp

@@ -131,6 +131,8 @@
         'common/manifest_handlers/incognito_info.h',
         'common/manifest_handlers/kiosk_mode_info.cc',
         'common/manifest_handlers/kiosk_mode_info.h',
+        'common/manifest_handlers/launcher_page_info.cc',
+        'common/manifest_handlers/launcher_page_info.h',
         'common/manifest_handlers/offline_enabled_info.cc',
         'common/manifest_handlers/offline_enabled_info.h',
         'common/manifest_handlers/permissions_parser.cc',

extensions/extensions_strings.grd

@@ -187,6 +187,9 @@
       <message name="IDS_EXTENSION_LOAD_ICON_FAILED" desc="">
         Could not load extension icon '<ph name="ICON">$1<ex>icon.png</ex></ph>'.
       </message>
+      <message name="IDS_EXTENSION_LOAD_LAUNCHER_PAGE_FAILED" desc="">
+        Could not load launcher page '<ph name="PAGE">$1<ex>index.html</ex></ph>'.
+      </message>
       <message name="IDS_EXTENSION_LOCALES_NO_DEFAULT_LOCALE_SPECIFIED" desc="">
         Localization used, but default_locale wasn't specified in the manifest.
       </message>