New issue
Advanced search Search tips

Issue 598452 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Jun 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Remove URLRequest::GetResponseCookies and URLRequestJob::GetResponseCookies.

Project Member Reported by mmenke@chromium.org, Mar 28 2016

Issue description

The cookies are already available via the response headers, this method isn't needed.  There's one consumer of this method, URLFetcher, which also makes the headers available to its consumers, and it's calling the method on every successful response needelessly, copying data needlessly when almost no one cares about it.

It looks like there are about 4-5 consumers of URLFetcher's GetResponseCookies method, however, only one of them looks to actually care about the cookies, the rest just ignore them.  So all the consumers that don't care can stop calling the method.  The one consumer that does care can just grab the response headers, and walk through the cookies itself.
 
Components: -Internals>Network Internals>Network>Cookies
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e74f617c9ca6471411b874fdf43db60a3ef59ccf

commit e74f617c9ca6471411b874fdf43db60a3ef59ccf
Author: martijn <martijn@martijnc.be>
Date: Wed Jun 15 17:19:23 2016

Remove URLRequest::GetResponseCookies and URLRequestJob::GetResponseCookies.

The response cookies are currently copied for every request and made available to consumers. The cookies are then passed through different parts of the code needlessly as they are only used in one place. This CL removes most of that logic. The sole consumer of the response cookies now retrieves them from the response headers.

Some tests that were relying on the old (cookies are already parsed) logic are updated as well.

BUG= 598452 

Review-Url: https://codereview.chromium.org/2035293002
Cr-Commit-Position: refs/heads/master@{#399939}

[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/chrome/browser/safe_browsing/client_side_detection_service.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/chrome/browser/safe_browsing/client_side_detection_service.h
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/chrome/service/cloud_print/cloud_print_url_fetcher.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/chrome/service/cloud_print/cloud_print_url_fetcher.h
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/chrome/service/cloud_print/cloud_print_url_fetcher_unittest.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/chrome/service/cloud_print/printer_job_handler.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/chrome/service/cloud_print/printer_job_handler.h
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/components/policy/core/common/cloud/device_management_service.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/google_apis/gaia/gaia_auth_fetcher.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/google_apis/gaia/gaia_auth_fetcher_unittest.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/google_apis/gaia/mock_url_fetcher_factory.h
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/google_apis/gaia/oauth2_access_token_fetcher_impl.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/google_apis/gaia/oauth2_access_token_fetcher_impl_unittest.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/google_apis/gaia/oauth2_api_call_flow.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/http/http_response_headers.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/http/http_response_headers.h
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/url_request/test_url_fetcher_factory.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/url_request/test_url_fetcher_factory.h
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/url_request/url_fetcher.h
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/url_request/url_fetcher_core.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/url_request/url_fetcher_core.h
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/url_request/url_fetcher_impl.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/url_request/url_fetcher_impl.h
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/url_request/url_fetcher_impl_unittest.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/url_request/url_request.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/url_request/url_request.h
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/url_request/url_request_http_job.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/url_request/url_request_http_job.h
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/url_request/url_request_job.cc
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/net/url_request/url_request_job.h
[modify] https://crrev.com/e74f617c9ca6471411b874fdf43db60a3ef59ccf/sync/internal_api/http_bridge_unittest.cc

Comment 3 by mmenke@chromium.org, Jun 15 2016

Status: Fixed (was: Available)
Thanks again, martijn!

Sign in to add a comment