Bug 1266747 P3 Sort output of Clients.matchAll(). r=smaug
☠☠ backed out by d90cb83b58bf ☠ ☠
authorBen Kelly <ben@wanderview.com>
Tue, 28 Feb 2017 10:48:51 -0500
changeset 394213 274999e28c0773393d07754c765fb627d79ecc37
parent 394212 e54c8d21e4c1fa34090ffb2190797be37da6f8fc
child 394214 b53d88cb7099db7cf46c687c22b28947d0f4ccc5
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1266747
milestone54.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 1266747 P3 Sort output of Clients.matchAll(). r=smaug
dom/workers/ServiceWorkerClient.cpp
dom/workers/ServiceWorkerClient.h
dom/workers/ServiceWorkerManager.cpp
--- a/dom/workers/ServiceWorkerClient.cpp
+++ b/dom/workers/ServiceWorkerClient.cpp
@@ -28,18 +28,19 @@ NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(Se
 NS_IMPL_CYCLE_COLLECTING_ADDREF(ServiceWorkerClient)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(ServiceWorkerClient)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ServiceWorkerClient)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   NS_INTERFACE_MAP_ENTRY(nsISupports)
 NS_INTERFACE_MAP_END
 
-ServiceWorkerClientInfo::ServiceWorkerClientInfo(nsIDocument* aDoc)
+ServiceWorkerClientInfo::ServiceWorkerClientInfo(nsIDocument* aDoc, uint32_t aOrdinal)
   : mType(ClientType::Window)
+  , mOrdinal(aOrdinal)
   , mWindowId(0)
   , mFrameType(FrameType::None)
 {
   MOZ_ASSERT(aDoc);
   nsresult rv = aDoc->GetOrCreateId(mClientId);
   if (NS_FAILED(rv)) {
     NS_WARNING("Failed to get the UUID of the document.");
   }
@@ -77,16 +78,46 @@ ServiceWorkerClientInfo::ServiceWorkerCl
     mFrameType = FrameType::Nested;
   } else if (outerWindow->HadOriginalOpener()) {
     mFrameType = FrameType::Auxiliary;
   } else {
     mFrameType = FrameType::Top_level;
   }
 }
 
+bool
+ServiceWorkerClientInfo::operator<(const ServiceWorkerClientInfo& aRight) const
+{
+  // Note: the mLastFocusTime comparisons are reversed because we need to
+  // put most recently focused values first.  The mOrdinal comparison is
+  // normal, though, because otherwise we want normal creation order.
+
+  if (mLastFocusTime == aRight.mLastFocusTime) {
+    return mOrdinal < aRight.mOrdinal;
+  }
+
+  if (mLastFocusTime.IsNull()) {
+    return false;
+  }
+
+  if (aRight.mLastFocusTime.IsNull()) {
+    return true;
+  }
+
+  return mLastFocusTime > aRight.mLastFocusTime;
+}
+
+bool
+ServiceWorkerClientInfo::operator==(const ServiceWorkerClientInfo& aRight) const
+{
+  return mLastFocusTime == aRight.mLastFocusTime &&
+         mOrdinal == aRight.mOrdinal &&
+         mClientId == aRight.mClientId;
+}
+
 JSObject*
 ServiceWorkerClient::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
 {
   return ClientBinding::Wrap(aCx, this, aGivenProto);
 }
 
 ClientType
 ServiceWorkerClient::Type() const
