Avoid parsing the webstore base url so much.

This is a followup to [1] which modified the extension url API to vend
GURLs rather than std::string for webstore update urls.

This patch does the same for the webstore base url
(the webstore "launch" url). We only parse this url in
ChromeResourceDispatcherHostDelegate::OnResponseStarted so there is less
of a performance win than [1], but still we should avoid doing needless
work.

Parsing this URL takes ~12% of the CPU time of
ResourceLoader::CompleteResponseStarted.

[1] https://codereview.chromium.org/2493053002/

BUG=664174

Review-Url: https://codereview.chromium.org/2506713002
Cr-Commit-Position: refs/heads/master@{#433963}

chrome/browser/chromeos/app_mode/kiosk_app_data.cc

@@ -662,8 +662,8 @@ void KioskAppData::OnWebstoreResponseParseSuccess(
                              &icon_url_string))
     return;
 
-  GURL icon_url = GURL(extension_urls::GetWebstoreLaunchURL()).Resolve(
-      icon_url_string);
+  GURL icon_url =
+      extension_urls::GetWebstoreLaunchURL().Resolve(icon_url_string);
   if (!icon_url.is_valid()) {
     LOG(ERROR) << "Webstore response error (icon url): "
                << ValueToString(*webstore_data);

chrome/browser/extensions/api/inline_install_private/inline_install_private_apitest.cc

@@ -40,7 +40,7 @@ class InlineInstallPrivateApiTestApp :
  public:
   InlineInstallPrivateApiTestApp() :
       InlineInstallPrivateApiTestBase(
-          "extensions/api_test/inline_install_private/",
+          "extensions/api_test/inline_install_private",
           "app.crx") {}
 };
 

chrome/browser/extensions/webstore_standalone_installer.cc

@@ -298,8 +298,7 @@ void WebstoreStandaloneInstaller::OnWebstoreResponseParseSuccess(
                       webstore_install::kInvalidWebstoreResponseError);
       return;
     }
-    icon_url = GURL(extension_urls::GetWebstoreLaunchURL()).Resolve(
-        icon_url_string);
+    icon_url = extension_urls::GetWebstoreLaunchURL().Resolve(icon_url_string);
     if (!icon_url.is_valid()) {
       CompleteInstall(webstore_install::INVALID_WEBSTORE_RESPONSE,
                       webstore_install::kInvalidWebstoreResponseError);

chrome/browser/prerender/prerender_browsertest.cc

@@ -2898,15 +2898,15 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
   // is partially controlled by the renderer, namely
   // ChromeContentRendererClient. This test instead relies on the Web
   // Store triggering such navigations.
-  std::string webstore_url = extension_urls::GetWebstoreLaunchURL();
+  GURL webstore_url = extension_urls::GetWebstoreLaunchURL();
 
   // Mock out requests to the Web Store.
   base::FilePath file(GetTestPath("prerender_page.html"));
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::Bind(&CreateMockInterceptorOnIO, GURL(webstore_url), file));
+      base::Bind(&CreateMockInterceptorOnIO, webstore_url, file));
 
-  PrerenderTestURL(CreateClientRedirect(webstore_url),
+  PrerenderTestURL(CreateClientRedirect(webstore_url.spec()),
                    FINAL_STATUS_OPEN_URL, 1);
 }
 

chrome/browser/ui/app_list/search/webstore/webstore_provider_browsertest.cc

@@ -169,8 +169,11 @@ class WebstoreProviderTest : public InProcessBrowserTest {
         base::Bind(&WebstoreProviderTest::HandleRequest,
                    base::Unretained(this)));
     ASSERT_TRUE(embedded_test_server()->Start());
+    // Minor hack: the gallery URL is expected not to end with a slash. Just
+    // append "path" to maintain this.
     base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-        ::switches::kAppsGalleryURL, embedded_test_server()->base_url().spec());
