Closed Bug 1365541 Opened 2 years ago Closed 2 years ago

Fix FX_SESSION_RESTORE_RESTORE_WINDOW_MS regression

Categories

(Firefox :: Session Restore, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 56
Tracking Status
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- wontfix
firefox56 --- verified

People

(Reporter: dao, Assigned: dao)

References

Details

(Keywords: perf, regression)

Attachments

(1 file)

(In reply to Chris H-C :chutten from bug 1054740 comment #56)
> It appears as though this bug may have contributed to a sudden change in the
> Telemetry probe FX_SESSION_RESTORE_RESTORE_WINDOW_MS[1] which seems to have
> occurred in Nightly 20170506[2][3].
> 
> It seems as though it got rather slower. And it seems from [2] that it's
> undoing a previous improvement in this arena.
> 
> Is this actually a regression? Is this intentional? Is it expected?

If I read the graph correctly, it's a regression and it's neither intentional nor expected.

> Is this probe measuring something useful?

I think so.

> [1]:
> http://alerts.telemetry.mozilla.org/index.html#/detectors/1/metrics/750/
> alerts/?from=2017-05-06&to=2017-05-06
> [2]:
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=0b255199db9d6a6f189b89b7906f99155bde3726&tochange=37a5
> b7f6f101df2eb292b1b6baaf1540c9920e20
> [3]: https://mzl.la/2rnqPjP
Keywords: regression
Blocks: 1054740
Wondering if bug 1364849 will help here...
Depends on: 1364849
I'm going to re-implement reusing the selected tab, hopefully with less code than that removed in bug 1054740.
Comment on attachment 8879105 [details]
Bug 1365541 - Re-implement reusing the selected tab when restoring a window to fix FX_SESSION_RESTORE_RESTORE_WINDOW_MS regression.

https://reviewboard.mozilla.org/r/150442/#review156188

LGTM, thanks for working on this Dão!
Attachment #8879105 - Flags: review?(mdeboer) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c6d27875faec
Re-implement reusing the selected tab when restoring a window to fix FX_SESSION_RESTORE_RESTORE_WINDOW_MS regression. r=mikedeboer
Duplicate of this bug: 1371884
https://hg.mozilla.org/mozilla-central/rev/c6d27875faec
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
I reported about white flashes when switching to a loading tab in this comment to bug 1367596:
https://bugzilla.mozilla.org/show_bug.cgi?id=1367596#c22

Mike Conley wrote a plan to help reduce the remaining flashes had been laid out in bug 1371884 that has been closed as a duplicate of this bug so I am writing here. I still see white flashes when switching to a loading tab. The easiest STR:
Enable bookmarks toolbar, create https://videocardz.com/ bookmark
Drag this bookmark to the tab bar in order to open it in the background
Count to 10 in order to make sure page has loaded
Switch to this tab

ER: Contents of the web page are rendered immediately
AR: Contents of the web page are rendered after annoying white flash

ni Dao Gottwald because I think this is a serious regression.
Flags: needinfo?(dao+bmo)
(In reply to ajfhajf from comment #10)
This should have its own bug. I needinfo'd mconley.
Flags: needinfo?(dao+bmo)
Duplicate of this bug: 1375755
Please request Beta approval on this when you get a chance.
Flags: needinfo?(dao+bmo)
I'm waiting for a second (Friday's) FX_SESSION_RESTORE_RESTORE_WINDOW_MS data point. Thursday's data point is promising, suggesting that FX_SESSION_RESTORE_RESTORE_WINDOW_MS is lower than it ever was.
Flags: needinfo?(dao+bmo)
Comment on attachment 8879105 [details]
Bug 1365541 - Re-implement reusing the selected tab when restoring a window to fix FX_SESSION_RESTORE_RESTORE_WINDOW_MS regression.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1054740
[User impact if declined]: slower startup
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: /
[Is the change risky?]: no
[Why is the change risky/not risky?]: this is central sessionstore code and as such stressed by hundreds of tests
[String changes made/needed]: /
Attachment #8879105 - Flags: approval-mozilla-beta?
Comment on attachment 8879105 [details]
Bug 1365541 - Re-implement reusing the selected tab when restoring a window to fix FX_SESSION_RESTORE_RESTORE_WINDOW_MS regression.

Backout for perf regression, let's take this for beta 5 (or 6, depending on when we go to build).
Attachment #8879105 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Depends on: 1388628
This was backed out from 55 for causing bug 1388628.
No longer blocks: 1054740
You need to log in before you can comment on or make changes to this bug.