Bug 1626570 - Improve handling of copying arrays in hal/. r=gsvelto
authorSimon Giesecke <sgiesecke@mozilla.com>
Tue, 05 May 2020 10:14:24 +0000
changeset 528128 078ce9d263f9fe78cd2d24e38b89008dec7962c8
parent 528127 704cedb103b8af91ee21bc75c9bdb3b9c5591cc9
child 528129 b045baf40b3b51ef111c689b253091557400e7f9
push id37382
push usermalexandru@mozilla.com
push dateTue, 05 May 2020 21:38:25 +0000
treeherdermozilla-central@385f49adaf00 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgsvelto
bugs1626570
milestone78.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 1626570 - Improve handling of copying arrays in hal/. r=gsvelto Differential Revision: https://phabricator.services.mozilla.com/D73637
hal/Hal.cpp
hal/Hal.h
hal/HalWakeLock.cpp
hal/WindowIdentifier.cpp
hal/WindowIdentifier.h
hal/android/AndroidHal.cpp
hal/fallback/FallbackVibration.cpp
hal/sandbox/SandboxHal.cpp
--- a/hal/Hal.cpp
+++ b/hal/Hal.cpp
@@ -71,17 +71,17 @@ bool WindowIsActive(nsPIDOMWindowInner* 
   NS_ENSURE_TRUE(document, false);
   return !document->Hidden();
 }
 
 StaticAutoPtr<WindowIdentifier::IDArrayType> gLastIDToVibrate;
 
 static void RecordLastIDToVibrate(const WindowIdentifier& aId) {
   if (!InSandbox()) {
-    *gLastIDToVibrate = aId.AsArray();
+    *gLastIDToVibrate = aId.AsArray().Clone();
   }
 }
 
 static bool MayCancelVibration(const WindowIdentifier& aId) {
   // Although only active windows may start vibrations, a window may
   // cancel its own vibration even if it's no longer active.
   //
   // After a window is marked as inactive, it sends a CancelVibrate
@@ -102,17 +102,17 @@ static bool MayCancelVibration(const Win
 }
 
 }  // namespace
 
 void Vibrate(const nsTArray<uint32_t>& pattern, nsPIDOMWindowInner* window) {
   Vibrate(pattern, WindowIdentifier(window));
 }
 
