851741 - Contacts API: make continue async

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Blocks a blocker

Comment 2

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

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.

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

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!

Comment 5

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/af48e32a1479

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/af48e32a1479

Comment 7

[Ryan VanderMeulen [:RyanVM]]

This doesn't apply cleanly to b2g18. Please post a branch-specific patch.

Comment 8

[Reuben Morais [:reuben]]

Attachment: Patch, rebased to mozilla-b2g18

Comment 9

[Reuben Morais [:reuben]]

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Of course, given that it's tef+, you need to land this on b2g18_v1_0_1 too ;)

Comment 11

[Reuben Morais [:reuben]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/fb7d33d358be

Comment 12

[Reuben Morais [:reuben]]

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?

Comment 13

[Gregor Wagner [:gwagner]]

(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.