Reenable framebusting deprecation, change it to allow navigation if iframe has ever had a user gestu

Reenable framebusting deprecation, change it to allow navigation if iframe has ever had a user gesture

Intent to remove: https://groups.google.com/a/chromium.org/d/msg/Blink-dev/Xi8-y4ySjA4/D80epeAMAAAJ

BUG=624061
TEST=http/tests/security/frameNavigation/xss-DENIED-top-navigation-user-gesture-in-parent.html
TEST=http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-async.html

Committed: https://crrev.com/5d8d5cef1e1e9e98f19cea4bcca21e9787a1f686
Cr-Commit-Position: refs/heads/master@{#427735}

[Nate Chapin, 2016-10-03 23:09:48]

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

[commit-bot: I haz the power, 2016-10-03 23:10:35]

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

[Nate Chapin, 2016-10-03 23:44:36]

Description was changed from

==========
Change framebusting deprecation to allow navigation if iframe has ever had a
user gesture

BUG=624064
TEST=http/tests/security/frameNavigation/xss-DENIED-top-navigation-user-gesture-in-parent.html
==========

to

==========
Change framebusting deprecation to allow navigation if iframe has ever had a
user gesture

BUG=624064
TEST=http/tests/security/frameNavigation/xss-DENIED-top-navigation-user-gesture-in-parent.html
TEST=http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-async.html
==========

[Nate Chapin, 2016-10-03 23:45:04]

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

[commit-bot: I haz the power, 2016-10-03 23:46:11]

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:51:59]

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

[commit-bot: I haz the power, 2016-10-04 00:51:59]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Nate Chapin, 2016-10-04 17:47:21]

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

[commit-bot: I haz the power, 2016-10-04 17:47:48]

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

[Nate Chapin, 2016-10-04 19:04:14]

japhet@chromium.org changed reviewers:
+ rbyers@chromium.org

[Nate Chapin, 2016-10-04 19:04:15]

rbyers, would you mind looking at this? I don't know if I hooked the input code
in the right place(s). This ensures that we set the flag that permits
framebusting for mousepress/tap/gesture, but not mouse moves.

[Nate Chapin, 2016-10-04 19:04:25]

japhet@chromium.org changed reviewers:
+ ojan@chromium.org

[Nate Chapin, 2016-10-04 19:05:33]

On 2016/10/04 19:04:15, Nate Chapin wrote:
> rbyers, would you mind looking at this? I don't know if I hooked the input
code
> in the right place(s). This ensures that we set the flag that permits
> framebusting for mousepress/tap/gesture, but not mouse moves.

BTW, do I need my Intent re-LGTMx3ed to change this intervention to be less
aggressive?

[commit-bot: I haz the power, 2016-10-04 19:27:23]

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

[commit-bot: I haz the power, 2016-10-04 19:27:23]

Dry run: This issue passed the CQ dry run.

[ojan, 2016-10-04 20:02:24]

We don't have an official policy on that AFAIK, but IMO, making the
intervention strictly less aggressive doesn't need another round of LGTMs.
It's worth doing a simple update on that same thread though for folks
following along.

+Rick Byers <rbyers@google.com> +Dru Knox <dknox@google.com> in case you
disagree.

On Tue, Oct 4, 2016 at 8:05 PM <japhet@chromium.org> wrote:

> On 2016/10/04 19:04:15, Nate Chapin wrote:
> > rbyers, would you mind looking at this? I don't know if I hooked the
> input
> code
> > in the right place(s). This ensures that we set the flag that permits
> > framebusting for mousepress/tap/gesture, but not mouse moves.
>
> BTW, do I need my Intent re-LGTMx3ed to change this intervention to be less
> aggressive?
>
> https://codereview.chromium.org/2392773002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[ojan, 2016-10-04 20:02:24]

We don't have an official policy on that AFAIK, but IMO, making the
intervention strictly less aggressive doesn't need another round of LGTMs.
It's worth doing a simple update on that same thread though for folks
following along.

+Rick Byers <rbyers@google.com> +Dru Knox <dknox@google.com> in case you
disagree.

On Tue, Oct 4, 2016 at 8:05 PM <japhet@chromium.org> wrote:

