New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 671445 link

Starred by 4 users

Issue metadata

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



Sign in to add a comment

DragAndDropBrowserTest might be flaky

Project Member Reported by lukasza@chromium.org, Dec 6 2016

Issue description

I can intermittently repro a timeout when running
$ DISPLAY=:20 out/gn/interactive_ui_tests --gtest_filter=*CrossSiteSubframe*DragAndDropBrowserTest.DragImageBetweenFrames* --site-per-process

When this happens
1) the test is stuck on the call to drag_start_waiter.WaitUntilDragStart
2) no "Got a dragstart event..." javascript console message is seen
Because of this, I suspect that the intermittent timeout is present in all of DragStartInFrame, DragImageBetweenFrames, CrossSiteDrag (pending in https://crrev.com/2549023003) tests.

 
Owner: lukasza@chromium.org
Status: Assigned (was: Untriaged)
2 hypothesis come to mind:

1. The test doesn't wait long enough, before starting to simulate mouse events leading to a drag start.  I've thought that waiting for 2 animation frames might be sufficient, but maybe it is not.

2. There is something wrong in OOPIFs handling in product code - so far I was not able to repro the timeout without --site-per-process switch (although this doesn't say much - maybe --site-per-process switch simply affects general timing and tickles problems pointed out in the other hypothesis).
Components: Blink>DataTransfer Internals>Sandbox>SiteIsolation
I think it might be okay to not disable the tests at this point, because

1) interactive_ui_tests is not run on CQ with --site-per-process (unless linux_site_isolation trybot is explicitly added to the CL)

2) we have not seen flakiness on linux_site_isolation bot yet (i.e. when looking at 200 builds between 12610 and 12808 at https://build.chromium.org/p/chromium.fyi/builders/Site%20Isolation%20Linux)

3) I don't see flakiness on https://chromium-try-flakes.appspot.com

4) the tests cover a recently changed portion of the code and I would hate to loose test coverage here.

Please let me know if you think we should reconsider.
It seems that in cases where the test hangs without seeing a dragstart, the state of the button seen by MouseEventManager::handleMouseDraggedEvent is always NoButton (it should be Left instead).  We need to trace down if this incorrect mouse buttons state originates from the browser side and why (it is probably somewhat related to the changes we had to make in r432491).
Cc: wjmaclean@chromium.org
The incorrect button state (-1 = NoButton VS 0 = Left) seems to be coming from the following callstack:

#2 0x7fb402e83078 content::InputRouterImpl::OfferToRenderer()
#3 0x7fb402e82bb4 content::InputRouterImpl::OfferToHandlers()
#4 0x7fb402e7cfc2 content::InputRouterImpl::FilterAndSendWebInputEvent()
#5 0x7fb403035825 content::RenderWidgetHostImpl::ForwardMouseEventWithLatencyInfo()
#6 0x7fb402950e09 content::CrossProcessFrameConnector::OnForwardInputEvent()

Still digging...
I think I understand what is happening in the test.  I think that 1.c. and 1.d. (see below) can lag behind and happen out of order - *after* 2.b. and *before* 3.b.  In this case the drag won't start and the test will hang.  At least this is my hypothesis.  I haven't verified all parts of it yet (i.e. step 1.e.)

1. Test simulates a mouse move without any buttons held.
   1.a. The browser sends the mouse move event to the renderer
   1.b. The renderer hit-tests to a RenderFrameProxy
   1.c. The renderer asks the browser to forward the event (RenderFrameProxy::forwardInputEvent)
   1.d. The browser forwards the event to another renderer (CrossProcessFrameConnector::OnForwardInputEvent)
   1.e. ??? The renderer remembers whether the mouse button is held (in MouseEventManager::m_mousePressed)

2. Test simulates pressing down the left button
   2.a. The browser sends the mouse move event to the renderer
   2.b. The renderer remembers whether the mouse button is held (in MouseEventManager::m_mousePressed)

