[Single Variant] 3rd party apps are deleted after a factory reset

VERIFIED FIXED in Firefox 28, Firefox OS v1.2

Status

VERIFIED FIXED
5 years ago
a year ago

People

(Reporter: rafael.marquez, Assigned: carmen.jimenezcabezas)

Tracking

unspecified
1.2 C5(Nov22)
x86_64
Windows 7

Firefox Tracking Flags

(blocking-b2g:koi+, firefox26 wontfix, firefox27 wontfix, firefox28 verified, b2g-v1.2 verified)

Details

(Whiteboard: [systemsfe] )

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
****Steps
1. Installing a v1.2 build on device
2. Clone the tests repository for sigle variant: https://github.com/telefonicaid/firefoxos-gaia-testsbuild.git
3. Configuring the file variant.json
4. Clone Gaia repository and select the v1.2 branch.
5. Executing the command "GAIA_DISTRIBUTION_DIR=URL/firefoxos-gaia-testsbuild  PRODUCTION=1 make reset-gaia" to install gaia with single variant in the device
6. Complete FTE  with a movistar SIM (in this case the configured in the file .json) and verify that 3º party apps(Calculator, Poppit and Wikipedia for this case) are installed.
7. Launch a "factory reset" from Settings/Device Information/More Information

***Expected Result
After the launch the factory reset, 3º party apps continue applications 

***Actual Result
3rd party apps(Calculator, Poppit and Wikipedia) are deleted after a factory reset

Variant.json file:
{
  "apps": {
    "Twitter":
      {
        "origin": "https://mobile.twitter.com",
        "manifestURL": "https://mobile.twitter.com/cache/twitter.webapp",
        "installOrigin": "https://marketplace.firefox.com"
      },
    "Facebook":
      {
        "origin": "http://m.facebook.com",
        "manifestURL": "http://m.facebook.com/openwebapp/manifest.webapp",
        "installOrigin": "https://marketplace.firefox.com"
      },
    "Wikipedia":
      {
        "origin": "https://bits.wikimedia.org",
        "manifestURL": "https://bits.wikimedia.org/WikipediaMobileFirefoxOS/manifest.webapp",
        "installOrigin": "https://marketplace.firefox.com"
      },
    "Accuweather":
      {
        "origin": "http://m.accuweather.com",
        "manifestURL": "http://m.accuweather.com/mozilla.webapp",
        "installOrigin": "https://marketplace.firefox.com"
      },
    "Calculator":
      {
        "origin": "app://calculator",
        "installOrigin": "https://marketplace.firefox.com",
        "manifestURL": "https://marketplace.firefox.com/app/9f96ce77-5b2d-42ca-a0d9-10a933dd84c4/manifest.webapp"
      },
    "Poppit":
      {
        "origin": "app://poppit",
        "installOrigin": "https://marketplace.firefox.com",
        "manifestURL": "https://marketplace.firefox.com/app/d37cb5ed-525a-408e-a7cf-ec848064e041/manifest.webapp"
      }
  },
  "operators": [
    {
      "id": "movistar",
      "mcc-mnc": [
        "214-001",
        "214-002",
        "214-005",
        "214-007"
      ],
      "apps": [
        {
          "id": "Calculator",
          "screen": 1,
          "location": 2
        },
        {
          "id": "Poppit",
          "screen": 1,
          "location": 3
        },
        {
          "id": "Wikipedia",
          "screen": 2,
          "location": 2
        }
      ],
      "support_contacts": "resources/support_contacts_movistar.json",
      "default_contacts": "resources/contacts_movistar.json",
      "ringtone": "resources/Movistar_Mid_ABR_128kbps.ogg",
      "wallpaper": "resources/customize.jpg"
    },
    {
      "id": "mexico",
      "mcc-mnc": [
        "334-004"
      ],
      "apps": [
        {
          "id": "Twitter",
          "screen": 1,
          "location": 2
        },
        {
          "id": "Facebook",
          "screen": 1,
          "location": 3
        },
        {
          "id": "Accuweather",
          "screen": 2,
          "location": 3
        },
        {
          "id": "Wikipedia",
          "screen": 2,
          "location": 2
        }
      ],
      "ringtone": "resources/ringer_dream_theme.ogg",
      "wallpaper": "resources/customize2.png"
    }
  ]
}
(Reporter)

Updated

