Move the parsing of homepage_url" and "devtools_page" out of Extension.

chrome/common/extensions/extension.cc

 // Copyright (c) 2012 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 "chrome/common/extensions/extension.h"
 
 #include <ostream>
 
 #include "base/base64.h"
 #include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/i18n/rtl.h"
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "base/stl_util.h"
 #include "base/string16.h"
 #include "base/string_number_conversions.h"
 #include "base/string_piece.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "base/version.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
+#include "chrome/common/extensions/api/url_parser/url_info.h"
 #include "chrome/common/extensions/csp_validator.h"
 #include "chrome/common/extensions/extension_manifest_constants.h"
 #include "chrome/common/extensions/extension_resource.h"
 #include "chrome/common/extensions/feature_switch.h"
 #include "chrome/common/extensions/features/base_feature_provider.h"
 #include "chrome/common/extensions/features/feature.h"
 #include "chrome/common/extensions/file_browser_handler.h"
 #include "chrome/common/extensions/manifest.h"
 #include "chrome/common/extensions/manifest_handler.h"
 #include "chrome/common/extensions/permissions/permission_set.h"
@@ skipping 438 matching lines @@
                              DictionaryValue* info) const {
   info->SetString(info_keys::kIdKey, id());
   info->SetString(info_keys::kNameKey, name());
   info->SetBoolean(info_keys::kEnabledKey, enabled);
   info->SetBoolean(info_keys::kOfflineEnabledKey, offline_enabled());
   info->SetString(info_keys::kVersionKey, VersionString());
   info->SetString(info_keys::kDescriptionKey, description());
   info->SetString(info_keys::kOptionsUrlKey,
                   options_url().possibly_invalid_spec());
   info->SetString(info_keys::kHomepageUrlKey,
-                  GetHomepageURL().possibly_invalid_spec());
+      URLInfo::url(this, URLInfo::HOMEPAGE_URL).possibly_invalid_spec());
   info->SetString(info_keys::kDetailsUrlKey,
                   details_url().possibly_invalid_spec());
   info->SetBoolean(info_keys::kPackagedAppKey, is_platform_app());
 }
 
 Extension::Type Extension::GetType() const {
   return converted_from_user_script() ? TYPE_USER_SCRIPT : manifest_->type();
 }
 
 // static
@@ skipping 444 matching lines @@
 }
 
 bool Extension::ShowConfigureContextMenus() const {
   // Don't show context menu for component extensions. We might want to show
   // options for component extension button but now there is no component
   // extension with options. All other menu items like uninstall have
   // no sense for component extensions.
   return location() != Extension::COMPONENT;
 }
 
-GURL Extension::GetHomepageURL() const {
-  if (homepage_url_.is_valid())
-    return homepage_url_;
-
-  return UpdatesFromGallery() ?
-      GURL(extension_urls::GetWebstoreItemDetailURLPrefix() + id()) : GURL();
-}
-
 std::set<FilePath> Extension::GetBrowserImages() const {
   std::set<FilePath> image_paths;
   // TODO(viettrungluu): These |FilePath::FromWStringHack(UTF8ToWide())|
   // indicate that we're doing something wrong.
 
   // Extension icons.
   for (ExtensionIconSet::IconMap::const_iterator iter = icons().map().begin();
        iter != icons().map().end(); ++iter) {
     image_paths.insert(FilePath::FromWStringHack(UTF8ToWide(iter->second)));
   }
@@ skipping 610 matching lines @@
     return false;
 
   if (!LoadThemeFeatures(error))
     return false;
 
   if (HasMultipleUISurfaces()) {
     *error = ASCIIToUTF16(errors::kOneUISurfaceOnly);
     return false;
   }
 
+  finished_parsing_manifest_ = true;

[Yoyo Zhou, 2012/12/27 17:16:17]

Why is this being moved? By the way, Devlin is changing this thread safety check
in

https://chromiumcodereview.appspot.com/11644057/

so it should work better.

[Joe Thomas, 2012/12/30 04:26:00]

PermissionSet constructor below queries for "devtools_page" URL through
PermissionSet::InitImplicitExtensionPermissions and hits DCHECK in
Extension::GetManifestData. So I think it's reasonable to move this setter up as
Manifest parsing is already completed. 

I will revisit this change once https://chromiumcodereview.appspot.com/11644057/
lands and move it back.

[Yoyo Zhou, 2013/01/03 01:45:54]

Once that lands, you should not need to change this. But your change here is not
a good workaround; finished_parsing_manifest_ tells us that the extension is
done being constructed and safe to read from other threads. But this is not true
if the permissions haven't been set yet.

+
   runtime_data_.SetActivePermissions(new PermissionSet(
       this, api_permissions, host_permissions));
   required_permission_set_ = new PermissionSet(
       this, api_permissions, host_permissions);
   optional_permission_set_ = new PermissionSet(
       optional_api_permissions, optional_host_permissions, URLPatternSet());
 
-  finished_parsing_manifest_ = true;
-
   return true;
 }
 
 bool Extension::LoadAppIsolation(const APIPermissionSet& api_permissions,
                                  string16* error) {
   // Platform apps always get isolated storage.
   if (is_platform_app()) {
     is_storage_isolated_ = true;
     return true;
   }
@@ skipping 318 matching lines @@
     extent_.ClearPatterns();
   }
 
   return true;
 }
 
 bool Extension::LoadSharedFeatures(
     const APIPermissionSet& api_permissions,
     string16* error) {
   if (!LoadDescription(error) ||
-      !LoadHomepageURL(error) ||
       !LoadUpdateURL(error) ||
       !LoadIcons(error) ||
       !LoadCommands(error) ||
       !LoadPlugins(error) ||
       !LoadNaClModules(error) ||
       !LoadWebAccessibleResources(error) ||
       !LoadSandboxedPages(error) ||
       !LoadRequirements(error) ||
       !LoadDefaultLocale(error) ||
       !LoadOfflineEnabled(error) ||
@@ skipping 38 matching lines @@
           switches::kAllowLegacyExtensionManifests)) {
     *error = ErrorUtils::FormatErrorMessageUTF16(
         errors::kInvalidManifestVersionOld,
         base::IntToString(kModernManifestVersion));
     return false;
   }
 
   return true;
 }
 
