Allow a unique origin to compare equal to itself

Allow a unique origin to compare equal to itself.

The HTML Same-origin and Same-origin-domain algorithms require that opaque origins can be considered to be equal to each other.

See https://html.spec.whatwg.org/multipage/browsers.html#same-origin


BUG=

[iclelland, 2017-02-08 19:43:31]

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

[commit-bot: I haz the power, 2017-02-08 19:44:32]

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-08 20:14:33]

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

[commit-bot: I haz the power, 2017-02-08 20:14:34]

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-10 05:36:09]

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

[commit-bot: I haz the power, 2017-02-10 05:36:36]

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

[iclelland, 2017-02-10 05:41:59]

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

[iclelland, 2017-02-10 05:42:00]

+r mkwst -- This still needs a bug#, spec link, and a bit more info in the
commit message than what's there, but you can take a look at the change itself
here.

[commit-bot: I haz the power, 2017-02-10 07:01:20]

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

[commit-bot: I haz the power, 2017-02-10 07:01:21]

Dry run: This issue passed the CQ dry run.

[Mike West, 2017-02-12 11:23:52]

LGTM if you flesh out the CL description a bit.

[iclelland, 2017-02-23 13:53:02]

Description was changed from

==========
Allow a unique origin to compare equal to itself

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

to

==========
Allow a unique origin to compare equal to itself.

The HTML Same-origin and Same-origin-domain algorithms require that opaque
origins can be considered to be equal to each other.

See https://html.spec.whatwg.org/multipage/browsers.html#same-origin


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

[Marijn Kruisselbrink, 2017-02-23 23:39:18]

drive-by question

https://codereview.chromium.org/2688573002/diff/20001/url/origin.cc
File url/origin.cc (right):

