[Extensions Cleanup] Sane-ify some extension action manifest constants

Clean up various extension action-related manifest constants:
- Instead of prefixing with "PageAction", just say "Action".
- Rename kPageActionId to kFileBrowserHandlerId, since that's the only
  thing that uses it.
- Remove undefined constant kInvalidPageActionPopupHeight
- Combine kInvalidPageActionPopup and kInvalidPageActionPopupPath
- Remove unused constants kPageActionTypePermanent and
  kPageActionTypeTab

Bug: 893373
Change-Id: Ic22f6d6f0ba1399c1f2dbbaeea3eac62e954e32d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1559767
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649652}

chrome/common/extensions/api/extension_action/action_info.cc

@@ -65,15 +65,15 @@ std::unique_ptr<ActionInfo> ActionInfo::Load(const Extension* extension,
   // Read the page action |default_icon| (optional).
   // The |default_icon| value can be either dictionary {icon size -> icon path}
   // or non empty string value.
-  if (dict->HasKey(keys::kPageActionDefaultIcon)) {
+  if (dict->HasKey(keys::kActionDefaultIcon)) {
     const base::DictionaryValue* icons_value = NULL;
     std::string default_icon;
-    if (dict->GetDictionary(keys::kPageActionDefaultIcon, &icons_value)) {
+    if (dict->GetDictionary(keys::kActionDefaultIcon, &icons_value)) {
       if (!manifest_handler_helpers::LoadIconsFromDictionary(
               icons_value, &result->default_icon, error)) {
         return nullptr;
       }
-    } else if (dict->GetString(keys::kPageActionDefaultIcon, &default_icon) &&
+    } else if (dict->GetString(keys::kActionDefaultIcon, &default_icon) &&
                manifest_handler_helpers::NormalizeAndValidatePath(
                    &default_icon)) {
       // Choose the most optimistic (highest) icon density regardless of the
@@ -82,26 +82,25 @@ std::unique_ptr<ActionInfo> ActionInfo::Load(const Extension* extension,
       result->default_icon.Add(extension_misc::EXTENSION_ICON_GIGANTOR,
                                default_icon);
     } else {
-      *error = base::ASCIIToUTF16(errors::kInvalidPageActionIconPath);
+      *error = base::ASCIIToUTF16(errors::kInvalidActionDefaultIcon);
       return nullptr;
     }
   }
 
   // Read the page action title from |default_title| if present, |name| if not
   // (both optional).
-  if (dict->HasKey(keys::kPageActionDefaultTitle)) {
-    if (!dict->GetString(keys::kPageActionDefaultTitle,
-                         &result->default_title)) {
-      *error = base::ASCIIToUTF16(errors::kInvalidPageActionDefaultTitle);
+  if (dict->HasKey(keys::kActionDefaultTitle)) {
+    if (!dict->GetString(keys::kActionDefaultTitle, &result->default_title)) {
+      *error = base::ASCIIToUTF16(errors::kInvalidActionDefaultTitle);
       return nullptr;
     }
   }
 
   // Read the action's default popup (optional).
-  if (dict->HasKey(keys::kPageActionDefaultPopup)) {
+  if (dict->HasKey(keys::kActionDefaultPopup)) {
     std::string url_str;
-    if (!dict->GetString(keys::kPageActionDefaultPopup, &url_str)) {
-      *error = base::ASCIIToUTF16(errors::kInvalidPageActionPopup);
+    if (!dict->GetString(keys::kActionDefaultPopup, &url_str)) {
+      *error = base::ASCIIToUTF16(errors::kInvalidActionDefaultPopup);
       return nullptr;
     }
 
@@ -110,8 +109,7 @@ std::unique_ptr<ActionInfo> ActionInfo::Load(const Extension* extension,
       result->default_popup_url = Extension::GetResourceURL(extension->url(),
                                                             url_str);
       if (!result->default_popup_url.is_valid()) {
-        *error = ErrorUtils::FormatErrorMessageUTF16(
-            errors::kInvalidPageActionPopupPath, url_str);
+        *error = base::ASCIIToUTF16(errors::kInvalidActionDefaultPopup);
         return nullptr;
       }
     } else {

chrome/common/extensions/api/extension_action/page_action_manifest_unittest.cc

@@ -57,7 +57,7 @@ TEST_F(PageActionManifestTest, ManifestVersion2DoesntAllowLegacyKeys) {
   EXPECT_TRUE(page_action_info->default_popup_url.is_empty());
 
   LoadAndExpectError("page_action_manifest_version_2b.json",
-                     errors::kInvalidPageActionPopup);
+                     errors::kInvalidActionDefaultPopup);
 }
 
 TEST_F(PageActionManifestTest, LoadPageActionHelper) {
@@ -86,7 +86,7 @@ TEST_F(PageActionManifestTest, LoadPageActionHelper) {
 
   // Invalid title should give an error.
   LoadAndExpectError("page_action_invalid_title.json",
-                     errors::kInvalidPageActionDefaultTitle);
+                     errors::kInvalidActionDefaultTitle);
 
   // Test the "default_popup" key.
   // These tests require an extension_url, so we also load the extension.

chrome/common/extensions/api/file_browser_handlers/file_browser_handler.cc

@@ -147,18 +147,19 @@ std::unique_ptr<FileBrowserHandler> LoadFileBrowserHandler(
 
   std::string handler_id;
   // Read the file action |id| (mandatory).
-  if (!file_browser_handler->HasKey(keys::kPageActionId) ||
-      !file_browser_handler->GetString(keys::kPageActionId, &handler_id)) {
-    *error = base::ASCIIToUTF16(errors::kInvalidPageActionId);
+  if (!file_browser_handler->HasKey(keys::kFileBrowserHandlerId) ||
+      !file_browser_handler->GetString(keys::kFileBrowserHandlerId,
+                                       &handler_id)) {
+    *error = base::ASCIIToUTF16(errors::kInvalidFileBrowserHandlerId);
     return nullptr;
   }
   result->set_id(handler_id);
 
   // Read the page action title from |default_title| (mandatory).
   std::string title;
-  if (!file_browser_handler->HasKey(keys::kPageActionDefaultTitle) ||
-      !file_browser_handler->GetString(keys::kPageActionDefaultTitle, &title)) {
-    *error = base::ASCIIToUTF16(errors::kInvalidPageActionDefaultTitle);
+  if (!file_browser_handler->HasKey(keys::kActionDefaultTitle) ||
+      !file_browser_handler->GetString(keys::kActionDefaultTitle, &title)) {
+    *error = base::ASCIIToUTF16(errors::kInvalidActionDefaultTitle);
     return nullptr;
   }
   result->set_title(title);
@@ -234,11 +235,11 @@ std::unique_ptr<FileBrowserHandler> LoadFileBrowserHandler(
 
   std::string default_icon;
   // Read the file browser action |default_icon| (optional).
-  if (file_browser_handler->HasKey(keys::kPageActionDefaultIcon)) {
-    if (!file_browser_handler->GetString(
-            keys::kPageActionDefaultIcon, &default_icon) ||
+  if (file_browser_handler->HasKey(keys::kActionDefaultIcon)) {
+    if (!file_browser_handler->GetString(keys::kActionDefaultIcon,
+                                         &default_icon) ||
         default_icon.empty()) {
-      *error = base::ASCIIToUTF16(errors::kInvalidPageActionIconPath);
+      *error = base::ASCIIToUTF16(errors::kInvalidActionDefaultIcon);
       return nullptr;
     }
     result->set_icon_path(default_icon);

chrome/common/extensions/api/file_browser_handlers/file_browser_handler_manifest_unittest.cc

@@ -85,9 +85,9 @@ TEST_F(FileBrowserHandlerManifestTest, InvalidFileBrowserHandlers) {
       Testcase("filebrowser_invalid_actions_2.json",
                errors::kInvalidFileBrowserHandler),
       Testcase("filebrowser_invalid_action_id.json",
-               errors::kInvalidPageActionId),
+               errors::kInvalidFileBrowserHandlerId),
       Testcase("filebrowser_invalid_action_title.json",
-               errors::kInvalidPageActionDefaultTitle),
+               errors::kInvalidActionDefaultTitle),
       Testcase("filebrowser_invalid_file_filters_1.json",
                errors::kInvalidFileFiltersList),
       Testcase("filebrowser_invalid_file_filters_2.json",

extensions/common/extension_l10n_util.cc

@@ -179,14 +179,14 @@ bool LocalizeManifest(const extensions::MessageBundle& messages,
   // Initialize browser_action.default_title
   std::string key(keys::kBrowserAction);
   key.append(".");
-  key.append(keys::kPageActionDefaultTitle);
+  key.append(keys::kActionDefaultTitle);
   if (!LocalizeManifestValue(key, messages, manifest, error))
     return false;
 
   // Initialize page_action.default_title
   key.assign(keys::kPageAction);
   key.append(".");
-  key.append(keys::kPageActionDefaultTitle);
+  key.append(keys::kActionDefaultTitle);
   if (!LocalizeManifestValue(key, messages, manifest, error))
     return false;
 
@@ -203,8 +203,8 @@ bool LocalizeManifest(const extensions::MessageBundle& messages,
         *error = errors::kInvalidFileBrowserHandler;
         return false;
       }
-      if (!LocalizeManifestValue(
-              keys::kPageActionDefaultTitle, messages, handler, error))
+      if (!LocalizeManifestValue(keys::kActionDefaultTitle, messages, handler,
+                                 error))
         return false;
     }
   }

extensions/common/extension_l10n_util_unittest.cc

@@ -384,7 +384,7 @@ TEST(ExtensionL10nUtil, LocalizeManifestWithNameDescriptionDefaultTitleMsgs) {
   manifest.SetString(keys::kDescription, "__MSG_description__");
   std::string action_title(keys::kBrowserAction);
   action_title.append(".");
-  action_title.append(keys::kPageActionDefaultTitle);
+  action_title.append(keys::kActionDefaultTitle);
   manifest.SetString(action_title, "__MSG_title__");
 
   std::string error;
@@ -437,8 +437,7 @@ TEST(ExtensionL10nUtil, LocalizeManifestWithNameDescriptionFileHandlerTitle) {
   manifest.SetString(keys::kDescription, "__MSG_description__");
 
   base::DictionaryValue handler;
-  handler.SetString(keys::kPageActionDefaultTitle,
-                    "__MSG_file_handler_title__");
+  handler.SetString(keys::kActionDefaultTitle, "__MSG_file_handler_title__");
   auto handlers = std::make_unique<base::ListValue>();
   handlers->GetList().push_back(std::move(handler));
   manifest.Set(keys::kFileBrowserHandlers, std::move(handlers));
@@ -460,7 +459,7 @@ TEST(ExtensionL10nUtil, LocalizeManifestWithNameDescriptionFileHandlerTitle) {
   manifest.GetList(keys::kFileBrowserHandlers, &handlers_raw);
   base::DictionaryValue* handler_raw = nullptr;
   handlers_raw->GetList()[0].GetAsDictionary(&handler_raw);
-  ASSERT_TRUE(handler_raw->GetString(keys::kPageActionDefaultTitle, &result));
+  ASSERT_TRUE(handler_raw->GetString(keys::kActionDefaultTitle, &result));
   EXPECT_EQ("file handler title", result);
 
   EXPECT_TRUE(error.empty());

extensions/common/manifest_constants.cc

@@ -11,7 +11,10 @@ namespace manifest_keys {
 
 const char kAboutPage[] = "about_page";
 const char kAction[] = "action";
+const char kActionDefaultIcon[] = "default_icon";
+const char kActionDefaultPopup[] = "default_popup";
 const char kActionDefaultState[] = "default_state";
+const char kActionDefaultTitle[] = "default_title";
 const char kAllFrames[] = "all_frames";
 const char kAltKey[] = "altKey";
 const char kApp[] = "app";
@@ -56,6 +59,7 @@ const char kExternallyConnectable[] = "externally_connectable";
 const char kEventRules[] = "event_rules";
 const char kFileAccessList[] = "file_access";
 const char kFileFilters[] = "file_filters";
+const char kFileBrowserHandlerId[] = "id";
 const char kFileBrowserHandlers[] = "file_browser_handlers";
 const char kFileHandlers[] = "file_handlers";
 const char kFileHandlerExtensions[] = "extensions";
@@ -122,10 +126,6 @@ const char kOverrideSearchProvider[] =
     "chrome_settings_overrides.search_provider";
 const char kOverrideStartupPage[] = "chrome_settings_overrides.startup_pages";
 const char kPageAction[] = "page_action";
-const char kPageActionDefaultIcon[] = "default_icon";
-const char kPageActionDefaultPopup[] = "default_popup";
-const char kPageActionDefaultTitle[] = "default_title";
-const char kPageActionId[] = "id";
 const char kPermissions[] = "permissions";
 const char kPlatformAppBackground[] = "app.background";
 const char kPlatformAppBackgroundPage[] = "app.background.page";
@@ -241,8 +241,6 @@ const char kRunAtDocumentStart[] = "document_start";
 const char kRunAtDocumentEnd[] = "document_end";
 const char kRunAtDocumentIdle[] = "document_idle";
 const char kPageActionCommandEvent[] = "_execute_page_action";
-const char kPageActionTypeTab[] = "tab";
-const char kPageActionTypePermanent[] = "permanent";
 const char kLaunchContainerPanelDeprecated[] = "panel";
 const char kLaunchContainerTab[] = "tab";
 const char kLaunchContainerWindow[] = "window";
@@ -310,7 +308,10 @@ const char kInvalidAboutPage[] = "Invalid value for 'about_page'.";
 const char kInvalidAboutPageExpectRelativePath[] =
     "Invalid value for 'about_page'. Value must be a relative path.";
 const char kInvalidAction[] = "Invalid value for 'action'.";
+const char kInvalidActionDefaultIcon[] = "Invalid value for 'default_icon'.";
+const char kInvalidActionDefaultPopup[] = "Invalid type for 'default_popup'.";
 const char kInvalidActionDefaultState[] = "Invalid value for 'default_state'.";
+const char kInvalidActionDefaultTitle[] = "Invalid value for 'default_title'.";
 const char kInvalidAllFrames[] =
     "Invalid value for 'content_scripts[*].all_frames'.";
 const char kInvalidAppIconColor[] = "Invalid value for app.icon_color.";
@@ -400,6 +401,8 @@ const char kInvalidFileAccessValue[] =
     "Invalid value for 'file_access[*]'.";
 const char kInvalidFileBrowserHandler[] =
     "Invalid value for 'file_browser_handlers'.";
+const char kInvalidFileBrowserHandlerId[] =
+    "Required value 'id' is missing or invalid.";
 const char kInvalidFileBrowserHandlerMissingPermission[] =
     "Declaring file browser handlers requires the fileBrowserHandler manifest "
     "permission.";
@@ -578,16 +581,6 @@ const char kInvalidOptionsUIOpenInTab[] =
     "Invalid value for 'options_ui.open_in_tab'.";
 const char kInvalidPageAction[] =
     "Invalid value for 'page_action'.";
-const char kInvalidPageActionDefaultTitle[] =
-    "Invalid value for 'default_title'.";
-const char kInvalidPageActionIconPath[] =
-    "Invalid value for 'page_action.default_icon'.";
-const char kInvalidPageActionId[] =
-    "Required value 'id' is missing or invalid.";
-const char kInvalidPageActionPopup[] =
-    "Invalid type for page action popup.";
-const char kInvalidPageActionPopupPath[] =
-    "Invalid value for page action popup path [*].";
 const char kInvalidPermissionWithDetail[] =
     "Invalid value for 'permissions[*]': *.";
 const char kInvalidPermission[] =

extensions/common/manifest_constants.h

@@ -14,7 +14,10 @@ namespace manifest_keys {
 
 extern const char kAboutPage[];
 extern const char kAction[];
+extern const char kActionDefaultIcon[];
+extern const char kActionDefaultPopup[];
 extern const char kActionDefaultState[];
+extern const char kActionDefaultTitle[];
 extern const char kAllFrames[];
 extern const char kAltKey[];
 extern const char kApp[];
@@ -62,6 +65,7 @@ extern const char kFileHandlerIncludeDirectories[];
 extern const char kFileHandlerTypes[];
 extern const char kFileHandlerVerb[];
 extern const char kFileFilters[];
+extern const char kFileBrowserHandlerId[];
 extern const char kFileBrowserHandlers[];
 extern const char kGlobal[];
 extern const char kHideBookmarkButton[];
@@ -124,10 +128,6 @@ extern const char kOverrideHomepage[];
 extern const char kOverrideSearchProvider[];
 extern const char kOverrideStartupPage[];
 extern const char kPageAction[];
-extern const char kPageActionDefaultIcon[];
-extern const char kPageActionDefaultPopup[];
-extern const char kPageActionDefaultTitle[];
-extern const char kPageActionId[];
 extern const char kPermissions[];
 extern const char kPlatformAppBackground[];
 extern const char kPlatformAppBackgroundPage[];
@@ -241,8 +241,6 @@ extern const char kLaunchContainerPanelDeprecated[];
 extern const char kLaunchContainerTab[];
 extern const char kLaunchContainerWindow[];
 extern const char kPageActionCommandEvent[];
-extern const char kPageActionTypePermanent[];
-extern const char kPageActionTypeTab[];
 extern const char kRunAtDocumentEnd[];
 extern const char kRunAtDocumentIdle[];
 extern const char kRunAtDocumentStart[];
@@ -279,7 +277,10 @@ extern const char kFileNotFound[];
 extern const char kInvalidAboutPage[];
 extern const char kInvalidAboutPageExpectRelativePath[];
 extern const char kInvalidAction[];
+extern const char kInvalidActionDefaultIcon[];
+extern const char kInvalidActionDefaultPopup[];
 extern const char kInvalidActionDefaultState[];
+extern const char kInvalidActionDefaultTitle[];
 extern const char kInvalidAllFrames[];
 extern const char kInvalidAppIconColor[];
 extern const char kInvalidAppThemeColor[];
@@ -326,6 +327,7 @@ extern const char kInvalidExportAllowlistString[];
 extern const char kInvalidFileAccessList[];
 extern const char kInvalidFileAccessValue[];
 extern const char kInvalidFileBrowserHandler[];
+extern const char kInvalidFileBrowserHandlerId[];
 extern const char kInvalidFileBrowserHandlerMissingPermission[];
 extern const char kInvalidFileFiltersList[];
 extern const char kInvalidFileFilterValue[];
@@ -415,12 +417,6 @@ extern const char kInvalidOptionsPage[];
 extern const char kInvalidOptionsPageExpectUrlInPackage[];
 extern const char kInvalidOptionsPageInHostedApp[];
 extern const char kInvalidPageAction[];
-extern const char kInvalidPageActionDefaultTitle[];
-extern const char kInvalidPageActionIconPath[];
-extern const char kInvalidPageActionId[];
-extern const char kInvalidPageActionPopup[];
-extern const char kInvalidPageActionPopupHeight[];
-extern const char kInvalidPageActionPopupPath[];
 extern const char kInvalidPermissionWithDetail[];
 extern const char kInvalidPermission[];
 extern const char kInvalidPermissions[];