1614465 - De-couple ASRouter initialization from Activity Stream

Status: RESOLVED FIXED

(Firefox :: Messaging System, task, P3)

Description

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

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

[Tim Spurway [:tspurway]]

hey mike, were you planning on working on this? we are doing some refactoring with separating about:welcome from newtab that might overlap

Comment 2

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

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.

Comment 3

[Ed Lee :Mardak]

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:

1. browser.js _delayedStartup
   https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/browser/base/content/browser.js#1926-1929
2. AboutNewTabService.jsm constructor
   https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/browser/components/newtab/AboutNewTabService.jsm#56
3. AboutNewTab.jsm onBrowserReady
   https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/browser/modules/AboutNewTab.jsm#70
4. ActivityStream.jsm init
   https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/browser/components/newtab/lib/ActivityStream.jsm#690-700
5. ASRouterFeed.jsm enable
   https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/browser/components/newtab/lib/ASRouterFeed.jsm#21-26
6. 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).

Comment 4

[Bernard Igiri]

Attachment: Bug 1614465 De-couple ASRouter initialization from Activity Stream r=MConley

Work in Progress. No tests have been altered or written yet.

Comment 5

[Bernard Igiri]

Attachment: Bug 1614465 - Replacing ASRouter calls to MessageChannel with JSWindowActors to eliminate ASRouterFeed r=mconley!,k88hudson!

Depends on D64388

Comment 6

[Bernard Igiri]

Attachment: Bug 1614465 Adding new files r?mconley

Depends on D71796

Comment 7

[Bernard Igiri]

Attachment: Bug 1614465 Adding new files r?mconley

Depends on D71797

Comment 8

[Bernard Igiri]

Attachment: Bug 1614465 Adding files r?mconley

Depends on D71798

Comment 9

[Bernard Igiri]

Attachment: Bug 1614465 Adding new files r?mconley

Depends on D71799

Comment 10

[Ron Manning]

Hey Bernard, we have this slated for Fx77 Release, will it be completed in time to merge to Beta on Friday?

Comment 11

[Bernard Igiri]

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

Comment 12

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

ni?ing andreio for https://phabricator.services.mozilla.com/D71796#3041259

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/01c4f36a13b8
Replacing ASRouter calls to MessageChannel with JSWindowActors to eliminate ASRouterFeed r=k88hudson

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0a719a2effaa
Revert mistakenly landed changes to SystemStatusBar.cpp

Comment 15

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/01c4f36a13b8
https://hg.mozilla.org/mozilla-central/rev/0a719a2effaa

Comment 16

[Ed Lee :Mardak]

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?