Closed
Bug 1362378
Opened 8 years ago
Closed 7 years ago
[e10s] Web content fails to load after reloading all tabs
Categories
(Firefox :: Tabbed Browser, defect)
Tracking
()
VERIFIED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox-esr45 | --- | unaffected |
firefox-esr52 | --- | unaffected |
firefox53 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | - | verified |
firefox56 | + | verified |
People
(Reporter: emilghitta, Assigned: kmag)
References
Details
(Keywords: dataloss, regression)
Attachments
(2 files)
[Affected versions]:
Firefox 55.0a1 (Build Id:20170505030252)
[Unaffected versions]:
Firefox 54.0b4 (Build Id:20170501133120)
Firefox 53.0 (Build Id:20170413192749)
[Affected platforms]:
Windows 10 64bit
Mac 10.11.6
Ubuntu 16.04 64bit
[Steps to reproduce]:
1.Launch Firefox with a clean profile.
2.Enable e10s (if it is not already enabled).
3.Access the https://www.youtube.com/ webpage.
4.Open several videos in new tabs.
5.Restart Firefox.
6.Reload all Tabs (without focusing them).
7.Focus the opened tabs.
8.Observe that the content is not displayed.
[Expected result]:
Content is displayed after reloading all tabs.
[Actual result]:
No content is displayed.
[Regression range]:
This is a regression:
Last good revision: f77767c9f2bcc78addef282be397ac869cb3dca8
First bad revision: fb4fca2b150a3715192e60e4dde593052fb563e2
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f77767c9f2bcc78addef282be397ac869cb3dca8&tochange=fb4fca2b150a3715192e60e4dde593052fb563e2
It seems that bug 1353060 may have caused this issue.
[Additional Notes]
Please note that the content is not displayed even if refreshing the tab.
It seems that this issue is not reproducible while having e10s disabled.
For further information regarding this issue, please observe the attached video.
Reporter | ||
Comment 1•8 years ago
|
||
Hi Kris!
Can you please have a look?
Thanks !
Flags: needinfo?(kmaglione+bmo)
Updated•8 years ago
|
status-firefox-esr52:
--- → unaffected
Comment 3•8 years ago
|
||
[Tracking Requested - why for this release]: Regression
Severity: major → critical
status-firefox-esr45:
--- → unaffected
tracking-firefox55:
--- → ?
Keywords: dataloss
Updated•8 years ago
|
Comment 5•8 years ago
|
||
I think it is time to backing the offending bug(bug 1353060) out
Updated•8 years ago
|
Severity: critical → blocker
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to Alice0775 White from comment #5)
> I think it is time to backing the offending bug(bug 1353060) out
It is definitely not. This only happens when extensions do something they should not be doing (reload a tab that hasn't been initialized yet). If it's not fixed before the merge, we can consider backing out.
Severity: blocker → critical
Flags: needinfo?(kmaglione+bmo)
Comment 7•8 years ago
|
||
(In reply to Kris Maglione [:kmag] (busy; behind on reviews) from comment #6)
> This only happens when extensions do something they should not be doing .
Extensions? Does it mean add-ons?
I can reproduce the data loss without any add-ons.
Assignee | ||
Comment 8•8 years ago
|
||
I mean extensions.
Hm. I didn't realize we had a Reload All Tabs feature with not extensions installed. That almost certainly should not be touching uninitialized tabs.
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8872194 [details]
Bug 1362378: Restore rather than reload pending background tabs when reloading all tabs.
https://reviewboard.mozilla.org/r/143648/#review150670
::: browser/base/content/tabbrowser.xml:3351
(Diff revision 1)
> + <parameter name="aKeepTemporaryPermissions"/>
> <body>
> <![CDATA[
> - let browser = this.getBrowserForTab(aTab);
> + if (!aKeepTemporaryPermissions) {
> - // Reset temporary permissions on the current tab. This is done here
> + // Reset temporary permissions on the current tab. This is done here
> - // because we only want to reset permissions on user reload.
> + // because we only want to reset permissions on user reload.
I'm not a fan of adding this boolean parameter. It doesn't help that the comment says "we only want to reset permissions on user reload" and "reload all tabs" is in fact a user action. Pretty obscure stuff.
Attachment #8872194 -
Flags: review?(dao+bmo) → review-
Comment 11•7 years ago
|
||
Kris, how important is this bug? Something we wanna land in 55, or uplift in early beta? Or can it wait for 56?
Flags: needinfo?(kmaglione+bmo)
Assignee | ||
Comment 12•7 years ago
|
||
The patches for bug 1362462 have fixed the immediate regression, but our behavior in this circumstance is still pretty flaky, so I think we should fix this in 55 regardless. I'll try to have an updated patch later today.
Flags: needinfo?(kmaglione+bmo)
Comment 13•7 years ago
|
||
(In reply to Kris Maglione [:kmag] (busy; behind on reviews) from comment #12)
> I'll try to have an updated patch later today.
Did you ever get around to that patch?
Flags: needinfo?(kmaglione+bmo)
Comment 14•7 years ago
|
||
Moving to Firefox:TabbedBrowser which seems a better fit for this issue.
Component: Layout → Tabbed Browser
Product: Core → Firefox
Updated•7 years ago
|
status-firefox56:
--- → affected
tracking-firefox56:
--- → ?
Updated•7 years ago
|
Updated•7 years ago
|
Dão, not sure if this is something you can help with (as it is now in "your" component or if Kris is still planning on a patch for 56/55.
Flags: needinfo?(dao+bmo)
Reporter | ||
Comment 16•7 years ago
|
||
Hi everyone !
It seems that I can't reproduce this issue anymore.
This issue was fixed by another patch:
First good revision: 63cc19a2300f356abf1fa03884a9ac3d4792ef57
Last bad revision: 72a455b224e63335c4cf6cce7588d2b9c9de7a5f
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=72a455b224e63335c4cf6cce7588d2b9c9de7a5f&tochange=63cc19a2300f356abf1fa03884a9ac3d4792ef57
Looks like the following bug has the changes which introduced the fix:
https://bugzilla.mozilla.org/show_bug.cgi?id=1362462
Comment 17•7 years ago
|
||
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #15)
> Dão, not sure if this is something you can help with (as it is now in "your"
> component or if Kris is still planning on a patch for 56/55.
I don't know what Kris is planning, his needinfo request is still pending. But based on comment 12, it seems that we don't need to track this for 55 anymore.
Flags: needinfo?(dao+bmo)
This may be a duplicate of bug 1362462. I'll continue discussion there.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Assignee: nobody → kmaglione+bmo
Target Milestone: --- → Firefox 55
Reporter | ||
Comment 19•7 years ago
|
||
This issue is verified fixed using Firefox 56.0.2 (BuildId:20171024165158) and Firefox 55.0.3 (BuildId:20170824053622) on Windows 10 64bit, macOS 10.11.6 and Ubuntu 16.04 64bit.
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(kmaglione+bmo)
You need to log in
before you can comment on or make changes to this bug.
Description
•