-void Vibrate(const nsTArray<uint32_t>& pattern, const WindowIdentifier& id) {
+void Vibrate(const nsTArray<uint32_t>& pattern, WindowIdentifier&& id) {
   AssertMainThread();
 
   // Only active windows may start vibrations.  If |id| hasn't gone
   // through the IPC layer -- that is, if our caller is the outside
   // world, not hal_proxy -- check whether the window is active.  If
   // |id| has gone through IPC, don't check the window's visibility;
   // only the window corresponding to the bottommost process has its
   // visibility state set correctly.
@@ -121,31 +121,33 @@ void Vibrate(const nsTArray<uint32_t>& p
     return;
   }
 
   RecordLastIDToVibrate(id);
 
   // Don't forward our ID if we are not in the sandbox, because hal_impl
   // doesn't need it, and we don't want it to be tempted to read it.  The
   // empty identifier will assert if it's used.
-  PROXY_IF_SANDBOXED(Vibrate(pattern, InSandbox() ? id : WindowIdentifier()));
+  PROXY_IF_SANDBOXED(
+      Vibrate(pattern, InSandbox() ? std::move(id) : WindowIdentifier()));
 }
 
 void CancelVibrate(nsPIDOMWindowInner* window) {
   CancelVibrate(WindowIdentifier(window));
 }
 
-void CancelVibrate(const WindowIdentifier& id) {
+void CancelVibrate(WindowIdentifier&& id) {
   AssertMainThread();
 
   if (MayCancelVibration(id)) {
     // Don't forward our ID if we are not in the sandbox, because hal_impl
     // doesn't need it, and we don't want it to be tempted to read it.  The
     // empty identifier will assert if it's used.
-    PROXY_IF_SANDBOXED(CancelVibrate(InSandbox() ? id : WindowIdentifier()));
+    PROXY_IF_SANDBOXED(
+        CancelVibrate(InSandbox() ? std::move(id) : WindowIdentifier()));
   }
 }
 
 template <class InfoType>
 class ObserversManager {
  public:
   void AddObserver(Observer<InfoType>* aObserver) {
     mObservers.AddObserver(aObserver);
--- a/hal/Hal.h
+++ b/hal/Hal.h
@@ -67,33 +67,32 @@ void Shutdown();
  * Only an active window within an active tab may call Vibrate; calls
  * from inactive windows and windows on inactive tabs do nothing.
  *
  * If you're calling hal::Vibrate from the outside world, pass an
  * nsIDOMWindow* in place of the WindowIdentifier parameter.
  * The method with WindowIdentifier will be called automatically.
  */
 void Vibrate(const nsTArray<uint32_t>& pattern, nsPIDOMWindowInner* aWindow);
-void Vibrate(const nsTArray<uint32_t>& pattern,
-             const hal::WindowIdentifier& id);
+void Vibrate(const nsTArray<uint32_t>& pattern, hal::WindowIdentifier&& id);
 
 /**
  * Cancel a vibration started by the content window identified by
  * WindowIdentifier.
  *
  * If the window was the last window to start a vibration, the
  * cancellation request will go through even if the window is not
  * active.
  *
  * As with hal::Vibrate(), if you're calling hal::CancelVibrate from the outside
  * world, pass an nsIDOMWindow*. The method with WindowIdentifier will be called
  * automatically.
  */
 void CancelVibrate(nsPIDOMWindowInner* aWindow);
-void CancelVibrate(const hal::WindowIdentifier& id);
+void CancelVibrate(hal::WindowIdentifier&& id);
 
 #define MOZ_DEFINE_HAL_OBSERVER(name_)                             \
   /**                                                              \
    * Inform the backend there is a new |name_| observer.           \
    * @param aObserver The observer that should be added.           \
    */                                                              \
   void Register##name_##Observer(hal::name_##Observer* aObserver); \
   /**                                                              \
--- a/hal/HalWakeLock.cpp
+++ b/hal/HalWakeLock.cpp
@@ -18,17 +18,17 @@ using namespace mozilla;
 using namespace mozilla::hal;
 
 namespace {
 
 struct LockCount {
   LockCount() : numLocks(0), numHidden(0) {}
   uint32_t numLocks;
   uint32_t numHidden;
-  nsTArray<uint64_t> processes;
+  CopyableTArray<uint64_t> processes;
 };
 
 typedef nsDataHashtable<nsUint64HashKey, LockCount> ProcessLockTable;
 typedef nsClassHashtable<nsStringHashKey, ProcessLockTable> LockTable;
 
 int sActiveListeners = 0;
 StaticAutoPtr<LockTable> sLockTable;
 bool sIsShuttingDown = false;
--- a/hal/WindowIdentifier.cpp
+++ b/hal/WindowIdentifier.cpp
@@ -7,32 +7,36 @@
 #include "WindowIdentifier.h"
 
 #include "mozilla/dom/ContentChild.h"
 #include "nsPIDOMWindow.h"
 
 namespace mozilla {
 namespace hal {
 
-WindowIdentifier::WindowIdentifier() : mWindow(nullptr), mIsEmpty(true) {}
+WindowIdentifier::WindowIdentifier()
+    : mWindow(nullptr)
+#ifdef DEBUG
+      ,
+      mIsEmpty(true)
+#endif
+{
+}
 
 WindowIdentifier::WindowIdentifier(nsPIDOMWindowInner* window)
-    : mWindow(window), mIsEmpty(false) {
+    : mWindow(window) {
   mID.AppendElement(GetWindowID());
 }
 
-WindowIdentifier::WindowIdentifier(const nsTArray<uint64_t>& id,
+WindowIdentifier::WindowIdentifier(nsTArray<uint64_t>&& id,
                                    nsPIDOMWindowInner* window)
-    : mID(id), mWindow(window), mIsEmpty(false) {
+    : mID(std::move(id)), mWindow(window) {
   mID.AppendElement(GetWindowID());
 }
 
-WindowIdentifier::WindowIdentifier(const WindowIdentifier& other)
-    : mID(other.mID), mWindow(other.mWindow), mIsEmpty(other.mIsEmpty) {}
-
 const nsTArray<uint64_t>& WindowIdentifier::AsArray() const {
   MOZ_ASSERT(!mIsEmpty);
   return mID;
 }
 
 bool WindowIdentifier::HasTraveledThroughIPC() const {
   MOZ_ASSERT(!mIsEmpty);
   return mID.Length() >= 2;
--- a/hal/WindowIdentifier.h
+++ b/hal/WindowIdentifier.h
@@ -42,36 +42,31 @@ class WindowIdentifier {
    * Create an empty WindowIdentifier.  Calls to any of this object's
    * public methods will assert -- an empty WindowIdentifier may be
    * used only as a placeholder to code which promises not to touch
    * the object.
    */
   WindowIdentifier();
 
   /**
-   * Copy constructor.
-   */
-  WindowIdentifier(const WindowIdentifier& other);
-
-  /**
    * Wrap the given window in a WindowIdentifier.  These two
    * constructors automatically grab the window's ID and append it to
    * the array of IDs.
    *
    * Note that these constructors allow an implicit conversion to a
    * WindowIdentifier.
    */
   explicit WindowIdentifier(nsPIDOMWindowInner* window);
 
   /**
    * Create a new WindowIdentifier with the given id array and window.
    * This automatically grabs the window's ID and appends it to the
    * array.
    */
-  WindowIdentifier(const nsTArray<uint64_t>& id, nsPIDOMWindowInner* window);
+  WindowIdentifier(nsTArray<uint64_t>&& id, nsPIDOMWindowInner* window);
 
   /**
    * Get the list of window and process IDs we contain.
    */
   typedef nsTArray<uint64_t> IDArrayType;
   const IDArrayType& AsArray() const;
 
   /**
@@ -95,15 +90,17 @@ class WindowIdentifier {
  private:
   /**
    * Get the ID of the window object we wrap.
    */
   uint64_t GetWindowID() const;
 
   AutoTArray<uint64_t, 3> mID;
   nsCOMPtr<nsPIDOMWindowInner> mWindow;
-  bool mIsEmpty;
+#ifdef DEBUG
+  bool mIsEmpty = false;
+#endif
 };
 
 }  // namespace hal
 }  // namespace mozilla
 
 #endif  // mozilla_hal_WindowIdentifier_h
--- a/hal/android/AndroidHal.cpp
+++ b/hal/android/AndroidHal.cpp
@@ -15,17 +15,17 @@
 using namespace mozilla::dom;
 using namespace mozilla::hal;
 
 namespace java = mozilla::java;
 
 namespace mozilla {
 namespace hal_impl {
 
-void Vibrate(const nsTArray<uint32_t>& pattern, const WindowIdentifier&) {
+void Vibrate(const nsTArray<uint32_t>& pattern, WindowIdentifier&&) {
   // Ignore the WindowIdentifier parameter; it's here only because hal::Vibrate,
   // hal_sandbox::Vibrate, and hal_impl::Vibrate all must have the same
   // signature.
 
   // Strangely enough, the Android Java API seems to treat vibrate([0]) as a
   // nop.  But we want to treat vibrate([0]) like CancelVibrate!  (Note that we
   // also need to treat vibrate([]) as a call to CancelVibrate.)
   bool allZero = true;
@@ -44,17 +44,17 @@ void Vibrate(const nsTArray<uint32_t>& p
   AndroidBridge* b = AndroidBridge::Bridge();
   if (!b) {
     return;
   }
 
   b->Vibrate(pattern);
 }
 
-void CancelVibrate(const WindowIdentifier&) {
+void CancelVibrate(WindowIdentifier&&) {
   // Ignore WindowIdentifier parameter.
 
   java::GeckoAppShell::CancelVibrate();
 }
 
 void EnableBatteryNotifications() {
   java::GeckoAppShell::EnableBatteryNotifications();
 }
--- a/hal/fallback/FallbackVibration.cpp
+++ b/hal/fallback/FallbackVibration.cpp
@@ -6,14 +6,14 @@
 
 #include "Hal.h"
 
 using mozilla::hal::WindowIdentifier;
 
 namespace mozilla {
 namespace hal_impl {
 
-void Vibrate(const nsTArray<uint32_t>& pattern, const hal::WindowIdentifier&) {}
+void Vibrate(const nsTArray<uint32_t>& pattern, hal::WindowIdentifier&&) {}
 
-void CancelVibrate(const hal::WindowIdentifier&) {}
+void CancelVibrate(hal::WindowIdentifier&&) {}
 
 }  // namespace hal_impl
 }  // namespace mozilla
--- a/hal/sandbox/SandboxHal.cpp
+++ b/hal/sandbox/SandboxHal.cpp
@@ -31,31 +31,29 @@ bool HalChildDestroyed() { return sHalCh
 static PHalChild* sHal;
 static PHalChild* Hal() {
   if (!sHal) {
     sHal = ContentChild::GetSingleton()->SendPHalConstructor();
   }
   return sHal;
 }
 
-void Vibrate(const nsTArray<uint32_t>& pattern, const WindowIdentifier& id) {
+void Vibrate(const nsTArray<uint32_t>& pattern, WindowIdentifier&& id) {
   HAL_LOG("Vibrate: Sending to parent process.");
 
-  AutoTArray<uint32_t, 8> p(pattern);
-
-  WindowIdentifier newID(id);
+  WindowIdentifier newID(std::move(id));
   newID.AppendProcessID();
-  Hal()->SendVibrate(p, newID.AsArray(),
+  Hal()->SendVibrate(pattern, newID.AsArray(),
                      BrowserChild::GetFrom(newID.GetWindow()));
 }
 
-void CancelVibrate(const WindowIdentifier& id) {
+void CancelVibrate(WindowIdentifier&& id) {
   HAL_LOG("CancelVibrate: Sending to parent process.");
 
-  WindowIdentifier newID(id);
+  WindowIdentifier newID(std::move(id));
   newID.AppendProcessID();
   Hal()->SendCancelVibrate(newID.AsArray(),
                            BrowserChild::GetFrom(newID.GetWindow()));
 }
 
 void EnableBatteryNotifications() { Hal()->SendEnableBatteryNotifications(); }
 
 void DisableBatteryNotifications() { Hal()->SendDisableBatteryNotifications(); }
@@ -161,30 +159,28 @@ class HalParent : public PHalParent,
       nsTArray<unsigned int>&& pattern, nsTArray<uint64_t>&& id,
       PBrowserParent* browserParent) override {
     // We give all content vibration permission.
     //    BrowserParent *browserParent = BrowserParent::GetFrom(browserParent);
     /* xxxkhuey wtf
     nsCOMPtr<nsIDOMWindow> window =
       do_QueryInterface(browserParent->GetBrowserDOMWindow());
     */
-    WindowIdentifier newID(id, nullptr);
-    hal::Vibrate(pattern, newID);
+    hal::Vibrate(pattern, WindowIdentifier(std::move(id), nullptr));
     return IPC_OK();
   }
 
   virtual mozilla::ipc::IPCResult RecvCancelVibrate(
       nsTArray<uint64_t>&& id, PBrowserParent* browserParent) override {
     // BrowserParent *browserParent = BrowserParent::GetFrom(browserParent);
     /* XXXkhuey wtf
     nsCOMPtr<nsIDOMWindow> window =
       browserParent->GetBrowserDOMWindow();
     */
-    WindowIdentifier newID(id, nullptr);
-    hal::CancelVibrate(newID);
+    hal::CancelVibrate(WindowIdentifier(std::move(id), nullptr));
     return IPC_OK();
   }
 
   virtual mozilla::ipc::IPCResult RecvEnableBatteryNotifications() override {
     // We give all content battery-status permission.
     hal::RegisterBatteryObserver(this);
     return IPC_OK();
   }