|
|
Created:
4 years, 2 months ago by Nate Chapin Modified:
4 years, 1 month ago CC:
blink-reviews, blink-reviews-dom_chromium.org, chromium-reviews, dcheng, dglazkov+blink, dtapuska+blinkwatch_chromium.org, eae+blinkwatch, mlamouri+watch-blink_chromium.org, nzolghadr+blinkwatch_chromium.org, rwlbuis, sof, site-isolation-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionReenable 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}
Patch Set 1 #Patch Set 2 : +test for previous user gesture #Patch Set 3 : Fix faliing content_browsertest #
Total comments: 2
Patch Set 4 : Check processingUserGesture() #Patch Set 5 : TODOs, TouchEventManager #Patch Set 6 : TODOs and TouchEventmanager #
Total comments: 5
Patch Set 7 : Rebase #Messages
Total messages: 62 (35 generated)
The CQ bit was checked by japhet@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
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 ==========
The CQ bit was checked by japhet@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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_...)
The CQ bit was checked by japhet@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
japhet@chromium.org changed reviewers: + rbyers@chromium.org
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.
japhet@chromium.org changed reviewers: + ojan@chromium.org
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?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
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.
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.
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.
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.
japhet@chromium.org changed reviewers: + mustaq@chromium.org
+mustaq
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.
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?
The CQ bit was checked by japhet@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
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.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
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?
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.
The CQ bit was checked by japhet@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
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 CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
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.
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 ==========
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
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.
The CQ bit was checked by japhet@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
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
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
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 ==========
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.
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!
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 ==========
lgtm
The CQ bit was checked by japhet@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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...)
japhet@chromium.org changed reviewers: + creis@chromium.org
creis, up for a quick 4-line test change in content/?
content/ LGTM, thanks!
The CQ bit was checked by japhet@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
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 ==========
Message was sent while issue was closed.
Committed patchset #7 (id:120001)
Message was sent while issue was closed.
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} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/5d8d5cef1e1e9e98f19cea4bcca21e9787a1f686 Cr-Commit-Position: refs/heads/master@{#427735} |