1040146 - [1.3 => 2.0] 2.0 tutorial is not displayed after an OTA update

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::First Time Experience, defect)

Description

[Johan Lorenzo [:jlorenzo]]

STR, follow the QA 1.3 to 2.0 workflow:
1) Flash OEM v122
2) Install v1.3 Gaia on top
git clone -b v1.3 https://github.com/mozilla-b2g/gaia.git v1.3_gaia
MOZILLA_OFFICIAL=1 NOFTU=0 make production
3) from the TWQA B2G-flash-tool repo run :
./change_channel.sh -v aurora
./change_ota_url.sh -u https://aus4.mozilla.org/update/3/B2G/28.0/20140709000201/flame/en-US/aurora/Boot2Gecko%202.0.0.0-prerelease/default/default/update.xml?force=1
4) Make you homescreen layout as displayed in attachment 8457843 [details] (bug 1037556)
5) Turn on Wifi
6) Open Settings > Device Information
7) Select 'Check for updates'
8) When update found select the notifications menu
9) Download and install the update

Expected result
The FTE tutorial should show up, explaining the Vertical Homescreen features

Actual result
The homescreen is immediately displayed. The problem is, if the FTE tutorial is not run, the homescreen is not migrated to the new vertical homescreen. As a result, you'll get the default vertical homescreen. 

Reproducibility rate: 4/4

Comment 1

[Johan Lorenzo [:jlorenzo]]

QA wanted to see if there is something wrong with the STR or if it's actually a regression since bug 1037556.

Comment 2

[Martin Shuman [:Marty]]

Attachment: logcat.txt

Comment 3

[Martin Shuman [:Marty]]

Attached logcat of the OTA

Environmental Variables post OTA:
Device: Flame 2.0
Build ID: 20140717000201
Gaia: aa4f795b81c6147d67c4f06009e166debcf8856e
Gecko: 0ec0b9ac39f0
Version: 32.0a2 (2.0)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 4

[Peter Bylenga [:PBylenga]]

We are able to confirm this issue, is a smoketest blocker. Note for the qawanted request, in order to do a regression window against 2.0 builds you have to use MAR files.  The OTA server will always return the latest 2.0 nightly build.

Comment 5

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I can't recall; I think we weren't able to get this in on time for 2.0.  It's in for 2.1

Pinging UX team to confirm.

Comment 6

[Stephany Wilkes]

Flagging Jacq to check status.

Comment 7

[Sam Foster [:sfoster] (he/him)]

After an upgrade, as the gaia system app bootstraps, the FTULauncher should have its retrieve method called.  On detection of an upgrade the FTULauncher loads the FTU app. The FTU app checks the version delta and if necessary shows the tutorial with the appropriate tutorial steps. So the tutorial is the last thing that happens, and it not being displayed simply means any one of the steps up to that have failed.

In bug 1038630 there's a separate issue that might compound this one, where the tutorial gets init'd twice. But the symptoms there are that the tutorial plays all steps not the select ones. I'm not sure how to reconcile the STR of that bug which indicate the upgrade/FTU process is basically working, with this one where they are apparently not. Johan, I think you filed both, what's the deal there?

Comment 8

[Johan Lorenzo [:jlorenzo]]

FTU tutorial is working when you upgrade from 1.4 to 2.0. This allowed to file bug 1038630. Because of this current issue, we're not able to to test bug 1038630 from 1.3 to 2.0. Does it make more sense?

Comment 9

[Jacqueline Savory [:jsavory] UX]

Hi Naoki, I believe that the update tutorial is still currently planned for 2.0.

Comment 10

[Sam Foster [:sfoster] (he/him)]

If I'm reading this right, the change to gecko's UpdatePrompt.js to write the deviceinfo.previous_os setting squeaked into 1.4: https://hg.mozilla.org/mozilla-central/rev/f646b79cf1e0, prior to that we'll just get undefined. 

In https://github.com/mozilla-b2g/gaia/blob/v2.0/shared/js/version_helper.js#L29 we explicitly treat lack of info about the previous version as *not* an upgrade. This bug appears to be the expected result of that. As deviceinfo.previous_os is only set by the UpdatePrompt, we would otherwise launch the FTU tutorial everytime gaia starts. In other words, the tutorial is explicitly opt-in. Remember we need to override the noftu value as the point is that we have *new* info to show since the FTU was last run.  

I'm all ears for ideas on how to solve this conundrum :)

Comment 11

[[:fabrice] Fabrice Desré]

UpdatePrompt.js is not the only place where you can change previous_os. Just grant settings write permission to the FTU and do something like:

if (previous_os == undefined || previous_os != CURRENT_VERSION) {
  runTutorial();
  previous_os = CURRENT_VERSION
}

Comment 12

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I did some testing and 1.3 => 2.1 also exhibits this issue where the FTE does not appear about the vertical homescreen.

Comment 13

[Sam Foster [:sfoster] (he/him)]

The problem with (previous_os == undefined || previous_os != CURRENT_VERSION) is that running a fresh profile with NOFTU=1 will be caught be this, which affects developers and I assume integration tests too.
 
Ideally, I'd write to the setting from some script that runs during the upgrade. I don't really have a good sense of what an upgrade entails - is there a hook for something like this to happen before we restart b2g? After the restart it seems difficult to reestablish this context and know what we just got upgraded from. This is the problem the original developer was trying to solve by adding this code to UpdatePrompt.js, but it misses all the cases where we are upgrading from a version before his change.