3. Test simulates mouse move
   3.a. The browser sends the mouse move event to the renderer
   3.b. The renderer checks if mouse button is pressed to decide whether to start a drag
        (early in MouseEventManager::handleMouseDraggedEvent)


If the hypothesis above is correct, then I think we have 2 questions to answer:

Q1: How much should we worry about this?  We kind of knew about this (it was mentioned in blink/hit-testing discussion), right?

Q2: How to unflake the test?  I guess I can wait until mousedown event is reported (via event_monitoring.js) from the real (non-proxy) frame.  Any comments / suggestions here?
nasko@ pointed out that the fact that forwarding happens is surprising in itself (i.e. the browser should have sent the event to the right renderer in 1.a./1.b. - the frame and inline css are static and the frame has been already loaded + has seen 2 animation frames after the load).

Comment 7 by lfg@chromium.org, Dec 13 2016

When you say that the frame was loaded and sent 2 animation frames, do you mean the parent? Or the parent after the subframe is loaded?

We've had flake tests in surface based hittesting in the past because of a race in the surface creation. Here's how it works:

1) Parent asks the browser to create a subframe.
2) Subframe process is created and a proxy replaces the subframe in the parent's process.
3) Subframe commits the first frame.
4) Browser notifies parent that the subframe is created and just commited a frame to a new surface.
5) Parent commits a new frame with a reference to the surface provided by the browser.

Browser-based hittesting relies on surfaces, so between 2 and 5 there's a time where an input would be directed to the parent's process, which will try to forward to the child.

So in order to unflake tests that rely on subframe input immediately after creation, you need to wait for the parent to commit a frame after the child commits a frame.

I've added a class that you can use to synchronize the parent's process (MainThreadFrameObserver in browser_test_utils.h) before sending input events.

Currently DragAndDropBrowserTest::NavigateNamedFrame navigates a child frame and waits until the child frame sent 2 animation frame with:

    script = std::string(
        "requestAnimationFrame(function() {\n"
        "  requestAnimationFrame(function() {\n"
        "    domAutomationController.send(43);\n"
        "  });\n"
        "});\n");
    if (!content::ExecuteScriptAndExtractInt(frame, script, &response))
      return false;

I've tried to also add waiting for the parent frame's animations with:

    if (frame->GetParent()) {
      script = std::string(
          "requestAnimationFrame(function() {\n"
          "  domAutomationController.send(44);\n"
          "});\n");
      if (!content::ExecuteScriptAndExtractInt(frame->GetParent(), script, &response))
        return false;
    }

or with:

    if (frame->GetParent()) {
      content::MainThreadFrameObserver parent_animation_observer(
        frame->GetParent()->GetView()->GetRenderWidgetHost());
      parent_animation_observer.Wait();
    }