5 years ago
Blocks: 893807
blocking-b2g: --- → koi?
QA Contact: rafael.marquez
Whiteboard: [systemsfe]
Do we know what happens with 3rd party preinstalled apps in the old customization format in 1.1 when you do a factory reset?
Flags: needinfo?(rafael.marquez)
In version 1.1 there is no local 3rd party apps, it was added in version 1.2 for single variant
(In reply to Marcelino Veiga Tuimil [:sonmarce] from comment #2)
> In version 1.1 there is no local 3rd party apps, it was added in version 1.2
> for single variant

No that's not right. Local 3rd party apps have existed since 1.01. The SIM customization variant support was introduced in 1.2.
Well, maybe they have the same name, but different meaning. In version 1.2 we have local 3rd party apps per MCC/MNC, to be installed the first time a SIM card is inserted. This behavior did not exist in version 1.1.
(In reply to Marcelino Veiga Tuimil [:sonmarce] from comment #4)
> Well, maybe they have the same name, but different meaning. In version 1.2
> we have local 3rd party apps per MCC/MNC, to be installed the first time a
> SIM card is inserted. This behavior did not exist in version 1.1.

Well right - but what I'm trying to understand here is if this bug exists in the 1.1 implementation of preinstalled apps. That will effect whether we block on this or not.
(Assignee)

Comment 6

5 years ago
I don't think this is a preexisting problem and in fact I don't think this will affect at all apps that are not single variant. 

External (third party) global apps are preinstalled on the phone at build time. The single variant apps on the other hand are installed at runtime. 

In any case, it should work since after the reset the first time the device is booted the apps should be reinstalled. I can think of two reasons why this could be failing:

1. The reset is erasing the whole data directory. That would erase also the single variant staging directory and thus the single variant apps would be lost. 
2. The SIM has changed between the original boot and the reset.

The most probable cause is 1. I'm taking this and if that's the case will patch the reset so it doesn't erase the single variant directory.
Okay, thanks for the clarification then. Removing needinfo then - sounds like we know we need to get the factory reset use case fixed.
Flags: needinfo?(rafael.marquez)
(Assignee)

Comment 8

5 years ago
I've been checking the code and we have a problem: 
The reset code is native (non open source) code. Apparently what we're doing at is just loading an external library file which is the one that actually does the reset. And it seems to be erasing the whole /data partition. Thus, the single variant configuration directory is erased. 

A easy solution (that requires two patches, one on the build system and other on the Gecko code) would be to move the sv configuration directory to /system. The problem with that is that it won't be possible to fulfill the requisite of erasing the non used apps.

There's something else we can try, which is checking what do that external library actually does, and check if it's configurable somehow. 

Adding our product people to the loop also.
(In reply to Carmen Jimenez Cabezas from comment #8)
> I've been checking the code and we have a problem: 
> The reset code is native (non open source) code. Apparently what we're doing
> at is just loading an external library file which is the one that actually
> does the reset. And it seems to be erasing the whole /data partition. Thus,
> the single variant configuration directory is erased. 
> 
> A easy solution (that requires two patches, one on the build system and
> other on the Gecko code) would be to move the sv configuration directory to
> /system. The problem with that is that it won't be possible to fulfill the
> requisite of erasing the non used apps.

Why can't the non-used apps be erased?
(Assignee)

Updated

5 years ago
Assignee: nobody → cjc
(Assignee)

Comment 10

5 years ago
(In reply to Jason Smith [:jsmith] from comment #9)

> 
> Why can't the non-used apps be erased?

Because /system is read-only directory
Discussed offline - this isn't a blocker for release. The only requirement for preloading is that the apps are preloaded correctly during first run of the phone. There isn't a requirement that factory reset has to keep the preloaded apps. However, this use case does make sense to support, so this is a valid bug to consider getting fixed.
blocking-b2g: koi? → -

Comment 12

5 years ago
(In reply to Jason Smith [:jsmith] from comment #11)
> Discussed offline - this isn't a blocker for release. The only requirement
> for preloading is that the apps are preloaded correctly during first run of
> the phone. There isn't a requirement that factory reset has to keep the
> preloaded apps. However, this use case does make sense to support, so this
> is a valid bug to consider getting fixed.

Agree with Jason.
This is a blocker in all Telefonica countries. We cannot face the calls to the customer care service department if we do not change this behaviour.
blocking-b2g: - → koi?

Comment 14

5 years ago
David - will you provide information as to whether the current behavior of the factory reset will negatively impact users who have purchased apps?  In other words, will they loose access to apps they have purchased? (or wiill the receipts persist allowing the end user to use the apps they have already paid for?)
Flags: needinfo?(dbialer)
(In reply to Karen Ward [:kward] from comment #14)
> David - will you provide information as to whether the current behavior of
> the factory reset will negatively impact users who have purchased apps?  In
> other words, will they loose access to apps they have purchased? (or wiill
> the receipts persist allowing the end user to use the apps they have already
> paid for?)

This bug has no relation to paid apps - none of our preinstalled apps are paid.
For purchased apps through the marketplace, these should not be lost.  Using the same login that they used, they should see the app on their "My Apps" tab with an Install button.
Agree with Beatriz. All preloaded apps must remain after a factory reset.
Just to add one last comment - we also have contractual commitments with 3rd parties so there is a commercial implication here as well
Mentioned this in System FE Planning:

This needs a product call on what to do here.
Flags: needinfo?(pdolanjski)
Keywords: productwanted
Flags: needinfo?(dbialer)
I think we're going to have to block on this.  It does sound like a factory reset would indeed be expected to bring the device back to the state it was in when it left the factory, whereby completing the FTE after reset would result in the preloads being shown to the user based on the SIM.
blocking-b2g: koi? → koi+
Flags: needinfo?(pdolanjski)
(In reply to Peter Dolanjski [:pdol] from comment #20)
> I think we're going to have to block on this.  It does sound like a factory
> reset would indeed be expected to bring the device back to the state it was
> in when it left the factory, whereby completing the FTE after reset would
> result in the preloads being shown to the user based on the SIM.
Thanks a lot Peter for your good understanding.
Target Milestone: --- → 1.3 Sprint 5 - 11/22
Target Milestone: 1.3 Sprint 5 - 11/22 → 1.2 C5(Nov22)

Updated

5 years ago
Keywords: productwanted

Updated

5 years ago
Blocks: 939342

Updated

5 years ago
No longer blocks: 939342
Gregor,

Do we have an update here? Is SystemFE team looking at this?
Flags: needinfo?(anygregor)
Carmen,

What are the next steps here? What would be the ETA for the fix?
Flags: needinfo?(cjc)
Flags: needinfo?(anygregor) → needinfo?(marce)
Carmen already started to work on it, she will give more feedback today
Flags: needinfo?(marce)
(Assignee)

Comment 25

5 years ago
Sorry for the delay, I going to upload a patch for this bug tomorrow.
Flags: needinfo?(cjc)
(Assignee)

Comment 26

5 years ago
Created attachment 8336171 [details] [diff] [review]
Proposed patch v1

The present bug solution is that we move the singleVariant files to a partition which it is not formatted in factory reset proccess and where we have write permisions.
With the objective of affecting as little as possible the build proccess, the SingleVariant directory will be initially instaled on the data partition.
The first time the phone boot, it will look up in the persist partition the singlevariantconf.json file. If it doesn't exist then it will search for the singlevariant apps in /data/local and if it exists, it will move the singlevariant dir to the persist partition. After that it will install the apps from the persist partition.

The persist partition will be configured in a preference.
Attachment #8336171 - Flags: review?(fabrice)
Comment on attachment 8336171 [details] [diff] [review]
Proposed patch v1

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

::: b2g/app/b2g.js
@@ +435,5 @@
>  #endif
>  
> +#ifdef MOZ_B2G_RIL
> +// SingleVariant
> +pref("dom.webapps.singleVariantSourceDir", "/persist/svoperapps");

Nit: we use snake_case in preferences, so single_variant_sourcedir

::: dom/apps/src/OperatorApps.jsm
@@ +140,5 @@
> +        }
> +      }
> +    } catch (e) {
> +      debug("Error copying " + aOrg.path + " to " + aDst.path + ". " + e);
> +    }

Please rewrite this using https://developer.mozilla.org/en-US/docs/JavaScript_OS.File/OS.File.DirectoryIterator_for_the_main_thread
Attachment #8336171 - Flags: review?(fabrice)
Status: NEW → ASSIGNED
(Assignee)

Comment 28

5 years ago
I don't make the copy proccess asyncronous intentionally because this proccess will only run in the first boot of the telephone. The move proccess won't even be executed on factory resets. And I want to ensure that the procces has finished before continuing because the sv apps installation proccess must ever start after the apps are in the correct location.
(Assignee)

Updated

5 years ago
Flags: needinfo?(fabrice)
(In reply to Carmen Jimenez Cabezas from comment #28)
> I don't make the copy proccess asyncronous intentionally because this
> proccess will only run in the first boot of the telephone. The move proccess
> won't even be executed on factory resets. And I want to ensure that the
> procces has finished before continuing because the sv apps installation
> proccess must ever start after the apps are in the correct location.

You can wrap the OS.File calls with a Task to get this behavior. That would be much better than doing main thread i/o.
Flags: needinfo?(fabrice)
(Assignee)

Comment 30

5 years ago
Created attachment 8339262 [details] [diff] [review]
Proposed patch v2

changes requested
Attachment #8336171 - Attachment is obsolete: true
Attachment #8339262 - Flags: review?(fabrice)
Comment on attachment 8339262 [details] [diff] [review]
Proposed patch v2

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

r=me with comments addressed

::: b2g/app/b2g.js
@@ +440,5 @@
>  #endif
>  
> +#ifdef MOZ_B2G_RIL
> +// SingleVariant
> +pref("dom.webapps.single_variant_sourcedir", "/persist/svoperapps");

we use dom.mozApps. as a prefix in preferences for this code.

::: dom/apps/src/OperatorApps.jsm
@@ +40,4 @@
>  const SINGLE_VARIANT_SOURCE_DIR = "svoperapps";
>  const SINGLE_VARIANT_CONF_FILE  = "singlevariantconf.json";
>  const PREF_FIRST_RUN_WITH_SIM   = "dom.webapps.firstRunWithSIM";
> +const PREF_SINGLE_VARIANT_DIR   = "dom.webapps.single_variant_sourcedir";

Update the pref name here to.

@@ +131,5 @@
> +  _copyDirectory: function(aOrg, aDst) {
> +    return Task.spawn(function() {
> +      try {
> +        let orgInfo = yield File.stat(aOrg);
> +        if (!aDst || !orgInfo.isDir) {

nit: you could return earlier if (!aDst).

@@ +171,5 @@
> +  },
> +
> +  _initializeSourceDir: function() {
> +    return Task.spawn(function() {
> +      var svFinalDirName;

s/var/let

@@ +191,5 @@
> +        yield File.makeDir(svFinalDirName, {ignoreExisting: true});
> +      }
> +
> +      let existsSvIndex = yield File.exists(OS.Path.join(svFinalDirName,
> +                                                   SINGLE_VARIANT_CONF_FILE));

nit: align SINGLE_VARIANT_CONF_FILE with OS.Path

@@ +200,5 @@
> +        debug("removing directory:" + svSourceDirName);
> +        File.removeDir(svSourceDirName, {
> +          ignoreAbsent: true,
> +          ignorePermissions: true
> +        });

don't you want to yield File.removeDir ?
Attachment #8339262 - Flags: review?(fabrice) → review+
(Assignee)

Comment 32

5 years ago
Created attachment 8339589 [details] [diff] [review]
Proposed patch v3

r=fabrice

I've made all the requested changes except for this one:

>@@ +200,5 @@
>> +        debug("removing directory:" + svSourceDirName);
>> +        File.removeDir(svSourceDirName, {
>> +          ignoreAbsent: true,
>> +          ignorePermissions: true
>> +        });

> don't you want to yield File.removeDir ?

I have omitted "yield" in this line because we don't need a synchronous removal of the source dir
Attachment #8339262 - Attachment is obsolete: true
Attachment #8339589 - Flags: review+
(Assignee)

Comment 34

5 years ago
Try looks good, requesting checkin
Keywords: checkin-needed
(Assignee)

Comment 35

5 years ago
Created attachment 8340818 [details] [diff] [review]
Proposed patch v3  B2G26_v1.2 version

r=fabrice

This is the b2g26 version. The other one is the mozilla central version and it doesn't apply directly to b2g26
Attachment #8340818 - Flags: review+

Updated

5 years ago
Component: Gaia::Homescreen → DOM: Apps
Product: Firefox OS → Core
https://hg.mozilla.org/mozilla-central/rev/67db0f727b94
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Keywords: verifyme
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/906c3568b844
status-b2g-v1.2: --- → fixed
status-firefox26: --- → wontfix
status-firefox27: --- → wontfix
status-firefox28: --- → fixed
(Reporter)

Comment 39

5 years ago
Verified:
Device -> Unagi
Branch -> v1.2
Gecko -> 4df52ee
Gaia -> 8d762f3
Status: RESOLVED → VERIFIED

Updated

5 years ago
Keywords: verifyme
status-b2g-v1.2: fixed → verified
status-firefox28: fixed → verified

Updated

a year ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.