Regression : Unable to scroll 'Subtitles' drop down list using mouse wheel for Google Doddle.
Reported by
rp...@etouch.net,
Sep 27
|
|||||||||||||
Issue descriptionChrome version: 71.0.3562.0 (Official Build)Revision 546b39eb7822fcd38f6c8093cb3ac1a70f8f2dcf-refs/branch-heads/3562@{#1}(32/64-bit) OS: Windows(7,8,8.1,10),Mac(10.12.6 , 10.13.1 , 10.14) and Linux(14.04 LTS) What steps will reproduce the problem? 1. Launch chrome, navigate to NTP and click on play button of 'Google Doddle' 2. Click on 'Settings' gear icon of video player and click to open 'Subtitles' drop down list. 3. Now try to scroll 'Subtitles' drop down list using mouse wheel and observe Actual Result: Unable to scroll 'Subtitles' drop down list using mouse wheel Expected Result: Should be able to scroll 'Subtitles' drop down list using mouse wheel This is regression issue broken in ‘M-71’ and below is the bisect info: Good build: 71.0.3561.0 (Revision: 593802) Bad build : 71.0.3562.0 (Revision: 594163) You are probably looking for a change made after 594020 (known good), but no later than 594024 (first known bad). CHANGE-LOG URL: https://chromium.googlesource.com/chromium/src/+log/126e7d72a19e66aac93fa7f4ecb6f7fee29591c5..b209442fa2c29021b06a6dcbbd0486b440011fe2?pretty=fuller&n=50 Suspecting: r594021 ? @danakj: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. NOTE: 1. Provided suspect through 'Chromium' bisect as unable to perform bisect using 'per-revision' bisect script. 2. Tried performing 'per revision' bisect on multiple Windows,Linux and Mac machines but unable to perform the same since getting error: "RuntimeError: We don't have enough builds to bisect". Thank you.
,
Oct 3
The google doodle does not have a play button for me, so I'm unable to try reproduce. How do I get into that state?
,
Oct 3
I found the youtube video on https://www.youtube.com/watch?v=-I_CIy3Uqx0 I can not reproduce the problem on Linux on da327162b3b76213b8846651d40bff1836bd45e1 which includes the above bisect range. The subtitles list scrolls properly for me. Can you bisect again, or try reproduce again on the latest Chrome?
,
Oct 4
With response to comment #3 : Able to reproduce this issue on Windows(7,8,8.1,10), Mac(10.12.6 , 10.13.1 , 10.14) and Linux(14.04 LTS) machines using latest Canary chrome version : 71.0.3570.0 with the steps mentioned in the description of the bug. URL : https://www.google.com/doodles/googles-20th-birthday-us To reproduce this issue navigate to above URL and perform the steps mentioned in the description of the bug. Attaching the screen cast for the same. NOTE : This issue is not reproducible for any www.youtube.com videos and it is only reproducible for Google Doodle. Re-bisected this issue on different machines and found the same range as mentioned in the description of the bug. @ danakj: Assigning this bug to you.Kindly have a look into this and if it is not related to you then please help to assign it to right owner. Thank you..!!
,
Oct 4
,
Oct 4
,
Oct 5
Confirmed my CL changes this. What happens is the scrolls in the captions list bubble up to the webpage, scrolling that instead. The difference vs on Youtube site would be its an embedded youtube video, so I guess it would be an OOPIF and then the captions list would be a.. WebPagePopup off the OOPIF maybe? My patch keeps the compositor thread scrolling for child local root widgets (based on for_oopif_) in RenderWidget, so must be something else about it.
,
Oct 5
Oh, CL was https://chromium-review.googlesource.com/1229417 not the one that changed the scrolling on thread logic. I'll have to compare before/after and see what's different I have no clue from the top of my head.
,
Oct 9
Looks to me like we're targeting the wrong renderer. +wjmaclean,+mcnee for OOPIF in case they have some advice.
,
Oct 9
Is this perhaps related to https://bugs.chromium.org/p/chromium/issues/detail?id=880122 ?
,
Oct 9
,
Oct 9
#10: I don't think it's the same. The difference was that this changed ordering of initialization for child local root renderwidgets, they were doing init of the compositor from within the WebFrameWidget constructor, which used to happen in the middle of RenderWidget init, but now was happening before. So then the RenderWidget was being used before it was ready. I think the solution is to move control of when the compositor is initialized into RenderWidget::Init so that it happens when the RenderWidget is ready for it to happen.
,
Oct 15
Friendly ping to get an update as it is marked as RBS. Thanks..!
,
Oct 16
,
Oct 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/17216f4d474e76a18eb2d0aacea4e3a771ccae09 commit 17216f4d474e76a18eb2d0aacea4e3a771ccae09 Author: danakj <danakj@chromium.org> Date: Tue Oct 16 22:57:32 2018 Initialize compositor in RenderWidget::Init() and pass it to WebWidget Previously the WebWidgets would each ask RenderWidget to initialize the compositor at different points in its initialization. WebPagePopup would do so after the widget was initialized, where as local root frames would do so before it was initialized (but importantly, after for_oopif_ was set - so really in the middle of initialization). WebView would do so before any initialization of RenderWidget. This made for 3 different ordering interactions of compositor initialization and RenderWidget initialization. Notably, compositor initialization goes through RenderWidget, and depends on that class. So using it before it's initialized is.. troublesome. This removes the WebWidgetClient::InitializeLayerTreeView() method, and has RenderWidget always initialize a compositor (since we always composite them now, thanks Always Forced Compositing Mode!). That allows RenderWidget to control the order of its initialization internally, so it is not used before it is initialized (or, at least not in this case.. there may be more to untangle). We add WebWidget::SetLayerTreeView() instead, which goes out to the WebWidget when the RenderWidget is being initialized, since the WebWidget is initialized first and passed to RenderWidget. This helps detangle the SimCompositor and FrameTestHelpers a bit, we no longer need 2 LayerTreeViews in SimTest - one in SimCompositor and one in TestWebViewClient. Instead we use the singular one in TestWebViewClient, and pass through the SimCompositor as the LayerTreeViewDelegate to it, so that it can implement callbacks from the compositor. Now the FrameTestHelpers must set the LayerTreeView on the various WebWidgets that it creates, either WebViewImpl or WebFrameWidgetImpls. R=ajwong@chromium.org, dcheng@chromium.org, jochen@chromium.org, piman@chromium.org Bug: 889759 Change-Id: I0e1b3ebf4132874d1151a6a770a02a1a40b650b1 Reviewed-on: https://chromium-review.googlesource.com/c/1271793 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Albert J. Wong <ajwong@chromium.org> Commit-Queue: danakj <danakj@chromium.org> Cr-Commit-Position: refs/heads/master@{#600165} [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/components/plugins/renderer/webview_plugin.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/components/plugins/renderer/webview_plugin.h [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/components/printing/renderer/print_render_frame_helper.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/content/renderer/render_widget.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/content/renderer/render_widget.h [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/content/renderer/render_widget_fullscreen_pepper.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/content/renderer/render_widget_unittest.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/content/shell/test_runner/web_view_test_proxy.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/content/shell/test_runner/web_view_test_proxy.h [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/content/shell/test_runner/web_widget_test_client.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/content/shell/test_runner/web_widget_test_client.h [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/public/web/web_widget.h [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/public/web/web_widget_client.h [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/exported/web_page_popup_impl.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/exported/web_page_popup_impl.h [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/exported/web_view_impl.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/exported/web_view_impl.h [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/exported/web_view_test.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/frame/frame_test_helpers.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/frame/frame_test_helpers.h [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/frame/web_frame_widget_impl.h [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/frame/web_view_frame_widget.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/frame/web_view_frame_widget.h [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/testing/sim/sim_compositor.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/testing/sim/sim_compositor.h [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/testing/sim/sim_test.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/testing/sim/sim_test.h [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/testing/sim/sim_web_view_client.cc [modify] https://crrev.com/17216f4d474e76a18eb2d0aacea4e3a771ccae09/third_party/blink/renderer/core/testing/sim/sim_web_view_client.h
,
Oct 17
Issue 896139 has been merged into this issue.
,
Oct 17
,
Oct 17
[Auto-generated comment by a script] We noticed that this issue is targeted for M-71; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-71 label, otherwise remove Merge-TBD label. Thanks.
,
Oct 17
,
Oct 18
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ddb97ad290e67725c4327c122aee751a00705bf6 commit ddb97ad290e67725c4327c122aee751a00705bf6 Author: danakj <danakj@chromium.org> Date: Thu Oct 18 16:22:44 2018 Initialize compositor in RenderWidget::Init() and pass it to WebWidget Previously the WebWidgets would each ask RenderWidget to initialize the compositor at different points in its initialization. WebPagePopup would do so after the widget was initialized, where as local root frames would do so before it was initialized (but importantly, after for_oopif_ was set - so really in the middle of initialization). WebView would do so before any initialization of RenderWidget. This made for 3 different ordering interactions of compositor initialization and RenderWidget initialization. Notably, compositor initialization goes through RenderWidget, and depends on that class. So using it before it's initialized is.. troublesome. This removes the WebWidgetClient::InitializeLayerTreeView() method, and has RenderWidget always initialize a compositor (since we always composite them now, thanks Always Forced Compositing Mode!). That allows RenderWidget to control the order of its initialization internally, so it is not used before it is initialized (or, at least not in this case.. there may be more to untangle). We add WebWidget::SetLayerTreeView() instead, which goes out to the WebWidget when the RenderWidget is being initialized, since the WebWidget is initialized first and passed to RenderWidget. This helps detangle the SimCompositor and FrameTestHelpers a bit, we no longer need 2 LayerTreeViews in SimTest - one in SimCompositor and one in TestWebViewClient. Instead we use the singular one in TestWebViewClient, and pass through the SimCompositor as the LayerTreeViewDelegate to it, so that it can implement callbacks from the compositor. Now the FrameTestHelpers must set the LayerTreeView on the various WebWidgets that it creates, either WebViewImpl or WebFrameWidgetImpls. R=ajwong@chromium.org, dcheng@chromium.org, jochen@chromium.org, piman@chromium.org TBR=danakj@chromium.org (cherry picked from commit 17216f4d474e76a18eb2d0aacea4e3a771ccae09) Bug: 889759 Change-Id: I0e1b3ebf4132874d1151a6a770a02a1a40b650b1 Reviewed-on: https://chromium-review.googlesource.com/c/1271793 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Albert J. Wong <ajwong@chromium.org> Commit-Queue: danakj <danakj@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#600165} Reviewed-on: https://chromium-review.googlesource.com/c/1288896 Reviewed-by: danakj <danakj@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#119} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/components/plugins/renderer/webview_plugin.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/components/plugins/renderer/webview_plugin.h [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/components/printing/renderer/print_render_frame_helper.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/content/renderer/render_widget.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/content/renderer/render_widget.h [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/content/renderer/render_widget_fullscreen_pepper.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/content/renderer/render_widget_unittest.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/content/shell/test_runner/web_view_test_proxy.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/content/shell/test_runner/web_view_test_proxy.h [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/content/shell/test_runner/web_widget_test_client.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/content/shell/test_runner/web_widget_test_client.h [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/public/web/web_widget.h [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/public/web/web_widget_client.h [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/exported/web_page_popup_impl.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/exported/web_page_popup_impl.h [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/exported/web_view_impl.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/exported/web_view_impl.h [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/exported/web_view_test.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/frame/frame_test_helpers.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/frame/frame_test_helpers.h [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/frame/web_frame_widget_impl.h [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/frame/web_view_frame_widget.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/frame/web_view_frame_widget.h [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/testing/sim/sim_compositor.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/testing/sim/sim_compositor.h [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/testing/sim/sim_test.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/testing/sim/sim_test.h [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/testing/sim/sim_web_view_client.cc [modify] https://crrev.com/ddb97ad290e67725c4327c122aee751a00705bf6/third_party/blink/renderer/core/testing/sim/sim_web_view_client.h
,
Oct 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ddb97ad290e67725c4327c122aee751a00705bf6 Commit: ddb97ad290e67725c4327c122aee751a00705bf6 Author: danakj@chromium.org Commiter: danakj@chromium.org Date: 2018-10-18 16:22:44 +0000 UTC Initialize compositor in RenderWidget::Init() and pass it to WebWidget Previously the WebWidgets would each ask RenderWidget to initialize the compositor at different points in its initialization. WebPagePopup would do so after the widget was initialized, where as local root frames would do so before it was initialized (but importantly, after for_oopif_ was set - so really in the middle of initialization). WebView would do so before any initialization of RenderWidget. This made for 3 different ordering interactions of compositor initialization and RenderWidget initialization. Notably, compositor initialization goes through RenderWidget, and depends on that class. So using it before it's initialized is.. troublesome. This removes the WebWidgetClient::InitializeLayerTreeView() method, and has RenderWidget always initialize a compositor (since we always composite them now, thanks Always Forced Compositing Mode!). That allows RenderWidget to control the order of its initialization internally, so it is not used before it is initialized (or, at least not in this case.. there may be more to untangle). We add WebWidget::SetLayerTreeView() instead, which goes out to the WebWidget when the RenderWidget is being initialized, since the WebWidget is initialized first and passed to RenderWidget. This helps detangle the SimCompositor and FrameTestHelpers a bit, we no longer need 2 LayerTreeViews in SimTest - one in SimCompositor and one in TestWebViewClient. Instead we use the singular one in TestWebViewClient, and pass through the SimCompositor as the LayerTreeViewDelegate to it, so that it can implement callbacks from the compositor. Now the FrameTestHelpers must set the LayerTreeView on the various WebWidgets that it creates, either WebViewImpl or WebFrameWidgetImpls. R=ajwong@chromium.org, dcheng@chromium.org, jochen@chromium.org, piman@chromium.org TBR=danakj@chromium.org (cherry picked from commit 17216f4d474e76a18eb2d0aacea4e3a771ccae09) Bug: 889759 Change-Id: I0e1b3ebf4132874d1151a6a770a02a1a40b650b1 Reviewed-on: https://chromium-review.googlesource.com/c/1271793 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Albert J. Wong <ajwong@chromium.org> Commit-Queue: danakj <danakj@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#600165} Reviewed-on: https://chromium-review.googlesource.com/c/1288896 Reviewed-by: danakj <danakj@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#119} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} |
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by pnangunoori@chromium.org
, Sep 27Labels: ReleaseBlock-Stable