Closed Bug 1478688 Opened 2 years ago Closed 2 months ago

See if we can stop fetching netmonitor response content from the stylesheet actor

Categories

(DevTools :: Netmonitor, enhancement, P3)

enhancement

Tracking

(Fission Milestone:M6c, firefox80 fixed, firefox81 fixed)

RESOLVED FIXED
81 Branch
Fission Milestone M6c
Tracking Status
firefox80 --- fixed
firefox81 --- fixed

People

(Reporter: ochameau, Assigned: daisuke)

References

(Blocks 1 open bug)

Details

(Whiteboard: dt-fission-m2-mvp)

Attachments

(5 files)

The stylesheet actor currently tries to fetch the response content saved in the network monitor actor.
The issue is that it breaks fission rule, where the content process should avoid fetching data from other processes.
Bug 1444132 is about to move the network monitor actor to the parent process, making that process even more complex, by involving the message manager...

It would be great to assess if we really have to fetch network monitor response content, or if we could pull the response from cache by some other platform API.
I think the main challenge here is that users my disable the cache via the checkbox in network monitor, and we wanted to avoid double loads from DevTools in that case.  So somehow, we need to reuse what DevTools has already seen.
Whiteboard: dt-fission
Priority: P2 → P3
Whiteboard: dt-fission → dt-fission-reserve

Tracking Fission DevTools bugs for Fission Nightly (M6)

Fission Milestone: --- → M6

dt-fission-reserve bugs do not need to block Fission Nightly (M6).

Fission Milestone: M6 → ---

Daisuke migrated to StyleSheet editor to the ResouceWatcher API on the client, making it Fission compatible.
But we are now hitting this bug, which prevents displaying stylesheets correctly when the page is already loaded:
https://phabricator.services.mozilla.com/D78240#2505525

This issue may actually prevent us from landing server side network request listening, i.e. bug 1644191.
So bumping this into the MVP list.

Blocks: 1644191
Whiteboard: dt-fission-reserve → dt-fission-m2-mvp

Tracking dt-fission-m2-mvp bugs for Fission Nightly milestone (M6c).

Fission Milestone: --- → M6c

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

This issue may actually prevent us from landing server side network request listening, i.e. bug 1625930.

Alex, are you pointing to the right bug here?

Honza

Flags: needinfo?(poirot.alex)

I meant bug 1644191. I updated my older comment.

Flags: needinfo?(poirot.alex)

Hello, Alex!
If the reason why we needed to see consoleActor.getRequestContentForURL(href) was to use the cache in the console actor, as DevTools has become to fetch all contents from our cache in the platform (to fix other issues), we may not need to see the console actor anymore?

Flags: needinfo?(poirot.alex)

This has been introduced in bug 1306892.
It is about preventing re-fetching the stylesheets.
Now, I don't know if the cache is populated as much as the netmonitor?
If the cache is complete as netmonitor inspection, then yes, it sounds like a good fix!

I have some doubt when I read Patrick's comment (bug 1306892 comment 11):

What I do remember is that if a stylesheet is set not to be cached, then the back-end has no other choices then to actually download it again from the server.
Could it be that some HTTP header will prevent the stylesheet from being cached, and we will redo the request here?

Flags: needinfo?(poirot.alex)

Thank you very much for your feedback, Alex!
Okay, let me check again whether the cache covers with all of the contents that the document loaded or not.

I have investigated with ignoring the result of consoleActor.getRequestContentForURL that where the stylesheet that DevTools got is taken from.

As a result, we found that most of stylesheets are taken from the cache in the platform expectedly.
e.g.
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=308778961&repo=try&lineNumber=8538

One thing we could not take from the cache was imported CSS.
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=308778961&repo=try&lineNumber=8691
But, as it seems we can get from the cache when browsing the page normally, it might be caused by the timing?

On the other hand, tests that tried to use consoleActor are browser_styleeditor_navigate.js and browser_styleeditor_fetch-from-netmonitor.js.
For the browser_styleeditor_navigate.js as well, we can confirm that the stylesheet can be taken from the cache in the platform.
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=308778961&repo=try&lineNumber=8994
For the browser_styleeditor_fetch-from-netmonitor.js, as this test checks whether we can get the proper content from consoleActor, it will be failed here, but can ignore.
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=308778961&repo=try&lineNumber=8617-8622

As a result, there will be no problem if we get rid of referring to consoleActor I think, but what do you think?
And, if you have any better ways that we can check whether the stylesheets are taken from the cache or not, please let me know!

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

I have some doubt when I read Patrick's comment (bug 1306892 comment 11):

What I do remember is that if a stylesheet is set not to be cached, then the back-end has no other choices then to actually download it again from the server.
Could it be that some HTTP header will prevent the stylesheet from being cached, and we will redo the request here?

I think this was fixed.
For example, even if set disable cache checkbox in the network monitor, DevTools can get from the cache (BF cache?) in the platform in order to get exactly the same content.

Flags: needinfo?(poirot.alex)

I heard from Honza on slack, as Alex and Honza agreed with using the browser cache, I will try to fix it with that way.
Thanks!

Assignee: nobody → daisuke
Status: NEW → ASSIGNED
Pushed by dakatsuka.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e16c7d81d82a
Avoid getting cached content from console actor. r=Honza
https://hg.mozilla.org/integration/autoland/rev/21078a5722b9
Remove codes that does not use anymore. r=Honza
https://hg.mozilla.org/integration/autoland/rev/e81d80110147
Remove test codes that does not use anymore. r=Honza
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 80
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Pushed by dakatsuka.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/153dc423b227
Remove no longer used code from network monitor actor. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/11e97601cbca
Revert browser_styleeditor_fetch-from-netmonitor test. r=ochameau
Regressions: 1656417
Status: REOPENED → RESOLVED
Closed: 3 months ago2 months ago
Resolution: --- → FIXED
Target Milestone: Firefox 80 → 81 Branch
Flags: needinfo?(poirot.alex)
You need to log in before you can comment on or make changes to this bug.