Bug 1550569 part 2. Stop using [array] in nsIPushService. r=dragana
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 10 May 2019 07:15:31 +0000
changeset 535339 11474fbdfdbf2d1dee9a0291173671114c6c28f7
parent 535338 a0efb14b79726f577beced5b0ef8f9e6419115ca
child 535340 043e58831ea8caac3c0d0a8ab19d2f3b892a122f
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana
bugs1550569
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1550569 part 2. Stop using [array] in nsIPushService. r=dragana Differential Revision: https://phabricator.services.mozilla.com/D30551
dom/interfaces/push/nsIPushService.idl
dom/push/Push.jsm
dom/push/PushComponents.jsm
dom/push/PushManager.cpp
dom/push/PushService.jsm
dom/push/test/xpcshell/test_service_child.js
--- a/dom/interfaces/push/nsIPushService.idl
+++ b/dom/interfaces/push/nsIPushService.idl
@@ -19,19 +19,17 @@ interface nsIPushSubscription : nsISuppo
   readonly attribute long long lastPush;
   readonly attribute long quota;
   readonly attribute bool isSystemSubscription;
   readonly attribute jsval p256dhPrivateKey;
 
   bool quotaApplies();
   bool isExpired();
 
-  void getKey(in AString name,
-              [optional] out uint32_t keyLen,
-              [array, size_is(keyLen), retval] out uint8_t key);
+  Array<uint8_t> getKey(in AString name);
 };
 
 /**
  * Called by methods that return a push subscription. A non-success
  * |status| indicates that there was a problem returning the
  * subscription, and the |subscription| argument should be ignored. Otherwise,
  * |subscription| will point to a valid push subscription, or |null| if the
  * subscription does not exist.
@@ -100,18 +98,17 @@ interface nsIPushService : nsISupports
                  in nsIPushSubscriptionCallback callback);
 
   /**
    * Creates a restricted push subscription with the given public |key|. The
    * application server must use the corresponding private key to authenticate
    * message delivery requests, as described in draft-thomson-webpush-vapid.
    */
   void subscribeWithKey(in AString scope, in nsIPrincipal principal,
-                        in uint32_t keyLength,
-                        [const, array, size_is(keyLength)] in uint8_t key,
+                        in Array<uint8_t> key,
                         in nsIPushSubscriptionCallback callback);
 
   /**
    * Removes a push subscription for the given |scope|.
    */
   void unsubscribe(in AString scope, in nsIPrincipal principal,
                    in nsIUnsubscribeResultCallback callback);
 
