1171720 - Measure the cost of calling loadFrameScript on all browser group members multiple times

Status: RESOLVED WORKSFORME

(Firefox :: General, defect, P4)

Description

[Mike Conley (:mconley) (:⚙️)]

In browser.js, we load a number of frame scripts one after the other into any browsers that fit the browser messenger group.

As seen in bug 1169723, loading more framescripts at that time can result in sessionrestore regressions.

I wonder how much sessionrestore time we could save by combining all of those scripts into a single file and sending over in one shot, as opposed to calling loadFrameScript several times.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Instead of the hacky solution in bug 1169723, shouldn't we have a better mechanism in general to load frame scripts only needed for a specific URL? I feel that if we had something like:

mm.loadFrameScriptForURLPattern("frame-script.js", "^(view-source:|data:.*MathML)");

Then 1) SessionStore wouldn't have to know about any of those (or any other place loading these URLs) and 2) we don't need to track browsers everywhere, that should be done by the module or mechanism.

We could as well just implement that in JS where you would have a module that you pass frame scripts to load per URL pattern and that listens for STATE_START notifications on the content side and load the appropriate frame script when needed.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

(In reply to Tim Taubert [:ttaubert] from comment #1)
> Instead of the hacky solution in bug 1169723, shouldn't we have a better
> mechanism in general to load frame scripts only needed for a specific URL? I
> feel that if we had something like:
> 
> mm.loadFrameScriptForURLPattern("frame-script.js",
> "^(view-source:|data:.*MathML)");
> 
> Then 1) SessionStore wouldn't have to know about any of those (or any other
> place loading these URLs) and 2) we don't need to track browsers everywhere,
> that should be done by the module or mechanism.
> 
> We could as well just implement that in JS where you would have a module
> that you pass frame scripts to load per URL pattern and that listens for
> STATE_START notifications on the content side and load the appropriate frame
> script when needed.

This also sounds an awful lot like what RemotePageManager / RemotePages does (although it doesn't use regex just yet).

Comment 3

[Dave Townsend [:mossop]]

Sort of. Loading a frame script for a URL doesn't really make sense as frame scripts are loaded outside the scope of the window with the URL. It sounds like you're talking about greasemonkey/page-mod style scripts loaded into the scope of the window itself. RemotePageManager doesn't do that, it just provides communication between whatever scripts load in the window and the main process, it probably could be extended to load a script too fairly easily. Or we have page-mod built in and it isn't difficult to instantiate it from a non-SDK context.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Well it wouldn't really hurt (or be different than from what we do now) if we load a frame script for a specific URL once when the matching page is loaded in a specific frameLoader. Yeah, it would stay around until the end but it doesn't hurt more than currently. And all the places loading these URLs wouldn't have to know about the frame scripts and trigger loading those. Looks like with the current optimization just entering "view-source:http://example.org/" into the location bar might break?

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Tim Taubert [:ttaubert] from comment #4)
> Well it wouldn't really hurt (or be different than from what we do now) if
> we load a frame script for a specific URL once when the matching page is
> loaded in a specific frameLoader. Yeah, it would stay around until the end
> but it doesn't hurt more than currently. And all the places loading these
> URLs wouldn't have to know about the frame scripts and trigger loading
> those. 

Right, that's basically the mechanism I'd like to have:

Once a <browser> starts loading some URL matching some known pattern(s), inject the corresponding frame script.  Also, start the chrome listener for any of that frame script's events as needed.

The frame script will stay around for the life of the <browser>, but that's okay, since they can protect against processing events for the wrong URLs (as they already do today).

The main goal is to delay load the frame script until the first visit of a URL pattern.

> Looks like with the current optimization just entering "view-source:http://example.org/" into the location bar might break?

You are correct that it does not have the extra view source features (toggle syntax, go to line, etc.), but so far that has never been the case when entering "view-source:http://example.org/" into a new tab.  This was not caused by the Talos optimization in bug 1169723.

So, for the moment, the extra features are only there if:

1. The view source tab was created by view source menu options / shortcuts
2. It was restored from session store

It seems like an improvement to also have them there when manually entering the location, so all the more reason to pursue something here.

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Since Tim is away on PTO, maybe Mossop can help...

Assuming we want to add some module that adds a frame script when you navigation to URL that matches a known pattern, how should the module track tabs that already have the frame script loaded?

For the current view source code, I used a WeakSet tracking <browser> elements[1] to do this.  However, in bug 1169723 comment 22, Tim said:

"This will break with docShell swapping. If drag&drop a view-source: tab to another window it will load the frame-script there again."

So, what's a good way to know "does this child browser have frame script X loaded already"?

[1]: https://dxr.allizom.org/mozilla-central/source/toolkit/components/viewsource/ViewSourceBrowser.jsm#24

Comment 7

[Dave Townsend [:mossop]]

You can use browser.permanentKey but that doesn't change if the browser switches from remote to non-remote when you do have to load the script again.

Does browser.messageManager remain the same when swapping frameloaders? I recall some oddness with what goes on there but I'd kind of expect it to be the same instance when in the new window.

Or outerWindowID should remain the same for the content window I think.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

We're getting rid of message managers, so closing this old bug.