> On 2016/10/04 19:04:15, Nate Chapin wrote:
> > rbyers, would you mind looking at this? I don't know if I hooked the
> input
> code
> > in the right place(s). This ensures that we set the flag that permits
> > framebusting for mousepress/tap/gesture, but not mouse moves.
>
> BTW, do I need my Intent re-LGTMx3ed to change this intervention to be less
> aggressive?
>
> https://codereview.chromium.org/2392773002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[blink-reviews, 2016-10-04 22:56:22]

Sounds reasonable to me!

On Tue, Oct 4, 2016 at 1:02 PM Ojan Vafai <ojan@chromium.org> wrote:

> We don't have an official policy on that AFAIK, but IMO, making the
> intervention strictly less aggressive doesn't need another round of LGTMs.
> It's worth doing a simple update on that same thread though for folks
> following along.
>
> +Rick Byers <rbyers@google.com> +Dru Knox <dknox@google.com> in case you
> disagree.
>
> On Tue, Oct 4, 2016 at 8:05 PM <japhet@chromium.org> wrote:
>
> On 2016/10/04 19:04:15, Nate Chapin wrote:
> > rbyers, would you mind looking at this? I don't know if I hooked the
> input
> code
> > in the right place(s). This ensures that we set the flag that permits
> > framebusting for mousepress/tap/gesture, but not mouse moves.
>
> BTW, do I need my Intent re-LGTMx3ed to change this intervention to be less
> aggressive?
>
> https://codereview.chromium.org/2392773002/
>
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-10-04 22:56:22]

Sounds reasonable to me!

On Tue, Oct 4, 2016 at 1:02 PM Ojan Vafai <ojan@chromium.org> wrote:

> We don't have an official policy on that AFAIK, but IMO, making the
> intervention strictly less aggressive doesn't need another round of LGTMs.
> It's worth doing a simple update on that same thread though for folks
> following along.
>
> +Rick Byers <rbyers@google.com> +Dru Knox <dknox@google.com> in case you
> disagree.
>
> On Tue, Oct 4, 2016 at 8:05 PM <japhet@chromium.org> wrote:
>
> On 2016/10/04 19:04:15, Nate Chapin wrote:
> > rbyers, would you mind looking at this? I don't know if I hooked the
> input
> code
> > in the right place(s). This ensures that we set the flag that permits
> > framebusting for mousepress/tap/gesture, but not mouse moves.
>
> BTW, do I need my Intent re-LGTMx3ed to change this intervention to be less
> aggressive?
>
> https://codereview.chromium.org/2392773002/
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nate Chapin, 2016-10-05 20:07:09]

japhet@chromium.org changed reviewers:
+ mustaq@chromium.org

[Nate Chapin, 2016-10-05 20:07:09]

+mustaq

[Rick Byers, 2016-10-05 20:32:31]

Agree this doesn't need API owner approval.

https://codereview.chromium.org/2392773002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/2392773002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Document.cpp:3372: m_hasReceivedUserGesture =
true;
This means that Document::m_hasReceivedUserGesture has a pretty different
definition for "user gesture" than our existing definition in
UserGestureIndicator.  For example, a hovering mousemove doesn't take a UGI (we
don't want just moving your mouse over a page to be able to launch pop-up
windows) but it appears to set this bool here.  The logic that decides what
exactly takes a UserGestureIndicator is fairly complex and spread out across a
number of scenarios (eg. see TouchEventManager::handleTouchEvent), I think it
could be dangerous to try to duplicate that logic, or do have some other
definition of "user gesture" with alternate semantics.

Unfortunately it appears to be non-trivial to re-use the existing logic. 
Basically we'd have to find all the places a UserGestureIndicator is created and
set the flag on some Document at that time.

One option for getting this closer to the right model would be to take each line
you've added like this and replace it with:

if (UserGestureIndicator::ProcessingUserGesture())
  m_hasReceivedUserGesture=true

That'll possibly still miss some cases (if we fail to instrument the right
places for every case that a UserGestureIndicator can be created), but at least
won't have problems with false positives.

Also note that this usage of UserGestureIndicator data will be a blind-spot for
the UserGestureUtilizedCallback stuff.  If you actually do something different
as a result of m_hasReceivedUserGesture being true, the UserGestureIndicator
framework won't know about that.  There are other blind spots and I believe
it'll only affect metrics (which try to measure how much breakage to expect by
requiring a UserGestureIndicator in a new place).  So it's probably OK, but
still worth being aware of as a problem with this design.

