DevTools: Fix getCookies to report for all resources

content/browser/devtools/protocol/network_handler.cc

 // Copyright 2014 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/devtools/protocol/network_handler.h"
 
 #include <stddef.h>
 
+#include "base/barrier_closure.h"
 #include "base/containers/hash_tables.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
 #include "content/browser/devtools/devtools_session.h"
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_client.h"
 #include "net/cookies/cookie_store.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 
 namespace content {
 namespace protocol {
 namespace {
 
+using ProtocolCookieArray = protocol::Array<Network::Cookie>;
 using GetCookiesCallback = protocol::Network::Backend::GetCookiesCallback;
 using GetAllCookiesCallback = protocol::Network::Backend::GetAllCookiesCallback;
 using SetCookieCallback = protocol::Network::Backend::SetCookieCallback;
 using DeleteCookieCallback = protocol::Network::Backend::DeleteCookieCallback;
 
 net::URLRequestContext* GetRequestContextOnIO(
     ResourceContext* resource_context,
     net::URLRequestContextGetter* context_getter,
     const GURL& url) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   net::URLRequestContext* context =
       GetContentClient()->browser()->OverrideRequestContextForURL(
           url, resource_context);
   if (!context)
     context = context_getter->GetURLRequestContext();
   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));
-}
+class CookieRetriever : public base::RefCountedThreadSafe<CookieRetriever> {
+  public:
+    CookieRetriever(std::unique_ptr<GetCookiesCallback> callback)
+        : callback_(std::move(callback)),
+          all_callback_(nullptr),
+          callback_count_(0) {}
 
-void GetCookiesForURLOnIO(
-    ResourceContext* resource_context,
-    net::URLRequestContextGetter* context_getter,
-    const GURL& url,
-    const net::CookieStore::GetCookieListCallback& 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));
-}
+    CookieRetriever(std::unique_ptr<GetAllCookiesCallback> callback)
+        : callback_(nullptr),
+          all_callback_(std::move(callback)),
+          callback_count_(0) {}
 
-void GetAllCookiesOnIO(
-    ResourceContext* resource_context,
-    net::URLRequestContextGetter* context_getter,
-    const net::CookieStore::GetCookieListCallback& callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  net::URLRequestContext* request_context =
-      context_getter->GetURLRequestContext();
-  request_context->cookie_store()->GetAllCookiesAsync(
-      base::Bind(&GotCookiesOnIO, callback));
-}
+    void RetrieveCookiesOnIO(
+        ResourceContext* resource_context,
+        net::URLRequestContextGetter* context_getter,
+        const std::vector<GURL>& urls) {
+      DCHECK_CURRENTLY_ON(BrowserThread::IO);
+      callback_count_ = urls.size();
+
+      if (callback_count_ == 0) {

[pfeldman, 2017/01/19 18:58:35]

nit: some would say !callback_count_.

[phulce, 2017/01/19 21:18:57]

Acknowledged.

+        GotAllCookies();
+        return;
+      }
+
+      for (const GURL& url : urls) {
+        net::URLRequestContext* request_context =
+            GetRequestContextOnIO(resource_context, context_getter, url);
+        request_context->cookie_store()->GetAllCookiesForURLAsync(url,
+            base::Bind(&CookieRetriever::GotCookies, this));
+      }
+    }
+
+    void RetrieveAllCookiesOnIO(
+        ResourceContext* resource_context,
+        net::URLRequestContextGetter* context_getter) {
+      DCHECK_CURRENTLY_ON(BrowserThread::IO);
+      callback_count_ = 1;
+
+      net::URLRequestContext* request_context =
+          context_getter->GetURLRequestContext();
+      request_context->cookie_store()->GetAllCookiesAsync(
+          base::Bind(&CookieRetriever::GotCookies, this));
+    }
+  protected:
+    virtual ~CookieRetriever() {}
+
+    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;
+      }
+
+      --callback_count_;
+      if (callback_count_ == 0) {

[pfeldman, 2017/01/19 18:58:35]

nit: some would drop {} and inline --callback_count_

[phulce, 2017/01/19 21:18:58]

Done.

+        GotAllCookies();
+      }
+    }
+
+    void GotAllCookies() {
+      net::CookieList master_cookie_list;
+      for (const auto& pair : cookies_) {

[pfeldman, 2017/01/19 18:58:35]

nit: ditto

[phulce, 2017/01/19 21:18:57]

Done.

+        master_cookie_list.push_back(pair.second);
+      }
+
+      BrowserThread::PostTask(
+          BrowserThread::UI,
+          FROM_HERE,
+          base::Bind(&CookieRetriever::SendCookiesResponseOnUI,
+                     this,
+                     master_cookie_list));
+    }
+
+    void SendCookiesResponseOnUI(const net::CookieList& cookie_list) {
+      DCHECK_CURRENTLY_ON(BrowserThread::UI);
+      std::unique_ptr<ProtocolCookieArray> cookies =
+          ProtocolCookieArray::create();
+
+      for (const net::CanonicalCookie& cookie : cookie_list) {
+       std::unique_ptr<Network::Cookie> devtools_cookie =
+            Network::Cookie::Create()
+            .SetName(cookie.Name())
+            .SetValue(cookie.Value())
+            .SetDomain(cookie.Domain())
+            .SetPath(cookie.Path())
+            .SetExpires(cookie.ExpiryDate().ToDoubleT() * 1000)
+            .SetSize(cookie.Name().length() + cookie.Value().length())
+            .SetHttpOnly(cookie.IsHttpOnly())
+            .SetSecure(cookie.IsSecure())
+            .SetSession(!cookie.IsPersistent())
+            .Build();
+
+       switch (cookie.SameSite()) {
+         case net::CookieSameSite::STRICT_MODE:
+           devtools_cookie->SetSameSite(Network::CookieSameSiteEnum::Strict);
+           break;
+         case net::CookieSameSite::LAX_MODE:
+           devtools_cookie->SetSameSite(Network::CookieSameSiteEnum::Lax);
+           break;
+         case net::CookieSameSite::NO_RESTRICTION:
+           break;
+       }
+
+       cookies->addItem(std::move(devtools_cookie));
+      }
+
+      if (callback_ != nullptr) {

[pfeldman, 2017/01/19 18:58:35]

if (callback_)

[phulce, 2017/01/19 21:18:57]

Done.

+        callback_->sendSuccess(std::move(cookies));
+      } else if (all_callback_ != nullptr) {

[pfeldman, 2017/01/19 18:58:35]

} else {
  DCHECK(all_callback_);
  ...
}

[phulce, 2017/01/19 21:18:57]

Done.

+        all_callback_->sendSuccess(std::move(cookies));
+      }
+    }
+
+    std::unique_ptr<GetCookiesCallback> callback_;
+    std::unique_ptr<GetAllCookiesCallback> all_callback_;
+    int callback_count_;