-bool Extension::LoadHomepageURL(string16* error) {
-  if (!manifest_->HasKey(keys::kHomepageURL))
-    return true;
-  std::string tmp_homepage_url;
-  if (!manifest_->GetString(keys::kHomepageURL, &tmp_homepage_url)) {
-    *error = ErrorUtils::FormatErrorMessageUTF16(
-        errors::kInvalidHomepageURL, "");
-    return false;
-  }
-  homepage_url_ = GURL(tmp_homepage_url);
-  if (!homepage_url_.is_valid() ||
-      (!homepage_url_.SchemeIs("http") &&
-       !homepage_url_.SchemeIs("https"))) {
-    *error = ErrorUtils::FormatErrorMessageUTF16(
-        errors::kInvalidHomepageURL, tmp_homepage_url);
-    return false;
-  }
-  return true;
-}
-
 bool Extension::LoadUpdateURL(string16* error) {
   if (!manifest_->HasKey(keys::kUpdateURL))
     return true;
   std::string tmp_update_url;
   if (!manifest_->GetString(keys::kUpdateURL, &tmp_update_url)) {
     *error = ErrorUtils::FormatErrorMessageUTF16(
         errors::kInvalidUpdateURL, "");
     return false;
   }
   update_url_ = GURL(tmp_update_url);
@@ skipping 758 matching lines @@
   return true;
 }
 
 bool Extension::LoadExtensionFeatures(APIPermissionSet* api_permissions,
                                       string16* error) {
   if (manifest_->HasKey(keys::kConvertedFromUserScript))
     manifest_->GetBoolean(keys::kConvertedFromUserScript,
                           &converted_from_user_script_);
 
   if (!LoadManifestHandlerFeatures(error) ||
-      !LoadDevToolsPage(error) ||
       !LoadInputComponents(*api_permissions, error) ||
       !LoadContentScripts(error) ||
       !LoadPageAction(error) ||
       !LoadBrowserAction(error) ||
       !LoadSystemIndicator(api_permissions, error) ||
       !LoadScriptBadge(error) ||
       !LoadFileBrowserHandlers(error) ||
       !LoadChromeURLOverrides(error) ||
       !LoadTextToSpeechVoices(error) ||
       !LoadIncognitoMode(error) ||
       !LoadFileHandlers(error) ||
       !LoadContentSecurityPolicy(error))
     return false;
 
   return true;
 }
 
 bool Extension::LoadManifestHandlerFeatures(string16* error) {
   std::vector<std::string> keys = ManifestHandler::GetKeys();
   for (size_t i = 0; i < keys.size(); ++i) {
     Value* value = NULL;
     if (!manifest_->Get(keys[i], &value))
       continue;
     if (!ManifestHandler::Get(keys[i])->Parse(value, this, error))
       return false;
   }
   return true;
 }
 
-bool Extension::LoadDevToolsPage(string16* error) {
-  if (!manifest_->HasKey(keys::kDevToolsPage))
-    return true;
-  std::string devtools_str;
-  if (!manifest_->GetString(keys::kDevToolsPage, &devtools_str)) {
-    *error = ASCIIToUTF16(errors::kInvalidDevToolsPage);
-    return false;
-  }
-  devtools_url_ = GetResourceURL(devtools_str);
-  return true;
-}
-
 bool Extension::LoadInputComponents(const APIPermissionSet& api_permissions,
                                     string16* error) {
   if (!manifest_->HasKey(keys::kInputComponents))
     return true;
   ListValue* list_value = NULL;
   if (!manifest_->GetList(keys::kInputComponents, &list_value)) {
     *error = ASCIIToUTF16(errors::kInvalidInputComponents);
     return false;
   }
 
@@ skipping 1378 matching lines @@
 
 UpdatedExtensionPermissionsInfo::UpdatedExtensionPermissionsInfo(
     const Extension* extension,
     const PermissionSet* permissions,
     Reason reason)
     : reason(reason),
       extension(extension),
       permissions(permissions) {}
 
 }   // namespace extensions