[ojan, 2016-10-05 20:44:10]

I think storing the bit on Document is fine, but divorcing the code from
UserGestureIndicator is probably more technical debt that we should take on?

Also, am I misreading the PointerEventManager code? It never creates
UserGestureIndicators today, so pointer events don't get a user gesture? I guess
the touch and mouse ones do, so it doesn't matter?

[Nate Chapin, 2016-10-05 21:07:21]

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

[commit-bot: I haz the power, 2016-10-05 21:08:09]

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

[Nate Chapin, 2016-10-05 21:43:56]

https://codereview.chromium.org/2392773002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/2392773002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Document.cpp:3372: m_hasReceivedUserGesture =
true;
On 2016/10/05 20:32:31, Rick Byers wrote:
> This means that Document::m_hasReceivedUserGesture has a pretty different
> definition for "user gesture" than our existing definition in
> UserGestureIndicator.  For example, a hovering mousemove doesn't take a UGI
(we
> don't want just moving your mouse over a page to be able to launch pop-up
> windows) but it appears to set this bool here.  The logic that decides what
> exactly takes a UserGestureIndicator is fairly complex and spread out across a
> number of scenarios (eg. see TouchEventManager::handleTouchEvent), I think it
> could be dangerous to try to duplicate that logic, or do have some other
> definition of "user gesture" with alternate semantics.
> 
> Unfortunately it appears to be non-trivial to re-use the existing logic. 
> Basically we'd have to find all the places a UserGestureIndicator is created
and
> set the flag on some Document at that time.
> 
> One option for getting this closer to the right model would be to take each
line
> you've added like this and replace it with:
> 
> if (UserGestureIndicator::ProcessingUserGesture())
>   m_hasReceivedUserGesture=true
> 
> That'll possibly still miss some cases (if we fail to instrument the right
> places for every case that a UserGestureIndicator can be created), but at
least
> won't have problems with false positives.
> 
> Also note that this usage of UserGestureIndicator data will be a blind-spot
for
> the UserGestureUtilizedCallback stuff.  If you actually do something different
> as a result of m_hasReceivedUserGesture being true, the UserGestureIndicator
> framework won't know about that.  There are other blind spots and I believe
> it'll only affect metrics (which try to measure how much breakage to expect by
> requiring a UserGestureIndicator in a new place).  So it's probably OK, but
> still worth being aware of as a problem with this design.

Ah, I'd missed the subtlety that a PlatfromEvent doesn't guarantee a
UserGestureIndicator. Done.

[commit-bot: I haz the power, 2016-10-05 22:53:52]

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

[commit-bot: I haz the power, 2016-10-05 22:53:53]

Dry run: This issue passed the CQ dry run.

[ojan, 2016-10-05 23:39:33]

If we do move forward with this as a rush for M55, I think there should be TODOs
to fix this more generally (right after) to have UserGestureIndicator take a
Document as an argument.

Also, do you not need to touch TouchEventManager?

[Nate Chapin, 2016-10-05 23:56:41]

On 2016/10/05 23:39:33, ojan wrote:
> If we do move forward with this as a rush for M55, I think there should be
TODOs
> to fix this more generally (right after) to have UserGestureIndicator take a
> Document as an argument.
> 
> Also, do you not need to touch TouchEventManager?

I have no idea! Tried to hunt down all of the various input managers and ensure
they were covered, but I'm ~clueless i core/input/.

Added TODOs.

[Nate Chapin, 2016-10-05 23:57:25]

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

[commit-bot: I haz the power, 2016-10-05 23:58:19]

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

[Navid Zolghadr, 2016-10-06 00:50:38]

On 2016/10/05 23:56:41, Nate Chapin wrote:
> On 2016/10/05 23:39:33, ojan wrote:
> > If we do move forward with this as a rush for M55, I think there should be
> TODOs
> > to fix this more generally (right after) to have UserGestureIndicator take a
> > Document as an argument.
> > 
> > Also, do you not need to touch TouchEventManager?
> 
> I have no idea! Tried to hunt down all of the various input managers and
ensure
> they were covered, but I'm ~clueless i core/input/.
> 
> Added TODOs.