[pfeldman, 2017/01/19 18:58:35]

in the new c++ you can say int callback_count_ = 0; and drop it from super
initializers.

[phulce, 2017/01/19 21:18:57]

Done.

+    base::hash_map<std::string, net::CanonicalCookie> cookies_;

[pfeldman, 2017/01/19 18:58:35]

use std::unordered_map (hash_map is just an alias)

[phulce, 2017/01/19 21:18:57]

Done.

+
+  private:
+    friend class base::RefCountedThreadSafe<CookieRetriever>;
+};
 
 void DeletedCookieOnIO(std::unique_ptr<DeleteCookieCallback> callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
       base::Bind(&DeleteCookieCallback::sendSuccess,
                  base::Passed(std::move(callback))));
 }
 
@@ skipping 48 matching lines @@
       expires,
       base::Time(),
       secure,
       http_only,
       same_site,
       are_experimental_cookie_features_enabled,
       net::COOKIE_PRIORITY_DEFAULT,
       base::Bind(&CookieSetOnIO, base::Passed(std::move(callback))));
 }
 
-template <typename Callback>
-class GetCookiesCommandBase {
- public:
-  GetCookiesCommandBase(std::unique_ptr<Callback> callback)
-      : callback_(std::move(callback)), request_count_(0) {}
-
- 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() {
-    std::unique_ptr<protocol::Array<Network::Cookie>> cookies =
-        protocol::Array<Network::Cookie>::create();
-    for (const auto& pair : cookies_) {
-      const net::CanonicalCookie& cookie = pair.second;
-      std::unique_ptr<Network::Cookie> devtools_cookie =
-           Network::Cookie::Create()
-           .SetName(cookie.Name())
-           .SetValue(cookie.Value())
-           .SetDomain(cookie.Domain())
-           .SetPath(cookie.Path())
-           .SetExpires(cookie.ExpiryDate().ToDoubleT() * 1000)
-           .SetSize(cookie.Name().length() + cookie.Value().length())
-           .SetHttpOnly(cookie.IsHttpOnly())
-           .SetSecure(cookie.IsSecure())
-           .SetSession(!cookie.IsPersistent())
-           .Build();
-
-      switch (cookie.SameSite()) {
-        case net::CookieSameSite::STRICT_MODE:
-          devtools_cookie->SetSameSite(Network::CookieSameSiteEnum::Strict);
-          break;
-        case net::CookieSameSite::LAX_MODE:
-          devtools_cookie->SetSameSite(Network::CookieSameSiteEnum::Lax);
-          break;
-        case net::CookieSameSite::NO_RESTRICTION:
-          break;
-      }
-      cookies->addItem(std::move(devtools_cookie));
-    }
-    callback_->sendSuccess(std::move(cookies));
-  }
-
-  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,
-                    std::unique_ptr<GetCookiesCallback> callback)
-      : GetCookiesCommandBase(std::move(callback)) {
-    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));
-    }
-  }
-};
-
-class GetAllCookiesCommand
-    : public GetCookiesCommandBase<GetAllCookiesCallback> {
- public:
-  GetAllCookiesCommand(RenderFrameHostImpl* frame_host,
-                       std::unique_ptr<GetAllCookiesCallback> callback)
-      : GetCookiesCommandBase(std::move(callback)) {
-    net::CookieStore::GetCookieListCallback got_cookies_callback = base::Bind(
-        &GetAllCookiesCommand::GotCookiesForURL, base::Unretained(this));
-
-    request_count_ = 1;
-    BrowserThread::PostTask(
-        BrowserThread::IO, FROM_HERE,
-        base::Bind(&GetAllCookiesOnIO,
-                   base::Unretained(frame_host->GetSiteInstance()
-                                        ->GetBrowserContext()
-                                        ->GetResourceContext()),
-                   base::Unretained(frame_host->GetProcess()
-                                        ->GetStoragePartition()
-                                        ->GetURLRequestContext()),
-                   got_cookies_callback));
-  }
-};
-
 }  // namespace
 
 NetworkHandler::NetworkHandler()
     : DevToolsDomainHandler(Network::Metainfo::domainName),
       host_(nullptr),
       enabled_(false) {
 }
 
 NetworkHandler::~NetworkHandler() {
 }
