Closed Bug 1093500 Opened 10 years ago Closed 10 years ago

Cleanup registration by pulling push URLs from the push handler

Categories

(Hello (Loop) :: Client, defect)

Product:
Hello (Loop)
Component:
Client
Type:
defect
Priority:
Not set
Severity:
normal
Points:
5

Tracking

(firefox35 fixed, firefox36 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla36
Iteration:
36.2
Tracking Flags:
Tracking Status
firefox35 --- fixed
firefox36 --- fixed

People

(Reporter: MattN, Assigned: MattN)

References

Details

User Story

* Factored push registration out to promiseRegisteredWithPushServer
* registerForNotification will use an existing endpoint and resolve the promise with it, if it exists
* pushURLs aren't passed around as registration function arguments so that we don't need to duplicate logic to pull existing endpoints from the pushHandler in loginToFxA and to implement retry (bug 1074670). registerWithLoopServer pulls them from the push handler.
* Removes the FxA login dependency on gRegisterredDeferred (and therefore the assumption that a guest registration is required before an FxA one) - Bug 1076428
* Replace gRegisteredDeferred with a map called deferredRegistrations which also handles FxA registrations
* Handling of the registration deferreds is more centralized making logic easier to follow
* client code now ensures the proper registration instead of always the guest one (which isn't useful and somewhat wasteful if the user is logged in)

Attachments

(1 file, 1 obsolete file)

MozReview Request: Bug 1093500 - Cleanup Loop registration by pulling push URLs from the push handler. r=pkerr
38 bytes, text/x-review-board-request
pkerr
: review+
Details
This is part of bug 1074670 which needs registration methods to be called more than once.
Flags: qe-verify-
Flags: firefox-backlog+
Attached file MozReview Request: bz://1093500/MattN (obsolete) —
/r/163 - Bug 1093500 - Cleanup Loop registration by pulling push URLs from the push handler. Pull down this commit: hg pull review -r 8e192f9fc25303121de23f6226e961fd4e412498
Attachment #8516542 - Flags: review?(pkerr)
/r/163 - Bug 1093500 - Cleanup Loop registration by pulling push URLs from the push handler. r=pkerr Pull down this commit: hg pull review -r 4b34ae4bb4ce46b00293f16619e6bdf78762997f
https://reviewboard.mozilla.org/r/161/#review73 ::: browser/components/loop/test/xpcshell/test_loopservice_restart.js (Diff revision 2) > - Assert.equal(MozLoopServiceInternal.pushHandler.pushUrl, kEndPointUrl, "Push URL should match"); > + Assert.equal(MozLoopServiceInternal.pushHandler.registrationPushURL, kEndPointUrl, "Push URL should match"); This was broken but went unnoticed until my patch exposed it. ::: browser/components/loop/test/xpcshell/test_loopservice_restart.js (Diff revision 2) > - mockPushHandler.pushUrl = kEndPointUrl; > + mockPushHandler.registrationPushURL = kEndPointUrl; ditto ::: browser/components/loop/test/desktop-local/conversation_test.js (Diff revision 2) > + LOOP_SESSION_TYPE: { > + GUEST: 1, > + FXA: 2 > + }, Needed for the move to use LOOP_SESSION_TYPE in requestCallUrl ::: browser/components/loop/MozLoopService.jsm (Diff revision 2) > + log.debug("hawkRequest: " + path, sessionType); This is useful since regular hawk debug logging doesn't clearly indicate the sessionType of the request.
https://reviewboard.mozilla.org/r/163/#review81 ::: browser/components/loop/MozLoopService.jsm (Diff revision 2) > + let deferred = Promise.defer(); How long will Promise.defer() be available for use in a JSM? There is already an issue in some of the test environments dropping support for this method.
Comment on attachment 8516542 [details] MozReview Request: bz://1093500/MattN Review comments posted at: https://reviewboard.mozilla.org/r/163/
Attachment #8516542 - Flags: review?(pkerr) → review+
https://reviewboard.mozilla.org/r/163/#review85 > How long will Promise.defer() be available for use in a JSM? There is already an issue in some of the test environments dropping support for this method. I'm not sure but I don't think it's a problem for now since there are other uses in this file.
https://hg.mozilla.org/mozilla-central/rev/6d819f81dcbd
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla36
status-firefox35: --- → fixed
status-firefox36: --- → fixed
Comment on attachment 8516542 [details] MozReview Request: bz://1093500/MattN Approval Request Comment Landed on aurora per IRC with lsblakk with a=loop-only
Attachment #8516542 - Flags: approval-mozilla-aurora?
Attachment #8516542 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8516542 - Attachment is obsolete: true
Attachment #8618546 - Flags: review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: