Bug 1551103. Stop using [array] in nsIServiceWorkerManager. r=asuth
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 14 May 2019 17:24:24 +0000
changeset 532633 5e5a2115d0da387463725c290d8f9b9a6cbd8e93
parent 532632 32b8f9006e8516679ca63880b1e029d42e1839d2
child 532634 1bb020dacad9998406a9bd9a7a6fe1568f718b19
push id11270
push userrgurzau@mozilla.com
push dateWed, 15 May 2019 15:07:19 +0000
treeherdermozilla-beta@571bc76da583 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1551103
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 1551103. Stop using [array] in nsIServiceWorkerManager. r=asuth Though I do wonder whether we could just remove the unused data byte arg and simplify the code... Differential Revision: https://phabricator.services.mozilla.com/D30835
dom/interfaces/base/nsIServiceWorkerManager.idl
dom/serviceworkers/ServiceWorkerManager.cpp
--- a/dom/interfaces/base/nsIServiceWorkerManager.idl
+++ b/dom/interfaces/base/nsIServiceWorkerManager.idl
@@ -167,18 +167,17 @@ interface nsIServiceWorkerManager : nsIS
                                   in AString aBody,
                                   in AString aTag,
                                   in AString aIcon,
                                   in AString aData,
                                   in AString aBehavior);
 
   [optional_argc] void sendPushEvent(in ACString aOriginAttributes,
                                      in ACString aScope,
-                                     [optional] in uint32_t aDataLength,
-                                     [optional, array, size_is(aDataLength)] in uint8_t aDataBytes);
+                                     [optional] in Array<uint8_t> aDataBytes);
   void sendPushSubscriptionChangeEvent(in ACString aOriginAttributes,
                                        in ACString scope);
 
   void addListener(in nsIServiceWorkerManagerListener aListener);
 
   void removeListener(in nsIServiceWorkerManagerListener aListener);
 
   bool isParentInterceptEnabled();
--- a/dom/serviceworkers/ServiceWorkerManager.cpp
+++ b/dom/serviceworkers/ServiceWorkerManager.cpp
@@ -946,24 +946,28 @@ RefPtr<ServiceWorkerRegistrationPromise>
   MOZ_ALWAYS_SUCCEEDS(NS_DispatchToCurrentThread(runnable));
 
   return runnable->Promise();
 }
 
 NS_IMETHODIMP
 ServiceWorkerManager::SendPushEvent(const nsACString& aOriginAttributes,
                                     const nsACString& aScope,
-                                    uint32_t aDataLength, uint8_t* aDataBytes,
+                                    const nsTArray<uint8_t>& aDataBytes,
                                     uint8_t optional_argc) {
-  if (optional_argc == 2) {
-    nsTArray<uint8_t> data;
-    if (!data.InsertElementsAt(0, aDataBytes, aDataLength, fallible)) {
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
-    return SendPushEvent(aOriginAttributes, aScope, EmptyString(), Some(data));
+  if (optional_argc == 1) {
+    // This does one copy here (while constructing the Maybe) and another when
+    // we end up copying into the SendPushEventRunnable.  We could fix that to
+    // only do one copy by making things between here and there take
+    // Maybe<nsTArray<uint8_t>>&&, but then we'd need to copy before we know
+    // whether we really need to in PushMessageDispatcher::NotifyWorkers.  Since
+    // in practice this only affects JS callers that pass data, and we don't
+    // have any right now, let's not worry about it.
+    return SendPushEvent(aOriginAttributes, aScope, EmptyString(),
+                         Some(aDataBytes));
   }
   MOZ_ASSERT(optional_argc == 0);
   return SendPushEvent(aOriginAttributes, aScope, EmptyString(), Nothing());
 }
 
 nsresult ServiceWorkerManager::SendPushEvent(
     const nsACString& aOriginAttributes, const nsACString& aScope,
     const nsAString& aMessageId, const Maybe<nsTArray<uint8_t>>& aData) {