Closed Bug 968774 Opened 6 years ago Closed 6 years ago

Metro interface is shown in a desktop window after clicking "Restart" in desktop crash reporter dialog

Categories

(Firefox for Metro Graveyard :: General, defect, P1)

28 Branch
All
Windows 8.1
defect

Tracking

(firefox28+ verified, firefox29+ verified, firefox30+ verified, b2g-v1.3 fixed)

VERIFIED FIXED
Firefox 30
Tracking Status
firefox28 + verified
firefox29 + verified
firefox30 + verified
b2g-v1.3 --- fixed

People

(Reporter: cornel_ionce, Assigned: jimm)

References

()

Details

(Whiteboard: [beta28] p=2 s=it-30c-29a-28b.1 r=ff28)

Attachments

(2 files)

Mozilla/5.0 (Windows NT 6.3; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Windows NT 6.3; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0

This issue was found on Firefox 28 beta 1 (build ID: 20140205162153).
After submitting a crash, the Metro interface is shown inside a desktop window.

Steps to reproduce:

1. Open Firefox and make it the default browser.
2. Install the crash me add-on (http://ted.mielczarek.org/mozilla/crashme.html)
3. Open 3 or more websites in different tabs.
4. Switch to Metro interface.
5. Switch back to Desktop interface.
6. Use the crash me add-on to simulate a crash.
7. Restart Firefox.

Expected Result:
Firefox restarts in desktop interface and the about:sessionrestore page is shown. 

Actual Result:
Metro mode is shown in a desktop window, without the session being restored.

Notes:
This issue also reproduces on latest Nightly and latest Aurora builds.
It not reproduces if Firefox is not the default browser.
Component: Session Restore → General
Product: Firefox → Firefox for Metro
Version: Trunk → 28 Branch
Whiteboard: [triage]
Whiteboard: [triage] → [triage] p=0
The browser can be used in this state, but is quite awkward to do so. If you select to Relaunch in Desktop it will open the same start page (see attachment), not the regular desktop mode.

After restarting FF, the desktop mode is correctly displayed and the pages that were opened before the crash are listed in the history. 
Also, the "Restore Previous Session" option from the History menu is grayed out and cannot be accessed.
Basically, you don't have an option to restore your session.
I don't know how this could happen. :) but we can try to reproduce. usually to get the desktop window you need to pass a special command line param to firefox.exe when launching it.
Tracking due to session loss, and poor user experience.
I wasn't able to reproduce on mc tip.
I can reproduce this in 28.0b1.  It happens only when restarting Firefox using the "Restart" button in the Crash Reporter dialog.  It does *not* happen if I start Firefox by launching it manually.
Summary: Metro interface is shown in a desktop window after crashing → Metro interface is shown in a desktop window after clicking "Restart" in desktop crash reporter dialog
I think we've tracked this down, patch forthcoming.
Attached patch fixSplinter Review
This bug stretches back a ways. When we first put metrofx together, we had separate app stubs for each browser. In the old stub we use to set the XUL_APP_FILE by default to the application ini. That code carried over when we merged the two app stubs -

http://mxr.mozilla.org/mozilla-central/source/browser/app/nsBrowserApp.cpp#301

This shouldn't be set here, there's no need for it. The crash reporter picks this up when it restarts the desktop browser after a crash, and we try to launch metrofx as an app.

One odd thing I've noticed about this that I can't explain. When we flip from metro to desktop, XUL_APP_FILE is set in metro startup. We *should* have seen this bug when first implemented app switching due to this code in nsBrowserApp -

http://mxr.mozilla.org/mozilla-central/source/browser/app/nsBrowserApp.cpp#180

Yet the variable isn't detected at this point. It is picked up once xul.dll loads up and the exception handler is set. We spawn a chain of processes to get from metro to desktop, which might play a role here.

Regardless, not setting XUL_APP_FILE in the first places fixes this bug. Weirdness in environment transfer with winrt apps can be investigated in a follow up bug.
Assignee: nobody → jmathies
Attachment #8371818 - Flags: review?(mbrubeck)
Whiteboard: [triage] p=0 → [triage] p=2
Status: NEW → ASSIGNED
Priority: -- → P1
QA Contact: jbecerra
Whiteboard: [triage] p=2 → p=2 s=it-30c-29a-28b.1
Target Milestone: --- → Firefox 28
Whiteboard: p=2 s=it-30c-29a-28b.1 → [beta28] p=2 s=it-30c-29a-28b.1
Two potential work arounds for beta 1 users:

A) if you crash after switching from metro to desktop, don't restart the browser from the crash reporter dialog, quit instead.

or if you have already restarted from the crash reporter and the browser isn't working right:

B) close the browser, then purge these files from your system to repair the damage:

1) delete file:   rootdrive/Users/(username)/Roaming/Mozilla/Firefox/Profiles/(profiledir)/sessionstore.js
2) delete folder: rootdrive/Users/(username)/Local/Mozilla/Firefox/Profiles/(profiledir)/startupCache
Attachment #8371818 - Flags: review?(mbrubeck) → review+
Let's get this uplifted before Monday's Beta 2 gtb.
https://hg.mozilla.org/mozilla-central/rev/ac822915b691
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Keywords: verifyme
Comment on attachment 8371818 [details] [diff] [review]
fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): long standing bug in metrofx app stub.
User impact if declined: desktop launch issues when restarting after a desktop crash.
Testing completed (on m-c, etc.): I've verified locally, but qa has yet to sign off.
Risk to taking this patch (and alternatives if risky): very low.
String or IDL/UUID changes made by this patch: none.
Attachment #8371818 - Flags: approval-mozilla-beta?
Attachment #8371818 - Flags: approval-mozilla-aurora?
Kamil, would you mind verifying this? The patch in in the latest mc nightly.
Flags: needinfo?(kamiljoz)
Whiteboard: [beta28] p=2 s=it-30c-29a-28b.1 → [beta28] p=2 s=it-30c-29a-28b.1 r=ff28
Triage drive-by, will wait for QA verify before approving for uplift.
Went through the following verification with the latest Nightly. Used the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-02-10-03-02-01-mozilla-central/

Before verification, reproduced the issue using the STR from comment #0 and comment #5. Used the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-02-06-03-02-01-mozilla-central/

- Ensured that the original issue didn't occur when following the STR from comment #0
- Crashed fxdesktop with several other combinations of tabs (1, 3, 5, 10, 15)
- Switched between fxmetro and fxdesktop many times and ensured it's correctly switching between the two environments
- Ensured that the user never loses the session and can restore without any issues
- Used both the "Restart", "Quite Firefox" and "X" without any issues (Couldn't reproduce the original issue)
- Used "Maximize" and "minimized" windows without any issues
- Used several configurations of snapped view while switching between environments and couldn't reproduce the original issue

I found Bug #970555 while testing this, doesn't seem to be related to this issue.
Flags: needinfo?(kamiljoz)
Status: RESOLVED → VERIFIED
Comment on attachment 8371818 [details] [diff] [review]
fix

QA has verified the patch. Ok for uplift!
Attachment #8371818 - Flags: approval-mozilla-beta?
Attachment #8371818 - Flags: approval-mozilla-beta+
Attachment #8371818 - Flags: approval-mozilla-aurora?
Attachment #8371818 - Flags: approval-mozilla-aurora+
Verified the fix on Firefox 28 beta 3 and on latest Aurora builds. Issue is no longer reproducing.
You need to log in before you can comment on or make changes to this bug.