Contacts: Fix error callbacks

RESOLVED FIXED in mozilla26

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gwagner, Assigned: gwagner)

Tracking

unspecified
mozilla26
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 793769 [details] [diff] [review]
errorcb.diff
Attachment #793769 - Flags: review?(reuben.bmo)
Comment on attachment 793769 [details] [diff] [review]
errorcb.diff

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

r=me. Let's see if some of those intermittent timeouts we've been seeing are actually hidden failures :)

::: dom/contacts/ContactManager.js
@@ +632,5 @@
>        case "Contact:Save:Return:OK":
>        case "Contacts:Clear:Return:OK":
>        case "Contact:Remove:Return:OK":
>          req = this.getRequest(msg.requestID);
>          if (req)

nit: Since you're touching nearby code, might as well add the missing braces here.

@@ +637,5 @@
>            Services.DOMRequest.fireSuccess(req.request, null);
>          break;
> +      case "Contacts:GetAll:Return:KO":
> +        req = this.getRequest(msg.requestID);
> +        let message = msg.errorMsg | "unknown error"

||, semicolon. Put the case body inside a block so |message| doesn't leak to the rest of the switch.

@@ +639,5 @@
> +      case "Contacts:GetAll:Return:KO":
> +        req = this.getRequest(msg.requestID);
> +        let message = msg.errorMsg | "unknown error"
> +        if (req)
> +          Services.DOMRequest.fireError(req.cursor, message);

nit: Braces.

@@ +648,5 @@
>        case "Contacts:Clear:Return:KO":
>        case "Contacts:GetRevision:Return:KO":
>        case "Contacts:Count:Return:KO":
>          req = this.getRequest(msg.requestID);
> +        message = msg.errorMsg | "unknown error"

||, semicolon. Put the case body inside a block and do |let message = …|.

@@ +653,2 @@
>          if (req)
> +          Services.DOMRequest.fireError(req.request, message);

nit: braces? It looks like your qparent is old, this code doesn't look like this anymore.
Attachment #793769 - Flags: review?(reuben.bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/2b62136bbc85
Assignee: nobody → anygregor
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.