1342849 - Tabbar would not scroll properly after session is restored. The current tab is not displayed in the tabbar.

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Alice0775 White]

Attachment: sessionstore.js

[Tracking Requested - why for this release]: The current tab is missing after session restore. Tabbar would not scroll properly after session is restored

Reproducible : always

Steps To Reproduce:
1. Open many tabs (50-60tabs)
2. Open several new tab after [web page2]
   [web page1][web page2][New Tab1][New Tab2][New Tab3][web page3]....[web pageN]
3. Select [New Tab1]
4. Quit and re-start Nightly and then Restore Previous Session

Actual Results:
No selected tab is displayed in the Tabbar.

Expected Results:
The current tab should be displayed in the Tabbar.

Comment 1

[Alice0775 White]

Also reproduced on Aurora53.0a2.

Steps To Reproduce:
1. Open many tabs (50-60tabs)
2. Open several new tab after [web page2]
   [web page1][web page2][New Tab1][New Tab2][New Tab3][web page3]....[web pageN]
3. Select [New Tab1]
4. Quit and re-start Nightly and then Restore Previous Session
5. (optionally, Repeat step4 if needed)

Actual Results:
After Stap4. New tabs will be removed due to Bug 1306294(maybe by design right now).
However, Current selected tab is missing in the tabbar. Tabbar scroll position is wrong.

Expected Results:
The Current selected tab should be visible in the tabbar.


The bunch of patch (Bug 1323987 and Bug 1306294) should be backed out.

Comment 2

[Alice0775 White]

I can reproduce the issue even if only 20 tabs are opened.

Comment 3

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

Hey mikedeboer,

Just giving you a heads up on this. How should we proceed?

Comment 4

[Matheus Longaray (:mlongaray)]

