New issue
Advanced search Search tips

Issue 650348 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Remove input event forwarding path from RenderFrameProxies

Project Member Reported by alex...@chromium.org, Sep 26 2016

Issue description

With --site-per-process:
(1) Go to http://alexmos17.github.io/tests/mouse-events.html
(2) Mouse down anywhere on main frame
(3) While holding the mouse button, move cursor over one of the subframes and release the button.

Expected:
Main frame prints MouseDown, subframe prints MouseUp.  This is what happens without --site-per-process.

Actual:
Main frame prints both MouseDown and MouseUp, subframe also prints MouseUp.  The extra mouseup event in the main frame seems incorrect.

Tries this on Mac Canary 55.0.2869 as well as Linux ToT.  The test frames just print a message for mousedown/mouseup/click events on the window.

Ken, is this expected to work after your cross-process mouse events CL in r418629, or is this an expected failure at this point?

Interestingly, doing the mousedown in a subframe and then releasing the button in another frame doesn't work the same way -- all events stay in the subframe both with and without --site-per-process in this case.  Not sure if that's expected behavior.


 

Comment 1 by kenrb@chromium.org, Sep 26 2016

Summary: Remove input event forwarding path from RenderFrameProxies (was: Two mouseup events dispatched when mousedown and mouseup land in frames from different processes)

Comment 2 by kenrb@chromium.org, Sep 26 2016

Oops, hit enter before typing comments on changing the title -- there are a couple of things happening here. The duplicate MouseUp is because the forwarding path is still alive, so the MouseUp getting targeted to the main frame is also going to the OOPIF based on renderer-side hit testing. That is waiting on browser side hit testing to be done on Android, but I don't think we have a bug open so it makes sense to leave it there.

The other issue is that it differs from non-OOPIF cases in that MouseUp only follows mouse capture with --site-per-process. That is kind of unavoidable at the moment and will have to wait for  bug 647378  to be dealt with.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 28 2017

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

commit e2da63fad40fc329932c04030bca51f366ed721c
Author: kenrb <kenrb@chromium.org>
Date: Tue Mar 28 18:44:55 2017

Remove renderer-to-renderer input event forwarding for OOPIFs

Now that the browser process supports focus tracking and
mouse/touch/gesture event hit testing for OOPIFs on all platforms, we
no longer need to ability to forward events from a parent frame
renderer to a child frame renderer. This patch removes that code.

BUG= 650348 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/content/browser/frame_host/cross_process_frame_connector.cc
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/content/browser/frame_host/cross_process_frame_connector.h
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/content/common/frame_messages.h
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/content/renderer/render_frame_proxy.cc
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/content/renderer/render_frame_proxy.h
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/third_party/WebKit/Source/core/frame/RemoteFrame.cpp
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/third_party/WebKit/Source/core/frame/RemoteFrame.h
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/third_party/WebKit/Source/core/frame/RemoteFrameClient.h
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/third_party/WebKit/Source/core/html/HTMLFrameElementBase.h
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/third_party/WebKit/Source/web/RemoteFrameClientImpl.cpp
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/third_party/WebKit/Source/web/RemoteFrameClientImpl.h
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/e2da63fad40fc329932c04030bca51f366ed721c/third_party/WebKit/public/web/WebRemoteFrameClient.h

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 31 2017

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

commit e8ff078cc3b53a20c192a72e5c09fe9a9d0d88c2
Author: kenrb <kenrb@chromium.org>
Date: Fri Mar 31 15:07:25 2017

Disable autoplay-crossorigin.html layout test with --site-per-process

This should have been disabled in r460177 but was missed.

BUG= 650348 
TBR=dcheng@chromium.org

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

[modify] https://crrev.com/e8ff078cc3b53a20c192a72e5c09fe9a9d0d88c2/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

Comment 5 by kenrb@chromium.org, May 8 2017

Status: Fixed (was: Assigned)
kenrb@: Is there a bug that tracks re-enabling the test?  Do we know what it would take to re-enable the test?

Sign in to add a comment