--- a/dom/workers/ServiceWorkerClient.h
+++ b/dom/workers/ServiceWorkerClient.h
@@ -26,25 +26,29 @@ class ServiceWorkerWindowClient;
 // Used as a container object for information needed to create
 // client objects.
 class ServiceWorkerClientInfo final
 {
   friend class ServiceWorkerClient;
   friend class ServiceWorkerWindowClient;
 
 public:
-  explicit ServiceWorkerClientInfo(nsIDocument* aDoc);
+  explicit ServiceWorkerClientInfo(nsIDocument* aDoc, uint32_t aOrdinal = 0);
 
   const nsString& ClientId() const
   {
     return mClientId;
   }
 
+  bool operator<(const ServiceWorkerClientInfo& aRight) const;
+  bool operator==(const ServiceWorkerClientInfo& aRight) const;
+
 private:
   const mozilla::dom::ClientType mType;
+  const uint32_t mOrdinal;
   nsString mClientId;
   uint64_t mWindowId;
   nsString mUrl;
 
   // Window Clients
   VisibilityState mVisibilityState;
   FrameType mFrameType;
   TimeStamp mLastFocusTime;
--- a/dom/workers/ServiceWorkerManager.cpp
+++ b/dom/workers/ServiceWorkerManager.cpp
@@ -2969,71 +2969,65 @@ ServiceWorkerManager::GetAllClients(nsIP
 
   nsCOMPtr<nsISimpleEnumerator> enumerator;
   nsresult rv = obs->EnumerateObservers("service-worker-get-client",
                                         getter_AddRefs(enumerator));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
 
-  auto ProcessDocument = [&aDocuments](nsIPrincipal* aPrincipal, nsIDocument* aDoc) {
-    if (!aDoc || !aDoc->GetWindow()) {
-      return;
+  // Get a list of Client documents out of the observer service
+  AutoTArray<nsCOMPtr<nsIDocument>, 32> docList;
+  bool loop = true;
+  while (NS_SUCCEEDED(enumerator->HasMoreElements(&loop)) && loop) {
+    nsCOMPtr<nsISupports> ptr;
+    rv = enumerator->GetNext(getter_AddRefs(ptr));
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      continue;
+    }
+
+    nsCOMPtr<nsIDocument> doc = do_QueryInterface(ptr);
+    if (!doc || !doc->GetWindow()) {
+      continue;
     }
 
     bool equals = false;
-    aPrincipal->Equals(aDoc->NodePrincipal(), &equals);
+    Unused << aPrincipal->Equals(doc->NodePrincipal(), &equals);
     if (!equals) {
-      return;
+      continue;
     }
 
     // Treat http windows with devtools opened as secure if the correct devtools
     // setting is enabled.
-    if (!aDoc->GetWindow()->GetServiceWorkersTestingEnabled() &&
+    if (!doc->GetWindow()->GetServiceWorkersTestingEnabled() &&
         !Preferences::GetBool("dom.serviceWorkers.testing.enabled") &&
-        !IsFromAuthenticatedOrigin(aDoc)) {
-      return;
+        !IsFromAuthenticatedOrigin(doc)) {
+      continue;
+    }
+
+    if (!aIncludeUncontrolled && !mControlledDocuments.Contains(doc)) {
+      continue;
     }
 
-    ServiceWorkerClientInfo clientInfo(aDoc);
-    aDocuments.AppendElement(aDoc);
-  };
-
-  // Since it's not simple to check whether a document is in
-  // mControlledDocuments, we take different code paths depending on whether we
-  // need to look at all documents.  The common parts of the two loops are
-  // factored out into the ProcessDocument lambda.
-  if (aIncludeUncontrolled) {
-    bool loop = true;
-    while (NS_SUCCEEDED(enumerator->HasMoreElements(&loop)) && loop) {
-      nsCOMPtr<nsISupports> ptr;
-      rv = enumerator->GetNext(getter_AddRefs(ptr));
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        continue;
-      }
-
-      nsCOMPtr<nsIDocument> doc = do_QueryInterface(ptr);
-      ProcessDocument(aPrincipal, doc);
-    }
-  } else {
-    for (auto iter = mControlledDocuments.Iter(); !iter.Done(); iter.Next()) {
-      ServiceWorkerRegistrationInfo* thisRegistration = iter.UserData();
-      MOZ_ASSERT(thisRegistration);
-      if (!registration->mScope.Equals(thisRegistration->mScope)) {
-        continue;
-      }
-
-      nsCOMPtr<nsIDocument> doc = do_QueryInterface(iter.Key());
-
-      // All controlled documents must have an outer window.
-      MOZ_ASSERT(doc->GetWindow());
-
-      ProcessDocument(aPrincipal, doc);
-    }
-  }
+    docList.AppendElement(doc.forget());
+  }
+
+  // The observer service gives us the list in reverse creation order.
+  // We need to maintain creation order, so reverse the list before
+  // processing.
+  docList.Reverse();
+
+  // Finally convert to the list of ServiceWorkerClientInfo objects.
+  uint32_t ordinal = 0;
+  for (uint32_t i = 0; i < docList.Length(); ++i) {
+    aDocuments.AppendElement(ServiceWorkerClientInfo(docList[i], ordinal));
+    ordinal += 1;
+  }
+
+  aDocuments.Sort();
 }
 
 void
 ServiceWorkerManager::MaybeClaimClient(nsIDocument* aDocument,
                                        ServiceWorkerRegistrationInfo* aWorkerRegistration)
 {
   MOZ_ASSERT(aWorkerRegistration);
   MOZ_ASSERT(aWorkerRegistration->GetActive());