https://codereview.chromium.org/2688573002/diff/20001/url/origin.cc#newcode170
url/origin.cc:170: bool Origin::IsSamePhysicalOriginWith(const Origin& other)
const {
Shouldn't an opaque origin also be considered same physical origin with itself?
I don't think this CL currently does that, as GetPhysicalOrigin() returns a copy
of the origin (so even if this == &other this->GetPhysicalOrigin() is not the
same as other.GetPhysicalOrigin())

[Mike West, 2017-02-24 07:59:59]

On 2017/02/23 at 23:39:18, mek wrote:
> drive-by question
> 
> https://codereview.chromium.org/2688573002/diff/20001/url/origin.cc
> File url/origin.cc (right):
> 
> https://codereview.chromium.org/2688573002/diff/20001/url/origin.cc#newcode170
> url/origin.cc:170: bool Origin::IsSamePhysicalOriginWith(const Origin& other)
const {
> Shouldn't an opaque origin also be considered same physical origin with
itself? I don't think this CL currently does that, as GetPhysicalOrigin()
returns a copy of the origin (so even if this == &other
this->GetPhysicalOrigin() is not the same as other.GetPhysicalOrigin())

FYI: dcheng@ has https://codereview.chromium.org/2714813003 which might obviate
this patch.

[iclelland, 2017-02-24 14:13:03]

On 2017/02/24 07:59:59, Mike West (sloooooow) wrote:
> On 2017/02/23 at 23:39:18, mek wrote:
> > drive-by question
> > 
> > https://codereview.chromium.org/2688573002/diff/20001/url/origin.cc
> > File url/origin.cc (right):
> > 
> >
https://codereview.chromium.org/2688573002/diff/20001/url/origin.cc#newcode170
> > url/origin.cc:170: bool Origin::IsSamePhysicalOriginWith(const Origin&
other)
> const {
> > Shouldn't an opaque origin also be considered same physical origin with
> itself? I don't think this CL currently does that, as GetPhysicalOrigin()
> returns a copy of the origin (so even if this == &other
> this->GetPhysicalOrigin() is not the same as other.GetPhysicalOrigin())
> 
> FYI: dcheng@ has https://codereview.chromium.org/2714813003 which might
obviate
> this patch.

[iclelland, 2017-02-24 14:16:19]

On 2017/02/23 23:39:18, Marijn Kruisselbrink wrote:
> drive-by question
> 
> https://codereview.chromium.org/2688573002/diff/20001/url/origin.cc
> File url/origin.cc (right):
> 
> https://codereview.chromium.org/2688573002/diff/20001/url/origin.cc#newcode170
> url/origin.cc:170: bool Origin::IsSamePhysicalOriginWith(const Origin& other)
> const {
> Shouldn't an opaque origin also be considered same physical origin with
itself?
> I don't think this CL currently does that, as GetPhysicalOrigin() returns a
copy
> of the origin (so even if this == &other this->GetPhysicalOrigin() is not the
> same as other.GetPhysicalOrigin())

(Rietveld ate the text of my last reply)

I think that the answer is yes, but
https://w3c.github.io/webappsec-suborigins/#comparing-physical-origins is silent
on the question; Physical origin isn't defined in a way that accounts for opaque
origins. mkwst or jww might know for sure, though.

[Mike West, 2017-02-24 14:31:07]

On 2017/02/24 at 14:16:19, iclelland wrote:
> I think that the answer is yes, but
https://w3c.github.io/webappsec-suborigins/#comparing-physical-origins is silent
on the question; Physical origin isn't defined in a way that accounts for opaque
origins. mkwst or jww might know for sure, though.

jww@ left the company, jochen@ is now responsible for coming up with a clever
answer to this question. :)

[jochen (gone - plz use gerrit), 2017-03-10 10:46:32]

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

[jochen (gone - plz use gerrit), 2017-03-10 10:46:33]

in the implementation in Blink, we say two origins that are unique but the same
origin object are equal, so this lgtm

[Mike West, 2017-03-10 10:50:12]

On 2017/03/10 at 10:46:33, jochen wrote:
> in the implementation in Blink, we say two origins that are unique but the
same origin object are equal, so this lgtm

I think Ian's question was more specifically about how/whether suborigins change
that calculation. I'm not sure it's possible in our current implementation for
this to happen, but consider two sandboxed `about:blank` frames that are pushed
into `suboriginA` and `suboriginB` respectively. Are those "equal"? Should they
be?

[jochen (gone - plz use gerrit), 2017-03-10 10:55:35]

On 2017/03/10 at 10:50:12, mkwst wrote:
> On 2017/03/10 at 10:46:33, jochen wrote:
> > in the implementation in Blink, we say two origins that are unique but the
same origin object are equal, so this lgtm
> 
> I think Ian's question was more specifically about how/whether suborigins
change that calculation. I'm not sure it's possible in our current
implementation for this to happen, but consider two sandboxed `about:blank`
frames that are pushed into `suboriginA` and `suboriginB` respectively. Are
those "equal"? Should they be?

they wouldn't be equal in anycase, suborigin or not, right, as they're two
distinct frames and so they'll have distinct SecurityOrigin objects.

the check for suborigins is "both sub-origins are equal" && "regular origin
equality check returns true"

[Marijn Kruisselbrink, 2017-03-10 19:48:38]

On 2017/03/10 at 10:55:35, jochen wrote:
> On 2017/03/10 at 10:50:12, mkwst wrote:
> > On 2017/03/10 at 10:46:33, jochen wrote:
> > > in the implementation in Blink, we say two origins that are unique but the
same origin object are equal, so this lgtm
> > 
> > I think Ian's question was more specifically about how/whether suborigins
change that calculation. I'm not sure it's possible in our current
implementation for this to happen, but consider two sandboxed `about:blank`
frames that are pushed into `suboriginA` and `suboriginB` respectively. Are
those "equal"? Should they be?
> 
> they wouldn't be equal in anycase, suborigin or not, right, as they're two
distinct frames and so they'll have distinct SecurityOrigin objects.
> 
> the check for suborigins is "both sub-origins are equal" && "regular origin
equality check returns true"

I don't know how suborigins interact with opaque origins, but in general "two
distinct frames" could very well have the same opaque origin (and hopefully same
SecurityOrigin object).

And yeah, as pointed out before, comparing the non-refcounted url::Origin
objects for object identity is pretty meaningless as they are copyable etc. So
something like the before mentioned CL from dcheng is really the only way to
have any hope of meaningful same-opaque-origin comparison in url::Origin.

[iclelland, 2017-08-16 17:41:15]

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:41:20]

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:49]

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

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

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