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

third_party/WebKit/Source/core/dom/Document.cpp

Index: third_party/WebKit/Source/core/dom/Document.cpp
diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp
index edecda6ba994935a308e864a4b511d98288d9a2b..aa012f7afebc1a01cf2a82ddfbe64e504e7b72fc 100644
--- a/third_party/WebKit/Source/core/dom/Document.cpp
+++ b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -465,6 +465,7 @@ Document::Document(const DocumentInit& initializer,
       m_hasXMLDeclaration(0),
       m_designMode(false),
       m_isRunningExecCommand(false),
+      m_hasReceivedUserGesture(false),
       m_hasAnnotatedRegions(false),
       m_annotatedRegionsDirty(false),
       m_useSecureKeyboardEntryWhenActive(false),
@@ -3368,6 +3369,7 @@ MouseEventWithHitTestResults Document::prepareMouseEvent(
     const LayoutPoint& documentPoint,
     const PlatformMouseEvent& event) {
   DCHECK(layoutViewItem().isNull() || layoutViewItem().isLayoutView());
+  m_hasReceivedUserGesture = true;

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

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.

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

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

 
   // LayoutView::hitTest causes a layout, and we don't want to hit that until
   // the first layout because until then, there is nothing shown on the screen -