Closed Bug 1200343 Opened 7 years ago Closed 7 years ago

Convert pref events to native calls

Categories

(Core Graveyard :: Widget: Android, defect)

All
Android
defect
Not set
normal

Tracking

(firefox44 fixed)

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: jchen, Assigned: jchen)

References

Details

Attachments

(4 files, 1 obsolete file)

No description provided.
This patch adds a specialization for jni::Ref<jni::ObjectArray>, which
includes members for getting the length of the array and accessing
array elements.
Attachment #8663011 - Flags: review?(snorp)
This patch adds two native calls to PrefsHelper to avoid using
GeckoEvent for getting prefs.
Attachment #8663012 - Flags: review?(snorp)
Now that we use native calls, we can remove the pref-related events from
GeckoEvent.
Attachment #8663013 - Flags: review?(snorp)
This patch implements the PrefsHelper native calls for getting prefs;
Attachment #8663014 - Flags: review?(snorp)
Comment on attachment 8663011 [details] [diff] [review]
Add JNI wrapper for object arrays (v1)

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

nice
Attachment #8663011 - Flags: review?(snorp) → review+
Comment on attachment 8663012 [details] [diff] [review]
Add native calls for pref events (v1)

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

Shouldn't the implementations of the new native calls be here somewhere?
Attachment #8663012 - Flags: review?(snorp) → review-
Attachment #8663013 - Flags: review?(snorp) → review+
Comment on attachment 8663014 [details] [diff] [review]
Implment PrefsHelper native calls (v1)

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

Looks good if you fix the one issue

::: widget/android/PrefsHelper.h
@@ +50,5 @@
> +        nsIAndroidBrowserApp* browserApp = nullptr;
> +        nsAppShell::gAppShell->GetBrowserApp(&browserApp);
> +        MOZ_ASSERT(browserApp);
> +
> +        if (observe) {

Looks like you have the logic or the bodies reversed here
Attachment #8663014 - Flags: review?(snorp) → review+
Comment on attachment 8663012 [details] [diff] [review]
Add native calls for pref events (v1)

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

I see you added the impl in a later patch, but I would just put it here, otherwise this patch is just busted on its own.
The problem was BaseTest.setPreferenceAndWaitForChange was calling the new
JNI pref methods before Gecko was ready. Blocking for Gecko ready should
fix the issue.
Attachment #8666770 - Flags: review+
Attachment #8663012 - Attachment is obsolete: true
Flags: needinfo?(nchen)
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.