1454493 - ContentParent::RecvCreateWindow can return a too-small set of dimensions for PuppetWidget

Status: RESOLVED DUPLICATE of bug 1358712

(Core :: DOM: Content Processes, defect, P1)

Description

[Mike Conley (:mconley) (:⚙️)]

We have some web platform tests that open up a new tab, and then the test in that new tab sets up some HTML and then synchronously flushes layout and compares it against some static, hard-coded values.

It seems that there are circumstances where when the ContentChild hears about the new TabChild that's being opened for the new tab, it gets dimensions from the parent that are too small for the purposes of the test, because the parent process hadn't flushed and run layout for the new remote <xul:browser> yet. So content gets these too-small dimensions, and runs the test content, and the layout ends up being affected by these too-small window dimensions. The parent, meanwhile, eventually figures out how big the <xul:browser> actually is, and updates the TabChild, but it's too late at that point.

While this causes us to fail some web platform tests sometimes, it could also break some sites that depend on layout early in their life-cycles to behave properly.

See bug 1358712 comment 30 for more context.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Here's the code path that ultimately computes the "too small" rect from RecvCreateWindow

https://searchfox.org/mozilla-central/rev/a30a60eadcff99d2a043241955d215dbeccad876/dom/ipc/ContentParent.cpp#4924
https://searchfox.org/mozilla-central/rev/a30a60eadcff99d2a043241955d215dbeccad876/dom/ipc/TabParent.cpp#2660
https://searchfox.org/mozilla-central/rev/bd326a2a6b729dc62a5aee57354a97ceac4d1dc0/dom/base/nsFrameLoader.cpp#772
https://searchfox.org/mozilla-central/rev/bd326a2a6b729dc62a5aee57354a97ceac4d1dc0/layout/generic/nsSubDocumentFrame.h#133
https://searchfox.org/mozilla-central/rev/bd326a2a6b729dc62a5aee57354a97ceac4d1dc0/layout/generic/nsSubDocumentFrame.cpp#190
https://searchfox.org/mozilla-central/rev/bd326a2a6b729dc62a5aee57354a97ceac4d1dc0/dom/base/nsFrameLoader.cpp#789
https://searchfox.org/mozilla-central/rev/bd326a2a6b729dc62a5aee57354a97ceac4d1dc0/layout/generic/nsSubDocumentFrame.cpp#281-283

I see two possible solutions:

1. In nsSubDocumentFrame::GetSubdocumentSize, in the event that the frame hasn't been laid out yet, flush layout, and then get the dimensions. This would, I think, be strictly better than what we're currently doing, since the front-end StatusPanel code is doing the layout flush for us (for this case, and _every time we hover a link_). The other upside is that this, I think, would be super trivial to implement. The bad side would be that this introduces another place where we flush layout.

2. Don't return the ContentParent::RecvCreateWindow Promise until the next reflow occurs. Basically, instead of forcing the layout flush, we wait until the next layout occurs, and when it does, grab the subframe dimensions and resolve the Promise with them. This is quite a bit more complicated, but allows us to avoid flushing layout in nsSubDocumentFrame. A potential downside is a regression in the tpaint talos test, which measures how long until MozAfterPaint in a window.open'd window (which might regress due to us waiting on the parent to find the time to reflow).

I'm going to see what (1) gets us.

Comment 2

[Jim Mathies [:jimm]]

Hey Mike, setting this to P1 since you assigned yourself recently. Feel free to downgrade if you're not working on this.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Actually, to avoid the possibility of this landing separately from bug 1358712, I'm just going to dupe this over, and put my patch in bug 1358712. Sorry for the bug noise.