Closed Bug 843821 Opened 8 years ago Closed 8 years ago

Support reading distribution resources from a /system location

Categories

(Firefox for Android :: General, defect)

15 Branch
x86_64
Linux
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 22
Tracking Status
firefox19 --- verified
firefox20 + verified
firefox21 + verified
firefox22 --- verified

People

(Reporter: mfinkle, Assigned: Margaret)

References

Details

Attachments

(1 file)

In order to better support OEMs that bundle Firefox on the ROM, we want to store the customization/distribution resources in a location that is protected from deletion or removal-via-updates.

Brad and I have been looking at various shipping devices and resources online about Android's file system layout. It seems like the /system folder is used as a readonly location for storing bundled APKs. We have also seen vendors (like Motorola, HTC and Samsung) store customization resources in the /system folder.

Therefore we propose to use this location for distribution resources: /system/org.mozilla.firefox/distribution

Where "org.mozilla.firefox" would match the Firefox APK packagename.

This means each channel would look in a slightly different place for distribution files. We don't see this as a problem since we only ever expect "org.mozilla.firefox" to be used and only the release version of Firefox would use the resources.

This also means that Firefox Beta could be installed on the device and it would NOT use the distribution resources meant for "org.mozilla.firefox" (Firefox Release).

The general rules for looking for distribution resources would be:
1. Look in APK and extract resources to /data/data/org.mozilla.xxx/distribution folder
2. Use resources in the /data/data/org.mozilla.xxx/distribution folder if it exists
3. Use resources in the /system/org.mozilla.xxx/distribution folder if it exists

Snorp mentioned a way to manually add files to the /system folder, in order to test the code.
Since we send "Distribution:Set" based on the SharedPreferences value, I feel like it would make more sense to just tweak the logic in copyFiles() to look in /system if if doesn't find anything in the APK. This would mean the files would still get copied over to /data and used from there, but I don't think that really matters, since this ROM solution is to address concerns that the APK might be overwritten before first run, right?

Otherwise, I feel like a patch for this will need to introduce a lot more new logic.
Or, we could have a new STATE_SYSTEM or something like that if we see the files in the /system location, and then pass that info along to gecko so that browser.js looks in there for the files.
(In reply to Mark Finkle (:mfinkle) from comment #0)

> Snorp mentioned a way to manually add files to the /system folder, in order
> to test the code.

snorp, how do I do this?
Attached patch patchSplinter Review
This patch lets Java still be in charge of telling Gecko when (and now where) to set up distribution stuff.

If we ever want to pave over a system distribution with one in the APK we'll still have to add some way to reset these prefs, but we'll have to do that anyway if we ever want to support changing distributions.
Attachment #716851 - Flags: feedback?(mark.finkle)
(In reply to :Margaret Leibovic from comment #3)
> (In reply to Mark Finkle (:mfinkle) from comment #0)
> 
> > Snorp mentioned a way to manually add files to the /system folder, in order
> > to test the code.
> 
> snorp, how do I do this?

You need a rooted device. It's easy-ish to root a Nexus device if you need to.

After that you can just run 'mount -o remount,rw /system' as root in adb shell.
Comment on attachment 716851 [details] [diff] [review]
patch

Very nice and clean. Since bookmarks uses Distribution.java to access the default bookmarks, this should handle all scenarios.

Let's see if you can test this locally using snorp's hint
Attachment #716851 - Flags: feedback?(mark.finkle) → feedback+
Comment on attachment 716851 [details] [diff] [review]
patch

I put distribution files at /system/org.mozilla.fennec_leibovic/distribution, and this used them successfully!

I also tested with a distribution folder in the APK, and it preferred using that over the distribution folder in /system.

I mentioned this on IRC, but this will require a follow-up for bookmarks to also get pulled out of a /system location, but that doesn't stop us from landing this.
Attachment #716851 - Flags: review?(mark.finkle)
Attachment #716851 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/3c11909c216a
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
Comment on attachment 716851 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 834681
User impact if declined: no distribution support (we want to uplift this so that partners can use it sooner rather than later)
Testing completed (on m-c, etc.): just landed on m-c
Risk to taking this patch (and alternatives if risky): low-risk, this code is pretty self-contained (it will only affect builds using distribution customizations)
String or UUID changes made by this patch: n/a
Attachment #716851 - Attachment description: WIP → patch
Attachment #716851 - Flags: approval-mozilla-beta?
Attachment #716851 - Flags: approval-mozilla-aurora?
(In reply to :Margaret Leibovic from comment #10)
> User impact if declined: no distribution support (we want to uplift this so
> that partners can use it sooner rather than later)

Do you have any more context around distribution support in the FF20 timeframe? If necessary, we'll definitely approve once this has been in a nightly.
Oops, noticed I accidentally left some logging in. Got rid of that:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b2c38a3b59dc
Attachment #716851 - Flags: approval-mozilla-beta?
Attachment #716851 - Flags: approval-mozilla-beta+
Attachment #716851 - Flags: approval-mozilla-aurora?
Attachment #716851 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-beta/rev/bdba94d329cb

There was a merge conflict on aurora, so I want to build to make sure it's all good before I push (I built and tested beta to verify a good merge).
Attachment #716851 - Flags: approval-mozilla-release+
Kevin, can you verify this one?
QA Contact: kbrosnan
We can verify on a device other than the vendor hardware. I think Mozilla China or Taiwan has access to the devices. Might prefer a verification on such hardware.
(In reply to Kevin Brosnan [:kbrosnan] from comment #18)
> We can verify on a device other than the vendor hardware. I think Mozilla
> China or Taiwan has access to the devices. Might prefer a verification on
> such hardware.

Hi Kevin, we tested some devices on Beijing and Taipei office, it works fine on 19.0.2. The testing devices are:
Series 1(SmartPhone Google Nexus One)
CPU:Qualcomm QSD8250 1GMHz
SIM:DSDA W+G
Dimension:119 x 59.8 x 11.5 mm
Memory:512M ROM+ 512M RAM
Display:3.7”
OS:Android 2.3.7

Series 2(Pad: Galaxy Tab GT-P7510)
CPU:Nvidia Tegra 2 
Dimension:256.7 x 175.3 x 8.6 mm
Memory:16G ROM+ 1G DDRII RAM
Display:10.1"
OS:Android 3.1
Launch time:2011/June

Series 3(Pad: Google Nexus 7)
CPU:Nvidia Tegra 3 1.3GHz 
Dimension:198.5 x 120 x 10.45 mm
Memory:8G ROM+ 1G RAM
Display:7"
OS:Android 4.2.2
Launch time:2012/Q4

Series 4(Pad: ASUS Transformer Pad TF300T)
CPU:Nvidia Tegra 3 
Dimension:263 x 180.8 x 9.9 mm
Memory:32G ROM+ 1G RAM
Display:10.1"
OS:Android 4.1.1
Launch time:2012/Q4

The Gigabyte tested on their hardware, too. All of the devices are worked fine.
Confirms what I have seen using my HTC G2. Going to call this verified across channels.
Status: RESOLVED → VERIFIED
Flags: in-moztrap?
Flags: in-moztrap? → in-moztrap?(kbrosnan)
Depends on: 872737
Flags: in-moztrap?(kbrosnan)
You need to log in before you can comment on or make changes to this bug.