Pinned tabs populating tab bar create visual noise on startup

REOPENED
Assigned to

Status

()

defect
P2
normal
REOPENED
a year ago
a month ago

People

(Reporter: dthayer, Assigned: dthayer, NeedInfo)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fxperf:p2])

Attachments

(5 attachments)

(Assignee)

Description

a year ago
STR:

- Have pinned tabs
- Start the browser
- Notice the pinned tabs come in after first paint, creating visual noise

I understand we want to get the first paint out as soon as possible, and knowing what pinned tabs to load before first paint might require more work than we'd like, but we should at least be able to store the number of pinned tabs somewhere quickly accessible and simply display a placeholder loading icon.
Component: Tabbed Browser → Session Restore
(Assignee)

Comment 1

a year ago
Looking into this to see how difficult it would be.
Assignee: nobody → dothayer
Status: NEW → ASSIGNED
Whiteboard: [fxperf:p1]
Comment hidden (mozreview-request)

Comment 3

a year ago
mozreview-review
Comment on attachment 8960382 [details]
Bug 1442694 - Preopen pinned tabs before session restore

https://reviewboard.mozilla.org/r/229166/#review235026

This doesn't seem to help in my testing, likely because gBrowser is initialized after the first pait.
Attachment #8960382 - Flags: review?(dao+bmo) → review-
(Assignee)

Comment 4

a year ago
That's strange - on my machine the dummy pinned tabs are present in the first paint with this change, so I'm not actually sure what's going on: are these changes just racing with the actual paint?
(Assignee)

Comment 5

a year ago
Posted video withchange.mp4
Attaching a video of startup with the change applied for me. There's one frame of what appears to be random GPU memory, then a blank paint, then the window with the pinned tabs visible.
(Assignee)

Comment 6

a year ago
Oh I see - this patch was bitrotted by your changes in bug 1443849. Looking into whether this approach is salvageable given those changes.
Comment hidden (mozreview-request)
Duplicate of this bug: 1314364

Comment 9

a year ago
mozreview-review
Comment on attachment 8960382 [details]
Bug 1442694 - Preopen pinned tabs before session restore

https://reviewboard.mozilla.org/r/229166/#review235546

::: toolkit/content/widgets/tabbox.xml:265
(Diff revision 2)
>          else
>            this.selectedIndex = 0;
>        ]]>
>        </constructor>
>  
> +      <method name="_preopenPinnedTabs">

This doesn't belong in tabbox.xml at all, and generally I don't think we want code to manually add tabs without going through gBrowser.addTab. What I think you really want is initialize gBrowser before the first paint.
Attachment #8960382 - Flags: review?(dao+bmo) → review-
(Assignee)

Comment 10

a year ago
mozreview-review-reply
Comment on attachment 8960382 [details]
Bug 1442694 - Preopen pinned tabs before session restore

https://reviewboard.mozilla.org/r/229166/#review235546

> This doesn't belong in tabbox.xml at all, and generally I don't think we want code to manually add tabs without going through gBrowser.addTab. What I think you really want is initialize gBrowser before the first paint.