This is expected to happen on version 53 (current Aurora) since when we first landed bug 1306294 we were removing about:newtab and about:blank tabs from session state (see https://hg.mozilla.org/mozilla-central/rev/cbe1b40baf57#l3.25).

Bug 1323987 was filed to solve this issue and now we're not removing such tabs from session state (see https://hg.mozilla.org/mozilla-central/rev/997080796cd3).

I used attached sessionstore.js file on version 54 (current Nightly) and could not reproduce the issue, as expected.

The issue can be reproduced on version 53 because (using attached sessionstore.js file) the current/selected tab is an about:newtab tab, thus, such tab will not be displayed in the tab bar (since we were removing such kind of tab from bug 1306294).

One thing we could do is to uplift commit https://hg.mozilla.org/mozilla-central/rev/997080796cd3 to Aurora as soon as possible before merge.

What do you think, Mike? Feel free to confirm what I just said. Please let me know what I can do to help.

Comment 5

[Mike de Boer [:mikedeboer]]

(In reply to Matheus Longaray (:mlongaray) from comment #4)
> What do you think, Mike? Feel free to confirm what I just said. Please let
> me know what I can do to help.

I also think we'll need to uplift these patches to 53 asap. Do you need help with that?

Comment 6

[Alice0775 White]

(In reply to Matheus Longaray (:mlongaray) from comment #4)

> 
> I used attached sessionstore.js file on version 54 (current Nightly) and
> could not reproduce the issue, as expected.
> 

I can reproduce the problem on Noghtly54.0a1 With STR in comment#0

Comment 7

[Matheus Longaray (:mlongaray)]

(In reply to Mike de Boer [:mikedeboer] from comment #5)
> (In reply to Matheus Longaray (:mlongaray) from comment #4)
> > What do you think, Mike? Feel free to confirm what I just said. Please let
> > me know what I can do to help.
> 
> I also think we'll need to uplift these patches to 53 asap. Do you need help
> with that?

I've never requested an uplift. I see that https://wiki.mozilla.org/Release_Management/Uplift_rules contains info in order to do that. Should I proceed?

Comment 8

[Mike de Boer [:mikedeboer]]

At this point it's also quite important to figure out _why_ Alice is able to reproduce it just fine and you can't using the same STR...

Comment 9

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

I concur. We need to understand what's going on here before we consider an uplift.

Comment 10

[Alice0775 White]

screen cast: https://youtu.be/3XQHh3xz3Fw
Current tab is not shown in tabbar after session restore.

Workaround, resize browser, then the current tab appears in tabbar.

Comment 11

[Matheus Longaray (:mlongaray)]

So, per Alice's screen cast, we can see that current tab is properly restored and selected after restoring previous session.

I see that our tabbar UI did not 're-flowed' itself to show the current tab though the selected tab's content was correct.

Are we sure that the regression range is correct?

Comment 12

[Matheus Longaray (:mlongaray)]

PS: current tab appears in tabbar on Linux (local development environment).

Comment 13

[u589863]

(In reply to Matheus Longaray (:mlongaray) from comment #12)
> PS: current tab appears in tabbar on Linux (local development environment).

can confirm works on linux but not in windows10 RS2

Comment 14

[Matheus Longaray (:mlongaray)]

Managed to reproduce the issue on Linux as well. After restoring session, navigated tabbar until the end, closed the browser, restored previous session -> tabbar UI did not re-flowed itself to show the current tab (however, tab's content was correct and was not removed from session state - no data loss).

Alice's workaround also caused expected effect on Linux.

Comment 15

[Matheus Longaray (:mlongaray)]

Any thoughts, mikedeboer?

Comment 16

[Alice0775 White]

Okay,
The STR is not good. The New tab thing hides the real culprit.


Steps To Reproduce(without New tab):
1. Open many tabs (20tabs is enough)
   [about:home][https://developer.mozilla.org/en-US/Add-ons/Code_snippets][about:home]....[about:home][about:home]
2. Select 2nd tab
3. Restart Browser, and then Restore Previous Session

Actual Results:
Tabbar scrolls at wrong position, so the current tab is not shown in the tabbar.

Expected results:
Tabbar should scroll so the current tab will be shown in the tabbar.



Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=69404501d4d8fc26cc4317ccaec8cf997e7b599a&tochange=910ceca7e8637c4c3ea963fd054e6c449eeff6a7

Regressed by:
910ceca7e863	Olli Pettay — Bug 1315570, vsync handling in parent process' main thread should use high priority runnables, r=ehsan



(:mconley, :mikedeboer, :mlongaraySorry , sorry for the confusion)

Comment 17

[Mike de Boer [:mikedeboer]]

^5 Alice, thanks for the update! I'll be keeping an eye on this bug.

Comment 18

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

Thanks for clearing this up, Alice!

Comment 19

[Marcia Knous [:marcia]]

Tracking 54+ for this regression, so that the tab functionality works as expected.

Comment 20

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

Attachment: wip

This helps, and as expected, the patch shows that we have some racy code somewhere and prioritizing vsync (requestAnimationFrame callbacks) triggers different execution for the racy code. 

Better would be of course to fix the racy code. Whether it has something to do with reflow and XBL constructors or some setTimeout somewhere or requestAnimationFrame... dunno yet.

Is this kind of patch acceptable, or does anyone familiar with this code have ideas how else to fix this?

Comment 21

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

Or maybe Mike knows

Comment 22

[Matheus Longaray (:mlongaray)]

I'm afraid I'm not well familiar with this piece of code. Maybe ttaubert can help you out as he also helped me with SessionStore related-code before.

Comment 23

[Tim Taubert [:ttaubert] (inactive)]

Mike's already CC'ed. He owns SessionStore these days :)

Comment 24

[Mike de Boer [:mikedeboer]]

Comment on attachment 8846458 [details] [diff] [review]
wip

Review of attachment 8846458 [details] [diff] [review]:
-----------------------------------------------------------------

Apology for my late reply here - I'm not yet 100% familiar with the code here, so I needed a bit more time to think about it.

Thanks for the patience!

::: browser/components/sessionstore/SessionStore.jsm
@@ +3450,5 @@
>      tabsDataArray.length = numTabsInWindow;
>  
>      // If provided, set the selected tab.
>      if (aSelectTab > 0 && aSelectTab <= aTabs.length) {
> +      let selectedTab = aTabs[aSelectTab - 1];

Hm, what I'd be OK with is to have a `setTimeout` at the end of this function and you keep a reference to the tab to select here:
```js
let tabToSelect = null;
if (aSelectTab > 0 && aSelectTab <= aTabs.length) {
  tabToSelect = aTabs[aSelectedTab - 1];
}
// ... rest of method ...
if (tabToSelect) {
  tabbrowser.ownerDocument.defaultView.setTimeout(() =>
    tabbrowser.selectedTab = selectedTab;
});
```

This is because I'm expecting more deferred operation to be added here very soon.

Comment 25

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

Attachment: patch

Let's see how many tests will be broken
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1ee0cb98540e74af3d767c0b363e05042b42404

Comment 26

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

Attachment: patch

Right patch this time

Comment 27

[Mike de Boer [:mikedeboer]]

Comment on attachment 8847122 [details] [diff] [review]
patch

Review of attachment 8847122 [details] [diff] [review]:
-----------------------------------------------------------------

So this is breaking quite a number of tests, it seems :( Not entirely surprising, but myeah, quite annoying.

OK, does this work when you just move the `tabbrowser.selectedTab = tabToSelect;` outside of the loop without the timeout?

Comment 28

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

You mean just not using setTimeout? That doesn't work.
bug 1315570 made rAF callbacks and layout to run possibly earlier, all asynchronous.
(It is now disabled in Bug 1346644 because of this bug)

Comment 29

[Mike de Boer [:mikedeboer]]

:'(

OK, moving on. I'm taking a quick look if we can do something smart about the tests. Meanwhile, do you think we can use a timer which finishes in the same stack as a Promise that resolves? Or simply use

```js
new Promise(resolve => {
  tabbrowser.selected = tabToSelect;
  resolve();
});
```

I know that the methods to propagate the sessionstore with tests data, restore windows & tabs use promises to wait for all the things. Assertion come after that and since timers are not reliable instruments, I'd conjure that this'd improve our situation.

Comment 30

[Mike de Boer [:mikedeboer]]

(In reply to Mike de Boer [:mikedeboer] from comment #29)
> ```js
> new Promise(resolve => {
>   tabbrowser.selected = tabToSelect;
>   resolve();
> });
> ```

This doesn't work. After crawling through code and flows a bit here, I stumbled upon the 'switcher' in tabbrowser.xml, which reacts to a whole bunch of events to keep the UI up-to-date asynchronously.
I think at this point it'd be OK to ask mconley what we can do here: Mike, what do you think the tab switcher can do wrt Olli's findings in comment 20?

Comment 31

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

(Promises are synchronous from Gecko point of view. They run within the current task.)

Comment 32

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

Hm, I think I know what's going on here.

SessionStore.jsm's restoreWindow method calls addTab when it's restoring state into a window that doesn't have enough tabs (which is true in the STR here). So a bunch of new tabs are added.

We're smart enough not to animate those tabs on creation - however, we also queue a bunch of setTimeout(f, 0)'s to fire soon after the <xul:tab> is added into the DOM:

http://searchfox.org/mozilla-central/rev/0079c7adf3b329bff579d3bbe6ac7ba2f6218a19/browser/base/content/tabbrowser.xml#2273

So those setTimeout's are all queued, but haven't fired by the time we reach here:

http://searchfox.org/mozilla-central/rev/0079c7adf3b329bff579d3bbe6ac7ba2f6218a19/browser/components/sessionstore/SessionStore.jsm#3436-3446

Which sets the selected tab to the one we actually want selected and visible, and then starts restoring the history state into the tabs.

Yes, we have an async tab switcher for e10s to account for layer trees not yet being ready from the content process - however, from the perspective of <xul:tabs>, the tab switch _is_ synchronous (there's an arguably unfortunate difference between "logically selected" and "visually selected" in this model).

So we've selected the tab, and at this point, because it's one of the first two tabs (using the STR from comment 16), the _handleTabSelect code (which fires once the tab is selected):

http://searchfox.org/mozilla-central/rev/0079c7adf3b329bff579d3bbe6ac7ba2f6218a19/browser/base/content/tabbrowser.xml#5912-5918

Decides that the tab is (rightly) still visible, and so we don't scroll anywhere.

Then those runnables we queued start firing. Those are calling into _handleNewTab. For the tabs _after_ the selected tab, we enter this condition:

http://searchfox.org/mozilla-central/rev/0079c7adf3b329bff579d3bbe6ac7ba2f6218a19/browser/base/content/tabbrowser.xml#6362

which enters here:

http://searchfox.org/mozilla-central/rev/0079c7adf3b329bff579d3bbe6ac7ba2f6218a19/browser/base/content/tabbrowser.xml#6227-6277

and that determines that the tab that we just added in the background is something that the user probably cares about and scrolls it into view. It does that for each tab that gets added. This ends up scrolling us to the end of the tab strip.

I don't know why smaug's patch caused this, to be honest. Those setTimeout's were always destined to run after the initial tab selection.

Anyhow, I have a patch that fixes this - and it might even give us a little performance improvement along the way, as we get to skip a lot of unnecessary work in _notifyBackgroundTab for restored tabs.

Comment 33

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

Attachment: Bug 1342849 - Don't do any notifications for newly added background tabs when restoring session.

Review commit: https://reviewboard.mozilla.org/r/122586/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/122586/

Comment 34

[Mike de Boer [:mikedeboer]]

Comment on attachment 8849851 [details]
Bug 1342849 - Don't do any notifications for newly added background tabs when restoring session.

https://reviewboard.mozilla.org/r/122586/#review125034

Well, this will certainly help perf! (More than you think at the moment, probably, especially since you're eliminating a sizeable chunk of heavy-duty code; I see multiple getBoundingClientRect() calls and a `_calcTabMargins` call, for example.)

In `_notifyBackgroundTab` the attribute name 'notifybgtab' is used. That's used on the scroll button, but I'd vote for that one to be renamed to something more self-explanatory and non-confusing.

Thanks!

::: browser/base/content/tabbrowser.xml:2263
(Diff revision 1)
>                ContextualIdentityService.setTabStyle(t);
>              }
>  
>              t.setAttribute("onerror", "this.removeAttribute('image');");
> +
> +            if (aSkipBackgroundNotify) {

Please add this attribute to `ATTRIBUTES_TO_SKIP` in TabAttributes.jsm

::: browser/base/content/tabbrowser.xml:6376
(Diff revision 1)
>            this.adjustTabstrip();
>  
>            if (tab.getAttribute("selected") == "true") {
>              this._fillTrailingGap();
>              this._handleTabSelect();
> -          } else {
> +          } else if (!tab.getAttribute("skipbackgroundnotify")) {

Please use `hasAttribute` here

Comment 35

[Mike de Boer [:mikedeboer]]

The late(r) review was due to me being sick this week... still am a little. Just sayin' ;-)

Comment 36

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

Comment on attachment 8849851 [details]
Bug 1342849 - Don't do any notifications for newly added background tabs when restoring session.

https://reviewboard.mozilla.org/r/122586/#review125034

Just so I'm clear, you want me to rename the notifybgtab attribute to something else (here, and in the CSS that selects on it)? What works - "backgroundtabnotification"? "highlight"?

Comment 37

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

Thanks for the review! ni? for comment 36.

Comment 38

[Mike de Boer [:mikedeboer]]

(In reply to Mike Conley (:mconley) (Catching up on reviews and needinfos) from comment #36)
> Just so I'm clear, you want me to rename the notifybgtab attribute to
> something else (here, and in the CSS that selects on it)? What works -
> "backgroundtabnotification"? "highlight"?

Yeah, 'highlight' works nicely, I think! (This is also the animation I'd like to disable during session restore, btw, just like we do with the others)

Comment 39

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

Comment on attachment 8849851 [details]
Bug 1342849 - Don't do any notifications for newly added background tabs when restoring session.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122586/diff/1-2/

Comment 40

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/838e5ace9f13
Don't do any notifications for newly added background tabs when restoring session. r=mikedeboer

Comment 41

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/838e5ace9f13

Comment 42

[u589863]

(In reply to Wes Kocher (:KWierso) from comment #41)
> https://hg.mozilla.org/mozilla-central/rev/838e5ace9f13

This seems to have brought a regression .
STR is still wip

but basically if there are 20 or so tabs(unloaded)
quickly ctrl+tab between them results a different tab being selected while ctrl+tab already starts to load another tab while still being on another.

Happens mostly switching from last tab to first.

Sorry if not clear

Comment 43

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

(In reply to Sunil Kumar from comment #42)
> (In reply to Wes Kocher (:KWierso) from comment #41)
> > https://hg.mozilla.org/mozilla-central/rev/838e5ace9f13
> 
> This seems to have brought a regression .
> STR is still wip
> 
> Sorry if not clear

Hey Sunil, thanks for the report! I think I understand what you're describing.

Are you certain that the patch for this bug caused the issue, and it did not exist in earlier builds? I ask, because what you're describing is a bug that I think has existed for a while, and wasn't just introduced.

If you take a March 20th Nightly, for example:

https://ftp.mozilla.org/pub/firefox/nightly/2017/03/2017-03-20-03-02-09-mozilla-central/

(Here's the Windows zip: https://ftp.mozilla.org/pub/firefox/nightly/2017/03/2017-03-20-03-02-09-mozilla-central/firefox-55.0a1.en-US.win32.zip )

do you still see the behaviour in that build?

Comment 44

[u589863]

(In reply to Mike Conley (:mconley) (Offsite until March 27) from comment #43)
> (In reply to Sunil Kumar from comment #42)
> > (In reply to Wes Kocher (:KWierso) from comment #41)
> > > https://hg.mozilla.org/mozilla-central/rev/838e5ace9f13
> > 
> > This seems to have brought a regression .
> > STR is still wip
> > 
> > Sorry if not clear
> 
> Hey Sunil, thanks for the report! I think I understand what you're
> describing.
> 
> Are you certain that the patch for this bug caused the issue, and it did not
> exist in earlier builds? I ask, because what you're describing is a bug that
> I think has existed for a while, and wasn't just introduced.
> 
> If you take a March 20th Nightly, for example:
> 
> https://ftp.mozilla.org/pub/firefox/nightly/2017/03/2017-03-20-03-02-09-
> mozilla-central/
> 
> (Here's the Windows zip:
> https://ftp.mozilla.org/pub/firefox/nightly/2017/03/2017-03-20-03-02-09-
> mozilla-central/firefox-55.0a1.en-US.win32.zip )
> 
> do you still see the behaviour in that build?

Thanks for getting it was not sure how to make the issue understandable

Got this after today's build, maybe just hit it today not sure.

okay testing it out and if will comment here.

Comment 45

[u589863]

(In reply to Mike Conley (:mconley) (QF workweek until March 31) from comment #43)
> (In reply to Sunil Kumar from comment #42)
> > (In reply to Wes Kocher (:KWierso) from comment #41)
> > > https://hg.mozilla.org/mozilla-central/rev/838e5ace9f13
> > 
> > This seems to have brought a regression .
> > STR is still wip
> > 
> > Sorry if not clear
> 
> Hey Sunil, thanks for the report! I think I understand what you're
> describing.
> 
> Are you certain that the patch for this bug caused the issue, and it did not
> exist in earlier builds? I ask, because what you're describing is a bug that
> I think has existed for a while, and wasn't just introduced.
> 
> If you take a March 20th Nightly, for example:
> 
> https://ftp.mozilla.org/pub/firefox/nightly/2017/03/2017-03-20-03-02-09-
> mozilla-central/
> 
> (Here's the Windows zip:
> https://ftp.mozilla.org/pub/firefox/nightly/2017/03/2017-03-20-03-02-09-
> mozilla-central/firefox-55.0a1.en-US.win32.zip )
> 
> do you still see the behaviour in that build?

Yes you were right!!
Just produced it on the build you provided.
Any fix for it yet?

Also found two bug which reproduced after landing this patch(or maybe some other patch landed with this one)

1. New profile, default settings, browser.sessionstore.restore_on_demand false,started to record performance profile while the session was being restored by loading 3 tabs at once in the background and I was slowly scrolling the page in the active tab. The result: http://i.imgur.com/wbBJln6.png
As you can see, fps constantly drops to 2-3 frames per second, the scrolling doesn't feel smooth at all

& browser.sessionstore.restore_on_demand true

cntrl + tabs quickly to load 10+ tabs and after most tabs load same thing as above.

2. multiple bookmarks in multiple folders say 10 in each folder
e10s =on and not in non e10s

Click folder to open all bookmarks
quickly followed by folder 2 then 3 then 4 etc

tab update to show bookmarks opening but sometimes tabs disappear 
eg Folder 1 and 2 all bookmarks open but folder 3 bookmarks appear to load and disappear(or 4 or 5) , folder 4 5 open fine.
clicking folder 3 after other tabs open load it fine.

some-throttling  tabs in e10s is happening when opening many bookmarks(and way to slow compared to e10s off  )

Any ideas?

sorry if not clear.

Comment 46

[Nathan Froyd [:froydnj]]

Please request Aurora approval on this when you get a chance.

Comment 47

[Nathan Froyd [:froydnj]]

(In reply to Sunil Kumar from comment #45)
> Also found two bug which reproduced after landing this patch(or maybe some
> other patch landed with this one)

Please file new bugs for these bugs, and please note (if possible) if they reproduce prior to the patch in this bug landing.  Thanks!

Comment 48

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

Comment on attachment 8849851 [details]
Bug 1342849 - Don't do any notifications for newly added background tabs when restoring session.

Approval Request Comment
[Feature/Bug causing the regression]:

Bug 1315570

[User impact if declined]:

Selected tab might not scroll into view after a window is restored (for example, when the session is restored during start-up).

[Is this code covered by automated tests?]:

No.

[Has the fix been verified in Nightly?]:

Yes.

[Needs manual test from QE? If yes, steps to reproduce]: 

Couldn't hurt. STR are in comment 16.

[List of other uplifts needed for the feature/fix]:

None.

[Is the change risky?]:

No.

[Why is the change risky/not risky?]:

It's a small scoped change that makes us skip doing a thing we don't need to do, which was causing the tab strip to scroll when it shouldn't have.

[String changes made/needed]:

None.

Comment 49

[u589863]

(In reply to Mike Conley (:mconley) (Very high latency until April 3rd) from comment #43)
> (In reply to Sunil Kumar from comment #42)
> > (In reply to Wes Kocher (:KWierso) from comment #41)
> > > https://hg.mozilla.org/mozilla-central/rev/838e5ace9f13
> > 
> > This seems to have brought a regression .
> > STR is still wip
> > 
> > Sorry if not clear
> 
> Hey Sunil, thanks for the report! I think I understand what you're
> describing.
> 
> Are you certain that the patch for this bug caused the issue, and it did not
> exist in earlier builds? I ask, because what you're describing is a bug that
> I think has existed for a while, and wasn't just introduced.
> 
> If you take a March 20th Nightly, for example:
> 
> https://ftp.mozilla.org/pub/firefox/nightly/2017/03/2017-03-20-03-02-09-
> mozilla-central/
> 
> (Here's the Windows zip:
> https://ftp.mozilla.org/pub/firefox/nightly/2017/03/2017-03-20-03-02-09-
> mozilla-central/firefox-55.0a1.en-US.win32.zip )
> 
> do you still see the behaviour in that build?

(In reply to Sunil Kumar from comment #45)
> (In reply to Mike Conley (:mconley) (QF workweek until March 31) from
> comment #43)
> > (In reply to Sunil Kumar from comment #42)
> > > (In reply to Wes Kocher (:KWierso) from comment #41)
> > > > https://hg.mozilla.org/mozilla-central/rev/838e5ace9f13
> > > 
> > > This seems to have brought a regression .
> > > STR is still wip
> > > 
> > > Sorry if not clear
> > 
> > Hey Sunil, thanks for the report! I think I understand what you're
> > describing.
> > 
> > Are you certain that the patch for this bug caused the issue, and it did not
> > exist in earlier builds? I ask, because what you're describing is a bug that
> > I think has existed for a while, and wasn't just introduced.
> > 
> > If you take a March 20th Nightly, for example:
> > 
> > https://ftp.mozilla.org/pub/firefox/nightly/2017/03/2017-03-20-03-02-09-
> > mozilla-central/
> > 
> > (Here's the Windows zip:
> > https://ftp.mozilla.org/pub/firefox/nightly/2017/03/2017-03-20-03-02-09-
> > mozilla-central/firefox-55.0a1.en-US.win32.zip )
> > 
> > do you still see the behaviour in that build?
> 
> Yes you were right!!
> Just produced it on the build you provided.
> Any fix for it yet?
> 
> Also found two bug which reproduced after landing this patch(or maybe some
> other patch landed with this one)
> 
> 1. New profile, default settings, browser.sessionstore.restore_on_demand
> false,started to record performance profile while the session was being
> restored by loading 3 tabs at once in the background and I was slowly
> scrolling the page in the active tab. The result:
> http://i.imgur.com/wbBJln6.png
> As you can see, fps constantly drops to 2-3 frames per second, the scrolling
> doesn't feel smooth at all
> 
> & browser.sessionstore.restore_on_demand true
> 
> cntrl + tabs quickly to load 10+ tabs and after most tabs load same thing as
> above.
> 
> 2. multiple bookmarks in multiple folders say 10 in each folder
> e10s =on and not in non e10s
> 
> Click folder to open all bookmarks
> quickly followed by folder 2 then 3 then 4 etc
> 
> tab update to show bookmarks opening but sometimes tabs disappear 
> eg Folder 1 and 2 all bookmarks open but folder 3 bookmarks appear to load
> and disappear(or 4 or 5) , folder 4 5 open fine.
> clicking folder 3 after other tabs open load it fine.
> 
> some-throttling  tabs in e10s is happening when opening many bookmarks(and
> way to slow compared to e10s off  )
> 
> Any ideas?
> 
> sorry if not clear.

Like you said this is a know bug, has it been filed anywhere where STR can be posted?

Comment 50

[Gerry Chang [:gchang]]

Hi Alice,
could you help verify if this issue was fixed as expected on a latest Nightly build? Thanks!

Comment 51

[Alice0775 White]

(In reply to Gerry Chang [:gchang] from comment #50)
> Hi Alice,
> could you help verify if this issue was fixed as expected on a latest
> Nightly build? Thanks!

no longer reproduce the problem on Nightly55.0a1[1].

[1]
https://hg.mozilla.org/mozilla-central/rev/38894655c89e68bcd8f45d31a0d3005f2c2b53db
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0 ID:20170403030207

Comment 52

[Gerry Chang [:gchang]]

Comment on attachment 8849851 [details]
Bug 1342849 - Don't do any notifications for newly added background tabs when restoring session.

Fix a regression related to scrolling issue in selected tab after session restore and was verified. Aurora54+.

Comment 53

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5587c1bbdf94

Comment 54

[Andrei Vaida [:avaida]]

Let's make sure this works as intended on Fx54 as well. Details available in Comment 16.

Comment 55

[Mihai Boldan, Desktop QA [:mboldan]]

I managed to reproduce the issue on Firefox 54.0a1 (2017-02-26), under Windows 10x64, using the STR from Comment 16.
The issue is no longer reproducible on Firefox 54.0b5.
Tests were performed under Windows 10x64 and under Windows 8.1x32.