Closed Bug 1182442 Opened 7 years ago Closed 7 years ago

Need a way to update distribution files post first install

Categories

(Firefox for Android Graveyard :: General, defect)

Product:
Firefox for Android Graveyard
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(p11+, fennec-)

Status:
RESOLVED WONTFIX
Tracking Flags:
Tracking Status
p11 + ---
fennec - ---

People

(Reporter: krudnitski, Unassigned)

Details

Attachments

(4 files)

MozReview Request: Bug 1182442 - (Part 1) Save version of distribution. r?
40 bytes, text/x-review-board-request
Details
MozReview Request: Bug 1182442 - (Part 2) Detect distribution updates. r?
40 bytes, text/x-review-board-request
Details
MozReview Request: Bug 1182442 - (Part 3) Update preferences from distribution. r?
40 bytes, text/x-review-board-request
Details
MozReview Request: Bug 1182442 - (Part 4) Update or insert bookmarks from distribution. r?
40 bytes, text/x-review-board-request
Details 
Reasoning:
Distribution files are unpacked once during first-run, then the browser ignores any new distribution files that may be bundled with an APK. The only way a user could get new distribution file settings would be to uninstall and re-install. However, there will be instances when a partner's search code may need changing or an add-on removed or replaced (such as an updated home panel with new static URLs). There could also be instances of bookmark URLs changing or wanting more granularity on metrics with a new Partner ID. However, some files should likely be ignored so as not to 'modify' an existing user's browser settings, unless some really crisp UX is inserted (but we could likely assess this at a later stage if deemed needed).

Requirement:
* Be able to over-write (at least some) distribution files post install (whether by OTA or by pre-installation)

