Bug 1550569. Stop using [array] in nsIPushNotifier. r=dragana
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 10 May 2019 07:15:08 +0000
changeset 535338 a0efb14b79726f577beced5b0ef8f9e6419115ca
parent 535337 a5fe44cee7723c38287dd5e4f5d04d335260bcb5
child 535339 11474fbdfdbf2d1dee9a0291173671114c6c28f7
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. Stop using [array] in nsIPushNotifier. r=dragana Differential Revision: https://phabricator.services.mozilla.com/D30549
dom/interfaces/push/nsIPushNotifier.idl
dom/push/PushNotifier.cpp
dom/push/PushService.jsm
dom/push/test/test_error_reporting.html
dom/push/test/xpcshell/test_observer_data.js
dom/push/test/xpcshell/test_observer_remoting.js
--- a/dom/interfaces/push/nsIPushNotifier.idl
+++ b/dom/interfaces/push/nsIPushNotifier.idl
@@ -34,18 +34,17 @@ interface nsIPushNotifier : nsISupports
 
   /**
    * Same as `notifyPush`, except the subject of the observer notification
    * receives an `nsIPushMessage` instance containing the |data|. Service
    * workers can access the |data| via the `PushMessageData` WebIDL interface.
    */
   void notifyPushWithData(in ACString scope, in nsIPrincipal principal,
                           in AString messageId,
-                          [optional] in uint32_t dataLen,
-                          [array, size_is(dataLen)] in uint8_t data);
+                          in Array<uint8_t> data);
 
   /**
    * Fires a `push-subscription-change` observer notification, and sends a
    * `pushsubscriptionchange` event to the service worker registered for the
    * |scope|.
    */
   void notifySubscriptionChange(in ACString scope, in nsIPrincipal principal);
 
@@ -73,18 +72,17 @@ interface nsIPushData : nsISupports
 {
   /** Extracts the data as a UTF-8 text string. */
   AString text();
 
   /** Extracts the data as a JSON value. */
   [implicit_jscontext] jsval json();
 
   /** Extracts the raw binary data. */
-  void binary([optional] out uint32_t dataLen,
-              [array, retval, size_is(dataLen)] out uint8_t data);
+  Array<uint8_t> binary();
 };
 
 /**
  * The subject of a `push-message` observer notification. |data| may be |null|
  * for messages without data.
  */
 [scriptable, builtinclass, uuid(b9d063ca-0e3f-4fee-be4b-ea9103263433)]
 interface nsIPushMessage : nsISupports
