Convert small instances of net::CookieStore -> network::mojom::CookieManager |
||||||
Issue descriptionThere are several instances in the code where consumers are accessing cookies through net::CookieStore, and it would be fairly easy to change those accesses to be through network::mojom::CookieManager, often removing IO thread specific implementation and doing all work on the UI thread. In general, these code locations access cookies by acquiring a URLRequestContextGetter from the profile, hopping to the IO thread, acquiring a URLRequestContext from the getter, acquiring a CookieStore from the URLRequestContext, doing the cookie operation, and transmitting the results of that (async) operation back to the UI thread. To transform this code, one would first climb the call stack to find where the URLRequestContextGetter came from. In all these cases I believe it's gotten from a Profile. One can then instead get the default storage partition from the Profile, get the NetworkContext associated with that storage partition, get a CookieManager associated with the NetworkContext, and use it. See https://chromium-review.googlesource.com/c/chromium/src/+/759696 for an example. The areas in which I believe this could easily be done are: content/browser/devtools/protocol content/shell/browser/layout_test chrome/browser/extensions/extension_service_unittest.cc. chrome/test/base/ui_test_utils.cc These instances are all separable; multiple people could work on them.
,
Dec 6 2017
I didn't consider this bug as addressing being robust against the network service dying; are we now considering that a requirement out of the gate for all conversions?
,
Dec 6 2017
If we don't, we'll have to go through and re-convert all conversions, inevitably missing a large number of them. My feeling is that if we're going to support this (I don't think we should, but that's not up to me), we basically have to go all-in, and do it from the start.
,
Dec 6 2017
,
Feb 17 2018
,
May 17 2018
,
May 18 2018
,
Jun 7 2018
This will be a natural by-product of removing URLRequestContext in the browser. for the test-only changes, either it's a unit test and we don't care, or it's a browser test and it'll be needed to make that test pass with NS. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by mmenke@chromium.org
, Dec 6 2017