Reproed on Windows, might not happen elsewhere (Mac scrollbars like to hide themselves)

This might be two separate issues -- the scrolling is not a pre-requisite to making the element unusable.

On Windows:
1. When I scroll with the popup open, it does not get dismissed.
2. If I close the popup and try to open it again, it does not get reappear, independently of whether I have previously scrolled before.

Neither of these problems repro on Linux, so it is possible they are related (presumably on Windows, something is being missed as part of the dialog dismissal).

I coudld not reproduce this bug on Linux. I assume "cannot be shown again" repeating step 3 will not show the popup?

I could repro this on Windows and I also agree with comment #2 that the bug repros without scrolling.

#3: Correct. And the problem with the popup not re-appearing looks like it is Windows only (I just tried it on Mac). The other problem, with the popup not disappearing when you scroll, appears on Mac and Windows. On Linux everything seems to work fine.

I noticed if we click outside Page/WebContents the popup widget can become active again (i.e., click inside OMNIBOX and then the arrow and the popup reappears).

I think when the popup disappears, we should hit RenderWidgetHostViewAura::SetPopupChild(nullptr) but it looks like clicking inside WebContents does not do it. It also works just fine when there are no OOPIFs.

ekaramad: Thanks for poking at it. If you need another bug then feel free to steal this one, although it isn't high priority so it doesn't need to derail any other work. I was just going through open input-related bugs today which made me think to cc you on this one.

Thanks Ken, I will take a deeper look at this. One question though: I noticed when I click the arrow key again, on Linux the event is routed to the RenderWidget of the date popup while on Windows it is going to the OOPIF's RenderWidget. I assume the windows behavior is correct (esp. given that the point I am clicking is outside the bounds of the date popup)?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e3cbfeec943500d99832b93b015d62937ff15443

commit e3cbfeec943500d99832b93b015d62937ff15443
Author: ekaramad <ekaramad@chromium.org>
Date: Thu Apr 20 17:45:50 2017

Hide popups when handling mouse down, mouse wheel, or gesture tap in a WebFrameWidget

When a mouse down is handled inside WebViewImpl (main frame), the current page
popup is hidden. The same behavior should apply to out of process renderers.
However, in OOPIF renderers the WebViewImpl does not handle mouse events and
the task is delegated to the WebFrameWidget corresponding to the frame which
has received the event. But since receiving and input inside a WebFrameWidget
suggests that the mouse down has been outside the bounds of the WebPagePopup,
we should dismiss any currently showing popups the same way as we do it for
WebViewImpls.

Specifically, not having this behavior led to a bug on Windows where date
picker inside OOPIFs does not show again after dismissing an older one by
clicking inside the OOPIF.

BUG= 671732 

Review-Url: https://codereview.chromium.org/2804813002
Cr-Commit-Position: refs/heads/master@{#466055}

[modify] https://crrev.com/e3cbfeec943500d99832b93b015d62937ff15443/chrome/browser/site_per_process_interactive_browsertest.cc
[modify] https://crrev.com/e3cbfeec943500d99832b93b015d62937ff15443/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp
[modify] https://crrev.com/e3cbfeec943500d99832b93b015d62937ff15443/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/e3cbfeec943500d99832b93b015d62937ff15443/third_party/WebKit/Source/web/WebViewImpl.h

Looks fixed in 60.0.3077.0 on the Mac Canary.  Thanks!

ekaramad@: Do you think this is worth merging to M59, in case it affects the OOPIF-based <webview> trial?

The fallthrough in line 916 of https://codereview.chromium.org/2804813002/diff/220001/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp is intentional, yes? (Sounds like it, but I'm not 100% sure)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1dbd8bf9b63becec870ee28ae9c26c2caa0677e6

commit 1dbd8bf9b63becec870ee28ae9c26c2caa0677e6
Author: Nico Weber <thakis@chromium.org>
Date: Mon Jan 29 18:27:59 2018

Remaining tricky bits to build webkit_unit_tests with -Wimplicit-fallthrough.

See https://bugs.chromium.org/p/chromium/issues/detail?id=671732#c11

This CL was uploaded by git cl split.

R=dcheng@chromium.org

Bug:  177475 , 671732 
Change-Id: I7b147b9bf9b5ed7853a9a98132812a3557972326
Reviewed-on: https://chromium-review.googlesource.com/890432
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532501}
[modify] https://crrev.com/1dbd8bf9b63becec870ee28ae9c26c2caa0677e6/third_party/WebKit/Source/core/frame/WebFrameWidgetImpl.cpp

Verified the fix on Windows 10 Chrome version #66.0.3335.0 as per the comment# 12
Attaching screen cast for reference.
Observed "popup widget is dismissed and shown again on clickng arrow icon after scrolling iframe window and it is in usable state"
Hence, the fix is working as expected.
Adding the verified labels.

Thanks!

Deleted: 671732.mp4 3.6 MB

	671732.mp4 3.6 MB View Download

re #11 I think this FALLTHROUGH was not intended but basically harmless because the code in the following case is not needed for any correctness, and I'm going to propose removing it in a CL.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/573f46751824b8dcfbec675291eacc8c581ae441

commit 573f46751824b8dcfbec675291eacc8c581ae441
Author: danakj <danakj@chromium.org>
Date: Tue Dec 11 22:50:40 2018