Comment 14

[Sam Foster [:sfoster] (he/him)]

from #gaia, fabrice suggests setting previous_os to the current os in build/settings.js when noftu !== 0. I think that gets us out of this corner and should work in all cases. I'll get a patch put together and running on gaia-try

Comment 15

[Kevin Grandon :kgrandon]

I don't think this is related to the vertical homescreen, so unblocking for now. I think we should be good here as we're already blocking 2.0+.

Comment 16

[Joshua Mitchell (Inactive)]

comment 14 indicates a cause is already known and a patch is in the works, removing qa-wanted keywords as a regression window should no longer be necessary.

Comment 17

[Sam Foster [:sfoster] (he/him)]

gaia/build/settings.js simply copies over the common-settings.json, so deviceinfo.os is an empty string at that point. It seems that b2g/chrome/settings.js is where that setting gets populated. And ISTM that this might be a better place to mark the upgrade for gaia by setting deviceinfo.previous_os if a deviceinfo.os exists already and doesn't match the value of '@MOZ_B2G_VERSION@'. As the newly upgraded client starts up, chrome/settings.js can populate deviceinfo.previous_os with the value returned by window.navigator.mozSettings.createLock().get('deviceinfo.os'). Back in gaia, when the settings app loads the VersionHelper, it finds both settings already populated and can give the right isUpgrade result to the FTULauncher and other callers. 

Does this plan sound viable? This is to fix a 2.0 blocker - is it managable risk? I'm making the assumption that deviceinfo.os will have a value pre- 1.4. And I'm not sure of the implications if any of adding that extra mozSettings read into the DeviceInfoToSettings function in chrome/settings.js

Comment 18

[[:fabrice] Fabrice Desré]

(In reply to Sam Foster [:sfoster] from comment #17)
> gaia/build/settings.js simply copies over the common-settings.json, so
> deviceinfo.os is an empty string at that point. It seems that
> b2g/chrome/settings.js is where that setting gets populated. And ISTM that
> this might be a better place to mark the upgrade for gaia by setting
> deviceinfo.previous_os if a deviceinfo.os exists already and doesn't match
> the value of '@MOZ_B2G_VERSION@'. As the newly upgraded client starts up,
> chrome/settings.js can populate deviceinfo.previous_os with the value
> returned by window.navigator.mozSettings.createLock().get('deviceinfo.os').
> Back in gaia, when the settings app loads the VersionHelper, it finds both
> settings already populated and can give the right isUpgrade result to the
> FTULauncher and other callers. 
> 
> Does this plan sound viable? This is to fix a 2.0 blocker - is it managable
> risk? I'm making the assumption that deviceinfo.os will have a value pre-
> 1.4. And I'm not sure of the implications if any of adding that extra
> mozSettings read into the DeviceInfoToSettings function in chrome/settings.js

That looks fine to me. Just make sure we don't race by launching the ftu before we update the settings.

Comment 19

[Sam Foster [:sfoster] (he/him)]

> That looks fine to me. Just make sure we don't race by launching the ftu
> before we update the settings.

I'm not sure how to accomplish that. On the face of it at least we already have a potential race-condition in there as the population of settings is async. I have to get up to speed though on the sequence of events here though and what loads what, when. I'll attach my WIP patch for reference.

Comment 20

[Sam Foster [:sfoster] (he/him)]

Attachment: Copy previous deviceinfo.os into deviceinfo.previous_os

Comment 21

[Sam Foster [:sfoster] (he/him)]

Attachment: Copy previous deviceinfo.os into deviceinfo.previous_os

Taking the approach discussed in earlier comments on this bug. As I understand it, by reusing the settings lock we should avoid race conditions as system and and FTU apps load and attempt to read the deviceinfo settings to determine if an upgrade has happened. 

This approach also makes manual testing easy: you simply give deviceinfo.os whatever value and restart b2g.

Comment 22

[[:fabrice] Fabrice Desré]

Comment on attachment 8462925 [details] [diff] [review]
Copy previous deviceinfo.os into deviceinfo.previous_os

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

lgtm, thanks Sam! Can you update the commit message of your patch with the bug number and r=fabrice, and set checkin-needed?

Comment 23

[Sam Foster [:sfoster] (he/him)]

Attachment: Copy previous deviceinfo.os into deviceinfo.previous_os

Carrying :fabrice' r+. Just adding bug number and r= to commit message.

Comment 24

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/integration/b2g-inbound/rev/404409aada71

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/404409aada71

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/4bf6c2d3512d

Comment 27

[Josh Schmitt [Joshs]]

Issue no longer repros on 2.0, the FTU tutorial is now shown after OTA.

Environmental Variables:
Device: Flame 2.0
Build ID: 20140729000201
Gaia: b11775fcbfe076a3fc560c2041f5b2fe1b345009
Gecko: 86b56e101512
Version: 32.0 (2.0)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 28

[Josh Schmitt [Joshs]]

Issue no longer repros 1.3 to 2.1 the FTU tutorial is now shown after OTA.

Environmental Variables:
Device: Flame Master
Build ID: 20140729040211
Gaia: fadfafa17f5175203b8b9457bfb95e5816f54f58
Gecko: b17cad2d1e5e
Version: 34.0a1 (Master)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0