But I can still repro test hang with the extra parent-waiting-code above. :-(


Below is the log I get from the test run with --site-per-process and sprinkled with some extra logging statements.  You can see when the test code finishes waiting for subframe navigation and for animation frames from the subframe and its parent.  You can see that after this waiting, we still route an event to the old widget which ends up requiring forwarding via the browser later on.

[ RUN      ] CrossSiteSubframe/DragAndDropBrowserTest.DragImageBetweenFrames/0
...
[146883:146883:1213/112750.879361:ERROR:input_router_impl.cc(430)] ; this = 0x62000000d080; input_event.type = 2
[146883:146883:1213/112750.879665:ERROR:input_router_impl.cc(430)] ; this = 0x62000000d080; input_event.type = 2
[146883:146970:1213/112752.444512:WARNING:simple_synchronous_entry.cc(1054)] Could not open platform files for entry.
[146883:146966:1213/112752.814341:WARNING:embedded_test_server.cc(219)] Request not handled. Returning 404: /favicon.ico
[146883:146883:1213/112753.072115:WARNING:render_frame_host_impl.cc(2127)] OnDidStopLoading was called twice.
[146883:146883:1213/112753.279629:ERROR:drag_and_drop_interactive_uitest.cc(715)] Finished navigating and waiting for left frame.
[146883:146883:1213/112753.425193:WARNING:render_frame_host_impl.cc(2127)] OnDidStopLoading was called twice.
[146883:146883:1213/112753.488577:ERROR:drag_and_drop_interactive_uitest.cc(715)] Finished navigating and waiting for right frame.
[146883:146883:1213/112753.488982:ERROR:drag_and_drop_interactive_uitest.cc(643)] SimulateMouseMove...
!!! |this| is still the old widget below:
[146883:146883:1213/112753.490810:ERROR:input_router_impl.cc(430)] ; this = 0x62000000d080; input_event.type = 2
[146883:146883:1213/112753.491054:ERROR:input_router_impl.cc(430)] ; this = 0x620000029080; input_event.type = 2
[146883:146883:1213/112753.505107:ERROR:drag_and_drop_interactive_uitest.cc(648)] SimulateMouseMove... done.
[146883:146883:1213/112753.505202:ERROR:drag_and_drop_interactive_uitest.cc(635)] SimulateMouseDown...
[146883:146883:1213/112753.507876:ERROR:input_router_impl.cc(430)] ; this = 0x620000029080; input_event.type = 0
!!! forwarding during the test is undesirable (the next 2 lines are for a mouse-enter + mouse-move events apparently which happen without the mouse button being held down):
[146883:146883:1213/112753.508858:ERROR:cross_process_frame_connector.cc(252)] CrossProcessFrameConnector::OnForwardInputEvent...
[146883:146883:1213/112753.508953:ERROR:input_router_impl.cc(430)] ; this = 0x620000029080; input_event.type = 3
[146883:146883:1213/112753.510255:ERROR:cross_process_frame_connector.cc(252)] CrossProcessFrameConnector::OnForwardInputEvent...
[146883:146883:1213/112753.510348:ERROR:input_router_impl.cc(430)] ; this = 0x620000029080; input_event.type = 2
[146883:146883:1213/112753.516768:ERROR:drag_and_drop_interactive_uitest.cc(638)] SimulateMouseDown... done.
[146883:146883:1213/112753.516849:ERROR:drag_and_drop_interactive_uitest.cc(643)] SimulateMouseMove...
[146883:146883:1213/112753.517725:ERROR:input_router_impl.cc(430)] ; this = 0x620000029080; input_event.type = 2
[146883:146883:1213/112753.518198:ERROR:drag_and_drop_interactive_uitest.cc(648)] SimulateMouseMove... done.
...
!!! test hangs / no dragstart happens
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 14 2016

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

commit eeb42c523def9bf6d2c514bf403f2cb67117cb8b
Author: lukasza <lukasza@chromium.org>
Date: Wed Dec 14 19:22:19 2016

Wait for mousemove DOM event, before simulating mousedown for dragstart tests.

Waiting until mousemove DOM event is received is important, because
otherwise (because of lag introduced by forwarding events between
renderers) button-less mousemove event can arrive at the target frame
*after* mousedown event (confusing the renderer whether mouse button is
held down or not and therefore preventing dragstart from happening).

This CL seems to fix the flakiness that used to be locally reproducible
with --site-per-process.  Right now we consistently pass 20 repetitions
of CrossSiteSubframe/DragAndDropBrowserTest.DragImageBetweenFrames/0.

BUG= 671445 

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

[modify] https://crrev.com/eeb42c523def9bf6d2c514bf403f2cb67117cb8b/chrome/browser/ui/views/drag_and_drop_interactive_uitest.cc
[modify] https://crrev.com/eeb42c523def9bf6d2c514bf403f2cb67117cb8b/chrome/test/data/drag_and_drop/image_source.html

Status: Fixed (was: Assigned)
Cc: lukasza@chromium.org
 Issue 688428  has been merged into this issue.
Status: Started (was: Fixed)
This is still happening.
I am observing the following sequence of events when running SameSiteSubframe/DragAndDropBrowserTest.CrossSiteDrag/0 with --site-per-process:

  1 drag_and_drop_interactive_uitest.cc(589)] Calling SimulateMouseDownAndDragStartInLeftFrame...
  2 drag_and_drop_interactive_uitest.cc(601)] Calling SimulateMouseDownAndDragStartInLeftFrame...
