Closed Bug 991160 Opened 6 years ago Closed 6 years ago

Run GeckoSharedPrefs migrations in the current thread

Categories

(Firefox for Android :: Awesomescreen, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 31

People

(Reporter: lucasr, Assigned: lucasr)

References

Details

Attachments

(1 file)

While still avoiding StrictMode warnings.
Attachment #8400748 - Flags: review?(rnewman)
Comment on attachment 8400748 [details] [diff] [review]
Run GeckoSharedPrefs migrations in the current thread (r=rnewman)

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

If it works, lgtm!

::: mobile/android/base/GeckoSharedPrefs.java
@@ +11,5 @@
>  import android.content.SharedPreferences;
>  import android.content.SharedPreferences.Editor;
>  import android.os.Build;
> +import android.os.StrictMode;
> +import android.os.SystemClock;

Dead import.

@@ +152,5 @@
>          if (migrationDone) {
>              return;
>          }
>  
> +        if (Build.VERSION.SDK_INT < 9) {

Add a comment before this block explaining that we migrate preferences on the main thread deliberately, because the total work time is insignificant when compared to context switching and runnable expenses.

@@ +157,3 @@
>              performMigration(context);
>          } else {
> +            // Avoid a strict mode warnings

Plural agreement, trailing period.
Attachment #8400748 - Flags: review?(rnewman) → review+
(In reply to Richard Newman [:rnewman] from comment #2)
> Comment on attachment 8400748 [details] [diff] [review]
> Run GeckoSharedPrefs migrations in the current thread (r=rnewman)
> 
> Review of attachment 8400748 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> If it works, lgtm!
> 
> ::: mobile/android/base/GeckoSharedPrefs.java
> @@ +11,5 @@
> >  import android.content.SharedPreferences;
> >  import android.content.SharedPreferences.Editor;
> >  import android.os.Build;
> > +import android.os.StrictMode;
> > +import android.os.SystemClock;
> 
> Dead import.

Fixed.

> @@ +152,5 @@
> >          if (migrationDone) {
> >              return;
> >          }
> >  
> > +        if (Build.VERSION.SDK_INT < 9) {
> 
> Add a comment before this block explaining that we migrate preferences on
> the main thread deliberately, because the total work time is insignificant
> when compared to context switching and runnable expenses.

Good idea, done.

> @@ +157,3 @@
> >              performMigration(context);
> >          } else {
> > +            // Avoid a strict mode warnings
> 
> Plural agreement, trailing period.

Fixed.
https://hg.mozilla.org/mozilla-central/rev/f8ff846a6396
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
You need to log in before you can comment on or make changes to this bug.