Replace usage of GURL(origin.Serialize()) with origin.GetURL()

Replace usage of GURL(origin.Serialize()) with origin.GetURL()

The bulk of this was via the following command:
git grep -l "GURL([a-zA-Z0-9\._]*\.Serialize())" | xargs sed -i \
 's/GURL(\([a-zA-Z0-9\._]*\)\.Serialize())/\1.GetURL()/g'

Which was then followed by a 'git cl format', and removing changes from
url unit tests, which check that this change has identical semantics.

renderer_host/media also had ConvertToGURL(Origin) methods
which were removed.

BUG=651554
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/aec2c5453d3e7769057aab74142b403d87791a8b
Cr-Commit-Position: refs/heads/master@{#424825}

[Charlie Harrison, 2016-09-29 23:34:02]

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

[commit-bot: I haz the power, 2016-09-29 23:34:36]

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-09-29 23:37:26]

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

[commit-bot: I haz the power, 2016-09-29 23:37:29]

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-09-29 23:38:10]

Description was changed from

==========
Replace usage of GURL(origin.Serialize()) with origin.GetURL()

The bulk of this was via the following command:
git grep -l "GURL([a-zA-Z0-9]*\.Serialize())" | xargs sed -i \
's/GURL(\([a-zA-Z0-9]*\)\.Serialize())/\1.GetURL()/g'

Which was then followed by a 'git cl format', and removing changes from
url unit tests, which check that this change has identical semantics.

BUG=651554
==========

to

==========
Replace usage of GURL(origin.Serialize()) with origin.GetURL()

The bulk of this was via the following command:
git grep -l "GURL([a-zA-Z0-9]*\.Serialize())" | xargs sed -i \
's/GURL(\([a-zA-Z0-9]*\)\.Serialize())/\1.GetURL()/g'

Which was then followed by a 'git cl format', and removing changes from
url unit tests, which check that this change has identical semantics.

renderer_host/media also had a ConvertToGURL(Origin) method
which was removed.

BUG=651554
==========

[Charlie Harrison, 2016-09-29 23:38:24]

Description was changed from

==========
Replace usage of GURL(origin.Serialize()) with origin.GetURL()

The bulk of this was via the following command:
git grep -l "GURL([a-zA-Z0-9]*\.Serialize())" | xargs sed -i \
's/GURL(\([a-zA-Z0-9]*\)\.Serialize())/\1.GetURL()/g'

Which was then followed by a 'git cl format', and removing changes from
url unit tests, which check that this change has identical semantics.

renderer_host/media also had a ConvertToGURL(Origin) method
which was removed.

BUG=651554
==========

to

==========
Replace usage of GURL(origin.Serialize()) with origin.GetURL()

The bulk of this was via the following command:
git grep -l "GURL([a-zA-Z0-9]*\.Serialize())" | xargs sed -i \
's/GURL(\([a-zA-Z0-9]*\)\.Serialize())/\1.GetURL()/g'

Which was then followed by a 'git cl format', and removing changes from
url unit tests, which check that this change has identical semantics.

renderer_host/media also had ConvertToGURL(Origin) methods
which were removed.

BUG=651554
==========

[commit-bot: I haz the power, 2016-09-30 00:37:49]

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

[commit-bot: I haz the power, 2016-09-30 00:37:50]

Dry run: This issue passed the CQ dry run.

[Charlie Harrison, 2016-10-03 21:59:40]

Description was changed from

==========
Replace usage of GURL(origin.Serialize()) with origin.GetURL()

The bulk of this was via the following command:
git grep -l "GURL([a-zA-Z0-9]*\.Serialize())" | xargs sed -i \
's/GURL(\([a-zA-Z0-9]*\)\.Serialize())/\1.GetURL()/g'

Which was then followed by a 'git cl format', and removing changes from
url unit tests, which check that this change has identical semantics.

renderer_host/media also had ConvertToGURL(Origin) methods
which were removed.

BUG=651554
==========

to

==========
Replace usage of GURL(origin.Serialize()) with origin.GetURL()

The bulk of this was via the following command:
git grep -l "GURL([a-zA-Z0-9\._]*\.Serialize())" | xargs sed -i \
 's/GURL(\([a-zA-Z0-9\._]*\)\.Serialize())/\1.GetURL()/g'

Which was then followed by a 'git cl format', and removing changes from
url unit tests, which check that this change has identical semantics.

renderer_host/media also had ConvertToGURL(Origin) methods
which were removed.