--- a/dom/push/Push.jsm
+++ b/dom/push/Push.jsm
@@ -91,18 +91,17 @@ Push.prototype = {
         }
 
         let keyView = this._normalizeAppServerKey(options.applicationServerKey);
         if (keyView.byteLength === 0) {
           callback._rejectWithError(Cr.NS_ERROR_DOM_PUSH_INVALID_KEY_ERR);
           return;
         }
         PushService.subscribeWithKey(this._scope, this._principal,
-                                     keyView.byteLength, keyView,
-                                     callback);
+                                     keyView, callback);
       })
     );
   },
 
   _normalizeAppServerKey(appServerKey) {
     let key;
     if (typeof appServerKey == "string") {
       try {
@@ -235,24 +234,23 @@ PushSubscriptionCallback.prototype = {
       // Avoid passing null keys to work around bug 1256449.
       options.appServerKey = appServerKey;
     }
     let sub = new pushManager._window.PushSubscription(options);
     this.resolve(sub);
   },
 
   _getKey(subscription, name) {
-    let outKeyLen = {};
-    let rawKey = Cu.cloneInto(subscription.getKey(name, outKeyLen),
+    let rawKey = Cu.cloneInto(subscription.getKey(name),
                               this.pushManager._window);
-    if (!outKeyLen.value) {
+    if (!rawKey.length) {
       return null;
     }
 
-    let key = new this.pushManager._window.ArrayBuffer(outKeyLen.value);
+    let key = new this.pushManager._window.ArrayBuffer(rawKey.length);
     let keyView = new this.pushManager._window.Uint8Array(key);
     keyView.set(rawKey);
     return key;
   },
 
   _rejectWithError(result) {
     let error;
     switch (result) {
--- a/dom/push/PushComponents.jsm
+++ b/dom/push/PushComponents.jsm
@@ -136,20 +136,20 @@ Object.assign(PushServiceParent.prototyp
     "Push:NotificationForOriginShown",
     "Push:NotificationForOriginClosed",
     "Push:ReportError",
   ],
 
   // nsIPushService methods
 
   subscribe(scope, principal, callback) {
-    this.subscribeWithKey(scope, principal, 0, null, callback);
+    this.subscribeWithKey(scope, principal, [], callback);
   },
 
-  subscribeWithKey(scope, principal, keyLen, key, callback) {
+  subscribeWithKey(scope, principal, key, callback) {
     this._handleRequest("Push:Register", principal, {
       scope,
       appServerKey: key,
     }).then(result => {
       this._deliverSubscription(callback, result);
     }, error => {
       this._deliverSubscriptionError(callback, error);
     }).catch(Cu.reportError);
@@ -343,20 +343,20 @@ Object.assign(PushServiceContent.prototy
     "PushService:Unregister:KO",
     "PushService:Clear:OK",
     "PushService:Clear:KO",
   ],
 
   // nsIPushService methods
 
   subscribe(scope, principal, callback) {
-    this.subscribeWithKey(scope, principal, 0, null, callback);
+    this.subscribeWithKey(scope, principal, [], callback);
   },
 
-  subscribeWithKey(scope, principal, keyLen, key, callback) {
+  subscribeWithKey(scope, principal, key, callback) {
     let requestID = this._addRequest(callback);
     this._mm.sendAsyncMessage("Push:Register", {
       scope,
       appServerKey: key,
       requestID,
     }, null, principal);
   },
 
@@ -524,39 +524,35 @@ PushSubscription.prototype = {
     return this.quota === 0;
   },
 
   /**
    * Returns a key for encrypting messages sent to this subscription. JS
    * callers receive the key buffer as a return value, while C++ callers
    * receive the key size and buffer as out parameters.
    */
-  getKey(name, outKeyLen) {
+  getKey(name) {
     switch (name) {
       case "p256dh":
-        return this._getRawKey(this._props.p256dhKey, outKeyLen);
+        return this._getRawKey(this._props.p256dhKey);
 
       case "auth":
-        return this._getRawKey(this._props.authenticationSecret, outKeyLen);
+        return this._getRawKey(this._props.authenticationSecret);
 
       case "appServer":
-        return this._getRawKey(this._props.appServerKey, outKeyLen);
+        return this._getRawKey(this._props.appServerKey);
     }
-    return null;
+    return [];
   },
 
-  _getRawKey(key, outKeyLen) {
+  _getRawKey(key) {
     if (!key) {
-      return null;
+      return [];
     }
-    let rawKey = new Uint8Array(key);
-    if (outKeyLen) {
-      outKeyLen.value = rawKey.length;
-    }
-    return rawKey;
+    return new Uint8Array(key);
   },
 };
 
 // Export the correct implementation depending on whether we're running in
 // the parent or content process.
 let Service = isParent ? PushServiceParent : PushServiceContent;
 
 const EXPORTED_SYMBOLS = ["Service"];
--- a/dom/push/PushManager.cpp
+++ b/dom/push/PushManager.cpp
@@ -56,74 +56,39 @@ nsresult GetPermissionState(nsIPrincipal
     aState = PushPermissionState::Denied;
   } else {
     aState = PushPermissionState::Prompt;
   }
 
   return NS_OK;
 }
 
-// A helper class that frees an `nsIPushSubscription` key buffer when it
-// goes out of scope.
-class MOZ_RAII AutoFreeKeyBuffer final {
-  uint8_t** mKeyBuffer;
-
- public:
-  explicit AutoFreeKeyBuffer(uint8_t** aKeyBuffer) : mKeyBuffer(aKeyBuffer) {
-    MOZ_ASSERT(mKeyBuffer);
-  }
-
-  ~AutoFreeKeyBuffer() { free(*mKeyBuffer); }
-};
-
-// Copies a subscription key buffer into an array.
-nsresult CopySubscriptionKeyToArray(nsIPushSubscription* aSubscription,
-                                    const nsAString& aKeyName,
-                                    nsTArray<uint8_t>& aKey) {
-  uint8_t* keyBuffer = nullptr;
-  AutoFreeKeyBuffer autoFree(&keyBuffer);
-
-  uint32_t keyLen;
-  nsresult rv = aSubscription->GetKey(aKeyName, &keyLen, &keyBuffer);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  if (!aKey.SetCapacity(keyLen, fallible) ||
-      !aKey.InsertElementsAt(0, keyBuffer, keyLen, fallible)) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-  return NS_OK;
-}
-
 nsresult GetSubscriptionParams(nsIPushSubscription* aSubscription,
                                nsAString& aEndpoint,
                                nsTArray<uint8_t>& aRawP256dhKey,
                                nsTArray<uint8_t>& aAuthSecret,
                                nsTArray<uint8_t>& aAppServerKey) {
   if (!aSubscription) {
     return NS_OK;
   }
 
   nsresult rv = aSubscription->GetEndpoint(aEndpoint);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
-  rv = CopySubscriptionKeyToArray(aSubscription, NS_LITERAL_STRING("p256dh"),
-                                  aRawP256dhKey);
+  rv = aSubscription->GetKey(NS_LITERAL_STRING("p256dh"), aRawP256dhKey);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
-  rv = CopySubscriptionKeyToArray(aSubscription, NS_LITERAL_STRING("auth"),
-                                  aAuthSecret);
+  rv = aSubscription->GetKey(NS_LITERAL_STRING("auth"), aAuthSecret);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
-  rv = CopySubscriptionKeyToArray(aSubscription, NS_LITERAL_STRING("appServer"),
-                                  aAppServerKey);
+  rv = aSubscription->GetKey(NS_LITERAL_STRING("appServer"), aAppServerKey);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   return NS_OK;
 }
 
 class GetSubscriptionResultRunnable final : public WorkerRunnable {
@@ -285,19 +250,18 @@ class GetSubscriptionRunnable final : pu
       callback->OnPushSubscriptionError(NS_ERROR_FAILURE);
       return NS_OK;
     }
 
     if (mAction == PushManager::SubscribeAction) {
       if (mAppServerKey.IsEmpty()) {
         rv = service->Subscribe(mScope, principal, callback);
       } else {
-        rv =
-            service->SubscribeWithKey(mScope, principal, mAppServerKey.Length(),
-                                      mAppServerKey.Elements(), callback);
+        rv = service->SubscribeWithKey(mScope, principal, mAppServerKey,
+                                       callback);
       }
     } else {
       MOZ_ASSERT(mAction == PushManager::GetSubscriptionAction);
       rv = service->GetSubscription(mScope, principal, callback);
     }
 
     if (NS_WARN_IF(NS_FAILED(rv))) {
       callback->OnPushSubscriptionError(NS_ERROR_FAILURE);
--- a/dom/push/PushService.jsm
+++ b/dom/push/PushService.jsm
@@ -1016,17 +1016,18 @@ var PushService = {
       this._db.getByIdentifiers(pageRecord)
     );
   },
 
   register(aPageRecord) {
     console.debug("register()", aPageRecord);
 
     let keyPromise;
-    if (aPageRecord.appServerKey) {
+    if (aPageRecord.appServerKey &&
+        aPageRecord.appServerKey.length != 0) {
       let keyView = new Uint8Array(aPageRecord.appServerKey);
       keyPromise = PushCrypto.validateAppServerKey(keyView)
         .catch(error => {
           // Normalize Web Crypto exceptions. `nsIPushService` will forward the
           // error result to the DOM API implementation in `PushManager.cpp` or
           // `Push.js`, which will convert it to the correct `DOMException`.
           throw errorWithResult("Invalid app server key",
                                 Cr.NS_ERROR_DOM_PUSH_INVALID_KEY_ERR);
--- a/dom/push/test/xpcshell/test_service_child.js
+++ b/dom/push/test/xpcshell/test_service_child.js
@@ -59,17 +59,16 @@ add_test(function test_subscribe_success
 
 add_test(function test_subscribeWithKey_error() {
   do_test_pending();
 
   let invalidKey = [0, 1];
   PushServiceComponent.subscribeWithKey(
     "https://example.com/sub-key/invalid",
     Services.scriptSecurityManager.getSystemPrincipal(),
-    invalidKey.length,
     invalidKey,
     (result, subscription) => {
       ok(!Components.isSuccessCode(result), "Expected error creating subscription with invalid key");
       equal(result, Cr.NS_ERROR_DOM_PUSH_INVALID_KEY_ERR, "Wrong error code for invalid key");
       strictEqual(subscription, null, "Unexpected subscription");
 
       do_test_finished();
       run_next_test();
@@ -79,17 +78,16 @@ add_test(function test_subscribeWithKey_
 
 add_test(function test_subscribeWithKey_success() {
   do_test_pending();
 
   generateKey().then(key => {
     PushServiceComponent.subscribeWithKey(
       "https://example.com/sub-key/ok",
       Services.scriptSecurityManager.getSystemPrincipal(),
-      key.length,
       key,
       (result, subscription) => {
         ok(Components.isSuccessCode(result), "Error creating subscription with key");
         notStrictEqual(subscription, null, "Expected subscription");
         done();
       }
     );
   }, error => {
@@ -100,17 +98,16 @@ add_test(function test_subscribeWithKey_
 
 add_test(function test_subscribeWithKey_conflict() {
   do_test_pending();
 
   generateKey().then(differentKey => {
     PushServiceComponent.subscribeWithKey(
       "https://example.com/sub-key/ok",
       Services.scriptSecurityManager.getSystemPrincipal(),
-      differentKey.length,
       differentKey,
       (result, subscription) => {
         ok(!Components.isSuccessCode(result), "Expected error creating subscription with conflicting key");
         equal(result, Cr.NS_ERROR_DOM_PUSH_MISMATCHED_KEY_ERR, "Wrong error code for mismatched key");
         strictEqual(subscription, null, "Unexpected subscription");
         done();
       }
     );