Closed Bug 1685268 Opened 2 years ago Closed 2 years ago

Enable STYLESHEET resource type by default

Categories

(DevTools :: Style Editor, task, P3)

task

Tracking

(Fission Milestone:M7a, firefox89 fixed)

RESOLVED FIXED
89 Branch
Fission Milestone M7a
Tracking Status
firefox89 --- fixed

People

(Reporter: Honza, Assigned: nchevobbe)

References

Details

(Whiteboard: dt-fission-m3-mvp)

Attachments

(5 files)

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

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

Fission Milestone: --- → M8

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

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

Fission Milestone: M8 → M7

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.)

Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED

(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

Tracking for Fission M8 milestone (Release experiment).

Fission Milestone: M7 → M8

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.

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

Blocks: 1700092
Attachment #9204768 - Attachment description: Bug 1685268 - [devtools] Use the StyleSheetManager in style-related actors. r=ochameau. → Bug 1685268 - [devtools] Use the StyleSheetManager in style-related actors. r=ochameau,jdescottes.

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

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

Attachment #9210955 - Attachment description: Bug 1685268 - [devtools] Clear StyleEditor UI on top-level target available. r=jdescottes. → Bug 1685268 - [devtools] Don't clear StyleEditor UI on redundant dom-loading event. r=jdescottes,ochameau.
Pushed by nchevobbe@mozilla.com:
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

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

Flags: needinfo?(nchevobbe)
Pushed by nchevobbe@mozilla.com:
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
Fission Milestone: M8 → M7a
Blocks: 1701862
Regressions: 1701785
You need to log in before you can comment on or make changes to this bug.