mouse move events:
  3 input_router_impl.cc(434)] ; button = -1
  4 input_router_impl.cc(434)] ; button = -1
  5 drag_and_drop_interactive_uitest.cc(604)] Calling SimulateMouseDownAndDragStartInLeftFrame...
  6 CONSOLE(16)] "Got a mousemove event from the left frame.", source: http://c.com:34816/drag_and_drop/event_monitoring.js (16)
  7 drag_and_drop_interactive_uitest.cc(608)] Calling SimulateMouseDownAndDragStartInLeftFrame...
mouse down event:
  8 input_router_impl.cc(434)] ; button = 0
  9 drag_and_drop_interactive_uitest.cc(611)] Calling SimulateMouseDownAndDragStartInLeftFrame...
 10 CONSOLE(16)] "Got a mousedown event from the left frame.", source: http://c.com:34816/drag_and_drop/event_monitoring.js (16)
mouse move events:
 11 input_router_impl.cc(434)] ; button = -1
 12 input_router_impl.cc(434)] ; button = -1
 13 drag_and_drop_interactive_uitest.cc(614)] Calling SimulateMouseDownAndDragStartInLeftFrame...
mouse down event!?  why is this *after* mouse move events above!?:
 14 input_router_impl.cc(434)] ; button = 0
 15 CONSOLE(16)] "Got a mousemove event from the left frame.", source: http://c.com:34816/drag_and_drop/event_monitoring.js (16)
 16 drag_and_drop_interactive_uitest.cc(618)] Calling SimulateMouseDownAndDragStartInLeftFrame...
 17 drag_and_drop_interactive_uitest.cc(215)] Calling WaitUntilDragStart...
 18 CONSOLE(16)] "Got a mousemove event from the left frame.", source: http://c.com:34816/drag_and_drop/event_monitoring.js (16)
 19 Still waiting for the following processes to finish:
 20 ...

The surprising / unexpected thing is that 14 happens after 11 and 12 (despite the fact that we wait for mousedown event [i.e. line 10 above], before simulating the mouse move that should initiate drag [i.e. lines 11 and 12]).
The flakiness is avoided (in 200 runs), if I insert a 5 seconds "sleep" (via RunLoop / PostDelayedTask / Run) after navigating the child frames, but before any mouse simulation.  This indicates that the timing of mouse simulations is not important and this also supports the hypothesis from #c7.
Cc: kenrb@chromium.org
Status: Assigned (was: Started)
kenrb@ / lfg@, would you be able to take a look?  I don't know what I should wait for to avoid flakiness associated with timing/interleaving of input events?  The hypothesis from #c7 (and #c5) sounds promising, but so far the prescribed treatment didn't help - even if I add the following code to the end of DragAndDropBrowserTest::NavigateNamedFrame, I can still repro the hang:

   if (frame->GetParent()) {
      content::MainThreadFrameObserver parent_animation_observer(
        frame->GetParent()->GetView()->GetRenderWidgetHost());
      parent_animation_observer.Wait();
    }
    content::MainThreadFrameObserver frame_animation_observer(
      frame->GetView()->GetRenderWidgetHost());
    frame_animation_observer.Wait();
I hadn't been following this bug at all so just reading up on it now.

Is this the problem that SurfaceHitTestReadyNotifier solves? Unfortunately that class isn't available outside of content, but wjmaclean was adding utility functions (ContainsSurfaceId() and  WaitForGuestSurfaceReady() in browser_test_utils.cc) to try to make similar utilities available in content's API. We might consider adding a more general interface.

I'm not sure how to interpret all the explanations you have posted, but that seems like it would address what you are seeing in c#5.

