BrowserSideFlingBrowserTest Failing on Viz Mac |
|||||||
Issue descriptionOS: Mac Test suite: viz_content_browsertests Tests: BrowserSideFlingBrowserTest.TouchpadFling BrowserSideFlingBrowserTest.TouchscreenFling BrowserSideFlingBrowserTest.AutoscrollFling Example failing build: https://ci.chromium.org/buildbot/chromium.fyi/Chromium%20Mac%2010.13/2797 The tests are all currently timing out while waiting for RenderFrameMetadata. However each test has while loops waiting for metadata to arrive with an expected scroll offset. So it's likely that we are receiving data, but that its not at the desired scroll offset. Then eventually frames stop being submitted. I'm assigning to ccameron@ whom has been working on Viz support for Mac. I'm CCing sahel@ whom added the test and may be able to comment further about this.
,
May 16 2018
Thanks for that info sahel@, sadrul@ mentioned that you may have work inflight which could address the viz support?
,
May 16 2018
https://chromium-review.googlesource.com/c/chromium/src/+/994181 is the cl that addresses viz support and it is already landed. Please see the todo comment here: https://chromium-review.googlesource.com/c/chromium/src/+/994181/22/content/browser/renderer_host/input/fling_scheduler_mac.mm when FlingSchedulerMac::GetCompositor() returns null then fling progress happens by calling setNeedsBeginFrame which is not allowed with viz enabled. On aura when FlingScheduler::GetCompositor() returns the proper compositor the tests pass with viz enabled.
,
May 16 2018
I think the fix here is to make sure that FlingSchedulerMac::GetCompositor() works on mac. sadrul@ or ccameron@ please correct me if I am wrong.
,
May 16 2018
https://docs.google.com/presentation/d/1miFAvKuz7tRT4IX_nCwZshFkUDX8bDYqfPlTOm2LS8U/edit?usp=sharing contains a class diagram for how fling controller and fling scheduler works.
,
May 22 2018
,
Jun 13 2018
Also: SitePerProcessBrowserTest.TouchpadGestureFlingStart SitePerProcessBrowserTest.TouchscreenGestureFlingStart
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a83e4da7b488f41b30c5d08491cad4723bf14bb0 commit a83e4da7b488f41b30c5d08491cad4723bf14bb0 Author: jonross <jonross@chromium.org> Date: Wed Jun 13 17:41:07 2018 Disable failing sling tests on Viz Mac Two new fling tests were added to SitePerProcessBrowserTest, these do not currently work on Mac with Viz. This filters those tests out. TBR=kylechar@chromium.org TEST=SitePerProcessBrowserTest.TouchpadGestureFlingStart, SitePerProcessBrowserTest.TouchscreenGestureFlingStart Bug: 842325 Change-Id: Ia45eaf8f5cfb2f8865a9f1d5e3aa4d677e37c1cd Reviewed-on: https://chromium-review.googlesource.com/1099372 Reviewed-by: Jonathan Ross <jonross@chromium.org> Cr-Commit-Position: refs/heads/master@{#566890} [modify] https://crrev.com/a83e4da7b488f41b30c5d08491cad4723bf14bb0/testing/buildbot/filters/viz.content_browsertests.filter
,
Jun 21 2018
Reproduced today ahead of triage
,
Jun 22 2018
Hey Chris, Would you be able to take a look at this? We'd like to clear up the known Mac issues before finch trials for Viz
,
Jun 25 2018
Sure, I'll try to get this soon.
,
Jul 11
Hey Chris, I was wondering if you'd had the chance to take a look at this? It's the main Mac-only failure in Viz tests at the moment.
,
Jul 11
I'm 110% on MacViews ATM, but I will find time eventually.
,
Jul 25
Fixing issue 833985 (uncommenting the "can't use ui::Compositor yet" part) seems to fix this.
,
Jul 25
Nice!! Based on that I've put together a patch which enables it: https://chromium-review.googlesource.com/c/chromium/src/+/1150210
,
Jul 25
Nevermind, ccameron@ is already working on landing a patch for this: https://chromium-review.googlesource.com/c/chromium/src/+/1149612
,
Jul 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d6ec81545fea9d795fe5655d8453c343bc61420a commit d6ec81545fea9d795fe5655d8453c343bc61420a Author: Christopher Cameron <ccameron@chromium.org> Date: Wed Jul 25 17:25:29 2018 Enable FlingSchedulerMac Enable tests that were disabled because of this functionality missing. Bug: 842325 Change-Id: I6f540460f4e5f81d7ff8dd36e4abf369f0bd4785 Reviewed-on: https://chromium-review.googlesource.com/1149612 Reviewed-by: Jonathan Ross <jonross@chromium.org> Commit-Queue: ccameron <ccameron@chromium.org> Cr-Commit-Position: refs/heads/master@{#577957} [modify] https://crrev.com/d6ec81545fea9d795fe5655d8453c343bc61420a/content/browser/renderer_host/browser_compositor_view_mac.h [modify] https://crrev.com/d6ec81545fea9d795fe5655d8453c343bc61420a/content/browser/renderer_host/browser_compositor_view_mac.mm [modify] https://crrev.com/d6ec81545fea9d795fe5655d8453c343bc61420a/content/browser/renderer_host/input/fling_scheduler_mac.mm [modify] https://crrev.com/d6ec81545fea9d795fe5655d8453c343bc61420a/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm [modify] https://crrev.com/d6ec81545fea9d795fe5655d8453c343bc61420a/testing/buildbot/filters/viz.content_browsertests.filter
,
Jul 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4c440b7c6b5edb7c01d41d498bc00f686c2251fa commit 4c440b7c6b5edb7c01d41d498bc00f686c2251fa Author: Yoichi Osato <yoichio@chromium.org> Date: Thu Jul 26 01:55:25 2018 Revert "Enable FlingSchedulerMac" This reverts commit d6ec81545fea9d795fe5655d8453c343bc61420a. Reason for revert: Findit identified the culprit r577957 with confidence 100.0% in the config "chromium.mac / Mac10.11 Tests" Original change's description: > Enable FlingSchedulerMac > > Enable tests that were disabled because of this functionality missing. > > Bug: 842325 > Change-Id: I6f540460f4e5f81d7ff8dd36e4abf369f0bd4785 > Reviewed-on: https://chromium-review.googlesource.com/1149612 > Reviewed-by: Jonathan Ross <jonross@chromium.org> > Commit-Queue: ccameron <ccameron@chromium.org> > Cr-Commit-Position: refs/heads/master@{#577957} TBR=ccameron@chromium.org,jonross@chromium.org Change-Id: I4a38151ba5333ca99fb28c00f57f93f057f5fd4b No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 842325 , 867668 Reviewed-on: https://chromium-review.googlesource.com/1150946 Reviewed-by: Yoichi Osato <yoichio@chromium.org> Commit-Queue: Yoichi Osato <yoichio@chromium.org> Cr-Commit-Position: refs/heads/master@{#578166} [modify] https://crrev.com/4c440b7c6b5edb7c01d41d498bc00f686c2251fa/content/browser/renderer_host/browser_compositor_view_mac.h [modify] https://crrev.com/4c440b7c6b5edb7c01d41d498bc00f686c2251fa/content/browser/renderer_host/browser_compositor_view_mac.mm [modify] https://crrev.com/4c440b7c6b5edb7c01d41d498bc00f686c2251fa/content/browser/renderer_host/input/fling_scheduler_mac.mm [modify] https://crrev.com/4c440b7c6b5edb7c01d41d498bc00f686c2251fa/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm [modify] https://crrev.com/4c440b7c6b5edb7c01d41d498bc00f686c2251fa/testing/buildbot/filters/viz.content_browsertests.filter
,
Jul 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3fff2b9f19ac86aec16ed5c0179cfe3cfe9dabc7 commit 3fff2b9f19ac86aec16ed5c0179cfe3cfe9dabc7 Author: Jonathan Ross <jonross@chromium.org> Date: Thu Jul 26 17:57:19 2018 Early Exit Failing Mac Fling Tests on Viz There are two remaining Mac Fling tests which fail under Viz. Work is ongoing to land a fix, but there was flakiness if the resolution. This change updates the failing tests to exit early when it is Viz on Mac. This will unblock removing our filter files. TEST=BrowserSideFlingBrowserTest.TouchpadFling BrowserSideFlingBrowserTest.TouchscreenFling Bug: 842325 Change-Id: I67e7c6165d821c9df4907ebf8803fe22fbdf1663 Reviewed-on: https://chromium-review.googlesource.com/1151392 Reviewed-by: ccameron <ccameron@chromium.org> Commit-Queue: Jonathan Ross <jonross@chromium.org> Cr-Commit-Position: refs/heads/master@{#578362} [modify] https://crrev.com/3fff2b9f19ac86aec16ed5c0179cfe3cfe9dabc7/content/browser/renderer_host/input/fling_browsertest.cc
,
Jul 26
Actually, the flake was some layout test ... see issue 867668. It starts passing 93% of the time on some ancient macOS version (grumble).
,
Aug 1
So the test in question is middleClickAutoscroll-click-hyperlink.html Looking into it further, this test was first marked with an expectation of failure by sheriffs on June 8, way before your patch. See issue 851090 It was also later one tracked by issue 867628 as well. So it looks like its a flaky test in general, and your patch may have helped reveal it further. Thoughts on relanding your patch, and coordinating with sahel@ about the flakiness of the test?
,
Aug 1
There are a few middleClickAutoscroll related tests that are flaky on Mac. I think it is ok to skip the test on Mac and land your patch. Me or sunyunjia@ will work on the test to make it less flaky and re-enable it.
,
Aug 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5b164dcf2b045ee787e69a20322a4a2808fbe9c0 commit 5b164dcf2b045ee787e69a20322a4a2808fbe9c0 Author: jonross <jonross@chromium.org> Date: Tue Aug 07 19:47:31 2018 Reland FlingSchedulerMac for Viz This relands the following change: Enable FlingSchedulerMac Enable tests that were disabled because of this functionality missing. Bug: 842325 Change-Id: I6f540460f4e5f81d7ff8dd36e4abf369f0bd4785 Reviewed-on: https://chromium-review.googlesource.com/1149612 Reviewed-by: Jonathan Ross <jonross@chromium.org> Commit-Queue: ccameron <ccameron@chromium.org> Cr-Commit-Position: refs/heads/master@{#577957} This was identified as a cause of flakiness in Mac webkit_layout_tests. But that test's flakiness also pre-dated this patch. The test in question has also been marked to expect fails on Mac. The source of that flakiness will be investigated separately. Change-Id: I22364cb8e18507b528c6c0c32f452fcf208caf7e Reviewed-on: https://chromium-review.googlesource.com/1162716 Commit-Queue: Jonathan Ross <jonross@chromium.org> Reviewed-by: ccameron <ccameron@chromium.org> Cr-Commit-Position: refs/heads/master@{#581309} [modify] https://crrev.com/5b164dcf2b045ee787e69a20322a4a2808fbe9c0/content/browser/renderer_host/browser_compositor_view_mac.h [modify] https://crrev.com/5b164dcf2b045ee787e69a20322a4a2808fbe9c0/content/browser/renderer_host/browser_compositor_view_mac.mm [modify] https://crrev.com/5b164dcf2b045ee787e69a20322a4a2808fbe9c0/content/browser/renderer_host/input/fling_browsertest.cc [modify] https://crrev.com/5b164dcf2b045ee787e69a20322a4a2808fbe9c0/content/browser/renderer_host/input/fling_scheduler_mac.mm [modify] https://crrev.com/5b164dcf2b045ee787e69a20322a4a2808fbe9c0/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm [modify] https://crrev.com/5b164dcf2b045ee787e69a20322a4a2808fbe9c0/third_party/WebKit/LayoutTests/TestExpectations
,
Aug 7
Fling is now re-enabled for Viz. With the flaky test expectations updated and issue 851090 tracking their re-work. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by sahel@chromium.org
, May 11 2018