Closed Bug 851741 Opened 7 years ago Closed 7 years ago

Contacts API: make continue async

Categories

(Core :: DOM: Device Interfaces, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22
blocking-b2g tef+
Tracking Status
firefox20 --- wontfix
firefox21 --- wontfix
firefox22 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed

People

(Reporter: gwagner, Assigned: gwagner)

References

Details

(Whiteboard: [FFOS_perf])

Attachments

(2 files)

No description provided.
blocking-b2g: --- → tef?
Blocks: 848316
Blocks: Contacts-Startup
No longer blocks: 848316
Blocks a blocker
blocking-b2g: tef? → tef+
Attached patch patchSplinter Review
Assignee: nobody → anygregor
Attachment #725675 - Flags: review?(bent.mozilla)
Comment on attachment 725675 [details] [diff] [review]
patch

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

Yay!

::: dom/contacts/ContactManager.js
@@ +637,5 @@
>      this.askPermission("find", cursor, allowCallback);
>      return cursor;
>    },
>  
> +  nextTick: function nextTick(aCallback) {

Nit: The others use CM_ for their name.
Attachment #725675 - Flags: review?(bent.mozilla) → review+
Comment on attachment 725675 [details] [diff] [review]
patch

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

::: dom/contacts/ContactManager.js
@@ +637,5 @@
>      this.askPermission("find", cursor, allowCallback);
>      return cursor;
>    },
>  
> +  nextTick: function nextTick(aCallback) {

Er, remove the only one that uses it!
https://hg.mozilla.org/mozilla-central/rev/af48e32a1479
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
This doesn't apply cleanly to b2g18. Please post a branch-specific patch.
Comment on attachment 726722 [details] [diff] [review]
Patch, rebased to mozilla-b2g18

Hey, I can do this myself now :)

https://hg.mozilla.org/releases/mozilla-b2g18/rev/2be82c7c9e22
Attachment #726722 - Flags: checkin?(ryanvm)
Of course, given that it's tef+, you need to land this on b2g18_v1_0_1 too ;)
Whiteboard: [FFOS_perf]
Comment on attachment 725675 [details] [diff] [review]
patch

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

::: dom/contacts/ContactManager.js
@@ +425,5 @@
>            if (DEBUG) debug("cursor waiting for contact, sending");
>            data.waitingForNext = false;
>            let contact = result.shift();
>            this._pushArray(data.cachedContacts, result);
> +          this.nextTick(this._fireSuccessOrDone.bind(this, data.cursor, contact));

This is called after receiving the message from the parent, is it really necessary to do this here?
If it is, shouldn't fireSuccess in the |case "Contacts:Find:Return:OK"| above also be changed?
(In reply to Reuben Morais [:reuben] from comment #12)
> Comment on attachment 725675 [details] [diff] [review]
> patch
> 
> Review of attachment 725675 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/contacts/ContactManager.js
> @@ +425,5 @@
> >            if (DEBUG) debug("cursor waiting for contact, sending");
> >            data.waitingForNext = false;
> >            let contact = result.shift();
> >            this._pushArray(data.cachedContacts, result);
> > +          this.nextTick(this._fireSuccessOrDone.bind(this, data.cursor, contact));
> 
> This is called after receiving the message from the parent, is it really
> necessary to do this here?
> If it is, shouldn't fireSuccess in the |case "Contacts:Find:Return:OK"|
> above also be changed?

Right, we are already waiting for the contact so we don't need it here.
You need to log in before you can comment on or make changes to this bug.