@Ojan, regarding the question you asked for user gestures in pointerevents I
have this bug that I need to create gesture token in PointerEventManager as the
pointerevents will be fired before the gesture token for touch/mouse are
generated. Here is the issue: crbug.com/609363

@Nate, I was looking at the bug you referenced in the CL and didn't quite
understand what it was. Can you give me a quick explanation of what this bit
does or supposed to do? I might be able to help you where you need to add these
then.

[commit-bot: I haz the power, 2016-10-06 03:38:41]

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

[commit-bot: I haz the power, 2016-10-06 03:38:42]

Dry run: This issue passed the CQ dry run.

[Rick Byers, 2016-10-06 15:45:25]

On 2016/10/06 00:50:38, Navid Zolghadr wrote:
> On 2016/10/05 23:56:41, Nate Chapin wrote:
> > On 2016/10/05 23:39:33, ojan wrote:
> > > If we do move forward with this as a rush for M55, I think there should be
> > TODOs
> > > to fix this more generally (right after) to have UserGestureIndicator take
a
> > > Document as an argument.

Yes something like that is the ideal solution, and I agree that if we're going
to depend on Document/UGI association then we should file a bug to track and add
TODO entries.  

It's not quite as simple as you describe though.  Often when we take a UGI we
don't know what Document is may be related to (eg. before we've done a hit-test
- though I don't know if ALL those cases could be moved to after the hit test). 
Also, might one UGI be associated with multiple documents?  Eg. when I click on
an iframe am I associating a UGI with the top document as well as the frame's
document?  From the user's perspective I think I probably am.  Perhaps it would
be good enough to have UserGestureIndicator have an 'associateWithDocument'
method, rather than require a Document in the constructor?

> > > 
> > > Also, do you not need to touch TouchEventManager?
> > 
> > I have no idea! Tried to hunt down all of the various input managers and
> ensure
> > they were covered, but I'm ~clueless i core/input/.
> > 
> > Added TODOs.
> 
> @Ojan, regarding the question you asked for user gestures in pointerevents I
> have this bug that I need to create gesture token in PointerEventManager as
the
> pointerevents will be fired before the gesture token for touch/mouse are
> generated. Here is the issue: crbug.com/609363

Whoa, that sounds scarier than I thought things were (or my tests seem to
indicate)!  Added a comment in the bug.

> @Nate, I was looking at the bug you referenced in the CL and didn't quite
> understand what it was. Can you give me a quick explanation of what this bit
> does or supposed to do? I might be able to help you where you need to add
these
> then.

[Rick Byers, 2016-10-06 16:13:54]

Description was changed from

==========
Change framebusting deprecation to allow navigation if iframe has ever had a
user gesture

BUG=624064
TEST=http/tests/security/frameNavigation/xss-DENIED-top-navigation-user-gesture-in-parent.html
TEST=http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-async.html
==========

to

==========
Change framebusting deprecation to allow navigation if iframe has ever had a
user gesture

BUG=624061
TEST=http/tests/security/frameNavigation/xss-DENIED-top-navigation-user-gesture-in-parent.html
TEST=http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-async.html
==========

[Rick Byers, 2016-10-06 16:14:51]

On 2016/10/06 00:50:38, Navid Zolghadr wrote:
> On 2016/10/05 23:56:41, Nate Chapin wrote:
> > On 2016/10/05 23:39:33, ojan wrote:
> > > If we do move forward with this as a rush for M55, I think there should be
> > TODOs
> > > to fix this more generally (right after) to have UserGestureIndicator take
a
> > > Document as an argument.
> > > 
> > > Also, do you not need to touch TouchEventManager?
> > 
> > I have no idea! Tried to hunt down all of the various input managers and
> ensure
> > they were covered, but I'm ~clueless i core/input/.
> > 
> > Added TODOs.
> 
> @Ojan, regarding the question you asked for user gestures in pointerevents I
> have this bug that I need to create gesture token in PointerEventManager as
the
> pointerevents will be fired before the gesture token for touch/mouse are
> generated. Here is the issue: crbug.com/609363
> 
> @Nate, I was looking at the bug you referenced in the CL and didn't quite
> understand what it was. Can you give me a quick explanation of what this bit
> does or supposed to do? I might be able to help you where you need to add
these
> then.

The BUG reference in the CL description was wrong, I updated it.  See
http://crbug.com/624061

[Rick Byers, 2016-10-06 16:26:47]

