Closed
Bug 1014338
Opened 11 years ago
Closed 11 years ago
Rework distribution processing to allow for delayed initialization
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 32
People
(Reporter: rnewman, Assigned: rnewman)
References
Details
Attachments
(4 files, 1 obsolete file)
2.48 KB,
patch
|
rnewman
:
review+
|
Details | Diff | Splinter Review |
7.84 KB,
patch
|
rnewman
:
review+
|
Details | Diff | Splinter Review |
17.05 KB,
patch
|
Margaret
:
review+
|
Details | Diff | Splinter Review |
7.85 KB,
patch
|
Margaret
:
review+
|
Details | Diff | Splinter Review |
Spinning this off from Bug 1013024.
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
Assignee | ||
Comment 3•11 years ago
|
||
This patch cleans up some of the distribution loading code, and provides a hook
point for callers to hang code that needs to run once the distribution has been
loaded.
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 8426707 [details] [diff] [review]
Pre: add ThreadUtils.assertNotOnUiThread.
Carrying forward Margaret's review from Bug 1013024.
Attachment #8426707 -
Flags: review+
Assignee | ||
Comment 5•11 years ago
|
||
Comment on attachment 8426708 [details] [diff] [review]
Part 0: move Distribution class into its own package.
Carrying forward Margaret's review from Bug 1013024.
Attachment #8426708 -
Flags: review+
Assignee | ||
Comment 6•11 years ago
|
||
Comment on attachment 8426709 [details] [diff] [review]
Part 1: rework distribution handling.
This is a reasonable amount of cleanup and reworking that I was doing as part of Bug 1013024, and as a prerequisite for the fixes in Bug 1013684 and friends. Figured it deserved its own patch, so here it is.
Attachment #8426709 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 7•11 years ago
|
||
Attachment #8426712 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 8•11 years ago
|
||
Now with restored imports.
Attachment #8426718 -
Flags: review?(margaret.leibovic)
Assignee | ||
Updated•11 years ago
|
Attachment #8426712 -
Attachment is obsolete: true
Attachment #8426712 -
Flags: review?(margaret.leibovic)
Comment 9•11 years ago
|
||
Comment on attachment 8426709 [details] [diff] [review]
Part 1: rework distribution handling.
Review of attachment 8426709 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM.
::: mobile/android/base/distribution/Distribution.java
@@ +253,5 @@
> +
> + // We try the APK, then the system directory.
> + final boolean distributionSet =
> + checkAPKDistribution() ||
> + checkSystemDistribution();
I like how your pulled these into helper methods.
Attachment #8426709 -
Flags: review?(margaret.leibovic) → review+
Comment 10•11 years ago
|
||
Comment on attachment 8426718 [details] [diff] [review]
Part 2: add delayed distro initialization hook. v2
Review of attachment 8426718 [details] [diff] [review]:
-----------------------------------------------------------------
::: mobile/android/base/distribution/Distribution.java
@@ +281,5 @@
>
> this.state = distributionSet ? STATE_SET : STATE_NONE;
> settings.edit().putInt(keyName, this.state).commit();
> +
> + runReadyQueue();
I was thinking it might make more sense to put one runReadyQueue() call after doInit() is called, but I realized it has multiple call sites, so this way is probably better, so that these calls are kept close together.
@@ +465,5 @@
> }
> +
> + /**
> + * The provided <code>Runnable</code> will be executed after the distribution
> + * is ready, or discarded if the distribution has already been processed.
Add a comment to let consumers know this Runnable will always be run on the background thread?
Attachment #8426718 -
Flags: review?(margaret.leibovic) → review+
Assignee | ||
Comment 11•11 years ago
|
||
Comment 12•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/207ea035d63e
https://hg.mozilla.org/mozilla-central/rev/90150f97a6de
https://hg.mozilla.org/mozilla-central/rev/5054df38fc14
https://hg.mozilla.org/mozilla-central/rev/74d77419ad61
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Assignee | ||
Comment 13•11 years ago
|
||
Follow-up to correct comment.
https://hg.mozilla.org/integration/fx-team/rev/498dc4b32c2c
Comment 14•11 years ago
|
||
Assignee | ||
Comment 15•10 years ago
|
||
Tested by uploading to Play. Even the dramatically delayed intent when installing over LTE works fine.
Assignee | ||
Comment 16•10 years ago
|
||
Oops. That was meant for Bug 1124492.
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•