1685268 - Enable STYLESHEET resource type by default

Status: RESOLVED FIXED

(DevTools :: Style Editor, task, P3)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thew new STYLESHEET resource and related Fission logic/code path should be enabled by default (we should stop using the pref entirely)

See server Watcher:

https://searchfox.org/mozilla-central/rev/a0ccd492719b1ad2106f6456549be62a76f45acb/devtools/server/actors/watcher.js#153

Honza

Comment 1

[Chris Peterson [:cpeterson]]

Tracking dt-fission-m3-mvp bugs for Fission M8 (blocking Release channel experiment, but not Beta experiment).

Comment 2

[Alexandre Poirot [:ochameau]]

Most of the STYLESHEET resource landed in bug 1644193 and last work was on bug 1664941.
Bug 1664941's dependencies already highlights followups and at least one breakage, bug 1673829.

A try run to see if tests are still failing when we turn this on:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7ab75317bc633f837164700fe6fe2f8257824b87

Comment 3

[Chris Peterson [:cpeterson]]

Moving dt-fission-m3-mvp bugs from Fission M8 to M7 (blocking Beta experiment).

Comment 4

[Alexandre Poirot [:ochameau]]

I'm note sure this code works:
https://searchfox.org/mozilla-central/source/devtools/client/fronts/style-rule.js#113-119
As I don't think any front will have resourceWatcher attribute set.
(This is actually a framework limitation.)

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Alexandre Poirot [:ochameau] from comment #4)

I'm note sure this code works:
https://searchfox.org/mozilla-central/source/devtools/client/fronts/style-rule.js#113-119
As I don't think any front will have resourceWatcher attribute set.
(This is actually a framework limitation.)

we do set the resourceWatcher here: https://searchfox.org/mozilla-central/rev/3ff133d19f87da2ba01ade55d6e7fdf0fc28b08c/devtools/client/inspector/inspector.js#277

Comment 6

[Chris Peterson [:cpeterson]]

Tracking for Fission M8 milestone (Release experiment).

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1685268 - [devtools] Move logic outside of StyleSheetWatcher to a new StyleSheetManager class. r=ochameau,jdescottes.

At the moment, when enabling the stylesheet resource watcher on the server there were a few issues.
The main one being a race when doing a target switch, when the client didn't yet
started watching for the needed resource. This caused an issue in the inspector
codepath where we tried to retrieve rules, as the pageStyle and ruleStyle actors
where both using the StylesheetWatcher to do some operation.

In order to not depend on the watcher status, we move most of the logic of the
StylesheetWatcher that wasn't purely watcher related to a StyleSheetsManager, that
listen for new stylesheet being created or modified, and emit event when it's the case,
so the StylesheetWatcher can listen to them.

Some methods are cleaned up, renamed, we add jsdoc where it's missing, and we
also simplify the main map where we store the stylesheet to not have them in
an object (since this was the only property in it).

StyleSheetManagers are kept in a Map keyed by target actor, and a function is exported
so we can clean the Map from the outside.

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1685268 - [devtools] Use the StyleSheetManager in style-related actors. r=ochameau,jdescottes.

Depends on D106078

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1685268 - [devtools] Enable Stylesheet resource server support for content toolbox. r=ochameau

Enabling the support for stylesheet resources highlighted some issue with the
current situation.
A few tests were failing and had to be adapted to pass.

Depends on D106079

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1685268 - [devtools] Watch for stylesheet resources from inspector front instead of inspector panel. r=ochameau.

This was previously done in the inspector panel initialization phase, after the
inspector front was created, which could lead to races.
Since the pageStyle actor requires stylesheets to being watched (when watcher
support is enabled), we do this in the inspector front, before creating the
page style actor.
We put the resourceWatcher instance on targetFront from resourceWatcher#_onTargetAvailable
so we can use it from everywhere.

Depends on D106079

Comment 11

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1685268 - [devtools] Don't clear StyleEditor UI on redundant dom-loading event. r=jdescottes,ochameau.

With the previous patches in this queue, it looks like the
timing is stlightly modified, and on initial load we can
reveive stylesheet resources before the intial dom-loading
event.
Since we're using dom-loading to clear the UI, we might destroy
stylesheets that we shouldn't.
In order to fix that, we clear the UI in onTargetAvailable, which
we do receive before any resources, and set a flag that we
check in dom-loading to avoid clearing the UI if it was already done.
We take this opportunity to clean a few things in the style editor:

• call watchResources once instead of twice.
• in onResourceAvailable, await for all the promises once, outside
  the loop so we don't have delays while looping through the resources.

Depends on D109470

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a127a2cb052
[devtools] Move logic outside of StyleSheetWatcher to a new StyleSheetManager class. r=ochameau,jdescottes.
https://hg.mozilla.org/integration/autoland/rev/01de742151af
[devtools] Use the StyleSheetManager in style-related actors. r=ochameau.
https://hg.mozilla.org/integration/autoland/rev/830e139a73a0
[devtools] Watch for stylesheet resources from inspector front instead of inspector panel. r=ochameau.
https://hg.mozilla.org/integration/autoland/rev/0068e7f023ce
[devtools] Don't clear StyleEditor UI on redundant dom-loading event. r=jdescottes,ochameau.
https://hg.mozilla.org/integration/autoland/rev/331d6b2d6f1b
[devtools] Enable Stylesheet resource server support for content toolbox. r=ochameau

Comment 13

[Atila Butkovits]

Backed out for causing failures on test_styles-applied.html.

Backout link: https://hg.mozilla.org/integration/autoland/rev/84ed889aec1619183b601e0c6138314d9e45f0e6

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=LGYCb7mMQmKA4SC_mF64CA.0&searchStr=linux%2C18.04%2Cx64%2Casan%2Copt%2Cmochitests%2Cwithout%2Ce10s%2Ctest-linux1804-64-asan%2Fopt-mochitest-chrome-1proc%2Cc3&revision=331d6b2d6f1b0ccde2c8c8ef1d002eebde01d355

Failure log: https://treeherder.mozilla.org/logviewer?job_id=334523745&repo=autoland&lineNumber=106673

Comment 14

[Nicolas Chevobbe [:nchevobbe]]

I'm looking, looks like something landed since last time I pushed to try

Comment 15

[Nicolas Chevobbe [:nchevobbe]]

new TRY push https://treeherder.mozilla.org/jobs?repo=try&revision=e72e46e13c57a38f97290b2df9d3f14d54733350

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb5287e6852c
[devtools] Move logic outside of StyleSheetWatcher to a new StyleSheetManager class. r=ochameau,jdescottes.
https://hg.mozilla.org/integration/autoland/rev/ff26aac6b8a6
[devtools] Use the StyleSheetManager in style-related actors. r=ochameau.
https://hg.mozilla.org/integration/autoland/rev/8c0ee03d5bb6
[devtools] Watch for stylesheet resources from inspector front instead of inspector panel. r=ochameau.
https://hg.mozilla.org/integration/autoland/rev/1020d2d180ce
[devtools] Don't clear StyleEditor UI on redundant dom-loading event. r=jdescottes,ochameau.
https://hg.mozilla.org/integration/autoland/rev/441a2c8df39b
[devtools] Enable Stylesheet resource server support for content toolbox. r=ochameau

Comment 17

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/cb5287e6852c
https://hg.mozilla.org/mozilla-central/rev/ff26aac6b8a6
https://hg.mozilla.org/mozilla-central/rev/8c0ee03d5bb6
https://hg.mozilla.org/mozilla-central/rev/1020d2d180ce
https://hg.mozilla.org/mozilla-central/rev/441a2c8df39b