Back out changeset f7e1ab1bd99c (bug 1207744) for X(3) failures.

MozReview-Commit-ID: 5mgyh20yXiI

--HG--
extra : rebase_source : d3adb0098eb1615fdd134cbb4f0afd8a151a5066

dom/push/PushDB.jsm

@@ -125,10 +125,10 @@ this.PushDB.prototype = {
       this.newTxn(
         "readwrite",
         this._dbStoreName,
-        (aTxn, aStore) => {
+        function txnCb(aTxn, aStore) {
           console.debug("delete: Removing record", aKeyID);
           aStore.get(aKeyID).onsuccess = event => {
-            aTxn.result = this.toPushRecord(event.target.result);
+            aTxn.result = event.target.result;
             aStore.delete(aKeyID);
           };
         },

dom/push/PushService.jsm

@@ -1023,8 +1023,10 @@ this.PushService = {
   _sendRequest(action, ...params) {
     if (this._state == PUSH_SERVICE_CONNECTION_DISABLE) {
       return Promise.reject(new Error("Push service disabled"));
-    }
-    if (this._state == PUSH_SERVICE_ACTIVE_OFFLINE) {
+    } else if (this._state == PUSH_SERVICE_ACTIVE_OFFLINE) {
+      if (this._service.serviceType() == "WebSocket" && action == "unregister") {
+        return Promise.resolve();
+      }
       return Promise.reject(new Error("Push service offline"));
     }
     // Ensure the backend is ready. `getByPageRecord` already checks this, but
@@ -1201,13 +1203,12 @@ this.PushService = {
         let reason = Ci.nsIPushErrorReporter.UNSUBSCRIBE_MANUAL;
         return Promise.all([
           this._sendUnregister(record, reason),
-          this._db.delete(record.keyID).then(record => {
-            if (record) {
-              gPushNotifier.notifySubscriptionModified(record.scope,
-                                                       record.principal);
-            }
-          }),
-        ]).then(([success]) => success);
+          this._db.delete(record.keyID),
+        ]).then(() => {
+          gPushNotifier.notifySubscriptionModified(record.scope,
+                                                   record.principal);
+          return true;
+        });
       });
   },
 

dom/push/PushServiceAndroidGCM.jsm

@@ -59,6 +59,10 @@ this.PushServiceAndroidGCM = {
                       PushRecordAndroidGCM);
   },
 
+  serviceType: function() {
+    return "AndroidGCM";
+  },
+
   validServerURI: function(serverURI) {
     if (!serverURI) {
       return false;

dom/push/PushServiceHttp2.jsm

@@ -430,6 +430,10 @@ this.PushServiceHttp2 = {
                       PushRecordHttp2);
   },
 
+  serviceType: function() {
+    return "http2";
+  },
+
   hasmainPushService: function() {
     return this._mainPushService !== null;
   },

dom/push/PushServiceWebSocket.jsm

@@ -143,6 +143,10 @@ this.PushServiceWebSocket = {
                       PushRecordWebSocket);
   },
 
+  serviceType: function() {
+    return "WebSocket";
+  },
+
   disconnect: function() {
     this._shutdownWS();
   },
