Rename Origin.unique() to opaque().

Rename Origin.unique() to opaque().

The Origin class has gotten out of date with respect to the spec (and with blink::SecurityOrigin); the concept of a "unique" origin has mostly been replaced with "opaque"ness. This almost purely mechanical CL updates that terminology; a follow-up CL will take care of the small semantic difference between the two concepts.

BUG=695622
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

[iclelland, 2017-02-23 15:03:32]

Description was changed from

==========
Rename Origin.unique to opaque.

BUG=
==========

to

==========
Rename Origin.unique to opaque.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[iclelland, 2017-02-23 15:03:33]

Description was changed from

==========
Rename Origin.unique to opaque.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Rename Origin.unique to opaque.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[iclelland, 2017-02-23 15:03:37]

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

[commit-bot: I haz the power, 2017-02-23 15:03:57]

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, 2017-02-23 15:29:06]

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

[commit-bot: I haz the power, 2017-02-23 15:29:07]

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

[iclelland, 2017-02-23 16:50:12]

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

[commit-bot: I haz the power, 2017-02-23 16:50:52]

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, 2017-02-23 17:16:54]

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

[commit-bot: I haz the power, 2017-02-23 17:16:54]

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_...)

[iclelland, 2017-02-23 18:29:47]

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

[commit-bot: I haz the power, 2017-02-23 18:30:39]

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, 2017-02-23 21:17:34]

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

[commit-bot: I haz the power, 2017-02-23 21:17:35]

Dry run: This issue passed the CQ dry run.

[iclelland, 2017-02-23 21:47:50]

Description was changed from

==========
Rename Origin.unique to opaque.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Rename Origin.unique() to opaque().

The Origin class has gotten out of date with respect to the spec (and with
blink::SecurityOrigin); the concept of a "unique" origin has mostly been
replaced with "opaque"ness. This almost purely mechanical CL updates that
terminology; a follow-up CL will take care of the small semantic difference
between the two concepts.

BUG=695622
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[iclelland, 2017-02-23 21:47:50]

iclelland@chromium.org changed reviewers:
+ mkwst@chromium.org

[iclelland, 2017-02-23 21:48:11]

+r mkwst -- can you PTAL?

[dcheng, 2017-02-23 21:57:44]

dcheng@chromium.org changed reviewers:
+ dcheng@chromium.org

[dcheng, 2017-02-23 21:57:45]

Will there be a followup patch on the Blink side?

I also think it's also a bit confusing to rename to opaque without laying the
groundwork for comparing opaque URLs in url::Origin.

[ortuno, 2017-02-23 21:59:34]

ortuno@chromium.org changed reviewers:
+ ortuno@chromium.org
- dcheng@chromium.org

[ortuno, 2017-02-23 21:59:34]

https://codereview.chromium.org/2716583003/diff/40001/content/browser/bluetoo...
File content/browser/bluetooth/bluetooth_device_chooser_controller.cc (right):

https://codereview.chromium.org/2716583003/diff/40001/content/browser/bluetoo...
content/browser/bluetooth/bluetooth_device_chooser_controller.cc:351: // The
above also excludes opaque origins, which are not even same-origin with
Do opaque origins still generate the same key in maps? If so could you change
this DCHECK to an if statement?

// Opaque origins generate the same key in maps, therefore are not
// supported.
// TODO(crbug.com/...): Change to REQUEST_DEVICE_FROM_UNSUPPORTED_ORIGIN.
if (request_origin.opaque()) {
  PostErrorCallback(blink::mojom::WebBluetoothResult::
                        REQUEST_DEVICE_FROM_CROSS_ORIGIN_IFRAME);
}

Please CC me and juncai in that bug.

[iclelland, 2017-02-23 23:32:37]

On 2017/02/23 21:57:45, dcheng wrote:
> Will there be a followup patch on the Blink side?

Yes, definitely.

> 
> I also think it's also a bit confusing to rename to opaque without laying the
> groundwork for comparing opaque URLs in url::Origin.

The CL that allows them to compare equal to themselves is
https://codereview.chromium.org/2688573002/; just not sure which one makes sense
to land first.

[dcheng, 2017-02-23 23:36:35]

On 2017/02/23 23:32:37, iclelland wrote:
> On 2017/02/23 21:57:45, dcheng wrote:
> > Will there be a followup patch on the Blink side?
> 
> Yes, definitely.
> 
> > 
> > I also think it's also a bit confusing to rename to opaque without laying
the
> > groundwork for comparing opaque URLs in url::Origin.
> 
> The CL that allows them to compare equal to themselves is
> https://codereview.chromium.org/2688573002/; just not sure which one makes
sense
> to land first.

