OOPIF viewport rects don't correct account for CSS scale |
|||||||||||
Issue descriptionThis bug is based on comment 32 of issue 690605 . Out-of-process iframes do not correctly account for CSS scale when calculating raster area, which is created by RenderFrameProxy::ComputeCompositingRect() and sent to the OOPIF over IPC. This causes scaled OOPIFs that are significantly larger than the window viewport to have unpainted areas when scrolled. The repro for this is from issue 817392 , load the attached page and then scroll the OOPIF.
,
Apr 2 2018
Let's make sure to get this fixed before stable in M67.
,
Apr 3 2018
The problem here is that we can't precisely use the same rect for intersection observer as for viewport clipping, because the former expects transforms to be applied to the rect, while the latter requires that they not be. I wrote a draft CL last week that turned out to only partially fix the problem (it corrected the position but not the size of the viewport rect), and will get back to it very soon.
,
Apr 12 2018
CL in progress: https://chromium-review.googlesource.com/1005993 Sounds like Ken's current plan is to land after branch, let it bake for a bit to ensure it's safe, then request merge to M67.
,
Apr 25 2018
M67 Stable promotion is coming soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. Thank you.
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/75dd4f69952c16ed3c5a987081d9c1e9d3c13927 commit 75dd4f69952c16ed3c5a987081d9c1e9d3c13927 Author: Ken Buchanan <kenrb@chromium.org> Date: Thu Apr 26 18:37:49 2018 Refactor OOPIF viewport clipping to support transforms The current computation of the display area for OOPIFs does not account for CSS transforms that can affect the size and offset of the iframe. This refactor moves most of the computation into Blink from content, where it is easier to account for transforms and requires less information to be plumbed out through the Blink API. Bug: 823433 Change-Id: I4d7ea05c466620d573f1166abf310291cfda6700 Reviewed-on: https://chromium-review.googlesource.com/1005993 Commit-Queue: Charlie Reis <creis@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Reviewed-by: Tien-Ren Chen <trchen@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#554097} [modify] https://crrev.com/75dd4f69952c16ed3c5a987081d9c1e9d3c13927/content/browser/site_per_process_browsertest.cc [modify] https://crrev.com/75dd4f69952c16ed3c5a987081d9c1e9d3c13927/content/renderer/render_frame_proxy.cc [modify] https://crrev.com/75dd4f69952c16ed3c5a987081d9c1e9d3c13927/content/renderer/render_frame_proxy.h [modify] https://crrev.com/75dd4f69952c16ed3c5a987081d9c1e9d3c13927/content/renderer/render_widget.cc [modify] https://crrev.com/75dd4f69952c16ed3c5a987081d9c1e9d3c13927/content/renderer/render_widget.h [add] https://crrev.com/75dd4f69952c16ed3c5a987081d9c1e9d3c13927/content/test/data/frame_tree/page_with_large_scrollable_frame.html [add] https://crrev.com/75dd4f69952c16ed3c5a987081d9c1e9d3c13927/content/test/data/frame_tree/page_with_scaled_large_frame.html [add] https://crrev.com/75dd4f69952c16ed3c5a987081d9c1e9d3c13927/content/test/data/frame_tree/page_with_scaled_large_frames_nested.html [modify] https://crrev.com/75dd4f69952c16ed3c5a987081d9c1e9d3c13927/third_party/blink/public/web/web_remote_frame.h [modify] https://crrev.com/75dd4f69952c16ed3c5a987081d9c1e9d3c13927/third_party/blink/renderer/core/exported/web_remote_frame_impl.cc [modify] https://crrev.com/75dd4f69952c16ed3c5a987081d9c1e9d3c13927/third_party/blink/renderer/core/exported/web_remote_frame_impl.h [modify] https://crrev.com/75dd4f69952c16ed3c5a987081d9c1e9d3c13927/third_party/blink/renderer/core/frame/remote_frame_view.cc [modify] https://crrev.com/75dd4f69952c16ed3c5a987081d9c1e9d3c13927/third_party/blink/renderer/core/frame/remote_frame_view.h
,
Apr 26 2018
Pls request a merge to M67 once change listed at #6 well baked/verified in canary. Thank you.
,
Apr 26 2018
Issue 836180 has been merged into this issue.
,
Apr 27 2018
The NextAction date has arrived: 2018-04-27
,
Apr 27 2018
I've confirmed that the repro steps show the bug in 67.0.3396.18 without the fix and that the bug is fixed in today's 68.0.3410.0 Canary with r554097. Thanks Ken! I'll note that the 68.0.3410.0 canary has a huge spike in crashes in issue 837541, but that appears to be from r554213 and not this CL. I don't see any crashes from this one. govind: It might make sense to let this bake over the weekend and have Ken request merge when he returns on Monday, giving him a chance to confirm it's safe. Does that make sense, or should we request the merge today to get it done?
,
Apr 27 2018
[Auto-generated comment by a script] We noticed that this issue is targeted for M-67; 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-67 label, otherwise remove Merge-TBD label. Thanks.
,
Apr 27 2018
Re #10: Pls update the bug with canary result on Monday morning. If all looks good, I will approve the merge. Thank you.
,
Apr 27 2018
Thanks! I can also confirm r554097 fixes issue 836180 in 68.0.3410.0 (per comment 8).
,
Apr 27 2018
This bug requires manual review: M67 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), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 30 2018
The NextAction date has arrived: 2018-04-30
,
Apr 30 2018
kenrb@: Can you verify the fix looks good on Canary and that it's safe to merge? We'd like to merge it to M67 today before 4 pm Pacific if possible. Thanks!
,
Apr 30 2018
All the test cases I have for this bug look fine on Canary, and I haven't seen any related bugs or crashes at this point. Can I have approval to merge?
,
Apr 30 2018
Approving merge to M67 branch 3396 based on comment #17. Pls merge ASAP so we can pick it up for this week M67 beta release. Thank you.
,
Apr 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f commit d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f Author: Ken Buchanan <kenrb@chromium.org> Date: Mon Apr 30 19:43:49 2018 Refactor OOPIF viewport clipping to support transforms The current computation of the display area for OOPIFs does not account for CSS transforms that can affect the size and offset of the iframe. This refactor moves most of the computation into Blink from content, where it is easier to account for transforms and requires less information to be plumbed out through the Blink API. TBR=kenrb@chromium.org (cherry picked from commit 75dd4f69952c16ed3c5a987081d9c1e9d3c13927) Bug: 823433 Change-Id: I4d7ea05c466620d573f1166abf310291cfda6700 Reviewed-on: https://chromium-review.googlesource.com/1005993 Commit-Queue: Charlie Reis <creis@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Reviewed-by: Tien-Ren Chen <trchen@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#554097} Reviewed-on: https://chromium-review.googlesource.com/1035910 Reviewed-by: Ken Buchanan <kenrb@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#394} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f/content/browser/site_per_process_browsertest.cc [modify] https://crrev.com/d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f/content/renderer/render_frame_proxy.cc [modify] https://crrev.com/d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f/content/renderer/render_frame_proxy.h [modify] https://crrev.com/d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f/content/renderer/render_widget.cc [modify] https://crrev.com/d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f/content/renderer/render_widget.h [add] https://crrev.com/d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f/content/test/data/frame_tree/page_with_large_scrollable_frame.html [add] https://crrev.com/d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f/content/test/data/frame_tree/page_with_scaled_large_frame.html [add] https://crrev.com/d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f/content/test/data/frame_tree/page_with_scaled_large_frames_nested.html [modify] https://crrev.com/d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f/third_party/blink/public/web/web_remote_frame.h [modify] https://crrev.com/d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f/third_party/blink/renderer/core/exported/web_remote_frame_impl.cc [modify] https://crrev.com/d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f/third_party/blink/renderer/core/exported/web_remote_frame_impl.h [modify] https://crrev.com/d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f/third_party/blink/renderer/core/frame/remote_frame_view.cc [modify] https://crrev.com/d88c3f7ef9531bbda42b8e166c5e7d6280c52e1f/third_party/blink/renderer/core/frame/remote_frame_view.h |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by creis@chromium.org
, Mar 19 2018Labels: -Pri-2 Target-67 M-67 Proj-SiteIsolation-LaunchBlocking Pri-1
41.0 KB
41.0 KB View Download