BUG=651554
==========

[Charlie Harrison, 2016-10-03 22:02:14]

Description was changed from

==========
Replace usage of GURL(origin.Serialize()) with origin.GetURL()

The bulk of this was via the following command:
git grep -l "GURL([a-zA-Z0-9\._]*\.Serialize())" | xargs sed -i \
 's/GURL(\([a-zA-Z0-9\._]*\)\.Serialize())/\1.GetURL()/g'

Which was then followed by a 'git cl format', and removing changes from
url unit tests, which check that this change has identical semantics.

renderer_host/media also had ConvertToGURL(Origin) methods
which were removed.

BUG=651554
==========

to

==========
Replace usage of GURL(origin.Serialize()) with origin.GetURL()

The bulk of this was via the following command:
git grep -l "GURL([a-zA-Z0-9\._]*\.Serialize())" | xargs sed -i \
 's/GURL(\([a-zA-Z0-9\._]*\)\.Serialize())/\1.GetURL()/g'

Which was then followed by a 'git cl format', and removing changes from
url unit tests, which check that this change has identical semantics.

renderer_host/media also had ConvertToGURL(Origin) methods
which were removed.

BUG=651554
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Charlie Harrison, 2016-10-03 22:02:17]

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

[commit-bot: I haz the power, 2016-10-03 22:03:13]

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-10-03 22:06:02]

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

[commit-bot: I haz the power, 2016-10-03 22:06:56]

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-10-03 22:14:12]

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

[commit-bot: I haz the power, 2016-10-03 22:14:13]

Dry run: Exceeded global retry quota

[Charlie Harrison, 2016-10-03 22:28:02]

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

[commit-bot: I haz the power, 2016-10-03 22:28:51]

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-10-04 00:05:45]

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

[commit-bot: I haz the power, 2016-10-04 00:05:46]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Charlie Harrison, 2016-10-04 00:12:26]

csharrison@chromium.org changed reviewers:
+ bauerb@chromium.org, johnme@chromium.org, nick@chromium.org,
pkasting@chromium.org, rdevlin.cronin@chromium.org, sdefresne@chromium.org

[Charlie Harrison, 2016-10-04 00:12:27]

Hello everyone,
We now have a way to get URLs from Origins without serializing + parsing. I've
converted some callsites here.

I've listed the files each reviewer needs to look at. Thanks!

bauerb:
  chrome/browser/browsing_data/origin_filter_builder.cc
  chrome/browser/ui/android/bluetooth_chooser_android.cc
johnme:
  chrome/browser/budget_service/budget_database.cc
  chrome/browser/budget_service/budget_manager.cc
nick:
  chrome/browser/site_details.cc
  content/browser/cache_storage/cache_storage_dispatcher_host.cc
  content/browser/frame_host/render_frame_message_filter.cc
  content/browser/indexed_db/indexed_db_backing_store.cc
  content/browser/indexed_db/indexed_db_context_impl.cc
  content/browser/indexed_db/indexed_db_internals_ui.cc
  content/browser/indexed_db/indexed_db_quota_client.cc
  content/browser/indexed_db/indexed_db_unittest.cc
  content/browser/permissions/permission_service_impl.cc
  content/browser/renderer_host/database_message_filter.cc
  content/browser/renderer_host/media/media_stream_manager.cc
  content/browser/renderer_host/media/media_stream_ui_proxy.cc
  content/browser/renderer_host/render_message_filter.cc
  content/renderer/dom_storage/local_storage_cached_area.cc
pkasting:
  chrome/browser/ui/android/bluetooth_chooser_android.cc
  chrome/browser/ui/login/login_handler.cc
  net/base/registry_controlled_domains/registry_controlled_domain.cc
rdevlin.cronin:
  extensions/browser/extension_web_contents_observer.cc
sdefresne:
  components/subresource_filter/core/common/indexed_ruleset.cc
  ios/web/public/origin_util.mm

[Charlie Harrison, 2016-10-04 01:46:39]

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

[commit-bot: I haz the power, 2016-10-04 01:47:18]

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-10-04 02:51:58]

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

[commit-bot: I haz the power, 2016-10-04 02:52:01]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Bernhard Bauer, 2016-10-04 09:24:14]

lgtm

[sdefresne, 2016-10-04 10:59:33]

https://codereview.chromium.org/2385533002/diff/100001/ios/web/public/origin_...
File ios/web/public/origin_util.mm (right):