Remove code to store the last popup transiently in WebFrameWidgetImpl

This code was copied over from WebViewImpl when adding the HidePopups
call in order to have a pinch zoom or scroll tap outside the current
popup in a non-main-frame-local-rooted iframe hide the current popup.

This is modifying state on WebViewImpl meant to track between TapDown
and Tap, to avoid the Tap showing a popup that was hidden by TapDown.

WebFrameWidgetImpl does not show popups on Tap, and if it did, it
should track this transient state locally not on WebViewImpl.

Also remove the undocumented FALLTHROUGH which was not explained
in 671732 as it now goes to nothing.

R=dcheng@chromium.org

Change-Id: I9d67539cf0afbb9db239b73bf83418e1a877e6db
Bug: 912193,  671732 
Reviewed-on: https://chromium-review.googlesource.com/c/1372539
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615708}
[modify] https://crrev.com/573f46751824b8dcfbec675291eacc8c581ae441/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc76bd1252ad56309ecad13729305c94b1f57ba7

commit dc76bd1252ad56309ecad13729305c94b1f57ba7
Author: Alice Boxhall <aboxhall@chromium.org>
Date: Wed Dec 12 02:11:12 2018

Revert "Remove code to store the last popup transiently in WebFrameWidgetImpl"

This reverts commit 573f46751824b8dcfbec675291eacc8c581ae441.

Reason for revert: Seems to be causing EffectiveTouchActionPropagatesAcrossNestedFrames to fail https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8927388648425983952/+/steps/content_browsertests_on_Intel_GPU_on_Mac_on_Mac-10.12.6/0/logs/SitePerProcessBrowserTouchActionTest.EffectiveTouchActionPropagatesAcrossNestedFrames/0

Original change's description:
> Remove code to store the last popup transiently in WebFrameWidgetImpl
> 
> This code was copied over from WebViewImpl when adding the HidePopups
> call in order to have a pinch zoom or scroll tap outside the current
> popup in a non-main-frame-local-rooted iframe hide the current popup.
> 
> This is modifying state on WebViewImpl meant to track between TapDown
> and Tap, to avoid the Tap showing a popup that was hidden by TapDown.
> 
> WebFrameWidgetImpl does not show popups on Tap, and if it did, it
> should track this transient state locally not on WebViewImpl.
> 
> Also remove the undocumented FALLTHROUGH which was not explained
> in 671732 as it now goes to nothing.
> 
> R=​dcheng@chromium.org
> 
> Change-Id: I9d67539cf0afbb9db239b73bf83418e1a877e6db
> Bug: 912193,  671732 
> Reviewed-on: https://chromium-review.googlesource.com/c/1372539
> Reviewed-by: James MacLean <wjmaclean@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Commit-Queue: danakj <danakj@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#615708}

TBR=danakj@chromium.org,dcheng@chromium.org,wjmaclean@chromium.org

Change-Id: Ifddbb605fc6165505774105d5618d57b62e3159c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 912193,  671732 
Reviewed-on: https://chromium-review.googlesource.com/c/1372110
Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Commit-Queue: Alice Boxhall <aboxhall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615783}
[modify] https://crrev.com/dc76bd1252ad56309ecad13729305c94b1f57ba7/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/72ac928f90e123b0a0fd54e3be0f6ab1fb79812d

commit 72ac928f90e123b0a0fd54e3be0f6ab1fb79812d
Author: danakj <danakj@chromium.org>
Date: Wed Dec 12 16:53:02 2018

Reland "Remove code to store the last popup transiently in WebFrameWidgetImpl"

This is a reland of 573f46751824b8dcfbec675291eacc8c581ae441

Was reverted because SitePerProcessBrowserTouchActionTest.EffectiveTouchActionPropagatesAcrossNestedFrames
failed when it landed. But the test went back to green for a few runs before
this was reverted, so I don't think it was at fault. We'll see if it starts
failing again on reland, and I'll watch the mac bots here in the CQ.

TBR=dcheng

Original change's description:
> Remove code to store the last popup transiently in WebFrameWidgetImpl
>
> This code was copied over from WebViewImpl when adding the HidePopups
> call in order to have a pinch zoom or scroll tap outside the current
> popup in a non-main-frame-local-rooted iframe hide the current popup.
>
> This is modifying state on WebViewImpl meant to track between TapDown
> and Tap, to avoid the Tap showing a popup that was hidden by TapDown.
>
> WebFrameWidgetImpl does not show popups on Tap, and if it did, it
> should track this transient state locally not on WebViewImpl.
>
> Also remove the undocumented FALLTHROUGH which was not explained
> in 671732 as it now goes to nothing.
>
> R=dcheng@chromium.org
>
> Change-Id: I9d67539cf0afbb9db239b73bf83418e1a877e6db
> Bug: 912193,  671732 
> Reviewed-on: https://chromium-review.googlesource.com/c/1372539
> Reviewed-by: James MacLean <wjmaclean@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Commit-Queue: danakj <danakj@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#615708}

Bug: 912193,  671732 
Change-Id: I18e8e146b5b525239d1667e267d44fa8a295599d
Reviewed-on: https://chromium-review.googlesource.com/c/1374193
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615925}
[modify] https://crrev.com/72ac928f90e123b0a0fd54e3be0f6ab1fb79812d/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc