[extensions] Stop parsing webstore urls so much

[extensions] Stop parsing webstore urls so much

The following hooks parse the webstore url per request, sometimes twice.
 - OnHeadersReceived
 - OnResponseStarted
 - OnBeforeRequest
 - OnBeforeSendHeaders
 - OnSendHeaders
 - OnCompleted

This accounts for ~3% of task CPU time on the IO thread from loading
http://cr.kungfoo.net/loading/1000spacers/ on Linux.

This patch just caches the url at the ExtensionsClient layer, so we never
have to do this more than once.

BUG=664174
Committed: https://crrev.com/4ff7b40b01046d703b33c4c96a491fdb923d20cd
Cr-Commit-Position: refs/heads/master@{#431771}

[Charlie Harrison, 2016-11-11 03:06:34]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-11 03:07:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Charlie Harrison, 2016-11-11 03:20:58]

Description was changed from

==========
[extensions] Stop parsing webstore urls so much

The following hooks parse the webstore url per request, sometimes twice.
 - OnHeadersReceived
 - OnResponseStarted
 - OnBeforeRequest
 - OnBeforeSendHeaders
 - OnSendHeaders
 - OnCompleted

This accounts for ~3% of task CPU time on the IO thread from loading
http://cr.kungfoo.net/loading/1000spacers/

This patch just caches the url at the ExtensionsClient layer, so we never
have to do this more than once.

BUG=664174
==========

to

==========
[extensions] Stop parsing webstore urls so much

The following hooks parse the webstore url per request, sometimes twice.
 - OnHeadersReceived
 - OnResponseStarted
 - OnBeforeRequest
 - OnBeforeSendHeaders
 - OnSendHeaders
 - OnCompleted

This accounts for ~3% of task CPU time on the IO thread from loading
http://cr.kungfoo.net/loading/1000spacers/ on Linux.

This patch just caches the url at the ExtensionsClient layer, so we never
have to do this more than once.

BUG=664174
==========

[commit-bot: I haz the power, 2016-11-11 03:41:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-11 03:41:09]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Charlie Harrison, 2016-11-11 15:05:07]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-11 15:05:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Charlie Harrison, 2016-11-11 15:11:58]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-11 15:12:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-11 15:49:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-11 15:49:36]

Dry run: This issue passed the CQ dry run.

[Charlie Harrison, 2016-11-11 15:57:40]

csharrison@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Charlie Harrison, 2016-11-11 15:57:41]

Hey Devlin, here's the followup I mentioned yesterday. This fixes a problem
where we are essentially doing GURL(GURL(string).spec()). There are a few things
left to be desired in this patch though:

1. To satisfy browser tests we need to check the command line more often than I
would like. I imagine fixing this would require changing the extension client
api.

2. The static GetWebstoreUpdateUrl in extension_urls.cc does not return a const
reference. This is because can't return the temporary
GURL(kChromeWebstoreUpdateURL) if there is no extension client. I don't think
this isn't a huge deal (GURL copies are cheap for small URLs). I'm not sure in
what circumstances we don't have a client, but I think it is generally bad
practice to have static local GURLs hanging around.

[Devlin, 2016-11-11 23:11:31]

lgtm w/ nit

https://codereview.chromium.org/2493053002/diff/20001/chrome/common/extension...
File chrome/common/extensions/chrome_extensions_client.cc (right):

https://codereview.chromium.org/2493053002/diff/20001/chrome/common/extension...
chrome/common/extensions/chrome_extensions_client.cc:285: // Browser tests like
to alter the command line at runtime with new update
:(

https://codereview.chromium.org/2493053002/diff/20001/chrome/common/extension...
File chrome/common/extensions/extension_constants.cc (right):

https://codereview.chromium.org/2493053002/diff/20001/chrome/common/extension...
chrome/common/extensions/extension_constants.cc:18: std::string
GetDefaultWebstoreUpdateUrl() {
Why std::string instead of const char*?  Most callers will probably construct an
url or do comparison, both of which can use a stringpiece.

[Charlie Harrison, 2016-11-12 02:23:05]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-12 02:23:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Charlie Harrison, 2016-11-12 02:24:34]

thanks!

https://codereview.chromium.org/2493053002/diff/20001/chrome/common/extension...
File chrome/common/extensions/extension_constants.cc (right):

https://codereview.chromium.org/2493053002/diff/20001/chrome/common/extension...
chrome/common/extensions/extension_constants.cc:18: std::string
GetDefaultWebstoreUpdateUrl() {
On 2016/11/11 23:11:31, Devlin (slow) wrote:
> Why std::string instead of const char*?  Most callers will probably construct
an
> url or do comparison, both of which can use a stringpiece.

Good catch. Done.

[Charlie Harrison, 2016-11-12 02:25:15]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2016-11-12 02:25:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/2493053002/#ps40001
(title: "rdevlin review")

[commit-bot: I haz the power, 2016-11-12 02:25:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-12 04:42:07]

Description was changed from

==========
[extensions] Stop parsing webstore urls so much

The following hooks parse the webstore url per request, sometimes twice.
 - OnHeadersReceived
 - OnResponseStarted
 - OnBeforeRequest
 - OnBeforeSendHeaders
 - OnSendHeaders
 - OnCompleted

This accounts for ~3% of task CPU time on the IO thread from loading
http://cr.kungfoo.net/loading/1000spacers/ on Linux.

This patch just caches the url at the ExtensionsClient layer, so we never
have to do this more than once.

BUG=664174
==========

to

==========
[extensions] Stop parsing webstore urls so much

The following hooks parse the webstore url per request, sometimes twice.
 - OnHeadersReceived
 - OnResponseStarted
 - OnBeforeRequest
 - OnBeforeSendHeaders
 - OnSendHeaders
 - OnCompleted

This accounts for ~3% of task CPU time on the IO thread from loading
http://cr.kungfoo.net/loading/1000spacers/ on Linux.

This patch just caches the url at the ExtensionsClient layer, so we never
have to do this more than once.

BUG=664174
==========

[commit-bot: I haz the power, 2016-11-12 04:42:08]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-12 04:46:41]

Description was changed from

==========
[extensions] Stop parsing webstore urls so much

The following hooks parse the webstore url per request, sometimes twice.
 - OnHeadersReceived
 - OnResponseStarted
 - OnBeforeRequest
 - OnBeforeSendHeaders
 - OnSendHeaders
 - OnCompleted

This accounts for ~3% of task CPU time on the IO thread from loading
http://cr.kungfoo.net/loading/1000spacers/ on Linux.

This patch just caches the url at the ExtensionsClient layer, so we never
have to do this more than once.

BUG=664174
==========

to

==========
[extensions] Stop parsing webstore urls so much

The following hooks parse the webstore url per request, sometimes twice.
 - OnHeadersReceived
 - OnResponseStarted
 - OnBeforeRequest
 - OnBeforeSendHeaders
 - OnSendHeaders
 - OnCompleted

This accounts for ~3% of task CPU time on the IO thread from loading
http://cr.kungfoo.net/loading/1000spacers/ on Linux.

This patch just caches the url at the ExtensionsClient layer, so we never
have to do this more than once.

BUG=664174

Committed: https://crrev.com/4ff7b40b01046d703b33c4c96a491fdb923d20cd
Cr-Commit-Position: refs/heads/master@{#431771}
==========

[commit-bot: I haz the power, 2016-11-12 04:46:41]

Patchset 3 (id:??) landed as
https://crrev.com/4ff7b40b01046d703b33c4c96a491fdb923d20cd
Cr-Commit-Position: refs/heads/master@{#431771}