We should not be forwarding events between renderers. The fact that it is happening just indicates hit testing isn't working properly.
I've uploaded a WIP CL @ https://codereview.chromium.org/2686683004, where I am using WaitForSurfaceReady from drag-and-drop tests.  Unexpectedly, I am still able to repro the problem where event forwarding happens and messes up the sequence of events that should start a drag - below I expect two events with button=0 (left) in a row, but the highlighted, forwarded event is unexpectedly happening and resetting the button state stored in the renderer.

Slightly edited output from running the test with the CL above applied:

     1	[ RUN      ] SameSiteSubframe/DragAndDropBrowserTest.CrossSiteDrag/0
     2	input_router_impl.cc(447): ; button = -1; local = (155, 150); global = (170, 241)
     3	input_router_impl.cc(447): ; button = -1; local = (155, 150); global = (170, 241)
     4	drag_and_drop_interactive_uitest.cc(673): Navigating left frame ...
     5	browser_test_utils.cc(1191): WaitForSurfaceReady...
     6	drag_and_drop_interactive_uitest.cc(702): Navigating left frame ... done.
     7	drag_and_drop_interactive_uitest.cc(673): Navigating right frame ...
     8	drag_and_drop_interactive_uitest.cc(702): Navigating right frame ... done.
     9	input_router_impl.cc(447): ; button = -1; local = (155, 150); global = (170, 241)
    10	input_router_impl.cc(447): ; button = -1; local = (55, 50); global = (170, 241)
    11	JS CONSOLE: "Got a mousemove event from the left frame.", source: http://c.com:43157/drag_and_drop/event_monitoring.js (16)
    12	input_router_impl.cc(447): ; button = 0; local = (55, 50); global = (170, 241)
    13	JS CONSOLE: "Got a mousedown event from the left frame.", source: http://c.com:43157/drag_and_drop/event_monitoring.js (16)
    14	input_router_impl.cc(447): button = -1; local = (55, 50); global = (170, 241)
    15	input_router_impl.cc(447): button = -1; local = (55, 50); global = (170, 241)
    16	input_router_impl.cc(455): stack = #0 0x000002b77d47 base::debug::StackTrace::StackTrace()
    17	#1 0x000000e898b0 content::InputRouterImpl::OfferToRenderer()
    18	#2 0x000000e8953b content::InputRouterImpl::OfferToHandlers()
    19	#3 0x000000e876e5 content::InputRouterImpl::FilterAndSendWebInputEvent()
    20	#4 0x000000f0abd0 content::RenderWidgetHostImpl::ForwardMouseEventWithLatencyInfo()
    21	#5 0x000000cf02c0 content::CrossProcessFrameConnector::OnForwardInputEvent()

You can see on line 5, that left frame is a cross-process frame and that we wait for the surface to be ready.