--- a/dom/push/PushNotifier.cpp
+++ b/dom/push/PushNotifier.cpp
@@ -36,27 +36,28 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(PushNotifier)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(PushNotifier)
 
 NS_IMETHODIMP
 PushNotifier::NotifyPushWithData(const nsACString& aScope,
                                  nsIPrincipal* aPrincipal,
-                                 const nsAString& aMessageId, uint32_t aDataLen,
-                                 uint8_t* aData) {
+                                 const nsAString& aMessageId,
+                                 const nsTArray<uint8_t>& aData) {
   NS_ENSURE_ARG(aPrincipal);
+  // We still need to do this copying business, if we want the copy to be
+  // fallible.  Just passing Some(aData) would do an infallible copy at the
+  // point where the Some() call happens.
   nsTArray<uint8_t> data;
-  if (!data.SetCapacity(aDataLen, fallible)) {
+  if (!data.AppendElements(aData, fallible)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
-  if (!data.InsertElementsAt(0, aData, aDataLen, fallible)) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-  PushMessageDispatcher dispatcher(aScope, aPrincipal, aMessageId, Some(data));
+  PushMessageDispatcher dispatcher(aScope, aPrincipal, aMessageId,
+                                   Some(std::move(data)));
   return Dispatch(dispatcher);
 }
 
 NS_IMETHODIMP
 PushNotifier::NotifyPush(const nsACString& aScope, nsIPrincipal* aPrincipal,
                          const nsAString& aMessageId) {
   NS_ENSURE_ARG(aPrincipal);
   PushMessageDispatcher dispatcher(aScope, aPrincipal, aMessageId, Nothing());
@@ -189,30 +190,18 @@ PushData::Json(JSContext* aCx, JS::Mutab
     return rv;
   }
   ErrorResult error;
   BodyUtil::ConsumeJson(aCx, aResult, mDecodedText, error);
   return error.StealNSResult();
 }
 
 NS_IMETHODIMP
-PushData::Binary(uint32_t* aDataLen, uint8_t** aData) {
-  NS_ENSURE_ARG_POINTER(aDataLen);
-  NS_ENSURE_ARG_POINTER(aData);
-
-  *aData = nullptr;
-  if (mData.IsEmpty()) {
-    *aDataLen = 0;
-    return NS_OK;
-  }
-  uint32_t length = mData.Length();
-  uint8_t* data = static_cast<uint8_t*>(moz_xmalloc(length * sizeof(uint8_t)));
-  memcpy(data, mData.Elements(), length * sizeof(uint8_t));
-  *aDataLen = length;
-  *aData = data;
+PushData::Binary(nsTArray<uint8_t>& aData) {
+  aData = mData;
   return NS_OK;
 }
 
 PushMessage::PushMessage(nsIPrincipal* aPrincipal, nsIPushData* aData)
     : mPrincipal(aPrincipal), mData(aData) {}
 
 PushMessage::~PushMessage() {}
 
--- a/dom/push/PushService.jsm
+++ b/dom/push/PushService.jsm
@@ -908,17 +908,17 @@ var PushService = {
     if (aPushRecord.quotaApplies()) {
       // Don't record telemetry for chrome push messages.
       Services.telemetry.getHistogramById("PUSH_API_NOTIFY").add();
     }
 
     if (payload) {
       gPushNotifier.notifyPushWithData(aPushRecord.scope,
                                        aPushRecord.principal,
-                                       messageID, payload.length, payload);
+                                       messageID, payload);
     } else {
       gPushNotifier.notifyPush(aPushRecord.scope, aPushRecord.principal,
                                messageID);
     }
 
     return Ci.nsIPushErrorReporter.ACK_DELIVERED;
   },
 
--- a/dom/push/test/test_error_reporting.html
+++ b/dom/push/test/test_error_reporting.html
@@ -57,17 +57,17 @@ http://creativecommons.org/licenses/publ
   function waitForDeliveryError(request) {
     return new Promise(resolve => {
       var data = new TextEncoder("utf-8").encode(JSON.stringify(request));
       var principal = SpecialPowers.wrap(document).nodePrincipal;
 
       let messageId = "message-" + (idCounter++);
       reporters.set(messageId, resolve);
       pushNotifier.notifyPushWithData(registration.scope, principal, messageId,
-                                      data.length, data);
+                                      data);
     });
   }
 
   add_task(async function reportDeliveryErrors() {
     var reason = await waitForDeliveryError({ type: "exception" });
     is(reason, SpecialPowers.Ci.nsIPushErrorReporter.DELIVERY_UNCAUGHT_EXCEPTION,
       "Should report uncaught exceptions");
 
--- a/dom/push/test/xpcshell/test_observer_data.js
+++ b/dom/push/test/xpcshell/test_observer_data.js
@@ -6,33 +6,33 @@ var systemPrincipal = Services.scriptSec
 
 add_task(async function test_notifyWithData() {
   let textData = '{"hello":"world"}';
   let payload = new TextEncoder("utf-8").encode(textData);
 
   let notifyPromise =
     promiseObserverNotification(PushServiceComponent.pushTopic);
   pushNotifier.notifyPushWithData("chrome://notify-test", systemPrincipal,
-    "" /* messageId */, payload.length, payload);
+    "" /* messageId */, payload);
 
   let data = (await notifyPromise).subject.QueryInterface(
     Ci.nsIPushMessage).data;
   deepEqual(data.json(), {
     hello: "world",
   }, "Should extract JSON values");
   deepEqual(data.binary(), Array.from(payload),
     "Should extract raw binary data");
   equal(data.text(), textData, "Should extract text data");
 });
 
 add_task(async function test_empty_notifyWithData() {
   let notifyPromise =
     promiseObserverNotification(PushServiceComponent.pushTopic);
   pushNotifier.notifyPushWithData("chrome://notify-test", systemPrincipal,
-    "" /* messageId */, 0, null);
+    "" /* messageId */, []);
 
   let data = (await notifyPromise).subject.QueryInterface(
     Ci.nsIPushMessage).data;
   throws(_ => data.json(),
     /InvalidStateError/,
     "Should throw an error when parsing an empty string as JSON");
   strictEqual(data.text(), "", "Should return an empty string");
   deepEqual(data.binary(), [], "Should return an empty array");
--- a/dom/push/test/xpcshell/test_observer_remoting.js
+++ b/dom/push/test/xpcshell/test_observer_remoting.js
@@ -72,17 +72,17 @@ var waitForNotifierObservers = async fun
     PushServiceComponent.subscriptionChangeTopic);
   let subModifiedPromise = promiseObserverNotification(
     PushServiceComponent.subscriptionModifiedTopic);
 
   let scope = "chrome://test-scope";
   let data = new TextEncoder("utf-8").encode(text);
 
   if (shouldNotify) {
-    pushNotifier.notifyPushWithData(scope, principal, "", data.length, data);
+    pushNotifier.notifyPushWithData(scope, principal, "", data);
     pushNotifier.notifySubscriptionChange(scope, principal);
     pushNotifier.notifySubscriptionModified(scope, principal);
   }
 
   let {
     data: notifyScope,
     subject: notifySubject,
   } = await notifyPromise;