Unfortunately, we can't really do identity comparison for url::Origin. This
works in Blink because SecurityOrigin is refcounted and noncopyable. However,
url::Origin is copyable, so this will fail in unexpected ways.

[Marijn Kruisselbrink, 2017-02-23 23:40:45]

On 2017/02/23 at 23:36:35, dcheng wrote:
> On 2017/02/23 23:32:37, iclelland wrote:
> > On 2017/02/23 21:57:45, dcheng wrote:
> > > Will there be a followup patch on the Blink side?
> > 
> > Yes, definitely.
> > 
> > > 
> > > I also think it's also a bit confusing to rename to opaque without laying
the
> > > groundwork for comparing opaque URLs in url::Origin.
> > 
> > The CL that allows them to compare equal to themselves is
> > https://codereview.chromium.org/2688573002/; just not sure which one makes
sense
> > to land first.
> 
> Unfortunately, we can't really do identity comparison for url::Origin. This
works in Blink because SecurityOrigin is refcounted and noncopyable. However,
url::Origin is copyable, so this will fail in unexpected ways.

Not that even in Blink it doesn't work entirely, because SecurityOrigin is not
thread safe (so we can't have same-opaque-origin across threads). This causes
problems for example around loading stuff from workers.

[dcheng, 2017-02-23 23:55:13]

On 2017/02/23 23:40:45, Marijn Kruisselbrink wrote:
> On 2017/02/23 at 23:36:35, dcheng wrote:
> > On 2017/02/23 23:32:37, iclelland wrote:
> > > On 2017/02/23 21:57:45, dcheng wrote:
> > > > Will there be a followup patch on the Blink side?
> > > 
> > > Yes, definitely.
> > > 
> > > > 
> > > > I also think it's also a bit confusing to rename to opaque without
laying
> the
> > > > groundwork for comparing opaque URLs in url::Origin.
> > > 
> > > The CL that allows them to compare equal to themselves is
> > > https://codereview.chromium.org/2688573002/; just not sure which one makes
> sense
> > > to land first.
> > 
> > Unfortunately, we can't really do identity comparison for url::Origin. This
> works in Blink because SecurityOrigin is refcounted and noncopyable. However,
> url::Origin is copyable, so this will fail in unexpected ways.
> 
> Not that even in Blink it doesn't work entirely, because SecurityOrigin is not
> thread safe (so we can't have same-opaque-origin across threads). This causes
> problems for example around loading stuff from workers.

Oh fun. Since I want origins to round trip through IPC, I think we'll want
something like https://codereview.chromium.org/2714813003/, which should help
the worker case.

[Mike West, 2017-02-24 08:01:45]

This LGTM, but I don't own most of the code. :)

https://codereview.chromium.org/2716583003/diff/40001/content/browser/bluetoo...
File content/browser/bluetooth/bluetooth_device_chooser_controller.cc (right):

https://codereview.chromium.org/2716583003/diff/40001/content/browser/bluetoo...
content/browser/bluetooth/bluetooth_device_chooser_controller.cc:351: // The
above also excludes opaque origins, which are not even same-origin with
On 2017/02/23 at 21:59:34, ortuno wrote:
> Do opaque origins still generate the same key in maps? If so could you change
this DCHECK to an if statement?
> 
> // Opaque origins generate the same key in maps, therefore are not
> // supported.
> // TODO(crbug.com/...): Change to REQUEST_DEVICE_FROM_UNSUPPORTED_ORIGIN.
> if (request_origin.opaque()) {
>   PostErrorCallback(blink::mojom::WebBluetoothResult::
>                         REQUEST_DEVICE_FROM_CROSS_ORIGIN_IFRAME);
> }
> 
> Please CC me and juncai in that bug.

I'd suggest doing this in a separate patch. Mechanical changes are best kept
mechanical.

[iclelland, 2017-08-16 17:28:16]

Description was changed from

==========
Rename Origin.unique() to opaque().

The Origin class has gotten out of date with respect to the spec (and with
blink::SecurityOrigin); the concept of a "unique" origin has mostly been
replaced with "opaque"ness. This almost purely mechanical CL updates that
terminology; a follow-up CL will take care of the small semantic difference
between the two concepts.

BUG=695622
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Rename Origin.unique() to opaque().

The Origin class has gotten out of date with respect to the spec (and with
blink::SecurityOrigin); the concept of a "unique" origin has mostly been
replaced with "opaque"ness. This almost purely mechanical CL updates that
terminology; a follow-up CL will take care of the small semantic difference
between the two concepts.

BUG=695622
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[iclelland, 2017-08-16 17:28:51]

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

[commit-bot: I haz the power, 2017-08-16 17:29:01]

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, 2017-08-16 18:06:54]

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

[commit-bot: I haz the power, 2017-08-16 18:06:55]

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...)