All right. I was assuming its initialization had been places after first paint for a concrete performance win, and didn't want to mess with that. To avoid wasting your time with another review, do you have any suggestions for where to initialize it? Maybe gBrowserInit.onBeforeInitialXULLayout()?
(In reply to Doug Thayer [:dthayer] from comment #10)
> Comment on attachment 8960382 [details]
> Bug 1442694 - Preopen pinned tabs before session restore
> 
> https://reviewboard.mozilla.org/r/229166/#review235546
> 
> > This doesn't belong in tabbox.xml at all, and generally I don't think we want code to manually add tabs without going through gBrowser.addTab. What I think you really want is initialize gBrowser before the first paint.
> 
> All right. I was assuming its initialization had been places after first
> paint for a concrete performance win, and didn't want to mess with that.

As a rule of thumb I think we want to initialize things both as early as needed (so they're ready when we want that) and as late as possible (so they don't delay other things that are needed before).

> To
> avoid wasting your time with another review, do you have any suggestions for
> where to initialize it? Maybe gBrowserInit.onBeforeInitialXULLayout()?

Yeah.
(Assignee)

Comment 12

a year ago
I'm a little bit out of my depth here. When I move the initial open of the pinned tabs to onBeforeInitialXULLayout, the tab's XBL properties are not initialized after this.addTab(...). I can hack around this by setting something like tab._initializer = () => {...} on the tab and checking it on the tabbrowser-tab constructor, but that hardly seems elegant, and I'd like to understand why this is manifesting first.
So using XBL in onBeforeInitialXULLayout won't work because XBL being ready depends on the initial layout. Instead gBrowserInit.onDOMContentLoaded is probably the right place to do this.
(Assignee)

Comment 14

a year ago
(In reply to Dão Gottwald [::dao] from comment #13)
> So using XBL in onBeforeInitialXULLayout won't work because XBL being ready
> depends on the initial layout. Instead gBrowserInit.onDOMContentLoaded is
> probably the right place to do this.

That was where this was occurring when you initially rejected it because it wasn't fixing the issue. That being said, it seems to be working in onDOMContentLoaded now. Is there any reason why this was happening after first paint before but after first paint now?
I'm not sure if there are meaningful guarantees about DOMContentLoaded and the first paint. I would normally expect DOMContentLoaded to fire first, but it seems that there are ways to trigger a paint before that. Maybe this doesn't happen anymore, which would be good, but I definitely saw it when I tried your first patch. You could also try requestAnimationFrame in onBeforeInitialXULLayout -- that should occur before the first paint, I think, but then you'd need to make sure it doesn't race with gBrowser.init in onDOMContentLoaded.
Usually onDOMContentLoaded is the last point at which we can do things before first paint in the browser window.
Comment hidden (mozreview-request)
How are you ensuring that browser.tabs.pinnedTabCount isn't the number of pinned tabs of some random window, but of the window that sessionstore is going to restore as the first one? This isn't really obvious to me from looking at the patch.
Flags: needinfo?(dothayer)
(Assignee)

Comment 19

a year ago
(In reply to Dão Gottwald [::dao] from comment #18)
> How are you ensuring that browser.tabs.pinnedTabCount isn't the number of
> pinned tabs of some random window, but of the window that sessionstore is
> going to restore as the first one? This isn't really obvious to me from
> looking at the patch.

These lines:

    let pinnedTabCount = Services.prefs.getIntPref("browser.tabs.pinnedTabCount", 0);
    let initialWindow = root.windows.find(w => {
      return w.tabs.filter(t => t.pinned).length == pinnedTabCount;


It catches most cases, but now that you mention it the patch could do more to ensure that this correctly finds a window. A case that breaks this would be

Window A has one pinned tab
Window B has one pinned tab
Unpin tab in Window A (updates the pref's value to be 0)
Close all windows
Start FF, it will look for a window with 0 pinned tabs, which won't exist if it's not an automatic session restore

I suppose on window close we could update the pref if that window's gBrowser._numPinnedTabs is nonzero?
Flags: needinfo?(dothayer)
(In reply to Doug Thayer [:dthayer] from comment #19)
> (In reply to Dão Gottwald [::dao] from comment #18)
> > How are you ensuring that browser.tabs.pinnedTabCount isn't the number of
> > pinned tabs of some random window, but of the window that sessionstore is
> > going to restore as the first one? This isn't really obvious to me from
> > looking at the patch.
> 
> These lines:
> 
>     let pinnedTabCount =
> Services.prefs.getIntPref("browser.tabs.pinnedTabCount", 0);
>     let initialWindow = root.windows.find(w => {
>       return w.tabs.filter(t => t.pinned).length == pinnedTabCount;

That's backwards. browser.tabs.pinnedTabCount shouldn't determine what window sessionstore restores first.

> I suppose on window close we could update the pref if that window's
> gBrowser._numPinnedTabs is nonzero?

Windows don't close in some particular order when quitting.
(Assignee)

Comment 21

a year ago
(In reply to Dão Gottwald [::dao] from comment #20)
> (In reply to Doug Thayer [:dthayer] from comment #19)
> > (In reply to Dão Gottwald [::dao] from comment #18)
> > > How are you ensuring that browser.tabs.pinnedTabCount isn't the number of
> > > pinned tabs of some random window, but of the window that sessionstore is
> > > going to restore as the first one? This isn't really obvious to me from
> > > looking at the patch.
> > 
> > These lines:
> > 
> >     let pinnedTabCount =
> > Services.prefs.getIntPref("browser.tabs.pinnedTabCount", 0);
> >     let initialWindow = root.windows.find(w => {
> >       return w.tabs.filter(t => t.pinned).length == pinnedTabCount;
> 
> That's backwards. browser.tabs.pinnedTabCount shouldn't determine what
> window sessionstore restores first.
> 
> > I suppose on window close we could update the pref if that window's
> > gBrowser._numPinnedTabs is nonzero?
> 
> Windows don't close in some particular order when quitting.

I'm fine to change it, but I went with this because there wasn't an established notion of what window ought to be restored first, since they are just collected from SessionStoreInternal._windows[1], which is an object, and which I believe has an undefined ordering of keys when iterated. If I misunderstood something in the implementation here let me know. If we want to establish which window should be restored first, I think that's a good idea, but it seemed like a separate bug.

[1] https://searchfox.org/mozilla-central/rev/12af4303ffd384bc2406b67f54ba68d8264d72c8/browser/components/sessionstore/SessionStore.jsm#3203
(In reply to Doug Thayer [:dthayer] from comment #21)
> (In reply to Dão Gottwald [::dao] from comment #20)
> > (In reply to Doug Thayer [:dthayer] from comment #19)
> > > (In reply to Dão Gottwald [::dao] from comment #18)
> > > > How are you ensuring that browser.tabs.pinnedTabCount isn't the number of
> > > > pinned tabs of some random window, but of the window that sessionstore is
> > > > going to restore as the first one? This isn't really obvious to me from
> > > > looking at the patch.
> > > 
> > > These lines:
> > > 
> > >     let pinnedTabCount =
> > > Services.prefs.getIntPref("browser.tabs.pinnedTabCount", 0);
> > >     let initialWindow = root.windows.find(w => {
> > >       return w.tabs.filter(t => t.pinned).length == pinnedTabCount;
> > 
> > That's backwards. browser.tabs.pinnedTabCount shouldn't determine what
> > window sessionstore restores first.
> > 
> > > I suppose on window close we could update the pref if that window's
> > > gBrowser._numPinnedTabs is nonzero?
> > 
> > Windows don't close in some particular order when quitting.
> 
> I'm fine to change it, but I went with this because there wasn't an
> established notion of what window ought to be restored first, since they are
> just collected from SessionStoreInternal._windows[1], which is an object,
> and which I believe has an undefined ordering of keys when iterated.

No, that doesn't define the restoration order. Bug 1034036 should have made this more predictable -- we should be restoring the previous session's top-most window first.
(Assignee)

Comment 23

a year ago
Dao, do you know why selectedWindow isn't changed with the patches for but 1034036? It produces a really noisy experience where the topmost window on close is opened first, but then any window can be the one that ends up on top after that. Adjusting the patch to use the topmost window after bug 1034036 is trivial, but I noticed this oddness while implementing it and it seems worth addressing in another bug.
Flags: needinfo?(dao+bmo)
(Assignee)

Comment 24

a year ago
(In reply to Doug Thayer [:dthayer] from comment #23)
> Dao, do you know why selectedWindow isn't changed with the patches for but
> 1034036? It produces a really noisy experience where the topmost window on
> close is opened first, but then any window can be the one that ends up on
> top after that. Adjusting the patch to use the topmost window after bug
> 1034036 is trivial, but I noticed this oddness while implementing it and it
> seems worth addressing in another bug.

Sorry, misread the code - selectedWindow is well defined, but I'm still seeing the behavior I mentioned.
Comment hidden (mozreview-request)
(In reply to Doug Thayer [:dthayer] from comment #24)
> (In reply to Doug Thayer [:dthayer] from comment #23)
> > Dao, do you know why selectedWindow isn't changed with the patches for but
> > 1034036? It produces a really noisy experience where the topmost window on
> > close is opened first, but then any window can be the one that ends up on
> > top after that. Adjusting the patch to use the topmost window after bug
> > 1034036 is trivial, but I noticed this oddness while implementing it and it
> > seems worth addressing in another bug.
> 
> Sorry, misread the code - selectedWindow is well defined, but I'm still
> seeing the behavior I mentioned.

Not sure what's wrong -- better file a new bug and involve Mike.
Flags: needinfo?(dao+bmo)
Priority: -- → P2
(Assignee)

Comment 27

11 months ago
Hey Dao, is this still on your radar to review? I should have been more clear but I think the issue in comment #23 is separate from this and doesn't need to block it.
Flags: needinfo?(dao+bmo)
(Assignee)

Comment 28

11 months ago
(In reply to Doug Thayer [:dthayer] (PTO on May 17) from comment #27)
> Hey Dao, is this still on your radar to review? I should have been more
> clear but I think the issue in comment #23 is separate from this and doesn't
> need to block it.

Also, AFAICT bug 1235231 covers this.

Comment 29

11 months ago
mozreview-review
Comment on attachment 8960382 [details]
Bug 1442694 - Preopen pinned tabs before session restore

https://reviewboard.mozilla.org/r/229166/#review253620

::: browser/base/content/tabbrowser.js:319
(Diff revision 4)
> +  _preopenPinnedTabs() {
> +    let pinnedTabCount = 0;
> +    // __pinnedTabCount is set by SessionStore when opening new windows
> +    // for a session restore.
> +    if (window.__pinnedTabCount) {
> +      pinnedTabCount = window.__pinnedTabCount;

So, as I understand is, the point of this bug is to let the first window prepare the tab strip before the session file has been read. When sessionstore itself opens the window, this optimization doesn't seem to make much sense.

::: browser/base/content/tabbrowser.js:326
(Diff revision 4)
> +      // This is a little weird, but it seems to be the simplest way to tell
> +      // if we're the first window. If only one window exists, it's us. This
> +      // will break if we open another browser window before running this.
> +      let windows = browserWindows();
> +      windows.next();
> +      let isFirstWindow = windows.next().done;

Isn't this wrong on Mac where you can close the last browser window, then open a new one without quitting?

::: browser/base/content/tabbrowser.js:341
(Diff revision 4)
> +        skipBackgroundNotify: true,
> +        createLazyBrowser: true,
> +      });
> +      tab._preopened = true;
> +      tab.setAttribute("busy", "true");
> +      this.pinTab(tab, {skipNotify: true});

should use addTab's pinned option

::: browser/components/sessionstore/SessionStore.jsm:3517
(Diff revision 4)
> -      }
> +        }
> -    }
> +      }
> +    }
> +
> +    if (aWindow == this.windowToFocus) {
> +      Services.prefs.setIntPref("browser.tabs.pinnedTabCount", numPinnedTabs);

Why are you setting this here?
Attachment #8960382 - Flags: review?(dao+bmo) → review-

Updated

11 months ago
Flags: needinfo?(dao+bmo)
Comment hidden (mozreview-request)
(Assignee)

Comment 31

10 months ago
mozreview-review-reply
Comment on attachment 8960382 [details]
Bug 1442694 - Preopen pinned tabs before session restore

https://reviewboard.mozilla.org/r/229166/#review253620

> So, as I understand is, the point of this bug is to let the first window prepare the tab strip before the session file has been read. When sessionstore itself opens the window, this optimization doesn't seem to make much sense.

I certainly added this for a reason, but I can't seem to reproduce any difference now with it gone, so I'll pull it out.

> Isn't this wrong on Mac where you can close the last browser window, then open a new one without quitting?

It's a bit of a misnomer in that scenario, and it's an unnecessary optimization, but I don't see it being harmful. If there's an easy way around this, I'm happy to implement it, but I can't think of one other than shoving some state in a jsm which we would then have to load? Or shoving it into an unrelated jsm that we've already loaded.

> should use addTab's pinned option

Updated, but we still have the problem of immediately inserting the browser due to the messageManager access from notifyPinnedStatus. I added a special parameter to avoid this rather than checking aCreateLazyBrowser to avoid giving people a footgun.

> Why are you setting this here?

Woops - outdated from when we were doing the tracking inside pinTab/unpinTab.
Comment hidden (mozreview-request)

Comment 33

10 months ago
mozreview-review
Comment on attachment 8960382 [details]
Bug 1442694 - Preopen pinned tabs before session restore

https://reviewboard.mozilla.org/r/229166/#review263094

::: browser/base/content/tabbrowser.js:2513
(Diff revision 6)
>          t.setAttribute("fadein", "true");
>        });
>      }
>  
>      // Additionally send pinned tab events
> -    if (aPinned) {
> +    if (aPinned && !aSkipPinnedNotification) {

Why not just (aPinned && !aCreateLazyBrowser)?

::: browser/components/sessionstore/SessionStore.jsm:3474
(Diff revision 6)
>            tabbrowser.updateBrowserRemoteness(tab.linkedBrowser, true);
>          }
>        }
>  
> +      if (tabData.pinned &&
> +          tabbrowser.tabs[pinnedTabsReused] &&

nit: use t here rather than pinnedTabsReused

::: browser/components/sessionstore/SessionStore.jsm:3475
(Diff revision 6)
>          }
>        }
>  
> +      if (tabData.pinned &&
> +          tabbrowser.tabs[pinnedTabsReused] &&
> +          tabbrowser.tabs[pinnedTabsReused]._preopened) {

Can't you just use linkedPanel instead of _preopened?

::: browser/modules/BrowserWindowTracker.jsm:96
(Diff revision 6)
>    if (idx >= 0)
>      _trackedWindows.splice(idx, 1);
>  }
>  
> +function _trackPinnedTabs(window) {
> +  Services.prefs.setIntPref("browser.tabs.pinnedTabCount",

Looks like this does't get updated when changing pinned tabs in the top-most window.

::: browser/modules/BrowserWindowTracker.jsm:96
(Diff revision 6)
>    if (idx >= 0)
>      _trackedWindows.splice(idx, 1);
>  }
>  
> +function _trackPinnedTabs(window) {
> +  Services.prefs.setIntPref("browser.tabs.pinnedTabCount",

nit: let's rename this pref to something more explicit to make it clear that this is a performance optimization for restoring the first window. Otherwise people might get confused about what this pref is about.
Attachment #8960382 - Flags: review?(dao+bmo)
(Assignee)

Comment 34

10 months ago
mozreview-review-reply
Comment on attachment 8960382 [details]
Bug 1442694 - Preopen pinned tabs before session restore

https://reviewboard.mozilla.org/r/229166/#review263094

> Why not just (aPinned && !aCreateLazyBrowser)?

I mentioned it briefly in one of the earlier comments. Essentially I wanted to make a special parameter to avoid the footgun. If I just used aCreateLazyBrowser, and someone tried to create a lazy, pinned browser in the future, they wouldn't know that they needed to ensure they sent the notification at a later time. This way they'll get a warning when the browser is prematurely inserted and (ideally) find this.
Comment hidden (mozreview-request)

Comment 36

9 months ago
mozreview-review
Comment on attachment 8960382 [details]
Bug 1442694 - Preopen pinned tabs before session restore

https://reviewboard.mozilla.org/r/229166/#review265660

::: browser/base/content/tabbrowser.js:2525
(Diff revision 7)
>        });
>      }
>  
>      // Additionally send pinned tab events
> -    if (aPinned) {
> +    if (aPinned && !aIsForFirstWindowRestore) {
>        this._notifyPinnedStatus(t);

This method both sends the Browser:AppTab message and dispatches the TabPinned event. The event shouldn't be deferred, I don't think; these kind of tab events generally doen't care about what's going on with the tab's browser.
Attachment #8960382 - Flags: review?(dao+bmo)
Comment hidden (mozreview-request)

Comment 38

9 months ago
mozreview-review
Comment on attachment 8960382 [details]
Bug 1442694 - Preopen pinned tabs before session restore

https://reviewboard.mozilla.org/r/229166/#review267126

::: browser/base/content/tabbrowser.js:568
(Diff revision 8)
>      this.tabContainer._positionPinnedTabs();
>      this.tabContainer._updateCloseButtons();
>    },
>  
> -  _notifyPinnedStatus(aTab) {
> +  _notifyPinnedStatus(aTab, aDeferContentMessage = false) {
> +    if (!aDeferContentMessage) {

Can you just check aTab.linkedPanel here?

::: browser/base/content/tabbrowser.js:587
(Diff revision 8)
> +  },
> +
> +  activatePreopenedPinnedTab(aTab) {
> +    delete aTab._preopened;
> +    this._insertBrowser(aTab);
> +    this.getBrowserForTab(aTab).messageManager.sendAsyncMessage("Browser:AppTab", { isAppTab: aTab.pinned });

Should we split the message and event parts of _notifyPinnedStatus into two methods so this code can be shared?

::: browser/base/content/tabbrowser.js:2213
(Diff revision 8)
>      referrerURI,
>      relatedToCurrent,
>      sameProcessAsFrameLoader,
>      skipAnimation,
>      skipBackgroundNotify,
> +    isForFirstWindowRestore,

Please maintain the alphabetical order.
Attachment #8960382 - Flags: review?(dao+bmo)
Whiteboard: [fxperf:p1] → [fxperf:p2]

Comment 39

4 months ago
Doug, when you get back from leave, can you check what (if any) the next steps for this bug would be? Thank you!
Flags: needinfo?(dothayer)
(Assignee)

Comment 40

4 months ago

Short answer: I don't know. There was work to be done when I last looked at this and I'm sure it has rotted.

Leaving the needinfo - I'll do some work to determine what this actually takes soon.

(Assignee)

Comment 41

3 months ago

When we open firefox with pinned tabs, we first paint a window with
one tab open, and then that tab gets displaced after the pinned tabs
come in. This aims to ensure that our first paint contains the
pinned tab, so that we don't have tabs moving around after first
paint.

MozReview-Commit-ID: GC1y6NlgLTd

(Assignee)

Comment 42

3 months ago

Dao, feel free to do a drive by review here - I put Gijs on the review in this case to not add more to your plate. Also Gijs feel free to punt this back if you don't feel comfortable reviewing it.

Also, regarding:

Can you just check aTab.linkedPanel here?

I went with an explicit parameter inside _notifyPinnedStatus to avoid a footgun where someone adds a lazy browser and we silently don't send the message. They will get a warning in this case when they prematurely create the browser.

Flags: needinfo?(dothayer)
(Assignee)

Comment 43

2 months ago

Depends on D18742

Comment 44

2 months ago
Pushed by dothayer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7a20c79a0e12
Preopen pinned tabs before session restore r=Gijs
https://hg.mozilla.org/integration/autoland/rev/f100f8631f78
Add tests for preopened pinned tabs r=Gijs

Comment 45

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

Comment 46

2 months ago

I think this change had me lose my whole session on Windows nightly rev/110ea2a7c3d4f34b5079c195f7ea57966748e6da

I think I hit a condition where something is null (checked browser console). That something was very likely this.getBrowserForTab(aTab) @ chrome://browser/content/tabbrowser.js:621 and so anything after that just failed.

Unfortunately I can't confirm this anymore that it failed exactly there but it would make sense. I got to a state where I had two pinned tabs at one window - both without any content. That window and two others also had one "unloaded" normal tab.

I had several pinned tabs there (6?) but it is pretty likely that at least one of them was not loaded since I hadn't activated the tab in a long while - days, at least maybe even few weeks.

Backed out 2 changesets (Bug 1442694) for breaking session restores on update / requested by :dthayer on irc.

Status: RESOLVED → REOPENED
Flags: needinfo?(dothayer)
Resolution: FIXED → ---

Comment 48

2 months ago
Backout by shindli@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/4a87b6ac14c0
Backed out 2 changesets for breaking session restores on update a=backout
Target Milestone: Firefox 67 → ---
Depends on: 1530683
(Assignee)

Comment 49

2 months ago

This adds test which reproduce the failure as well as the fix. Essentially,
if we hit the edited case in SessionStore with tab equal to tabbrowser.tabs[t],
we remove the tab and then try to pin it, which obviously blows up.

Note: the additional method in SessionStore.jsm was largely to get around
complexity requirements inside restoreWindow. Cleaner solutions welcome.

Depends on D19463

(Assignee)

Comment 50

2 months ago

All right. I think I've done all I can reasonably do to be confident about this, but I am feeling a little timid about landing it due to the previous failures. I'm going to wait until Monday though and not land this on a Friday afternoon just in case. If anyone has any concerns in the mean time and would like me to test a particular case please let me know.

Flags: needinfo?(dothayer)

Comment 51

2 months ago
Pushed by dothayer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/59c8fffe9e41
Preopen pinned tabs before session restore r=Gijs
https://hg.mozilla.org/integration/autoland/rev/3bd9591627ce
Add tests for preopened pinned tabs r=Gijs
https://hg.mozilla.org/integration/autoland/rev/8b3fe0426ffc
Fix failures due to removing selected tab r=Gijs

Comment 52

2 months ago
bugherder
Status: REOPENED → RESOLVED
Last Resolved: 2 months ago2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
Depends on: 1532498
Target Milestone: Firefox 67 → ---
You need to log in before you can comment on or make changes to this bug.