Implement referrerpolicy attr for iframes

Implement referrerpolicy attr for iframes

BUG=490608
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201921

[estark, 2015-09-08 03:05:17]

estark@chromium.org changed reviewers:
+ yoav@yoav.ws

[estark, 2015-09-08 03:05:17]

Yoav: do you think you might be able to review this, since you reviewed the
corresponding CL for imgs? Thanks!

[Yoav Weiss, 2015-09-08 09:55:08]

Sure! :)
A few comments below. Also - can you `git cl try` so that we'd see the tests are
passing?

https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLFr...
File Source/core/html/HTMLFrameOwnerElement.cpp (right):

https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLFr...
Source/core/html/HTMLFrameOwnerElement.cpp:268: if
(referrerPolicyAttribute(&policy)) {
I think it would be cleaner to change referrerPolicyAttribute to return the
policy, and have this section conditional on the run time flag.

https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLIF...
File Source/core/html/HTMLIFrameElement.cpp (right):

https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLIF...
Source/core/html/HTMLIFrameElement.cpp:43: , m_didLoadNonEmptyDocument(false)
m_referrerPolicy should be initialized regardless of runtime flag.

https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLIF...
Source/core/html/HTMLIFrameElement.cpp:181: bool
HTMLIFrameElement::referrerPolicyAttribute(ReferrerPolicy* policy)
Like I wrote at the caller site, it'd be better to just return the policy and
check the runtime flag at the caller.

[estark, 2015-09-08 14:40:35]

https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLFr...
File Source/core/html/HTMLFrameOwnerElement.cpp (right):

https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLFr...
Source/core/html/HTMLFrameOwnerElement.cpp:268: if
(referrerPolicyAttribute(&policy)) {
On 2015/09/08 09:55:08, Yoav Weiss wrote:
> I think it would be cleaner to change referrerPolicyAttribute to return the
> policy, and have this section conditional on the run time flag.

Done.

https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLIF...
File Source/core/html/HTMLIFrameElement.cpp (right):

https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLIF...
Source/core/html/HTMLIFrameElement.cpp:181: bool
HTMLIFrameElement::referrerPolicyAttribute(ReferrerPolicy* policy)
On 2015/09/08 09:55:08, Yoav Weiss wrote:
> Like I wrote at the caller site, it'd be better to just return the policy and
> check the runtime flag at the caller.

Done.

https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLIF...
Source/core/html/HTMLIFrameElement.cpp:181: bool
HTMLIFrameElement::referrerPolicyAttribute(ReferrerPolicy* policy)
On 2015/09/08 09:55:08, Yoav Weiss wrote:
> Like I wrote at the caller site, it'd be better to just return the policy and
> check the runtime flag at the caller.

Done.

[Yoav Weiss, 2015-09-08 15:24:30]

On 2015/09/08 14:40:35, estark wrote:
>
https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLFr...
> File Source/core/html/HTMLFrameOwnerElement.cpp (right):
> 
>
https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLFr...
> Source/core/html/HTMLFrameOwnerElement.cpp:268: if
> (referrerPolicyAttribute(&policy)) {
> On 2015/09/08 09:55:08, Yoav Weiss wrote:
> > I think it would be cleaner to change referrerPolicyAttribute to return the
> > policy, and have this section conditional on the run time flag.
> 
> Done.
> 
>
https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLIF...
> File Source/core/html/HTMLIFrameElement.cpp (right):
> 
>
https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLIF...
> Source/core/html/HTMLIFrameElement.cpp:181: bool
> HTMLIFrameElement::referrerPolicyAttribute(ReferrerPolicy* policy)
> On 2015/09/08 09:55:08, Yoav Weiss wrote:
> > Like I wrote at the caller site, it'd be better to just return the policy
and
> > check the runtime flag at the caller.
> 
> Done.
> 
>
https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLIF...
> Source/core/html/HTMLIFrameElement.cpp:181: bool
> HTMLIFrameElement::referrerPolicyAttribute(ReferrerPolicy* policy)
> On 2015/09/08 09:55:08, Yoav Weiss wrote:
> > Like I wrote at the caller site, it'd be better to just return the policy
and
> > check the runtime flag at the caller.
> 
> Done.


The webexposed failure seems relevant.

Other than that, LGTM! :)

[estark, 2015-09-08 16:48:14]

On 2015/09/08 15:24:30, Yoav Weiss wrote:
> On 2015/09/08 14:40:35, estark wrote:
> >
>
https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLFr...
> > File Source/core/html/HTMLFrameOwnerElement.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLFr...
> > Source/core/html/HTMLFrameOwnerElement.cpp:268: if
> > (referrerPolicyAttribute(&policy)) {
> > On 2015/09/08 09:55:08, Yoav Weiss wrote:
> > > I think it would be cleaner to change referrerPolicyAttribute to return
the
> > > policy, and have this section conditional on the run time flag.
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLIF...
> > File Source/core/html/HTMLIFrameElement.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLIF...
> > Source/core/html/HTMLIFrameElement.cpp:181: bool
> > HTMLIFrameElement::referrerPolicyAttribute(ReferrerPolicy* policy)
> > On 2015/09/08 09:55:08, Yoav Weiss wrote:
> > > Like I wrote at the caller site, it'd be better to just return the policy
> and
> > > check the runtime flag at the caller.
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1328183002/diff/20001/Source/core/html/HTMLIF...
> > Source/core/html/HTMLIFrameElement.cpp:181: bool
> > HTMLIFrameElement::referrerPolicyAttribute(ReferrerPolicy* policy)
> > On 2015/09/08 09:55:08, Yoav Weiss wrote:
> > > Like I wrote at the caller site, it'd be better to just return the policy
> and
> > > check the runtime flag at the caller.
> > 
> > Done.
> 
> 
> The webexposed failure seems relevant.

Yep, fixed in patch set 5. Thanks Yoav!

> 
> Other than that, LGTM! :)

[estark, 2015-09-08 16:48:17]

The CQ bit was checked by estark@chromium.org

[commit-bot: I haz the power, 2015-09-08 16:48:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1328183002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1328183002/80001

[commit-bot: I haz the power, 2015-09-08 17:26:17]

Committed patchset #5 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201921