@@ -230,15 +234,17 @@ this.PushServiceWebSocket = {
       requestTimedOut = true;
 
     } else {
-      for (let [key, request] of this._pendingRequests) {
+      for (let [channelID, request] of this._registerRequests) {
         let duration = now - request.ctime;
         // If any of the registration requests time out, all the ones after it
         // also made to fail, since we are going to be disconnecting the
         // socket.
         requestTimedOut |= duration > this._requestTimeout;
         if (requestTimedOut) {
-          request.reject(new Error("Request timed out"));
-          this._pendingRequests.delete(key);
+          request.reject(new Error(
+            "Register request timed out for channel ID " + channelID));
+
+          this._registerRequests.delete(channelID);
         }
       }
     }
@@ -272,7 +278,7 @@ this.PushServiceWebSocket = {
   },
 
   _ws: null,
-  _pendingRequests: new Map(),
+  _registerRequests: new Map(),
   _currentState: STATE_SHUT_DOWN,
   _requestTimeout: 0,
   _requestTimeoutTimer: null,
@@ -370,7 +376,7 @@ this.PushServiceWebSocket = {
     }
 
     if (shouldCancelPending) {
-      this._cancelPendingRequests();
+      this._cancelRegisterRequests();
     }
 
     if (this._notifyRequestQueue) {
@@ -431,7 +437,7 @@ this.PushServiceWebSocket = {
 
   /** Indicates whether we're waiting for pongs or requests. */
   _hasPendingRequests() {
-    return this._lastPingTime > 0 || this._pendingRequests.size > 0;
+    return this._lastPingTime > 0 || this._registerRequests.size > 0;
   },
 
   /**
@@ -616,7 +622,7 @@ this.PushServiceWebSocket = {
         this._notifyRequestQueue();
         this._notifyRequestQueue = null;
       }
-      this._sendingPendingRequests();
+      this._sendRegisterRequests();
     };
 
     function finishHandshake() {
@@ -663,12 +669,17 @@ this.PushServiceWebSocket = {
    */
   _handleRegisterReply: function(reply) {
     console.debug("handleRegisterReply()");
-
-    let tmp = this._takeRequestForReply(reply);
-    if (!tmp) {
+    if (typeof reply.channelID !== "string" ||
+        !this._registerRequests.has(reply.channelID)) {
       return;
     }
 
+    let tmp = this._registerRequests.get(reply.channelID);
+    this._registerRequests.delete(reply.channelID);
+    if (!this._hasPendingRequests()) {
+      this._requestTimeoutTimer.cancel();
+    }
+
     if (reply.status == 200) {
       try {
         Services.io.newURI(reply.pushEndpoint, null, null);
@@ -697,18 +708,6 @@ this.PushServiceWebSocket = {
     }
   },
 
-  _handleUnregisterReply(reply) {
-    console.debug("handleUnregisterReply()");
-
-    let request = this._takeRequestForReply(reply);
-    if (!request) {
-      return;
-    }
-
-    let success = reply.status === 200;
-    request.resolve(success);
-  },
-
   _handleDataUpdate: function(update) {
     let promise;
     if (typeof update.channelID != "string") {
@@ -846,6 +845,9 @@ this.PushServiceWebSocket = {
   register(record) {
     console.debug("register() ", record);
 
+    // start the timer since we now have at least one request
+    this._startRequestTimeoutTimer();
+
     let data = {channelID: this._generateID(),
                 messageType: "register"};
 
@@ -856,7 +858,15 @@ this.PushServiceWebSocket = {
       });
     }
 
-    return this._sendRequest(record, data).then(record => {
+    return new Promise((resolve, reject) => {
+      this._registerRequests.set(data.channelID, {
+        record: record,
+        resolve: resolve,
+        reject: reject,
+        ctime: Date.now(),
+      });
+      this._queueRequest(data);
+    }).then(record => {
       if (!this._dataEnabled) {
         return record;
       }
@@ -873,24 +883,15 @@ this.PushServiceWebSocket = {
   unregister(record, reason) {
     console.debug("unregister() ", record, reason);
 
-    return Promise.resolve().then(_ => {
-      let code = kUNREGISTER_REASON_TO_CODE[reason];
-      if (!code) {
-        throw new Error('Invalid unregister reason');
-      }
-      let data = {channelID: record.channelID,
-                  messageType: "unregister",
-                  code: code};
-
-      // If we're connected to a Web Push server, wait for an unregister
-      // response. Simple Push servers aren't required to support
-      // unregistration, so we return immediately.
-      if (this._dataEnabled) {
-        return this._sendRequest(record, data);
-      }
-      this._queueRequest(data);
-      return true;
-    });
+    let code = kUNREGISTER_REASON_TO_CODE[reason];
+    if (!code) {
+      return Promise.reject(new Error('Invalid unregister reason'));
+    }
+    let data = {channelID: record.channelID,
+                messageType: "unregister",
+                code: code};
+    this._queueRequest(data);
+    return Promise.resolve();
   },
 
   _queueStart: Promise.resolve(),
@@ -908,15 +909,22 @@ this.PushServiceWebSocket = {
 
   _send(data) {
     if (this._currentState == STATE_READY) {
-      // check if request has not been cancelled
-      this._wsSendMessage(data);
+      if (data.messageType != "register" ||
+          this._registerRequests.has(data.channelID)) {
+
+        // check if request has not been cancelled
+        this._wsSendMessage(data);
+      }
     }
   },
 
-  _sendingPendingRequests() {
+  _sendRegisterRequests() {
     this._enqueue(_ => {
-      for (let request of this._pendingRequests.values()) {
-        this._send(request.data);
+      for (let channelID of this._registerRequests.keys()) {
+        this._send({
+          messageType: "register",
+          channelID: channelID,
+        });
       }
     });
   },
@@ -1051,7 +1059,7 @@ this.PushServiceWebSocket = {
 
     // A whitelist of protocol handlers. Add to these if new messages are added
     // in the protocol.
-    let handlers = ["Hello", "Register", "Unregister", "Notification"];
+    let handlers = ["Hello", "Register", "Notification"];
 
     // Build up the handler name to call from messageType.
     // e.g. messageType == "register" -> _handleRegisterReply.
@@ -1097,53 +1105,11 @@ this.PushServiceWebSocket = {
   /**
    * Rejects all pending register requests with errors.
    */
-  _cancelPendingRequests() {
-    for (let request of this._pendingRequests.values()) {
-      request.reject(new Error("Request aborted"));
+  _cancelRegisterRequests: function() {
+    for (let request of this._registerRequests.values()) {
+      request.reject(new Error("Register request aborted"));
     }
-    this._pendingRequests.clear();
-  },
-
-  _makePendingRequestKey(request) {
-    return request.messageType.toLowerCase() + "|" + request.channelID;
-  },
-
-  _sendRequest(record, data) {
-    // start the timer since we now have at least one request
-    this._startRequestTimeoutTimer();
-
-    let key = this._makePendingRequestKey(data);
-    if (!this._pendingRequests.has(key)) {
-      let request = {
-        data: data,
-        record: record,
-        ctime: Date.now(),
-      };
-      request.promise = new Promise((resolve, reject) => {
-        request.resolve = resolve;
-        request.reject = reject;
-        this._queueRequest(data);
-      });
-      this._pendingRequests.set(key, request);
-    }
-
-    return this._pendingRequests.get(key).promise;
-  },
-
-  _takeRequestForReply(reply) {
-    if (typeof reply.channelID !== "string") {
-      return null;
-    }
-    let key = this._makePendingRequestKey(reply);
-    let request = this._pendingRequests.get(key);
-    if (!request) {
-      return null;
-    }
-    this._pendingRequests.delete(key);
-    if (!this._hasPendingRequests()) {
-      this._requestTimeoutTimer.cancel();
-    }
-    return request;
+    this._registerRequests.clear();
   },
 };
 

dom/push/test/test_utils.js

@@ -106,11 +106,7 @@
     },
 
     onUnregister(request) {
-      this.serverSendMsg(JSON.stringify({
-        messageType: "unregister",
-        channelID: request.channelID,
-        status: 200,
-      }));
+      // Do nothing.
     },
 
     onAck(request) {

dom/push/test/xpcshell/test_permissions.js

@@ -120,11 +120,6 @@ add_task(function* setUp() {
           equal(request.code, 202,
             'Expected permission revoked unregister reason');
           resolve();
-          this.serverSendMsg(JSON.stringify({
-            messageType: 'unregister',
-            status: 200,
-            channelID: request.channelID,
-          }));
         },
         onACK(request) {},
       });

dom/push/test/xpcshell/test_unregister_invalid_json.js

@@ -50,8 +50,7 @@ add_task(function* test_unregister_invalid_json() {
           this.serverSendMsg(JSON.stringify({
             messageType: 'hello',
             status: 200,
-            uaid: userAgentID,
-            use_webpush: true,
+            uaid: userAgentID
           }));
         },
         onUnregister(request) {
@@ -62,27 +61,21 @@ add_task(function* test_unregister_invalid_json() {
     }
   });
 
-  yield rejects(
-    PushService.unregister({
-      scope: 'https://example.edu/page/1',
-      originAttributes: '',
-    }),
-    'Expected error for first invalid JSON response'
-  );
-
+  // "unregister" is fire-and-forget: it's sent via _send(), not
+  // _sendRequest().
+  yield PushService.unregister({
+    scope: 'https://example.edu/page/1',
+    originAttributes: '',
+  });
   let record = yield db.getByKeyID(
     '87902e90-c57e-4d18-8354-013f4a556559');
   ok(!record, 'Failed to delete unregistered record');
 
-  yield rejects(
-    PushService.unregister({
-      scope: 'https://example.net/page/1',
-      originAttributes: ChromeUtils.originAttributesToSuffix(
-        { appId: Ci.nsIScriptSecurityManager.NO_APP_ID, inIsolatedMozBrowser: false }),
-    }),
-    'Expected error for second invalid JSON response'
-  );
-
+  yield PushService.unregister({
+    scope: 'https://example.net/page/1',
+    originAttributes: ChromeUtils.originAttributesToSuffix(
+      { appId: Ci.nsIScriptSecurityManager.NO_APP_ID, inIsolatedMozBrowser: false }),
+  });
   record = yield db.getByKeyID(
     '057caa8f-9b99-47ff-891c-adad18ce603e');
   ok(!record,

dom/push/test/xpcshell/test_unregister_success.js

@@ -5,14 +5,11 @@
 
 const {PushDB, PushService, PushServiceWebSocket} = serviceExports;
 
-const userAgentID = 'fbe865a6-aeb8-446f-873c-aeebdb8d493c';
 const channelID = 'db0a7021-ec2d-4bd3-8802-7a6966f10ed8';
 
 function run_test() {
   do_get_profile();
-  setPrefs({
-    userAgentID: userAgentID,
-  });
+  setPrefs();
   run_next_test();
 }
 
@@ -39,8 +36,7 @@ add_task(function* test_unregister_success() {
           this.serverSendMsg(JSON.stringify({
             messageType: 'hello',
             status: 200,
-            uaid: userAgentID,
-            use_webpush: true,
+            uaid: 'fbe865a6-aeb8-446f-873c-aeebdb8d493c'
           }));
         },
         onUnregister(request) {