https://codereview.chromium.org/2385533002/diff/100001/ios/web/public/origin_...
ios/web/public/origin_util.mm:44: return origin_tuple.GetURL();
It look like this code broke the iOS bots with a failing DCHECK.

[Charlie Harrison, 2016-10-04 12:15:22]

https://codereview.chromium.org/2385533002/diff/100001/ios/web/public/origin_...
File ios/web/public/origin_util.mm (right):

https://codereview.chromium.org/2385533002/diff/100001/ios/web/public/origin_...
ios/web/public/origin_util.mm:44: return origin_tuple.GetURL();
On 2016/10/04 10:59:33, sdefresne wrote:
> It look like this code broke the iOS bots with a failing DCHECK.

Yup, I think this indicates problems with the dependent PS that already landed
:/ I'll investigate.

[Charlie Harrison, 2016-10-04 13:13:19]

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

[commit-bot: I haz the power, 2016-10-04 13:13:38]

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-10-04 13:16:10]

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

[commit-bot: I haz the power, 2016-10-04 13:16:11]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  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-10-04 13:37:31]

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

[commit-bot: I haz the power, 2016-10-04 13:37:49]

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-10-04 14:17:32]

I've added a dependent PS fix that seems to have fixed the iOS problems.

The bug was because SchemeHostPort::GetURL() was returning wrong data for
invalid SchemeHostPorts, but the bug is masked for invalid Origins which are
handled properly.

[commit-bot: I haz the power, 2016-10-04 14:36:36]

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

[commit-bot: I haz the power, 2016-10-04 14:36:37]

Dry run: This issue passed the CQ dry run.

[Devlin, 2016-10-04 14:49:07]

extensions lgtm

[sdefresne, 2016-10-04 15:10:47]

components/subresource_filter/core/common/indexed_ruleset.cc
  ios/web/public/origin_util.mm

both lgtm

[sdefresne, 2016-10-04 15:10:47]

components/subresource_filter/core/common/indexed_ruleset.cc
  ios/web/public/origin_util.mm

both lgtm

[ncarter (slow), 2016-10-04 18:21:06]

content lgtm with one further simplification

https://codereview.chromium.org/2385533002/diff/140001/content/browser/frame_...
File content/browser/frame_host/render_frame_message_filter.cc (right):

https://codereview.chromium.org/2385533002/diff/140001/content/browser/frame_...
content/browser/frame_host/render_frame_message_filter.cc:497:
main_frame_origin.unique() ? GURL() : main_frame_origin.GetURL();
GURL policy_url = main_frame_origin.GetURL();

[Charlie Harrison, 2016-10-04 19:09:56]

Thanks! ncarter: would you also mind taking a look at site_details.cc? You're
the only reviewer I have for that one.

https://codereview.chromium.org/2385533002/diff/140001/content/browser/frame_...
File content/browser/frame_host/render_frame_message_filter.cc (right):

https://codereview.chromium.org/2385533002/diff/140001/content/browser/frame_...
content/browser/frame_host/render_frame_message_filter.cc:497:
main_frame_origin.unique() ? GURL() : main_frame_origin.GetURL();
On 2016/10/04 18:21:06, ncarter wrote:
> GURL policy_url = main_frame_origin.GetURL();

Done.

[ncarter (slow), 2016-10-04 20:01:46]

site_details lgtm too

[Peter Kasting, 2016-10-05 00:32:08]

LGTM

https://codereview.chromium.org/2385533002/diff/160001/chrome/browser/ui/logi...
File chrome/browser/ui/login/login_handler.cc (right):

https://codereview.chromium.org/2385533002/diff/160001/chrome/browser/ui/logi...
chrome/browser/ui/login/login_handler.cc:485: }
I don't want to force you to clean this up in this patch, but this whole
conditional looks strange to me.

It seems like it should equivalently* be:

  dialog_form.origin = auth_info.challenger.GetURL();
  DCHECK(auth_info.is_proxy ||
         auth_info.challenger.IsSameOriginWith(url::Origin(request_url)));

