Avoid parsing the webstore base url so much.

chrome/common/extensions/chrome_extensions_client.cc

Index: chrome/common/extensions/chrome_extensions_client.cc
diff --git a/chrome/common/extensions/chrome_extensions_client.cc b/chrome/common/extensions/chrome_extensions_client.cc
index 9a4a929ae7b1dddc2643342e158c230cbd045d26..d568e6caf7e6fb25728aa23af64e25ca5d0de7cd 100644
--- a/chrome/common/extensions/chrome_extensions_client.cc
+++ b/chrome/common/extensions/chrome_extensions_client.cc
@@ -269,16 +269,19 @@ 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);
+  } else if (webstore_base_url_.is_empty()) {

[Devlin, 2016/11/16 19:11:52]

Is the savings from lazily populating webstore_base_url_ worth the cost of the
constant if-branch here?  I'd think we could just instantiate it at construction
of the client.

[Charlie Harrison, 2016/11/16 21:45:19]

That's a better solution. I don't think lazy initialization wins us much. I'll
move the update url to this pattern too.

These are moved to Initalize().

+    webstore_base_url_ = GURL(extension_urls::kChromeWebstoreBaseURL);
+  }
+  return webstore_base_url_;
 }
 
 const GURL& ChromeExtensionsClient::GetWebstoreUpdateURL() const {
@@ -289,7 +292,7 @@ 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());