* Updates to the following files would be seen as definitely required:
** Search provider (updating search code / favicon, not changing the user's default)
** Add-ons

* Updates to the following files would be seen as nice-to-have:
** Bookmarks
** Partner ID

* No update would be wanted (ie ignore these bits) for:
** First-run thumbnails
** Preferences

Putting this in a bug for discussion to figure out what's possible before turning this over for a break-down.
I tweaked the requirements a bit and included user stories in the Aha Feature Card: https://mozilla.aha.io/features/6169843846517514115
Be warned: some bits of this might get hairy.

So to take a step back: is bundling a replacement distro with a newer version of the APK the best way forward? Don't we expect most users to start with a distro APK then migrate onto a vanilla Play APK? Is this mechanism intended for OTA partner OS upgrades with bundled Firefox?

Would it be better to define an add-on based way of doing this? That'll be able to make changes for all users who ever started with that distro, not just ones that are willing to install another 35MB APK.

Feel free to grab some time on my calendar if you think it'll help to talk this through.
This is something we may try to target for 45.

Richard, let's find a time to get together with product to talk about the details of what's possible here.
tracking-fennec: --- → ?
Some further comments and honing in on actual requirements have been made in the Aha feature card here https://mozilla.aha.io/features/6169843846517514115. 

There is no preconceived notion on how best to implement this, provided it gives us the ability to modify / add certain components of the distribution when a distro is 'in the wild'. I think some of the funnel review should be spent discussing this one, if we could!
Karen - I want to push back on this bug. I don't think this is a priority and would rather WONTFIX this until we have a very explicit need. Changing the homepage can be done by the partner by just redirecting. I'd rather spend time working on other features.
Flags: needinfo?(krudnitski)
Scope has reduced a bit (and detailed in the Aha card as per comment 1).

Not worried about how it's done, provided there could be a mechanism. We need this for a vetted viable opportunity.
Flags: needinfo?(krudnitski)
Looking at the Aha card, we know that partners use PartnerBookmarksProvider [1] to get bookmarks to appear in Chrome. These seem to be loaded from a combination of resources and a flat file [2].

These can only be updated via a firmware update. No OTA updates here.

We could consider using a similar approach where we create dynamic bookmarks from a file in the system distribution folder. Those would be displayed in the application like normal bookmarks but could not be edited or deleted, sadly. They would be updated any time the system distribution folder is updated.

I would love to know if Chrome displays these bookmarks as top level bookmarks, or if they are shown in a folder.


[1] http://androidxref.com/6.0.0_r1/xref/packages/providers/PartnerBookmarksProvider/
[2] http://androidxref.com/6.0.0_r1/xref/packages/providers/PartnerBookmarksProvider/src/com/android/providers/partnerbookmarks/PartnerBookmarksProvider.java
Assignee: nobody → s.kaspari
tracking-fennec: ? → 45+
This is my current set of patches. I can update bookmarks and Android preferences for a distribution.

However I'm having a hard time to update favicons in a batch operation. I can't update existing favicons because they might be used by other distribution bookmarks (deduplication) or the new guessed(!) favicon URL is already used (unique constraint).

Even if I do not use batch operations and query/insert/update manually then I still have to handle somehow updating a favicon that might be used by multiple distribution bookmarks:

Bookmark A - Favicon X    ->   Bookmark A - Favicon K
Bookmark B - Favicon X    ->   Bookmark B - Favicon X

In this situation I can't update Favicon X because it's used by Bookmark B too. I can't insert Favicon K because the ContentProvider will guess a favicon URL and it will be the same like Favicon X -> UNIQUE constraint fails.
Flags: needinfo?(rnewman)
Flags: needinfo?(mark.finkle)
Flags: needinfo?(margaret.leibovic)
Actually a variation of this issue can already happen at first-run - without distribution updates:

Bookmark A: http://www.example.org/ - Favicon X
Bookmark B: http://www.example.org/news - Favicon Y

Inserting the favicons fail because for both favicons we guess the favicon URL to be http://www.example.org/ and run into the UNIQUE constraint failure. The bookmarks will be inserted but will have the default favicon.
(In reply to Sebastian Kaspari (:sebastian) from comment #13)
> because for both favicons we guess the favicon
> URL to be http://www.example.org/ and run into the UNIQUE constraint
> failure.

This should be http://www.example.org/favicon.ico of course.
https://reviewboard.mozilla.org/r/25961/#review23421

::: mobile/android/base/distribution/Distribution.java:948
(Diff revision 1)
>              settings = GeckoSharedPrefs.forApp(context);

Will it cause problems that we're currently saving distribution preferences per app instead of per profile?

In the short term, it probably won't cause any problems, but technically it's not correct, since we're doing things like writing the bookmarks into a DB in the profile.
https://reviewboard.mozilla.org/r/25965/#review23423

::: mobile/android/base/distribution/Distribution.java:475
(Diff revision 1)
> +                    update();

This could have problems if `runReadyQueue` depends on an updated shared preferences state, since we post a runnable in `update`. Probably not an issue for the homepage pref, but something to consider if we're supporting more in the future.
I was going to point you at testDistribution, but I don't see any current test coverage there for bookmarks... However, it probably wouldn't be that bad to add a testcase that does a DB query to see if the bookmarks ended up where they should.

I'm not sure how hard it would be to add a test for this update path, but it would certainly be nice to have one. Right now we include a .zip file with a dummy distribution directory in the test assets, and we use that to create a test distribution. Maybe we could add a second test asset, and point our update logic at that to see if it works.
Flags: needinfo?(margaret.leibovic)
(In reply to Sebastian Kaspari (:sebastian) from comment #12)
> This is my current set of patches. I can update bookmarks and Android
> preferences for a distribution.
> 
> However I'm having a hard time to update favicons in a batch operation. I
> can't update existing favicons because they might be used by other
> distribution bookmarks (deduplication) or the new guessed(!) favicon URL is
> already used (unique constraint).
> 
> Even if I do not use batch operations and query/insert/update manually then
> I still have to handle somehow updating a favicon that might be used by
> multiple distribution bookmarks:
> 
> Bookmark A - Favicon X    ->   Bookmark A - Favicon K
> Bookmark B - Favicon X    ->   Bookmark B - Favicon X
> 
> In this situation I can't update Favicon X because it's used by Bookmark B
> too. I can't insert Favicon K because the ContentProvider will guess a
> favicon URL and it will be the same like Favicon X -> UNIQUE constraint
> fails.

I would not worry about favicons. If the user visits the bookmark ever, I think (hope?) that we update the favicon. And in practice, I don't think the favicon is likely to change in an update anyway.

(In reply to Sebastian Kaspari (:sebastian) from comment #13)
> Actually a variation of this issue can already happen at first-run - without
> distribution updates:
> 
> Bookmark A: http://www.example.org/ - Favicon X
> Bookmark B: http://www.example.org/news - Favicon Y
> 
> Inserting the favicons fail because for both favicons we guess the favicon
> URL to be http://www.example.org/ and run into the UNIQUE constraint
> failure. The bookmarks will be inserted but will have the default favicon.

Let's file a separate bug for the favicon problems. We can land that patch separately, since it's less important to have that ready for uplift.
Status: NEW → ASSIGNED
Flags: needinfo?(rnewman)
Version: Firefox 41 → Trunk
NOTE: We might be pausing this work.
Flags: needinfo?(mark.finkle)
*finger hovers over WONTFIX*
On hold.
Assignee: s.kaspari → nobody
Status: ASSIGNED → NEW
This doesn't need to track anymore.

mfinkle, can we WONTFIX this?
tracking-fennec: 45+ → -
Flags: needinfo?(mark.finkle)
(In reply to :Margaret Leibovic from comment #22)
> This doesn't need to track anymore.
> 
> mfinkle, can we WONTFIX this?

Yes. Done.
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(mark.finkle)
Resolution: --- → WONTFIX
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.