Move Origin::GetURL() out of resource request critical path

Move Origin::GetURL() out of resource request critical path

We still call GetURL() when we receive the response. But it is not really
necessary to do so, and we can eventually remove that and make code that
operates on the GURL "origin" operate on an Origin.

This patch also removes some dead code I found.

BUG=348655
Committed: https://crrev.com/f32ade75b611e31c6f43914a93c3f7067bcc8cce
Cr-Commit-Position: refs/heads/master@{#437380}

[Charlie Harrison, 2016-12-08 19:47:32]

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

[commit-bot: I haz the power, 2016-12-08 19:48:12]

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-12-08 19:57:18]

Description was changed from

==========
Move Origin::GetURL() out of resource request critical path

We still call GetURL() when we receive the response. But it is not really
necessary to do so, and we can eventually remove that and make code that
operates on the GURL "origin" operate on an Origin.

BUG=348655
==========

to

==========
Move Origin::GetURL() out of resource request critical path

We still call GetURL() when we receive the response. But it is not really
necessary to do so, and we can eventually remove that and make code that
operates on the GURL "origin" operate on an Origin.

This patch also removes some dead code I found.

BUG=348655
==========

[Charlie Harrison, 2016-12-08 21:15:27]

csharrison@chromium.org changed reviewers:
+ nick@chromium.org

[Charlie Harrison, 2016-12-08 21:15:27]

nick@ (also cc creis@), here's another quick CL concerning origins.

I did some profiling, and actually WebSecurityOrigin::operator Origin, and
Origin::GetURL() aren't as cheap as I thought they were. I'm going to see about
optimizing it a bit, but in the meantime we may as well move part of this out of
the critical path (it is also a bit more semantic).

Note that I have https://codereview.chromium.org/2562813003 in flight which will
hopefully remove the need to call GetURL() on the response.

[ncarter (slow), 2016-12-08 21:39:49]

lgtm

https://codereview.chromium.org/2557673006/diff/1/content/child/site_isolatio...
File content/child/site_isolation_stats_gatherer.cc (right):

https://codereview.chromium.org/2557673006/diff/1/content/child/site_isolatio...
content/child/site_isolation_stats_gatherer.cc:128: // Origin.
Another option here is to delete this class, or enable it fractionally (e.g. for
1% of renderers or requests).

[commit-bot: I haz the power, 2016-12-08 21:40:11]

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

[commit-bot: I haz the power, 2016-12-08 21:40:12]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Charlie Harrison, 2016-12-08 21:57:33]

Thanks!

https://codereview.chromium.org/2557673006/diff/1/content/child/site_isolatio...
File content/child/site_isolation_stats_gatherer.cc (right):

https://codereview.chromium.org/2557673006/diff/1/content/child/site_isolatio...
content/child/site_isolation_stats_gatherer.cc:128: // Origin.
On 2016/12/08 21:39:49, ncarter wrote:
> Another option here is to delete this class, or enable it fractionally (e.g.
for
> 1% of renderers or requests).

I think once the CL I referenced lands, it will be easy to do this plumbing. If
not I'll consider it. No big deal though.

[Charlie Harrison, 2016-12-08 21:57:38]

The CQ bit was checked by csharrison@chromium.org

[commit-bot: I haz the power, 2016-12-08 21:58:15]

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-12-08 22:50:01]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1481234258972870, "parent_rev":
"ed460b9cd3884576d5253bd0cba6c32c05cdbad2", "commit_rev":
"2729c417ef1ca0f2f1e8626e60d7e734dc5f6a0e"}

[commit-bot: I haz the power, 2016-12-08 22:50:27]

Description was changed from

==========
Move Origin::GetURL() out of resource request critical path

We still call GetURL() when we receive the response. But it is not really
necessary to do so, and we can eventually remove that and make code that
operates on the GURL "origin" operate on an Origin.

This patch also removes some dead code I found.

BUG=348655
==========

to

==========
Move Origin::GetURL() out of resource request critical path

We still call GetURL() when we receive the response. But it is not really
necessary to do so, and we can eventually remove that and make code that
operates on the GURL "origin" operate on an Origin.

This patch also removes some dead code I found.

BUG=348655
==========

[commit-bot: I haz the power, 2016-12-08 22:50:28]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-12-08 22:52:32]

Description was changed from

==========
Move Origin::GetURL() out of resource request critical path

We still call GetURL() when we receive the response. But it is not really
necessary to do so, and we can eventually remove that and make code that
operates on the GURL "origin" operate on an Origin.

This patch also removes some dead code I found.

BUG=348655
==========

to

==========
Move Origin::GetURL() out of resource request critical path

We still call GetURL() when we receive the response. But it is not really
necessary to do so, and we can eventually remove that and make code that
operates on the GURL "origin" operate on an Origin.

This patch also removes some dead code I found.

BUG=348655

Committed: https://crrev.com/f32ade75b611e31c6f43914a93c3f7067bcc8cce
Cr-Commit-Position: refs/heads/master@{#437380}
==========

[commit-bot: I haz the power, 2016-12-08 22:52:33]

Patchset 1 (id:??) landed as
https://crrev.com/f32ade75b611e31c6f43914a93c3f7067bcc8cce
Cr-Commit-Position: refs/heads/master@{#437380}