1968903 - Avoid creating script and CSS loaders for documents loaded as data

Status: REOPENED

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

Description

[Henri Sivonen (:hsivonen)]

It seems that loadedAsData=true docs never become loadedAsData=false. Therefore, it seems feasible to pursue not creating script and CSS loaders for them in order to speed up creation from DOMParser.

Comment 1

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=151508

Comment 2

[Henri Sivonen (:hsivonen)]

It turns out that print preview wants a document that counts as loaded as data but can be styled and rendered.

Comment 3

[Henri Sivonen (:hsivonen)]

Emilio, is print preview a special case where the browser has special powers and can be addressed as a special case, or is it possible to start styling and displaying a loaded-as-data document using Web-exposed APIs?

Comment 4

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=151539

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Static clones for both print preview and printing need styling, yeah. I think SVG-as-image also counts as loaded-as-data? But with those special cases out of the way I think you're good to go.

Comment 6

[Henri Sivonen (:hsivonen)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)

Static clones for both print preview and printing need styling, yeah. I think SVG-as-image also counts as loaded-as-data? But with those special cases out of the way I think you're good to go.

Thanks. Some SVG cases that one might expect to be loaded-as-data already aren't. Let's see what happens with allowing styling from clone but not otherwise:

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=151846

Comment 7

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/jobs?repo=try&revision=13ee2499967b1d96ad12fca775af682e4bf23585

Comment 8

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=152387

Comment 9

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1968903 - Avoid creating script and CSS loaders for documents loaded as data. r?emilio!

Comment 10

[Henri Sivonen (:hsivonen)]

https://perf.compare/compare-lando-results?baseLando=152962&newLando=152968&baseRepo=try&newRepo=try&framework=13

Base revision's try run: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=152962
Local revision's try run: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=152968

Comment 11

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=152972

Comment 12

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=153676

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f83cb2878925
https://hg.mozilla.org/integration/autoland/rev/58ff60616c74
Avoid creating script and CSS loaders for documents loaded as data. r=firefox-style-system-reviewers,emilio,layout-reviewers

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/0a11d7413f2e
https://hg.mozilla.org/integration/autoland/rev/958717d69d7a
Revert "Bug 1968903 - Avoid creating script and CSS loaders for documents loaded as data. r=firefox-style-system-reviewers,emilio,layout-reviewers" for causing multiple wpt failures.

Comment 15

[agoloman]

Backed out for causing multiple wpt failures.

• Backout link
• Push with failures
• Failure Log @table-fixed-minmax.html
• Failure Log @caretPositionFromPoint.html
• Failure Log @elementFromPoint-001.html

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/6cbdfe15a061
https://hg.mozilla.org/integration/autoland/rev/c5b047ab1e56
Avoid creating script and CSS loaders for documents loaded as data. r=firefox-style-system-reviewers,emilio,layout-reviewers

Comment 17

[agoloman]

Relanded because it was not the actual culprit.

Comment 18

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/c5b047ab1e56

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f21b53b7045f
https://hg.mozilla.org/mozilla-central/rev/75a595a9d65d
Revert "Bug 1968903 - Avoid creating script and CSS loaders for documents loaded as data. r=firefox-style-system-reviewers,emilio,layout-reviewers" for causing bug 1991800

Comment 20

[Cristina Horotan [:chorotan]]

Backed out from central https://hg.mozilla.org/mozilla-central/rev/75a595a9d65d529c86a28bc528c86caf24891d85

Comment 21

[Henri Sivonen (:hsivonen)]

Let's use bug 1991800 for investigation.

Comment 22

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

There are 6 crash reports from 2 installs of the latest Nightly with [@ mozilla::dom::ScriptLoader::AddParserBlockingScriptExecutionBlocker ] as signature, e.g. bp-4db92bb6-dfaa-4613-9323-fe6960251001. If these were a result of bug 1991800, there should be more reports.

Comment 23

[Henri Sivonen (:hsivonen)]

(In reply to Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout) from comment #22)

There are 6 crash reports from 2 installs of the latest Nightly with [@ mozilla::dom::ScriptLoader::AddParserBlockingScriptExecutionBlocker ] as signature, e.g. bp-4db92bb6-dfaa-4613-9323-fe6960251001. If these were a result of bug 1991800, there should be more reports.

Thanks! This is a null check oversight.

Comment 24

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=157592

Comment 25

[Henri Sivonen (:hsivonen)]

layout/base/tests/chrome/printpreview_pps9.html fails with the current patch even though other print preview cases pass. Emilio, do you see some obvious reason that could make that one fail when the others don't fail?

Comment 26

[Henri Sivonen (:hsivonen)]

(The current state of the patch accomplishes the goal on Speedometer3 of not creating the CSS loader for loaded-as-data docs during the Speedometer3 run.)

Comment 27

[Emilio Cobos Álvarez [:emilio]]

Not sure but it seems the patch is pretty orange, so that might have to do with it.

Comment 28

[Emilio Cobos Álvarez [:emilio]]

Maybe that's not using quirks mode and the others do? But just guessing really.

Comment 29

[Henri Sivonen (:hsivonen)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #27)

Not sure but it seems the patch is pretty orange, so that might have to do with it.

Whoa. The patch went really bad in my latest tweak that was supposed to be an easy fix.

Comment 30

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=158062

Comment 31

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #30)

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=158062

This run is considerably less orange. With this code, layout/base/tests/chrome/printpreview_pps9.html still fails for me locally. Furthermore, even if I try to cause more eager creation of the CSS loader from DocumentOrShadowRoot::CloneAdoptedSheetsFrom, layout/base/tests/chrome/printpreview_pps9.html still fails locally here.

Comment 32

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #31)

(In reply to Henri Sivonen (:hsivonen) from comment #30)

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=158062

This run is considerably less orange. With this code, layout/base/tests/chrome/printpreview_pps9.html still fails for me locally. Furthermore, even if I try to cause more eager creation of the CSS loader from DocumentOrShadowRoot::CloneAdoptedSheetsFrom, layout/base/tests/chrome/printpreview_pps9.html still fails locally here.

The null-returning Document::GetExistingCSSLoader look in Pernosco like they are not the cause.

Comment 33

[Henri Sivonen (:hsivonen)]

Still fails for me locally even if I cause CSS loader creation in Document::CloneDocHelper right after creating the cloned doc and before the compat mode is set.

Comment 34

[Henri Sivonen (:hsivonen)]

It seems that the test now passes on try. OTOH, a test that fails on try passes locally. Probably both are unreliable somehow.

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b1685935b1c2
https://hg.mozilla.org/integration/autoland/rev/d6edfd154d6b
Avoid creating script and CSS loaders for documents loaded as data. r=firefox-style-system-reviewers,emilio,layout-reviewers

Comment 36

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/55550 for changes under testing/web-platform/tests