Clients.matchAll() should return ordered clients by type/focus time/creation time

content/browser/service_worker/service_worker_client_utils.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/service_worker/service_worker_client_utils.h"
 
 #include <algorithm>
 #include <tuple>
 
 #include "base/location.h"
@@ skipping 84 matching lines @@
 
   int frame_tree_node_id_;
   const OpenURLCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(OpenURLObserver);
 };
 
 ServiceWorkerClientInfo GetWindowClientInfoOnUI(
     int render_process_id,
     int render_frame_id,
+    base::TimeTicks create_time,
     const std::string& client_uuid) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RenderFrameHostImpl* render_frame_host =
       RenderFrameHostImpl::FromID(render_process_id, render_frame_id);
   if (!render_frame_host)
     return ServiceWorkerClientInfo();
 
   // TODO(mlamouri,michaeln): it is possible to end up collecting information
   // for a frame that is actually being navigated and isn't exactly what we are
   // expecting.
   return ServiceWorkerClientInfo(
       client_uuid, render_frame_host->GetVisibilityState(),
       render_frame_host->IsFocused(), render_frame_host->GetLastCommittedURL(),
       render_frame_host->GetParent() ? REQUEST_CONTEXT_FRAME_TYPE_NESTED
                                      : REQUEST_CONTEXT_FRAME_TYPE_TOP_LEVEL,
-      render_frame_host->frame_tree_node()->last_focus_time(),
+      render_frame_host->frame_tree_node()->last_focus_time(), create_time,
       blink::kWebServiceWorkerClientTypeWindow);
 }
 
 ServiceWorkerClientInfo FocusOnUI(int render_process_id,
                                   int render_frame_id,
+                                  base::TimeTicks create_time,
                                   const std::string& client_uuid) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RenderFrameHostImpl* render_frame_host =
       RenderFrameHostImpl::FromID(render_process_id, render_frame_id);
   WebContentsImpl* web_contents = static_cast<WebContentsImpl*>(
       WebContents::FromRenderFrameHost(render_frame_host));
 
   if (!render_frame_host || !web_contents)
     return ServiceWorkerClientInfo();
 
   FrameTreeNode* frame_tree_node = render_frame_host->frame_tree_node();
 
   // Focus the frame in the frame tree node, in case it has changed.
   frame_tree_node->frame_tree()->SetFocusedFrame(
       frame_tree_node, render_frame_host->GetSiteInstance());
 
   // Focus the frame's view to make sure the frame is now considered as focused.
   render_frame_host->GetView()->Focus();
 
   // Move the web contents to the foreground.
   web_contents->Activate();
 
   return GetWindowClientInfoOnUI(render_process_id, render_frame_id,
-                                 client_uuid);
+                                 create_time, client_uuid);
 }
 
 // This is only called for main frame navigations in OpenWindowOnUI().
 void DidOpenURLOnUI(const OpenURLCallback& callback,
                     WebContents* web_contents) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (!web_contents) {
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
@@ skipping 107 matching lines @@
            context->GetClientProviderHostIterator(origin);
        !it->IsAtEnd(); it->Advance()) {
     ServiceWorkerProviderHost* provider_host = it->GetProviderHost();
     if (provider_host->process_id() != render_process_id ||
         provider_host->frame_id() != render_frame_id) {
       continue;
     }
     BrowserThread::PostTaskAndReplyWithResult(
         BrowserThread::UI, FROM_HERE,
         base::Bind(&GetWindowClientInfoOnUI, provider_host->process_id(),
-                   provider_host->route_id(), provider_host->client_uuid()),
+                   provider_host->route_id(), provider_host->create_time(),
+                   provider_host->client_uuid()),
         base::Bind(callback, SERVICE_WORKER_OK));
     return;
   }
 
   // If here, it means that no provider_host was found, in which case, the
   // renderer should still be informed that the window was opened.
   callback.Run(SERVICE_WORKER_OK, ServiceWorkerClientInfo());
 }
 
 void AddWindowClient(
     ServiceWorkerProviderHost* host,
-    std::vector<std::tuple<int, int, std::string>>* client_info) {
+    std::vector<std::tuple<int, int, base::TimeTicks, std::string>>*
+        client_info) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (host->client_type() != blink::kWebServiceWorkerClientTypeWindow)
     return;
   client_info->push_back(std::make_tuple(host->process_id(), host->frame_id(),
+                                         host->create_time(),
                                          host->client_uuid()));
 }
 
 void AddNonWindowClient(ServiceWorkerProviderHost* host,
                         const ServiceWorkerClientQueryOptions& options,
                         ServiceWorkerClients* clients) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   blink::WebServiceWorkerClientType host_client_type = host->client_type();
   if (host_client_type == blink::kWebServiceWorkerClientTypeWindow)
     return;
   if (options.client_type != blink::kWebServiceWorkerClientTypeAll &&
       options.client_type != host_client_type)
     return;
 
   ServiceWorkerClientInfo client_info(
       host->client_uuid(), blink::kWebPageVisibilityStateHidden,
       false,  // is_focused
       host->document_url(), REQUEST_CONTEXT_FRAME_TYPE_NONE, base::TimeTicks(),
-      host_client_type);
+      host->create_time(), host_client_type);
   clients->push_back(client_info);
 }
 
 void OnGetWindowClientsOnUI(
     // The tuple contains process_id, frame_id, client_uuid.
-    const std::vector<std::tuple<int, int, std::string>>& clients_info,
+    const std::vector<std::tuple<int, int, base::TimeTicks, std::string>>&
+        clients_info,
     const GURL& script_url,
     const GetWindowClientsCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   std::unique_ptr<ServiceWorkerClients> clients(new ServiceWorkerClients);

[shimazu, 2017/05/25 08:56:26]

Could you use base::MakeUnique<ServiceWorkerClients>() instead?

[xiaofengzhang, 2017/05/26 04:01:28]

Done.

   for (const auto& it : clients_info) {
     ServiceWorkerClientInfo info = GetWindowClientInfoOnUI(
-        std::get<0>(it), std::get<1>(it), std::get<2>(it));
+        std::get<0>(it), std::get<1>(it), std::get<2>(it), std::get<3>(it));
 
     // If the request to the provider_host returned an empty
     // ServiceWorkerClientInfo, that means that it wasn't possible to associate
     // it with a valid RenderFrameHost. It might be because the frame was killed
     // or navigated in between.
     if (info.IsEmpty())
       continue;
 
     // We can get info for a frame that was navigating end ended up with a
     // different URL than expected. In such case, we should make sure to not
     // expose cross-origin WindowClient.
     if (info.url.GetOrigin() != script_url.GetOrigin())
       continue;
 
     clients->push_back(info);
   }
 
   BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
                           base::Bind(callback, base::Passed(&clients)));
 }
 