Ojan's comments got me thinking through the details some more.  I think you guys
need to decide how bad it is to be missing some UserGestureIndicator cases.

https://codereview.chromium.org/2392773002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/Document.h (right):

https://codereview.chromium.org/2392773002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/Document.h:1287: bool
hasReceivedUserGesture() const { return m_hasReceivedUserGesture; }
nit: add a comment documenting the semantics here, especially the limitation
that it isn't yet aware of ALL sources of user gestures.

https://codereview.chromium.org/2392773002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/Frame.cpp (right):

https://codereview.chromium.org/2392773002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/Frame.cpp:175: ?
toLocalFrame(this)->document()->hasReceivedUserGesture()
I've been looking over all the places that take a UserGestureIndicator and I'm
nervous about the other cases you haven't yet instrumented.  For example, it
seems there are accessibility scenarios.  It would be bad it frame-busting
worked for input-driven navigation but not accessibility-tool driven navigation,
right?

One mitigation could be to also check processingUserGesture here.  But really
the only full solution is probably to audit every single instance of
UserGestureIndicator and associate a Document with it :-(

https://codereview.chromium.org/2392773002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/GestureManager.cpp (right):

https://codereview.chromium.org/2392773002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/GestureManager.cpp:144:
UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture);
for really simple cases like this, perhaps we should just add the Document
argument to the UserGestureIndicator constructor and use it?  Then you only need
TODOs for the cases where you don't know the Document as UserGestureIndicator
construction time.

https://codereview.chromium.org/2392773002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/PointerEventManager.cpp (right):

https://codereview.chromium.org/2392773002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:292: if
(UserGestureIndicator::processingUserGesture())
I think this is also always the root frame document.  But it doesn't matter
because (I think) there's never a UGI at this point anyway
(http://crbug.com/609363).  Maybe just drop this for now and ask Navid to make
sure he adds the necessary call when he fixes his bug?

https://codereview.chromium.org/2392773002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/TouchEventManager.cpp (right):

https://codereview.chromium.org/2392773002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/TouchEventManager.cpp:546:
m_frame->document()->setHasReceivedUserGesture();
I believe this is the wrong document.  Here m_frame is, I believe, always the
local root frame (though there should at least be asserts for that - see
http://crbug.com/449649).  Instead, for touch events, you want
m_touchSequenceDocument.

[Nate Chapin, 2016-10-25 22:51:58]

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

[commit-bot: I haz the power, 2016-10-25 22:52:26]

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

[Nate Chapin, 2016-10-25 22:55:04]

With https://codereview.chromium.org/2408333004/ landed, this change should be
down to just making the framebusting intervention depend on
Document::hasReceivedUserGesture and flipping framebusting back to enabled. PTAL

[commit-bot: I haz the power, 2016-10-26 00:13:27]

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

[commit-bot: I haz the power, 2016-10-26 00:13:28]

Dry run: This issue passed the CQ dry run.

[Rick Byers, 2016-10-26 13:45:53]

Description was changed from

==========
Change framebusting deprecation to allow navigation if iframe has ever had a
user gesture

BUG=624061
TEST=http/tests/security/frameNavigation/xss-DENIED-top-navigation-user-gesture-in-parent.html
TEST=http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-async.html
==========

to

==========
Change framebusting deprecation to allow navigation if iframe has ever had a
user gesture

Intent to remove:
https://groups.google.com/a/chromium.org/d/msg/Blink-dev/Xi8-y4ySjA4/D80epeAM...

BUG=624061
TEST=http/tests/security/frameNavigation/xss-DENIED-top-navigation-user-gesture-in-parent.html
TEST=http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-async.html
==========

[Rick Byers, 2016-10-26 13:46:33]

On 2016/10/25 22:55:04, Nate Chapin wrote:
> With https://codereview.chromium.org/2408333004/ landed, this change should be
> down to just making the framebusting intervention depend on
> Document::hasReceivedUserGesture and flipping framebusting back to enabled.
PTAL

LGTM

I added a link to the intent thread to your CL description.

[Rick Byers, 2016-10-26 13:47:11]

On 2016/10/26 13:46:33, Rick Byers wrote:
> On 2016/10/25 22:55:04, Nate Chapin wrote:
> > With https://codereview.chromium.org/2408333004/ landed, this change should
be
> > down to just making the framebusting intervention depend on
> > Document::hasReceivedUserGesture and flipping framebusting back to enabled.
> PTAL
> 
> LGTM
> 
> I added a link to the intent thread to your CL description.

Oh and please update the CL description to make it clear that this CL is
shipping the intervention!

[Nate Chapin, 2016-10-26 16:38:15]

Description was changed from

==========
Change framebusting deprecation to allow navigation if iframe has ever had a
user gesture

Intent to remove:
https://groups.google.com/a/chromium.org/d/msg/Blink-dev/Xi8-y4ySjA4/D80epeAM...

BUG=624061
TEST=http/tests/security/frameNavigation/xss-DENIED-top-navigation-user-gesture-in-parent.html
TEST=http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-async.html
==========

to

==========
Reenable framebusting deprecation, change it to allow navigation if iframe has
ever had a user gesture

Intent to remove:
https://groups.google.com/a/chromium.org/d/msg/Blink-dev/Xi8-y4ySjA4/D80epeAM...

BUG=624061
TEST=http/tests/security/frameNavigation/xss-DENIED-top-navigation-user-gesture-in-parent.html
TEST=http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-async.html
==========

[ojan, 2016-10-26 16:39:16]

lgtm

[Nate Chapin, 2016-10-26 16:39:44]

The CQ bit was checked by japhet@chromium.org

[commit-bot: I haz the power, 2016-10-26 16:40:13]

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-26 16:50:25]

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

[commit-bot: I haz the power, 2016-10-26 16:50:26]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Nate Chapin, 2016-10-26 16:53:52]