+        ::switches::kAppsGalleryURL,
+        embedded_test_server()->base_url().spec() + "path");
 
     mock_controller_.reset(new AppListControllerDelegateForTest);
     webstore_provider_.reset(new WebstoreProvider(

chrome/browser/ui/webui/ntp/ntp_resource_cache.cc

@@ -444,9 +444,10 @@ void NTPResourceCache::CreateNewTabHTML() {
   load_time_data.SetString("learnMore",
       l10n_util::GetStringUTF16(IDS_LEARN_MORE));
   const std::string& app_locale = g_browser_process->GetApplicationLocale();
-  load_time_data.SetString("webStoreLink",
-      google_util::AppendGoogleLocaleParam(
-          GURL(extension_urls::GetWebstoreLaunchURL()), app_locale).spec());
+  load_time_data.SetString(
+      "webStoreLink", google_util::AppendGoogleLocaleParam(
+                          extension_urls::GetWebstoreLaunchURL(), app_locale)
+                          .spec());
   load_time_data.SetString("appInstallHintText",
       l10n_util::GetStringUTF16(IDS_NEW_TAB_APP_INSTALL_HINT_LABEL));
   load_time_data.SetString("learn_more",

chrome/common/extensions/chrome_extensions_client.cc

@@ -132,6 +132,9 @@ void ChromeExtensionsClient::Initialize() {
   // TODO(dmazzoni): remove this once we have an extension API that
   // allows any extension to request read-only access to webui pages.
   scripting_whitelist_.push_back(extension_misc::kChromeVoxExtensionId);
+
+  webstore_base_url_ = GURL(extension_urls::kChromeWebstoreBaseURL);
+  webstore_update_url_ = GURL(extension_urls::GetDefaultWebstoreUpdateUrl());
 }
 
 const PermissionMessageProvider&
@@ -269,16 +272,17 @@ void ChromeExtensionsClient::RecordDidSuppressFatalError() {
                             NUM_CHANNELS_FOR_HISTOGRAM);
 }
 
-std::string ChromeExtensionsClient::GetWebstoreBaseURL() const {
-  std::string gallery_prefix = extension_urls::kChromeWebstoreBaseURL;
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kAppsGalleryURL))
-    gallery_prefix =
-        base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-            switches::kAppsGalleryURL);
-  if (base::EndsWith(gallery_prefix, "/", base::CompareCase::SENSITIVE))
-    gallery_prefix = gallery_prefix.substr(0, gallery_prefix.length() - 1);
-  return gallery_prefix;
+const GURL& ChromeExtensionsClient::GetWebstoreBaseURL() const {
+  // Browser tests like to alter the command line at runtime with new update
+  // URLs. Just update the cached value of the base url (to avoid reparsing
+  // it) if the value has changed.
+  base::CommandLine* cmdline = base::CommandLine::ForCurrentProcess();
+  if (cmdline->HasSwitch(switches::kAppsGalleryURL)) {
+    std::string url = cmdline->GetSwitchValueASCII(switches::kAppsGalleryURL);
+    if (webstore_base_url_.possibly_invalid_spec() != url)
+      webstore_base_url_ = GURL(url);
+  }
+  return webstore_base_url_;
 }
 
 const GURL& ChromeExtensionsClient::GetWebstoreUpdateURL() const {
@@ -289,10 +293,8 @@ const GURL& ChromeExtensionsClient::GetWebstoreUpdateURL() const {
   if (cmdline->HasSwitch(switches::kAppsGalleryUpdateURL)) {
     std::string url =
         cmdline->GetSwitchValueASCII(switches::kAppsGalleryUpdateURL);
-    if (webstore_update_url_ != url)
+    if (webstore_update_url_.possibly_invalid_spec() != url)
       webstore_update_url_ = GURL(url);
-  } else if (webstore_update_url_.is_empty()) {
-    webstore_update_url_ = GURL(extension_urls::GetDefaultWebstoreUpdateUrl());
   }
   return webstore_update_url_;
 }

chrome/common/extensions/chrome_extensions_client.h

@@ -47,7 +47,7 @@ class ChromeExtensionsClient : public ExtensionsClient {
   base::StringPiece GetAPISchema(const std::string& name) const override;
   bool ShouldSuppressFatalErrors() const override;
   void RecordDidSuppressFatalError() override;
-  std::string GetWebstoreBaseURL() const override;
+  const GURL& GetWebstoreBaseURL() const override;
   const GURL& GetWebstoreUpdateURL() const override;
   bool IsBlacklistUpdateURL(const GURL& url) const override;
   std::set<base::FilePath> GetBrowserImagePaths(
@@ -69,6 +69,7 @@ class ChromeExtensionsClient : public ExtensionsClient {
   ScriptingWhitelist scripting_whitelist_;
 
   // Mutable to allow caching in a const method.
+  mutable GURL webstore_base_url_;
   mutable GURL webstore_update_url_;
 
   friend struct base::DefaultLazyInstanceTraits<ChromeExtensionsClient>;

extensions/common/extension_urls.cc

@@ -33,35 +33,40 @@ const char kChromeWebstoreBaseURL[] = "https://chrome.google.com/webstore";
 const char kChromeWebstoreUpdateURL[] =
     "https://clients2.google.com/service/update2/crx";
 
-std::string GetWebstoreLaunchURL() {
+GURL GetWebstoreLaunchURL() {
   extensions::ExtensionsClient* client = extensions::ExtensionsClient::Get();
   if (client)
     return client->GetWebstoreBaseURL();
-  return kChromeWebstoreBaseURL;
+  return GURL(kChromeWebstoreBaseURL);
 }
 
+// TODO(csharrison,devlin): Migrate the following methods to return
+// GURLs.
+// TODO(devlin): Try to use GURL methods like Resolve instead of string
+// concatenation.
 std::string GetWebstoreExtensionsCategoryURL() {
-  return GetWebstoreLaunchURL() + "/category/extensions";
+  return GetWebstoreLaunchURL().spec() + "/category/extensions";
 }
 
 std::string GetWebstoreItemDetailURLPrefix() {
-  return GetWebstoreLaunchURL() + "/detail/";
+  return GetWebstoreLaunchURL().spec() + "/detail/";
 }
 
 GURL GetWebstoreItemJsonDataURL(const std::string& extension_id) {
-  return GURL(GetWebstoreLaunchURL() + "/inlineinstall/detail/" + extension_id);
+  return GURL(GetWebstoreLaunchURL().spec() + "/inlineinstall/detail/" +
+              extension_id);
 }
 
 GURL GetWebstoreJsonSearchUrl(const std::string& query,
                               const std::string& host_language_code) {
-  GURL url(GetWebstoreLaunchURL() + "/jsonsearch");
+  GURL url(GetWebstoreLaunchURL().spec() + "/jsonsearch");
   url = net::AppendQueryParameter(url, "q", query);
   url = net::AppendQueryParameter(url, "hl", host_language_code);
   return url;
 }
 
 GURL GetWebstoreSearchPageUrl(const std::string& query) {
-  return GURL(GetWebstoreLaunchURL() + "/search/" +
+  return GURL(GetWebstoreLaunchURL().spec() + "/search/" +
               net::EscapeQueryParamValue(query, false));
 }
 
@@ -75,7 +80,7 @@ GURL GetWebstoreUpdateUrl() {
 GURL GetWebstoreReportAbuseUrl(const std::string& extension_id,
                                const std::string& referrer_id) {
   return GURL(base::StringPrintf("%s/report/%s?utm_source=%s",
-                                 GetWebstoreLaunchURL().c_str(),
+                                 GetWebstoreLaunchURL().spec().c_str(),
                                  extension_id.c_str(), referrer_id.c_str()));
 }
 

extensions/common/extension_urls.h

@@ -38,7 +38,7 @@ extern const char kChromeWebstoreUpdateURL[];
 // Returns the URL prefix for the extension/apps gallery. Can be set via the
 // --apps-gallery-url switch. The URL returned will not contain a trailing
 // slash. Do not use this as a prefix/extent for the store.
-std::string GetWebstoreLaunchURL();
+GURL GetWebstoreLaunchURL();
 
 // Returns the URL to the extensions category on the Web Store. This is
 // derived from GetWebstoreLaunchURL().

extensions/common/extensions_client.h

@@ -103,7 +103,7 @@ class ExtensionsClient {
   virtual void RecordDidSuppressFatalError() = 0;
 
   // Returns the base webstore URL prefix.
-  virtual std::string GetWebstoreBaseURL() const = 0;
+  virtual const GURL& GetWebstoreBaseURL() const = 0;
 
   // Returns the URL to use for update manifest queries.
   virtual const GURL& GetWebstoreUpdateURL() const = 0;

extensions/shell/common/shell_extensions_client.cc

@@ -77,6 +77,7 @@ base::LazyInstance<ShellPermissionMessageProvider>
 
 ShellExtensionsClient::ShellExtensionsClient()
     : extensions_api_permissions_(ExtensionsAPIPermissions()),
+      webstore_base_url_(extension_urls::kChromeWebstoreBaseURL),
       webstore_update_url_(extension_urls::kChromeWebstoreUpdateURL) {}
 
 ShellExtensionsClient::~ShellExtensionsClient() {
@@ -181,8 +182,8 @@ bool ShellExtensionsClient::ShouldSuppressFatalErrors() const {
 void ShellExtensionsClient::RecordDidSuppressFatalError() {
 }
 
-std::string ShellExtensionsClient::GetWebstoreBaseURL() const {
-  return extension_urls::kChromeWebstoreBaseURL;
+const GURL& ShellExtensionsClient::GetWebstoreBaseURL() const {
+  return webstore_base_url_;
 }
 
 const GURL& ShellExtensionsClient::GetWebstoreUpdateURL() const {

extensions/shell/common/shell_extensions_client.h

@@ -41,7 +41,7 @@ class ShellExtensionsClient : public ExtensionsClient {
   base::StringPiece GetAPISchema(const std::string& name) const override;
   bool ShouldSuppressFatalErrors() const override;
   void RecordDidSuppressFatalError() override;
-  std::string GetWebstoreBaseURL() const override;
+  const GURL& GetWebstoreBaseURL() const override;
   const GURL& GetWebstoreUpdateURL() const override;
   bool IsBlacklistUpdateURL(const GURL& url) const override;
 
@@ -50,6 +50,7 @@ class ShellExtensionsClient : public ExtensionsClient {
 
   ScriptingWhitelist scripting_whitelist_;
 
+  const GURL webstore_base_url_;
   const GURL webstore_update_url_;
 
   DISALLOW_COPY_AND_ASSIGN(ShellExtensionsClient);

extensions/test/test_extensions_client.cc

@@ -38,7 +38,8 @@ SimpleFeature* CreateFeature() {
 }  // namespace
 
 TestExtensionsClient::TestExtensionsClient()
-    : webstore_update_url_(extension_urls::kChromeWebstoreUpdateURL) {}
+    : webstore_base_url_(extension_urls::kChromeWebstoreBaseURL),
+      webstore_update_url_(extension_urls::kChromeWebstoreUpdateURL) {}
 
 TestExtensionsClient::~TestExtensionsClient() {
 }
@@ -147,8 +148,8 @@ bool TestExtensionsClient::ShouldSuppressFatalErrors() const {
 void TestExtensionsClient::RecordDidSuppressFatalError() {
 }
 
-std::string TestExtensionsClient::GetWebstoreBaseURL() const {
-  return extension_urls::kChromeWebstoreBaseURL;
+const GURL& TestExtensionsClient::GetWebstoreBaseURL() const {
+  return webstore_base_url_;
 }
 
 const GURL& TestExtensionsClient::GetWebstoreUpdateURL() const {

extensions/test/test_extensions_client.h

@@ -49,7 +49,7 @@ class TestExtensionsClient : public ExtensionsClient {
   base::StringPiece GetAPISchema(const std::string& name) const override;
   bool ShouldSuppressFatalErrors() const override;
   void RecordDidSuppressFatalError() override;
-  std::string GetWebstoreBaseURL() const override;
+  const GURL& GetWebstoreBaseURL() const override;
   const GURL& GetWebstoreUpdateURL() const override;
   bool IsBlacklistUpdateURL(const GURL& url) const override;
   std::set<base::FilePath> GetBrowserImagePaths(
@@ -63,6 +63,7 @@ class TestExtensionsClient : public ExtensionsClient {
 
   std::set<BrowserImagePathsFilter*> browser_image_filters_;
 
+  const GURL webstore_base_url_;
   const GURL webstore_update_url_;
 
   DISALLOW_COPY_AND_ASSIGN(TestExtensionsClient);