See spec discussion:
https://github.com/whatwg/html/issues/1813
and ancient webkit bug:
https://bugs.webkit.org/show_bug.cgi?id=107236
I don't think we can avoid creating the LayoutView without everything coming tumbling down on us since everything with a frame is assumed to have one, but we can probably just add a check in LayoutTreeBuilderForElement::shouldCreateLayoutObject() and then we need some bits to propagate for OOPIF.
I wonder how this would work for remote frames when the <iframe> goes from display:none to not display:none. We wouldn't be able to replicate the CSS property change immediately, which means there'd be some visual delay before the <iframe>'s contents go through layout and painting, right? Maybe that's OK?
I think that's fine, this is the same as any resize. Today display: none frames are doing layout with a 0x0 viewport, OOPIF is already replicating that state, this would just be replicating another bit so we can skip layout tree building entirely.
This has always been a problem right? I agree it's important to fix, but unless something has changed to make this significantly worse, I'm not sure about the RB-S label here (especially when there's no owner).
This would likely also need an intent to implement and ship, as it has web-visible effects.
Given that the issue stems from lack of clarity in spec, the solution will require some time [spec ratification, intent to implement and ship], and this has been Chrome's behavior for quite some time, there's no need to mark this as RB-S.
I've updated the internal bug, since Chrome is behaving according to [the current] spec: https://buganizer.corp.google.com/issues/32124393
Going to try to land the main CL. dcheng notes we will want OOPIF tests.
"""
On 2017/02/13 23:13:55, dcheng wrote:
> Sorry for the delayed review. You're right that I think this Just Works
> (unfortunately, we have two different ways of plumbing replicated state and I
> mixed things up). That being said, I'm not sure that any of the layout tests
> actually cover the OOPIF case: they would need to be http tests with
> cross-origin frames to actually trigger coverage.
>
> In any case, IPC LGTM but we may want to consider adding OOPIF specific tests in
> a followup.
"""
This CL was reverted because of failures on chromium.webkit/WebKit Linux Trusty Leak
https://bugs.chromium.org/p/chromium/issues/detail?id=692872
"""
12:01:41.854 31767 worker/1 virtual/mojo-loading/http/tests/security/contentSecurityPolicy/object-src-url-allowed.html leaked
12:01:41.862 31665 [20757/48425] virtual/mojo-loading/http/tests/security/contentSecurityPolicy/object-src-url-allowed.html failed unexpectedly (leak detected: ({"numberOfLiveDocuments":[1,2],"numberOfLiveNodes":[4,10],"numberOfLiveSuspendableObjects":[2,3]}))
12:01:41.854 31767 worker/1 virtual/mojo-loading/http/tests/security/contentSecurityPolicy/object-src-url-allowed.html failed:
"""
I will attempt to investigate this when I get some time.
+szager
Status update:
The original CL landed ~2 months ago as a RuntimeEnabledFeature=experimental.
1) This caused a known fullscreen regression: [ Issue 676432 ]. I've been waiting for foolip@'s fullscreen refactor to land before continuing.
2) Meanwhile, this has caused a lifecycle-related clusterfuzz regression: https://bugs.chromium.org/p/chromium/issues/detail?id=730398&q=owner%3Aszager%40chromium.org%20&colspec=ID%20Pri%20M%20Stars%20ReleaseBlock%20Component%20Status%20Owner%20Summary%20OS%20Modified.
szager@ has been digging into this, and it turns out to be fairly hairy:
https://chromium-review.googlesource.com/c/527606/
We seem to be running into issues with ForceLayoutParentViewIfNeeded for embedded svg elements.
szager@ has suggested that we can accomplish the desired effect [preventing RAFs] by allowing the layout object to be created, but latching onto the same mechanism that we use to throttle offscreen iframes, e.g.
"""
3374 void LocalFrameView::UpdateStyleAndLayoutIfNeededRecursiveInternal() {
3375 if (ShouldThrottleRendering() || !frame_->GetDocument()->IsActive())
3376 return;
"""
It's not immediately clear to me whether this meets the specification for the upcoming CSS changes: https://github.com/whatwg/html/issues/1813, but it certainly sounds like an easier change.
dcheng, esprehn: thoughts?
skyostil: My reading of https://github.com/whatwg/html/issues/1813 is that this *may* indeed break some sites, but this is already a case where behavior has diverged between Firefox/Edge and Chrome/Safari, and that the we [meaning the CSS spec] wants to converge towards the Firefox/Edge behavior.
This issue has been automatically relabelled type=task because type=launch-owp issues are now officially deprecated. The deprecation is because they were creating confusion about how to get launch approvals, which should be instead done via type=launch issues.
We recommend this issue be used for implementation tracking (for public visibility), but if you already have an issue for that, you may mark this as duplicate.
For more details see here: https://docs.google.com/document/d/1JA6RohjtZQc26bTrGoIE_bSXGXUDQz8vc6G0n_sZJ2o/edit
For any questions, please contact owencm, sshruthi, larforge
Comment 1 by andymutton@chromium.org
, Sep 26 2016Status: Available (was: Untriaged)