japhet@chromium.org changed reviewers:
+ creis@chromium.org

[Nate Chapin, 2016-10-26 16:53:52]

creis, up for a quick 4-line test change in content/?

[Charlie Reis, 2016-10-26 17:18:10]

content/ LGTM, thanks!

[Nate Chapin, 2016-10-26 17:19:06]

The CQ bit was checked by japhet@chromium.org

[commit-bot: I haz the power, 2016-10-26 17:19:44]

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-26 17:38:40]

Description was changed from

==========
Reenable framebusting deprecation, change it to allow navigation if iframe has
ever had a user gesture

Intent to remove:
https://groups.google.com/a/chromium.org/d/msg/Blink-dev/Xi8-y4ySjA4/D80epeAM...

BUG=624061
TEST=http/tests/security/frameNavigation/xss-DENIED-top-navigation-user-gesture-in-parent.html
TEST=http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-async.html
==========

to

==========
Reenable framebusting deprecation, change it to allow navigation if iframe has
ever had a user gesture

Intent to remove:
https://groups.google.com/a/chromium.org/d/msg/Blink-dev/Xi8-y4ySjA4/D80epeAM...

BUG=624061
TEST=http/tests/security/frameNavigation/xss-DENIED-top-navigation-user-gesture-in-parent.html
TEST=http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-async.html
==========

[commit-bot: I haz the power, 2016-10-26 17:38:43]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-10-26 17:51:03]

Description was changed from

==========
Reenable framebusting deprecation, change it to allow navigation if iframe has
ever had a user gesture

Intent to remove:
https://groups.google.com/a/chromium.org/d/msg/Blink-dev/Xi8-y4ySjA4/D80epeAM...

BUG=624061
TEST=http/tests/security/frameNavigation/xss-DENIED-top-navigation-user-gesture-in-parent.html
TEST=http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-async.html
==========

to

==========
Reenable framebusting deprecation, change it to allow navigation if iframe has
ever had a user gesture

Intent to remove:
https://groups.google.com/a/chromium.org/d/msg/Blink-dev/Xi8-y4ySjA4/D80epeAM...

BUG=624061
TEST=http/tests/security/frameNavigation/xss-DENIED-top-navigation-user-gesture-in-parent.html
TEST=http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-async.html

Committed: https://crrev.com/5d8d5cef1e1e9e98f19cea4bcca21e9787a1f686
Cr-Commit-Position: refs/heads/master@{#427735}
==========

[commit-bot: I haz the power, 2016-10-26 17:51:04]

Patchset 7 (id:??) landed as
https://crrev.com/5d8d5cef1e1e9e98f19cea4bcca21e9787a1f686
Cr-Commit-Position: refs/heads/master@{#427735}