Events on lines 14 and 15 are unexpected (the button should be held down the whole time - otherwise drag won't start).

I assume that the callstack frame on line 21 indicates that the event from line 15 is trigerred by event forwarding, because of inaccurate hit testing.  I thought that this shouldn't have happened because of the waiting on line 5.  FWIW, I checked that the event from line 14 has the same callstack as the event from line 15.
Status: Available (was: Assigned)
I am happy to push https://codereview.chromium.org/2686683004 forward (e.g. removing polling in WaitForSurfaceReady and replacing it with waiting for OnSwapCompositorFrame instead), but I am have a hard time trying to explain why this CL in its current state doesn't prevent the test flakiness / hangs.  I need help please? :-)
Do you want to try disabling input event forwarding (in CrossProcessFrameConnector, possibly), and see if it helps with the flakiness (or, alternatively, if it breaks the test)? Now that I think about this, it is possible that some event are getting needlessly forwarded, although this doesn't usually cause bugs. We do, for instance, send a MouseMove to both the frame and its parent when the mouse enters an iframe, so that the parent can fire MouseOut handlers. With forwarding still enabled, it likely results in duplicate MouseMoves going to the child frame (one routed, one forwarded). With that disabled, it would be worth verifying that all the right events go to the correct renderers in this test.

Comment 21 by kenrb@chromium.org, Feb 17 2017

I can take it but won't be able to start work on it immediately, so anyone else can feel free to steal if they have time.
 Issue 702565  has been merged into this issue.
 Issue 704264  has been merged into this issue.
Project Member

Comment 24 by chromium...@appspot.gserviceaccount.com, Mar 26 2017

Labels: Sheriff-Chromium
Detected 3 new flakes for test/step "CrossSiteSubframe/DragAndDropBrowserTest.DragImageBetweenFrames/0". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyTAsSBUZsYWtlIkFDcm9zc1NpdGVTdWJmcmFtZS9EcmFnQW5kRHJvcEJyb3dzZXJUZXN0LkRyYWdJbWFnZUJldHdlZW5GcmFtZXMvMAw. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
Project Member

Comment 25 by chromium...@appspot.gserviceaccount.com, Mar 26 2017

Detected 3 new flakes for test/step "SameSiteSubframe/DragAndDropBrowserTest.DragImageFromDisappearingFrame/0". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyUwsSBUZsYWtlIkhTYW1lU2l0ZVN1YmZyYW1lL0RyYWdBbmREcm9wQnJvd3NlclRlc3QuRHJhZ0ltYWdlRnJvbURpc2FwcGVhcmluZ0ZyYW1lLzAM. This message was posted automatically by the chromium-try-flakes app.
Project Member

Comment 26 by chromium...@appspot.gserviceaccount.com, Mar 27 2017

Detected 3 new flakes for test/step "CrossSiteSubframe/DragAndDropBrowserTest.DragImageBetweenFrames/0". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyTAsSBUZsYWtlIkFDcm9zc1NpdGVTdWJmcmFtZS9EcmFnQW5kRHJvcEJyb3dzZXJUZXN0LkRyYWdJbWFnZUJldHdlZW5GcmFtZXMvMAw. This message was posted automatically by the chromium-try-flakes app.
Project Member

Comment 27 by chromium...@appspot.gserviceaccount.com, Mar 27 2017

Detected 3 new flakes for test/step "SameSiteSubframe/DragAndDropBrowserTest.DragImageFromDisappearingFrame/0". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyUwsSBUZsYWtlIkhTYW1lU2l0ZVN1YmZyYW1lL0RyYWdBbmREcm9wQnJvd3NlclRlc3QuRHJhZ0ltYWdlRnJvbURpc2FwcGVhcmluZ0ZyYW1lLzAM. This message was posted automatically by the chromium-try-flakes app.
Labels: -Sheriff-Chromium
Project Member

Comment 29 by chromium...@appspot.gserviceaccount.com, Mar 28 2017

Labels: Sheriff-Chromium
Detected 3 new flakes for test/step "SameSiteSubframe/DragAndDropBrowserTest.DragImageFromDisappearingFrame/0". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyUwsSBUZsYWtlIkhTYW1lU2l0ZVN1YmZyYW1lL0RyYWdBbmREcm9wQnJvd3NlclRlc3QuRHJhZ0ltYWdlRnJvbURpc2FwcGVhcmluZ0ZyYW1lLzAM. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
Labels: -Sheriff-Chromium
This is still popping up in the Sheriff queue. Any progress that we can make here?
Does the flakiness still repro after r460177?  In #c19 we suspected that event forwarding might be causing the flakiness - maybe r460177 is sufficient to unflake these tests?
Project Member

Comment 32 by bugdroid1@chromium.org, Apr 3 2017

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

commit 66c46290f9774951731d1d38ad75d7aa2d1ac23d
Author: kenrb <kenrb@chromium.org>
Date: Mon Apr 03 18:47:36 2017

Fix flakiness in DragAndDropBrowserTests

Multiple drag and drop tests in interactive_ui_tests have been flaky
with OOPIFs present. One cause was addressed in r460177, and this CL
fixes another, which is that UI interaction can happen before surface-
based hit testing is ready in the browser process.

This CL includes a small refactor of SurfaceHitTestReadyNotifier to
unify code paths for tests inside and outside of content/.

Most of this patch was written by lukasza@.

BUG= 671445 , 704603 

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

[modify] https://crrev.com/66c46290f9774951731d1d38ad75d7aa2d1ac23d/chrome/browser/ui/views/drag_and_drop_interactive_uitest.cc
[modify] https://crrev.com/66c46290f9774951731d1d38ad75d7aa2d1ac23d/content/browser/accessibility/touch_accessibility_aura_browsertest.cc
[modify] https://crrev.com/66c46290f9774951731d1d38ad75d7aa2d1ac23d/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/66c46290f9774951731d1d38ad75d7aa2d1ac23d/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/66c46290f9774951731d1d38ad75d7aa2d1ac23d/content/public/test/browser_test_utils.h
[modify] https://crrev.com/66c46290f9774951731d1d38ad75d7aa2d1ac23d/content/test/content_browser_test_utils_internal.cc
[modify] https://crrev.com/66c46290f9774951731d1d38ad75d7aa2d1ac23d/content/test/content_browser_test_utils_internal.h

Project Member

Comment 33 by chromium...@appspot.gserviceaccount.com, Apr 4 2017

Labels: Sheriff-Chromium
Detected 3 new flakes for test/step "SameSiteSubframe/DragAndDropBrowserTest.DragImageFromDisappearingFrame/0". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyUwsSBUZsYWtlIkhTYW1lU2l0ZVN1YmZyYW1lL0RyYWdBbmREcm9wQnJvd3NlclRlc3QuRHJhZ0ltYWdlRnJvbURpc2FwcGVhcmluZ0ZyYW1lLzAM. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
 Issue 708047  has been merged into this issue.
Looks like it could still be flaky. https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/422394 is a recent (includes the patch from #32) tryjob run with these tests failing.
 Issue 706483  has been merged into this issue.
Labels: -Sheriff-Chromium
kenrb@, lukasza@: 
This is still popping up in the Sheriff queue. Any progress that we can make here?
Sorry for not commenting. I am actively working on this. Unfortunately I haven't been able to reproduce the flakiness locally, and since addressing the known issues with these tests didn't seem to fix the problem, I am still working in a somewhat speculative mode.
Project Member

Comment 39 by chromium...@appspot.gserviceaccount.com, Apr 7 2017

Labels: Sheriff-Chromium
Detected 3 new flakes for test/step "CrossSiteSubframe/DragAndDropBrowserTest.DragImageBetweenFrames/0". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyTAsSBUZsYWtlIkFDcm9zc1NpdGVTdWJmcmFtZS9EcmFnQW5kRHJvcEJyb3dzZXJUZXN0LkRyYWdJbWFnZUJldHdlZW5GcmFtZXMvMAw. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
Labels: -Sheriff-Chromium
Okay, thanks, kenrb@!
 Issue 704603  has been merged into this issue.
Prior to kenrb@'s fixes the start of the drag would be flaky.  Right now I am seeing (*) that the "drop" event is sometimes not firing in the target/right frame (but surprisingly "dragend" *is* firing in the source/left frame).

(*) https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_asan_rel_ng/builds/344233
and
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/425194
and
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isolation/builds/6922

Comment 43 by kenrb@chromium.org, Apr 12 2017

Is anybody able to reproduce this locally? I haven't been able to do so, including by inserting delays in various places in the hope of triggering a race condition.

Tracing the flow of the input events hasn't been helpful. The tests look sound as far as I can see, and with no way of tracing a failing test run, it's difficult to progress on this.

Comment 44 by kenrb@chromium.org, Apr 24 2017

I got stuck on this, not being able to repro. However I also notice that we haven't seen any flakes for 12 days at this point. At some point should we close this and re-open if the flakes come back?

Comment 45 by kenrb@chromium.org, Aug 15 2017

Status: Fixed (was: Assigned)
Looks like these stopped flaking a long time ago.

Sign in to add a comment