843248 - Upgrading with maintenance service from a non metro build to a metro build does not show tile

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: Shell, defect)

Description

[Kamil Jozwiak [:kjozwiak]]

1. Install a fresh Windows 8 installation
2. Install this build which does not have Metro enabled.
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-02-08-03-10-53-mozilla-central/firefox-21.0a1.en-US.win32.installer.exe
3. Set the browser as your default.
4. Open your about box and perform an update. 

Actual results:
Start screen has a firefox icon until you reboot

Expected results:
Firefox tile shows right away

If you don't use the maintenance service for the upgrade everything works like it should.

This bug only happens if you are upgrading with the maintenance service.

Comment 1

[Brian R. Bondy [:bbondy]]

Reproduced as you said only when the maintenance service is used.  Since updates are hard to test before it lands I was previously only testing by running helper.exe /PostUpdate once. 

My best guess is that it's caused by helper.exe being run twice for maintenance service upgrades. 

I verified this with 2 small test programs. 

The first opens up the property store for the shortcut with:
SHGetPropertyStoreFromParsingName
And leaves it open.

The second tries to open up the property store again but fails because it's already opened.

I think the fix is to not run this code when helper.exe is being run from the service I.e. when the session ID is 0.

Comment 2

[Brian R. Bondy [:bbondy]]

Blocking the story Marco since upgrading with the service is the default. What we'd end up with is a ton of users who should get the metro browser when we finally release it, but they'd have to wait an additional 6 weeks to see the browser.

Comment 3

[Brian R. Bondy [:bbondy]]

Ran procmon and did confirm helper.exe is running under session 1.  So race condition is likely.

Comment 4

[Marco Mucci [:MarcoM]]

Hi Brian.  Thanks for the clarification.

Comment 5

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

Comment 6

[Brian R. Bondy [:bbondy]]

Pushed here:
https://hg.mozilla.org/mozilla-central/rev/d57a813c77a4

Hard to test this without a build though so may need to be re-opened. Took a guess at what I thought the problem was.

Comment 7

[Brian R. Bondy [:bbondy]]

I'm still getting this on a fresh VM so unfortunately it wasn't because of a race condition on ApplicationID::Set.  I'll try to find a better way to test this other than running helper.exe /PostUpdate

Comment 8

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1 - always get session ID

OK I found a better way to test this by replacing the helper.exe after the update is staged inside the <installdir>/updated.  

I found out the cause of the failure. 
The problem is that with session 0 $9 is never set to the session ID, so it remains its default value which is "".

The check below does "${If} $9 != 0 then install the CEH" and "" != 0 with NSIS so it goes in there with session 0 when it shouldn't.

I verified that always calculating $9 fixes the problem.

Comment 9

[Brian R. Bondy [:bbondy]]

By the way I tried removing the previous fixes but they are all still needed.

Comment 10

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/mozilla-central/rev/7f95f7e03542

Comment 11

[Kamil Jozwiak [:kjozwiak]]

Went through the following on a fresh Windows 8 x86 VM created in VirtualBox

Downloaded the following build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-02-08-03-10-53-mozilla-central/firefox-21.0a1.en-US.win32.installer.exe

- Checked and ensured that the Firefox Nightly icon was being used
- Selected Help -> About Nightly and updated to the latest version of Firefox Nightly (21.0a1 2013-02-22)
- Checked and ensured that the Firefox Metro tile was being used (without needing to restart as before)
- Double checked and made sure Firefox launches in Metro and in Desktop mode

Comment 12

[Brian R. Bondy [:bbondy]]

\o/

Finally worked :) Thanks!
Works for me too on my VM with the same build.