*Except that we now don't handle DCHECK failure, since the style guide bans
that.  If there is a security issue here, then probably the DCHECK should be a
CHECK instead?  The referenced Chromium bug is long fixed, so I'm not really
sure what the reference is supposed to mean (maybe "this bugfix should have
resulted in this being unreachable").

If you don't want to make this change in this CL, would you be willing to post
it as a separate CL and find a reviewer familiar enough with this specific code
to look?

[Charlie Harrison, 2016-10-05 12:32:03]

https://codereview.chromium.org/2385533002/diff/160001/chrome/browser/ui/logi...
File chrome/browser/ui/login/login_handler.cc (right):

https://codereview.chromium.org/2385533002/diff/160001/chrome/browser/ui/logi...
chrome/browser/ui/login/login_handler.cc:485: }
On 2016/10/05 00:32:08, Peter Kasting wrote:
> I don't want to force you to clean this up in this patch, but this whole
> conditional looks strange to me.
> 
> It seems like it should equivalently* be:
> 
>   dialog_form.origin = auth_info.challenger.GetURL();
>   DCHECK(auth_info.is_proxy ||
>          auth_info.challenger.IsSameOriginWith(url::Origin(request_url)));
> 
> *Except that we now don't handle DCHECK failure, since the style guide bans
> that.  If there is a security issue here, then probably the DCHECK should be a
> CHECK instead?  The referenced Chromium bug is long fixed, so I'm not really
> sure what the reference is supposed to mean (maybe "this bugfix should have
> resulted in this being unreachable").
> 
> If you don't want to make this change in this CL, would you be willing to post
> it as a separate CL and find a reviewer familiar enough with this specific
code
> to look?

Yikes you're right, this should get cleaned up. I threw up a dependent patch
here:
https://codereview.chromium.org/2392243002/

[Charlie Harrison, 2016-10-07 18:24:08]

johnme, friendly ping :)

[johnme, 2016-10-12 16:35:56]

On 2016/10/07 18:24:08, Charlie Harrison wrote:
> johnme, friendly ping :)

chrome/browser/budget_service/ lgtm (sorry, I was OOO so missed the ping)

[Charlie Harrison, 2016-10-12 16:36:32]

On 2016/10/12 16:35:56, johnme wrote:
> On 2016/10/07 18:24:08, Charlie Harrison wrote:
> > johnme, friendly ping :)
> 
> chrome/browser/budget_service/ lgtm (sorry, I was OOO so missed the ping)

No worries! thanks :)

[Charlie Harrison, 2016-10-12 16:36:41]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2016-10-12 16:36:41]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
rdevlin.cronin@chromium.org, sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2385533002/#ps160001
(title: "ncarter comments")

[commit-bot: I haz the power, 2016-10-12 16:36:55]

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-10-12 16:39:34]

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

[commit-bot: I haz the power, 2016-10-12 16:39:36]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Charlie Harrison, 2016-10-12 17:02:29]

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

[commit-bot: I haz the power, 2016-10-12 17:03:00]

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-10-12 17:03:30]

The CQ bit was unchecked by csharrison@chromium.org

[Charlie Harrison, 2016-10-12 17:03:30]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2016-10-12 17:03:30]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
johnme@chromium.org, nick@chromium.org, pkasting@chromium.org,
rdevlin.cronin@chromium.org, sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2385533002/#ps180001
(title: "sync to #424762")

[commit-bot: I haz the power, 2016-10-12 17:04:01]

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-10-12 18:49:21]

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

[commit-bot: I haz the power, 2016-10-12 18:49:23]

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-10-12 18:59:44]

The CQ bit was checked by csharrison@chromium.org

[commit-bot: I haz the power, 2016-10-12 19:00:27]

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-10-12 19:41:02]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-10-12 19:42:33]

Description was changed from

==========
Replace usage of GURL(origin.Serialize()) with origin.GetURL()

The bulk of this was via the following command:
git grep -l "GURL([a-zA-Z0-9\._]*\.Serialize())" | xargs sed -i \
 's/GURL(\([a-zA-Z0-9\._]*\)\.Serialize())/\1.GetURL()/g'

Which was then followed by a 'git cl format', and removing changes from
url unit tests, which check that this change has identical semantics.

renderer_host/media also had ConvertToGURL(Origin) methods
which were removed.

BUG=651554
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Replace usage of GURL(origin.Serialize()) with origin.GetURL()

The bulk of this was via the following command:
git grep -l "GURL([a-zA-Z0-9\._]*\.Serialize())" | xargs sed -i \
 's/GURL(\([a-zA-Z0-9\._]*\)\.Serialize())/\1.GetURL()/g'

Which was then followed by a 'git cl format', and removing changes from
url unit tests, which check that this change has identical semantics.

renderer_host/media also had ConvertToGURL(Origin) methods
which were removed.

BUG=651554
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[commit-bot: I haz the power, 2016-10-12 19:42:35]

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