@@ skipping 29 matching lines @@
     GetContentClient()->browser()->ClearCache(host_);
   return Response::OK();
 }
 
 Response NetworkHandler::ClearBrowserCookies() {
   if (host_)
     GetContentClient()->browser()->ClearCookies(host_);
   return Response::OK();
 }
 
+static std::vector<GURL> ComputeCookieURLs(

[pfeldman, 2017/01/19 18:58:35]

nit: since you have anonymous namespace declared few lines above, you can move
this method there and drop 'static' modifier. it'll result in the same thing.

[phulce, 2017/01/19 21:18:57]

Done.

+    RenderFrameHostImpl* frame_host,
+    Maybe<protocol::Array<String>>& protocol_urls) {
+  std::vector<GURL> urls;
+
+  if (protocol_urls.isJust()) {
+    std::unique_ptr<protocol::Array<std::string>> actual_urls =
+        protocol_urls.takeJust();
+
+    for (size_t i = 0; i < actual_urls->length(); i++)
+      urls.push_back(GURL(actual_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())

[pfeldman, 2017/01/19 18:58:35]

nit: you don't need this check, we can report cross process for now.

[phulce, 2017/01/19 21:18:57]

Done.

+        continue;
+
+      urls.push_back(node->current_url());
+      for (size_t i = 0; i < node->child_count(); ++i)
+        queue.push(node->child_at(i));
+    }
+  }
+
+  return urls;
+}
+
 void NetworkHandler::GetCookies(
+    Maybe<protocol::Array<String>> protocol_urls,
     std::unique_ptr<GetCookiesCallback> callback) {
-  if (!host_)
+  if (!host_) {
     callback->sendFailure(Response::InternalError());
-  else
-    new GetCookiesCommand(host_, std::move(callback));
+    return;
+  }
+
+  std::vector<GURL> urls = ComputeCookieURLs(host_, protocol_urls);
+  scoped_refptr<CookieRetriever> retriever =
+      new CookieRetriever(std::move(callback));
+
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&CookieRetriever::RetrieveCookiesOnIO,
+                 retriever,
+                 base::Unretained(host_->GetSiteInstance()
+                                       ->GetBrowserContext()
+                                       ->GetResourceContext()),
+                 base::Unretained(host_->GetProcess()
+                                       ->GetStoragePartition()
+                                       ->GetURLRequestContext()),
+                 urls));
 }
 
 void NetworkHandler::GetAllCookies(
     std::unique_ptr<GetAllCookiesCallback> callback) {
-  if (!host_)
+  if (!host_) {
     callback->sendFailure(Response::InternalError());
-  else
-    new GetAllCookiesCommand(host_, std::move(callback));
+    return;
+  }
+
+  scoped_refptr<CookieRetriever> retriever =
+      new CookieRetriever(std::move(callback));
+
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&CookieRetriever::RetrieveAllCookiesOnIO,
+                 retriever,
+                 base::Unretained(host_->GetSiteInstance()
+                                       ->GetBrowserContext()
+                                       ->GetResourceContext()),
+                 base::Unretained(host_->GetProcess()
+                                       ->GetStoragePartition()
+                                       ->GetURLRequestContext())));
 }
 
 void NetworkHandler::SetCookie(
     const std::string& url,
     const std::string& name,
     const std::string& value,
     Maybe<std::string> domain,
     Maybe<std::string> path,
     Maybe<bool> secure,
     Maybe<bool> http_only,
@@ skipping 64 matching lines @@
   *result = false;
   return Response::OK();
 }
 
 std::string NetworkHandler::UserAgentOverride() const {
   return enabled_ ? user_agent_ : std::string();
 }
 
 }  // namespace protocol
 }  // namespace content