OOPIF clipping incorrectly when device emulation mode enabled. |
||||||||
Issue descriptionWhat steps will reproduce the problem? 1. Turn on --site-per-process. 2. Navigate to https://slashdot.org 3. Open up dev tools and enable device emulation mode. 4. Set to Responsive and show the device pixel ratio controls. 5. Play around with device pixel ratio and notice the odd OOPIF clipping change. This is likely caused by the RenderWidgetScreenMetricsEmulator (RWSME). RWSME lies about the ScreenInfo (including the device scale factor). When ScreenInfo changes, we inform BrowserPlugins and RenderFrameProxys of the change. The problem is the emulated ScreenInfo gets propagated to the child renderers which clip the viewport incorrectly (since the parent Blink already accounts for the scaling). The solution is to always use the "original" ScreenInfo for RenderFrameProxy and BrowserPlugin.
,
Mar 8 2018
,
Mar 8 2018
,
Mar 8 2018
I don't think this warrants merging in M66. M67 sgtm
,
Mar 8 2018
Yep, 67 should be fine.
,
Mar 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a863f15cf84a128c419ba8e2b6d3da3b91499f55 commit a863f15cf84a128c419ba8e2b6d3da3b91499f55 Author: Fady Samuel <fsamuel@chromium.org> Date: Fri Mar 09 16:10:03 2018 Surface synchronization: Pass original ScreenInfo to OOPIF/BrowserPlugin In device emulation mode, the top level renderer devtools lies to Blink about the size and scale factor by constructing an emulated ScreenInfo object. Prior to this patch, we propagated that ScreenInfo down to OOPIFs and BrowserPlugins. This caused double scaling of the compositor viewport size in OOPIFs because the top level renderer had already scaled down the surface embedded. This CL instead plumbs down the original ScreenInfo which fixes the device emulation problem. Bug: 819903 , 672962 Change-Id: Ie32c2798edcbe8738c463edf86d45c86f33d0d5b Reviewed-on: https://chromium-review.googlesource.com/956554 Commit-Queue: Fady Samuel <fsamuel@chromium.org> Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Reviewed-by: Lucas Gadani <lfg@chromium.org> Cr-Commit-Position: refs/heads/master@{#542130} [modify] https://crrev.com/a863f15cf84a128c419ba8e2b6d3da3b91499f55/content/renderer/browser_plugin/browser_plugin.cc [modify] https://crrev.com/a863f15cf84a128c419ba8e2b6d3da3b91499f55/content/renderer/input/render_widget_input_handler.cc [modify] https://crrev.com/a863f15cf84a128c419ba8e2b6d3da3b91499f55/content/renderer/render_frame_impl.cc [modify] https://crrev.com/a863f15cf84a128c419ba8e2b6d3da3b91499f55/content/renderer/render_frame_proxy.cc [modify] https://crrev.com/a863f15cf84a128c419ba8e2b6d3da3b91499f55/content/renderer/render_view_browsertest.cc [modify] https://crrev.com/a863f15cf84a128c419ba8e2b6d3da3b91499f55/content/renderer/render_view_impl.cc [modify] https://crrev.com/a863f15cf84a128c419ba8e2b6d3da3b91499f55/content/renderer/render_view_impl.h [modify] https://crrev.com/a863f15cf84a128c419ba8e2b6d3da3b91499f55/content/renderer/render_widget.cc [modify] https://crrev.com/a863f15cf84a128c419ba8e2b6d3da3b91499f55/content/renderer/render_widget.h
,
Mar 9 2018
,
Mar 19 2018
I would like to merge this into m66 because crrev.com/50d1e0c doesn't merge nicely without it. I already talked to Fady and he believes this CL is safe.
,
Mar 19 2018
This bug requires manual review: M66 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 20 2018
Approving merge for M66. Branch:3359
,
Mar 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/33ce7de78d85aacdb69bc5e122b9e86e53e1a696 commit 33ce7de78d85aacdb69bc5e122b9e86e53e1a696 Author: Fady Samuel <fsamuel@chromium.org> Date: Tue Mar 20 12:51:34 2018 Surface synchronization: Pass original ScreenInfo to OOPIF/BrowserPlugin In device emulation mode, the top level renderer devtools lies to Blink about the size and scale factor by constructing an emulated ScreenInfo object. Prior to this patch, we propagated that ScreenInfo down to OOPIFs and BrowserPlugins. This caused double scaling of the compositor viewport size in OOPIFs because the top level renderer had already scaled down the surface embedded. This CL instead plumbs down the original ScreenInfo which fixes the device emulation problem. Bug: 819903 , 672962 Change-Id: Ie32c2798edcbe8738c463edf86d45c86f33d0d5b Reviewed-on: https://chromium-review.googlesource.com/956554 Commit-Queue: Fady Samuel <fsamuel@chromium.org> Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Reviewed-by: Lucas Gadani <lfg@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#542130}(cherry picked from commit a863f15cf84a128c419ba8e2b6d3da3b91499f55) Reviewed-on: https://chromium-review.googlesource.com/970781 Reviewed-by: Fady Samuel <fsamuel@chromium.org> Cr-Commit-Position: refs/branch-heads/3359@{#340} Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276} [modify] https://crrev.com/33ce7de78d85aacdb69bc5e122b9e86e53e1a696/content/renderer/browser_plugin/browser_plugin.cc [modify] https://crrev.com/33ce7de78d85aacdb69bc5e122b9e86e53e1a696/content/renderer/input/render_widget_input_handler.cc [modify] https://crrev.com/33ce7de78d85aacdb69bc5e122b9e86e53e1a696/content/renderer/render_frame_impl.cc [modify] https://crrev.com/33ce7de78d85aacdb69bc5e122b9e86e53e1a696/content/renderer/render_frame_proxy.cc [modify] https://crrev.com/33ce7de78d85aacdb69bc5e122b9e86e53e1a696/content/renderer/render_view_browsertest.cc [modify] https://crrev.com/33ce7de78d85aacdb69bc5e122b9e86e53e1a696/content/renderer/render_view_impl.cc [modify] https://crrev.com/33ce7de78d85aacdb69bc5e122b9e86e53e1a696/content/renderer/render_view_impl.h [modify] https://crrev.com/33ce7de78d85aacdb69bc5e122b9e86e53e1a696/content/renderer/render_widget.cc [modify] https://crrev.com/33ce7de78d85aacdb69bc5e122b9e86e53e1a696/content/renderer/render_widget.h |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by fsam...@chromium.org
, Mar 8 2018