Closed Bug 922703 Opened 7 years ago Closed 7 years ago

[Gaia] Refine the error handler for card lock functionality

Categories

(Firefox OS Graveyard :: Gaia, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: edgar, Assigned: edgar)

References

Details

Attachments

(1 file)

In bug 873380, we would like to improve the error report policy for card lock related functionality, like setCardLock() and unlockCardLock(). When these operations is failed, we need to provide 'retryCount' information to Gaia. And currently we dispatch it via |onicccardlockerror| event. Now we are going to dispatch it via |onerror| event of DOMRequest, so that we can have a symmetric design and don't need |onicccardlockerror| any more.

The change of api usage is as below:

* current:

var request = iccManager.unlockCardLock();
request.onsuccess = function onsuccess() {
  // handle unlock success
};

iccManager.onicccardlockerror = function handleCardLockError() {
  // handle unlock failed
};


* after:

var request = iccManager.unlockCardLock();
request.onsuccess = function onsuccess() {
  // handle unlock success
};

request.onerror = function onerror() {
  // handle unlock failed
};

Thanks
Assignee: nobody → echen
Comment on attachment 814063 [details]
Gaia pull request #12703

Hi Fernando, I separated the patch into 4 parts to make them easier to be reviewed (After reviewed, I will squash them). And I have tested this patch with bug 873380, the card lock error handler works well. Could you help to review it for me? Or you want bug 873380 land first? Thanks.
Attachment #814063 - Flags: review?(fernando.campo)
Comment on attachment 814063 [details]
Gaia pull request #12703

Hi Edgar, thanks for taking care of this.

And thanks for dividing the PR, is much easier to review. All of the changes look good, so got the r+

But for the merge, I'd like to wait till bug 873380 is also merged, to avoid collisions

Cheers!
Attachment #814063 - Flags: review?(fernando.campo) → review+
(In reply to Fernando Campo (:fcampo) from comment #4)
> Comment on attachment 814063 [details]
> Gaia pull request #12703
> 
> Hi Edgar, thanks for taking care of this.
> 
> And thanks for dividing the PR, is much easier to review. All of the changes
> look good, so got the r+
> 
> But for the merge, I'd like to wait till bug 873380 is also merged, to avoid
> collisions
> 
> Cheers!

Sure, after bug 873380 is landed, I will let you know, thanks for your review. :)
(In reply to Edgar Chen [:edgar][:echen] from comment #5)
> (In reply to Fernando Campo (:fcampo) from comment #4)
> > Comment on attachment 814063 [details]
> > Gaia pull request #12703
> > 
> > Hi Edgar, thanks for taking care of this.
> > 
> > And thanks for dividing the PR, is much easier to review. All of the changes
> > look good, so got the r+
> > 
> > But for the merge, I'd like to wait till bug 873380 is also merged, to avoid
> > collisions
> > 
> > Cheers!
> 
> Sure, after bug 873380 is landed, I will let you know, thanks for your
> review. :)

Hi Fernando, bug 873380 is landed, could you help to merge this patch? I am not authorized to do that, thanks.
Done! merged on master 0cb86c7d87020525efe363bdd4d48485c39d8c99

Thanks for your work
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.