De-couple ASRouter initialization from Activity Stream
Categories
(Firefox :: Messaging System, task, P3)
Tracking
()
People
(Reporter: mconley, Assigned: bigiri)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 5 obsolete files)
ASRouter grew out of work from the Activity Stream team, but at this point seems to be a pretty distinct component.
Bug 1614351 and its dependencies hopes to eventually rearchitect how about:home starts up a bit. One of the first things we'd like to do is make it so that ASRouter doesn't depend on Activity Stream for initialization.
We'll still need it so that ASRouter can send snippets to about:home, but there should be a well-defined API for this... I'm not sure having ASRouter send messages down directly through the message channel is the right move. Perhaps ASRouter can talk to AboutNewTabService or something, and then AboutNewTabService does the job of sending the update down to all of the about:home pages.
Comment 1•5 years ago
|
||
hey mike, were you planning on working on this? we are doing some refactoring with separating about:welcome from newtab that might overlap
Reporter | ||
Comment 2•5 years ago
|
||
I'm hoping bigiri can work on it soon - we're meeting today about it, and then he'll likely reach out to your team to coordinate.
Reporter | ||
Updated•5 years ago
|
Updated•5 years ago
|
Comment 3•5 years ago
|
||
From our chat, it seems like the first step is removing the blocking initialization dependency of ASRouter from new tab page's startup path. So we need to find an appropriate place to initialize and communicate with ActivityStream/new tab modules.
For reference, this is how ASRouter currently initializes at startup:
- browser.js _delayedStartup
https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/browser/base/content/browser.js#1926-1929 - AboutNewTabService.jsm constructor
https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/browser/components/newtab/AboutNewTabService.jsm#56 - AboutNewTab.jsm onBrowserReady
https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/browser/modules/AboutNewTab.jsm#70 - ActivityStream.jsm init
https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/browser/components/newtab/lib/ActivityStream.jsm#690-700 - ASRouterFeed.jsm enable
https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/browser/components/newtab/lib/ASRouterFeed.jsm#21-26 - ASRouter init
https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/browser/components/newtab/lib/ASRouter.jsm#893-902
The last step 6 actually initializing ASRouter takes parameters for the channel (to communicate with content pages), indexeddb storage (to persist data), and dispatch function (to send actions to activity stream).
Ideally somehow we just straight to step 6 to not rely on any of the other new tab startup behavior. It seems likely the entry point will be similar to existing step 1 as that's where many other things get initialized, but potentially we can delay even more.
I believe this specific bug should focus on breaking the dependency and followup bugs will move various pieces out of the main process. And hopefully with the first step done, new tab page can start sooner without explicitly blocking on ASRouter related code (although might be indirectly delayed when competing for cpu/disk resources).
Updated•5 years ago
|
Assignee | ||
Comment 4•5 years ago
|
||
Work in Progress. No tests have been altered or written yet.
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 5•5 years ago
|
||
Depends on D64388
Assignee | ||
Comment 6•5 years ago
|
||
Depends on D71796
Assignee | ||
Comment 7•5 years ago
|
||
Depends on D71797
Assignee | ||
Comment 8•5 years ago
|
||
Depends on D71798
Assignee | ||
Comment 9•5 years ago
|
||
Depends on D71799
Comment 10•5 years ago
|
||
Hey Bernard, we have this slated for Fx77 Release, will it be completed in time to merge to Beta on Friday?
Assignee | ||
Comment 11•5 years ago
|
||
(In reply to Ron Manning from comment #10)
Hey Bernard, we have this slated for Fx77 Release, will it be completed in time to merge to Beta on Friday?
No, I don't expect it will.
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Reporter | ||
Comment 12•4 years ago
|
||
ni?ing andreio for https://phabricator.services.mozilla.com/D71796#3041259
Updated•4 years ago
|
Comment 13•4 years ago
|
||
Updated•4 years ago
|
Comment 14•4 years ago
|
||
Comment 15•4 years ago
|
||
bugherder |
Updated•4 years ago
|
Updated•4 years ago
|
Comment 16•4 years ago
|
||
Looks like there was a package.json change that didn't update package-lock.json. Bug 1674433 seems to have updated it although curious why tests didn't fail. I suppose we check for bundle differences not package-lock.json differences?
Description
•