+struct ServiceWorkerClientTypeIsWindow {
+  bool operator()(const ServiceWorkerClientInfo& a,
+                  const ServiceWorkerClientInfo& b) const {
+    return a.client_type == blink::kWebServiceWorkerClientTypeWindow;
+  }
+};

[xiaofengzhang, 2017/05/26 04:01:28]

Sorry, this part is not needed

[shimazu, 2017/05/26 08:10:07]

Acknowledged.

+
 struct ServiceWorkerClientInfoSortMRU {
   bool operator()(const ServiceWorkerClientInfo& a,
                   const ServiceWorkerClientInfo& b) const {
     return a.last_focus_time > b.last_focus_time;
   }
 };
 
+struct ServiceWorkerClientInfoSortCreateTime {
+  bool operator()(const ServiceWorkerClientInfo& a,
+                  const ServiceWorkerClientInfo& b) const {
+    return a.create_time < b.create_time;
+  }
+};
+
 void DidGetClients(const ClientsCallback& callback,
                    ServiceWorkerClients* clients) {

[shimazu, 2017/05/25 08:56:26]

Could you change the type to std::unique_ptr<ServiceWorkerClients> and use
std::move() to passing the argument?

[xiaofengzhang, 2017/05/26 04:01:27]

Done.

   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  // Sort clients so that the most recently active tab is in the front.
-  std::sort(clients->begin(), clients->end(), ServiceWorkerClientInfoSortMRU());
+  // Window clients are always placed before worker clients.
+  auto window_clients_bound = std::stable_partition(
+      clients->begin(), clients->end(), [](ServiceWorkerClientInfo info) {

[shimazu, 2017/05/25 08:56:26]

I feel it's more simple to implement operator< in ServiceWorkerClientInfo. 
WDYT?

[xiaofengzhang, 2017/05/26 04:01:27]

Thanks shimazu. please see the cl#2.
The reasons I keep it here instead of ServiceWorkerClientInfo is that I think
ServiceWorkerClientInfo is only for holding the information of service worker
client, and std::sort() maybe not clear enough without a comparison function.
WDYT?

[shimazu, 2017/05/26 08:10:07]

It looks good, thanks!

+        return info.client_type == blink::kWebServiceWorkerClientTypeWindow;
+      });
+
+  // Window clients that have ever been focused are placed first.
+  auto focused_bound = std::stable_partition(
+      clients->begin(), window_clients_bound, [](ServiceWorkerClientInfo info) {
+        return info.last_focus_time > base::TimeTicks();
+      });
+
+  // Sort focused window clients so that the most recently active tab is in the
+  // front.
+  std::sort(clients->begin(), focused_bound, ServiceWorkerClientInfoSortMRU());
+
+  // Sort non-focused window clients so that the earliest created tab is in the
+  // front.
+  std::sort(focused_bound, clients->end(),
+            ServiceWorkerClientInfoSortCreateTime());
+
+  // Sort worker clients so that the earliest created one is in the front.
+  std::sort(window_clients_bound, clients->end(),
+            ServiceWorkerClientInfoSortCreateTime());
 
   callback.Run(clients);
 }
 
 void GetNonWindowClients(const base::WeakPtr<ServiceWorkerVersion>& controller,
                          const ServiceWorkerClientQueryOptions& options,
                          ServiceWorkerClients* clients) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (!options.include_uncontrolled) {
     for (auto& controllee : controller->controllee_map())
@@ skipping 17 matching lines @@
   DidGetClients(callback, clients.get());
 }
 
 void GetWindowClients(const base::WeakPtr<ServiceWorkerVersion>& controller,
                       const ServiceWorkerClientQueryOptions& options,
                       const ClientsCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(options.client_type == blink::kWebServiceWorkerClientTypeWindow ||
          options.client_type == blink::kWebServiceWorkerClientTypeAll);
 
-  std::vector<std::tuple<int, int, std::string>> clients_info;
+  std::vector<std::tuple<int, int, base::TimeTicks, std::string>> clients_info;
   if (!options.include_uncontrolled) {
-    for (auto& controllee : controller->controllee_map())
+    for (auto& controllee : controller->controllee_map()) {
       AddWindowClient(controllee.second, &clients_info);
+    }
   } else if (controller->context()) {
     GURL origin = controller->script_url().GetOrigin();
     for (auto it = controller->context()->GetClientProviderHostIterator(origin);
          !it->IsAtEnd(); it->Advance()) {
       AddWindowClient(it->GetProviderHost(), &clients_info);
     }
   }
 
   if (clients_info.empty()) {
     DidGetWindowClients(controller, options, callback,
@@ skipping 11 matching lines @@
 }  // namespace
 
 void FocusWindowClient(ServiceWorkerProviderHost* provider_host,
                        const ClientCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK_EQ(blink::kWebServiceWorkerClientTypeWindow,
             provider_host->client_type());
   BrowserThread::PostTaskAndReplyWithResult(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&FocusOnUI, provider_host->process_id(),
-                 provider_host->frame_id(), provider_host->client_uuid()),
+                 provider_host->frame_id(), provider_host->create_time(),
+                 provider_host->client_uuid()),
       callback);
 }
 
 void OpenWindow(const GURL& url,
                 const GURL& script_url,
                 int worker_process_id,
                 const base::WeakPtr<ServiceWorkerContextCore>& context,
                 const NavigationCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   BrowserThread::PostTask(
@@ skipping 25 matching lines @@
   blink::WebServiceWorkerClientType client_type = provider_host->client_type();
   DCHECK(client_type == blink::kWebServiceWorkerClientTypeWindow ||
          client_type == blink::kWebServiceWorkerClientTypeWorker ||
          client_type == blink::kWebServiceWorkerClientTypeSharedWorker)
       << client_type;
 
   if (client_type == blink::kWebServiceWorkerClientTypeWindow) {
     BrowserThread::PostTaskAndReplyWithResult(
         BrowserThread::UI, FROM_HERE,
         base::Bind(&GetWindowClientInfoOnUI, provider_host->process_id(),
-                   provider_host->route_id(), provider_host->client_uuid()),
+                   provider_host->route_id(), provider_host->create_time(),
+                   provider_host->client_uuid()),
         callback);
     return;
   }
 
   ServiceWorkerClientInfo client_info(
       provider_host->client_uuid(), blink::kWebPageVisibilityStateHidden,
       false,  // is_focused
       provider_host->document_url(), REQUEST_CONTEXT_FRAME_TYPE_NONE,
-      base::TimeTicks(), provider_host->client_type());
+      base::TimeTicks(), provider_host->create_time(),
+      provider_host->client_type());
   BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
                           base::Bind(callback, client_info));
 }
 
 void GetClients(const base::WeakPtr<ServiceWorkerVersion>& controller,
                 const ServiceWorkerClientQueryOptions& options,
                 const ClientsCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   ServiceWorkerClients clients;

[shimazu, 2017/05/25 08:56:26]

How about creating std::unique_ptr<ServiceWorkerClients> here and pass it by
std::move() in the both paths:
- GetWindowClients -> DidGetWindowClients -> DidGetClients
- DidGetClients

[xiaofengzhang, 2017/05/26 04:01:28]

Done.

   if (!controller->HasControllee() && !options.include_uncontrolled) {
     DidGetClients(callback, &clients);
     return;
   }
 
   // For Window clients we want to query the info on the UI thread first.
   if (options.client_type == blink::kWebServiceWorkerClientTypeWindow ||
       options.client_type == blink::kWebServiceWorkerClientTypeAll) {
     GetWindowClients(controller, options, callback);
     return;
   }
 
   GetNonWindowClients(controller, options, &clients);
   DidGetClients(callback, &clients);
 }
 
 }  // namespace service_worker_client_utils
 }  // namespace content