2068549 - Content in a new remote browser can load and fire load with a 0x0 viewport

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Florian Quèze [:florian]]

A newly created remote <browser> has no viewport size until the parent process reflows it and sends PBrowser::Msg_UpdateDimensions. That reflow only happens off a refresh driver tick, and nothing forces one, so when no tick lands while the tab is opening the content process runs style and layout, fires DOMContentLoaded, load and pageshow, and reaches readyState complete while window.innerWidth is 0.

This is web observable: media queries evaluate against an empty viewport, getBoundingClientRect() returns zeros, and hit testing finds nothing because the root element's border box covers no point, so elementFromPoint() returns null everywhere. It also means layout-triggered subresource loads (fonts, images, image prioritisation by position) happen against a viewport that is not the real one.

Profiles of uriloader/exthandler/tests/mochitest/browser_pdf_save_as.js show the failing ordering as load and pageshow at 7.876s, then Msg_UpdateDimensions at 7.899s. In the passing cases the dimensions arrive first, 6-10ms after PBrowser::Msg_Show and before load.

In bug 1970802 comment 71 and comment 77, Gijs suggested best-guessing the initial viewport size when creating the docshell, so that there is never a 0x0 state. nsSubDocumentFrame::GetInitialSubdocumentSize() already has a fallback for this, but it returns 10x10 unless there is a detached presentation to copy from, and nsFrameLoader::ShowRemoteFrame() deliberately does not send a size before the first reflow (bug 820571).

Bug 1970802 addressed the test harness consequence, by making BrowserTestUtils.openNewForegroundTab wait for a tick before resolving. This bug is for the underlying behaviour.

Comment 1

[:Gijs (he/him)]

Gonna transplant my needinfo from 1970802 to Emilio here.

Can we add optional args to whatever IPC is creating the content viewport/docshell and, for tabbed browsers, prefill them with what we expect, and/or is there a better/alternative way to avoid this race and improve the status quo here?

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Well, yeah, we could, in order of my gut feeling easier-to-harder stuff:

• Fallback to a better size (WIP incoming, I don't know how that'll fare on try, we'll see).
• Give the <browser> element a way of providing a fallback size in the DOM (what about width / height attributes? Something else).
• Try to make sure that layout is clean before showing remote frames. This changes scheduling enough that makes me a bit scared. Though in practice it should be more consistent. Right now if you append a <browser> element there's nothing flushing layout by default until the refresh driver tick. But if you flush style via getComputedStyle you get into this weird situation where you have frames and trigger the Show(), but don't have layout and thus don't send a size.

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 2068549 - Use a more reasonable default size for remote browsers. r=#layout

The fallback intrinsic size seems way better than 0, and reasonable
enough?

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d2f7c679438a
https://hg.mozilla.org/integration/autoland/rev/140fb57c0552
Use a more reasonable default size for remote browsers. r=layout-reviewers,dshin

Comment 5

[Silaghi Andreea]

https://hg.mozilla.org/mozilla-central/rev/140fb57c0552

Comment 6

[Timothy Nikkel (:tnikkel)]

I think the intermittent failure I debugged at https://bugzilla.mozilla.org/show_bug.cgi?id=1924676#c58 had the same root cause.