DevTools: Fix getCookies to report for all resources

content/browser/devtools/protocol/network_handler.cc

Index: content/browser/devtools/protocol/network_handler.cc
diff --git a/content/browser/devtools/protocol/network_handler.cc b/content/browser/devtools/protocol/network_handler.cc
index 60f76e22fe0c94fb9ed50cba64e1b9c8ff5fdeae..a053adea1f743d533a62cba69ea68f439ebfb09f 100644
--- a/content/browser/devtools/protocol/network_handler.cc
+++ b/content/browser/devtools/protocol/network_handler.cc
@@ -34,6 +34,52 @@ using GetAllCookiesCallback = protocol::Network::Backend::GetAllCookiesCallback;
 using SetCookieCallback = protocol::Network::Backend::SetCookieCallback;
 using DeleteCookieCallback = protocol::Network::Backend::DeleteCookieCallback;
 
+class CookieReceiver {

[pfeldman, 2017/01/18 01:41:11]

nit: inherit from base::NonThreadSafe and DCHECK(CalledOnValidThread()) below.

nit: also, making it ref-counted-thread-safe would allow graceful threads
termination.

+  public:
+    CookieReceiver(const net::CookieStore::GetCookieListCallback& callback)
+        : callback_(callback), request_count_(0) {}
+
+    void increment(int count) {
+      request_count_ += count;

[pfeldman, 2017/01/18 01:41:11]

nit: use base::BarrierClosure instead - your callback would receive it, do the
work and call barrier's closure.

+    }
+
+    net::CookieStore::GetCookieListCallback bound() {
+      return base::Bind(&CookieReceiver::GotCookies, base::Unretained(this));
+    }
+
+  protected:
+    void GotCookies(const net::CookieList& cookie_list) {
+      DCHECK_CURRENTLY_ON(BrowserThread::IO);
+      for (const net::CanonicalCookie& cookie : cookie_list) {
+        std::string key = base::StringPrintf(
+            "%s::%s::%s::%d", cookie.Name().c_str(), cookie.Domain().c_str(),
+            cookie.Path().c_str(), cookie.IsSecure());
+        cookies_[key] = cookie;
+      }
+      --request_count_;
+      if (!request_count_) {
+        SendResponse();
+        delete this;
+      }
+    }
+
+    void SendResponse() {
+      net::CookieList master_cookie_list;
+      for (const auto& pair : cookies_) {
+        master_cookie_list.push_back(pair.second);
+      }
+
+      BrowserThread::PostTask(
+          BrowserThread::UI,
+          FROM_HERE,
+          base::Bind(callback_, master_cookie_list));
+    }
+
+    const net::CookieStore::GetCookieListCallback& callback_;
+    int request_count_;
+    base::hash_map<std::string, net::CanonicalCookie> cookies_;
+};
+
 net::URLRequestContext* GetRequestContextOnIO(
     ResourceContext* resource_context,
     net::URLRequestContextGetter* context_getter,
@@ -47,37 +93,36 @@ net::URLRequestContext* GetRequestContextOnIO(
   return context;
 }
 
-void GotCookiesOnIO(
-    const net::CookieStore::GetCookieListCallback& callback,
-    const net::CookieList& cookie_list) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  BrowserThread::PostTask(
-      BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(callback, cookie_list));
-}
-
-void GetCookiesForURLOnIO(
+void GetCookiesForURLsOnIO(
     ResourceContext* resource_context,
     net::URLRequestContextGetter* context_getter,
-    const GURL& url,
-    const net::CookieStore::GetCookieListCallback& callback) {
+    const std::vector<GURL>& urls,
+    const net::CookieStore::GetCookieListCallback& finished_callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  net::URLRequestContext* request_context =
-      GetRequestContextOnIO(resource_context, context_getter, url);
-  request_context->cookie_store()->GetAllCookiesForURLAsync(
-      url, base::Bind(&GotCookiesOnIO, callback));
+  CookieReceiver* receiver = new CookieReceiver(finished_callback);
+  receiver->increment(urls.size());
+
+  net::CookieStore::GetCookieListCallback callback = receiver->bound();
+
+  for (const GURL& url : urls) {
+    net::URLRequestContext* request_context =
+        GetRequestContextOnIO(resource_context, context_getter, url);
+    request_context->cookie_store()->GetAllCookiesForURLAsync(url, callback);
+  }
 }
 
 void GetAllCookiesOnIO(
     ResourceContext* resource_context,
     net::URLRequestContextGetter* context_getter,
-    const net::CookieStore::GetCookieListCallback& callback) {
+    const net::CookieStore::GetCookieListCallback& finished_callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  CookieReceiver* receiver = new CookieReceiver(finished_callback);
+  receiver->increment(1);
+
+  net::CookieStore::GetCookieListCallback callback = receiver->bound();
   net::URLRequestContext* request_context =
       context_getter->GetURLRequestContext();
-  request_context->cookie_store()->GetAllCookiesAsync(
-      base::Bind(&GotCookiesOnIO, callback));
+  request_context->cookie_store()->GetAllCookiesAsync(callback);
 }
 
 void DeletedCookieOnIO(std::unique_ptr<DeleteCookieCallback> callback) {
@@ -151,29 +196,13 @@ template <typename Callback>
 class GetCookiesCommandBase {
  public:
   GetCookiesCommandBase(std::unique_ptr<Callback> callback)
-      : callback_(std::move(callback)), request_count_(0) {}
+      : callback_(std::move(callback)) {}
 
  protected:
-  void GotCookiesForURL(const net::CookieList& cookie_list) {
-    DCHECK_CURRENTLY_ON(BrowserThread::UI);
-    for (const net::CanonicalCookie& cookie : cookie_list) {
-      std::string key = base::StringPrintf(
-          "%s::%s::%s::%d", cookie.Name().c_str(), cookie.Domain().c_str(),
-          cookie.Path().c_str(), cookie.IsSecure());
-      cookies_[key] = cookie;
-    }
-    --request_count_;
-    if (!request_count_) {
-      SendResponse();
-      delete this;
-    }
-  }
-
-  void SendResponse() {
+  void SendResponse(const net::CookieList& cookie_list) {
     std::unique_ptr<protocol::Array<Network::Cookie>> cookies =
         protocol::Array<Network::Cookie>::create();
-    for (const auto& pair : cookies_) {
-      const net::CanonicalCookie& cookie = pair.second;
+    for (const auto& cookie : cookie_list) {
       std::unique_ptr<Network::Cookie> devtools_cookie =
            Network::Cookie::Create()
            .SetName(cookie.Name())
@@ -203,42 +232,56 @@ class GetCookiesCommandBase {
   }
 
   std::unique_ptr<Callback> callback_;
-  int request_count_;
-  base::hash_map<std::string, net::CanonicalCookie> cookies_;
 };
 
 class GetCookiesCommand : public GetCookiesCommandBase<GetCookiesCallback> {
  public:
   GetCookiesCommand(RenderFrameHostImpl* frame_host,
+                    Maybe<protocol::Array<std::string>>& maybe_urls,
                     std::unique_ptr<GetCookiesCallback> callback)
       : GetCookiesCommandBase(std::move(callback)) {
+    std::vector<GURL> urls_to_fetch;
     net::CookieStore::GetCookieListCallback got_cookies_callback = base::Bind(
-        &GetCookiesCommand::GotCookiesForURL, base::Unretained(this));
-
-    std::queue<FrameTreeNode*> queue;
-    queue.push(frame_host->frame_tree_node());
-    while (!queue.empty()) {
-      FrameTreeNode* node = queue.front();
-      queue.pop();
-
-      // Only traverse nodes with the same local root.
-      if (node->current_frame_host()->IsCrossProcessSubframe())
-        continue;
-      ++request_count_;
-      BrowserThread::PostTask(
-          BrowserThread::IO, FROM_HERE,
-          base::Bind(&GetCookiesForURLOnIO,
-                     base::Unretained(frame_host->GetSiteInstance()
-                                          ->GetBrowserContext()
-                                          ->GetResourceContext()),
-                     base::Unretained(frame_host->GetProcess()
-                                          ->GetStoragePartition()
-                                          ->GetURLRequestContext()),
-                     node->current_url(), got_cookies_callback));
-
-      for (size_t i = 0; i < node->child_count(); ++i)
-        queue.push(node->child_at(i));
+        &GetCookiesCommand::SendResponse, base::Unretained(this));

[pfeldman, 2017/01/18 01:41:11]

This is dangerous - when your callback triggers, this object can no longer be
there. You need to use WeakPtr factory here.

+
+    if (maybe_urls.isJust()) {
+      std::unique_ptr<protocol::Array<std::string>> urls =
+          maybe_urls.takeJust();
+
+      for (size_t i = 0; i < urls->length(); i++)
+        urls_to_fetch.push_back(GURL(urls->get(i)));
+    } else {
+      std::queue<FrameTreeNode*> queue;
+      queue.push(frame_host->frame_tree_node());
+      while (!queue.empty()) {
+        FrameTreeNode* node = queue.front();
+        queue.pop();
+
+        // Only traverse nodes with the same local root.
+        if (node->current_frame_host()->IsCrossProcessSubframe())
+          continue;
+
+        urls_to_fetch.push_back(node->current_url());
+        for (size_t i = 0; i < node->child_count(); ++i)
+          queue.push(node->child_at(i));
+      }
     }
+
+    if (urls_to_fetch.size() == 0) {
+      got_cookies_callback();
+      return;
+    }
+
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&GetCookiesForURLsOnIO,
+                   base::Unretained(frame_host->GetSiteInstance()
+                                        ->GetBrowserContext()
+                                        ->GetResourceContext()),
+                   base::Unretained(frame_host->GetProcess()
+                                        ->GetStoragePartition()
+                                        ->GetURLRequestContext()),
+                   urls_to_fetch, got_cookies_callback));
   }
 };
 
@@ -249,9 +292,8 @@ class GetAllCookiesCommand
                        std::unique_ptr<GetAllCookiesCallback> callback)
       : GetCookiesCommandBase(std::move(callback)) {
     net::CookieStore::GetCookieListCallback got_cookies_callback = base::Bind(
-        &GetAllCookiesCommand::GotCookiesForURL, base::Unretained(this));
+        &GetAllCookiesCommand::SendResponse, base::Unretained(this));
 
-    request_count_ = 1;
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         base::Bind(&GetAllCookiesOnIO,
@@ -315,11 +357,12 @@ Response NetworkHandler::ClearBrowserCookies() {
 }
 
 void NetworkHandler::GetCookies(
+    Maybe<protocol::Array<String>> urls,
     std::unique_ptr<GetCookiesCallback> callback) {
   if (!host_)
     callback->sendFailure(Response::InternalError());
   else
-    new GetCookiesCommand(host_, std::move(callback));
+    new GetCookiesCommand(host_, urls, std::move(callback));
 }
 
 void NetworkHandler::GetAllCookies(