DevTools: Fix Network.clearBrowserCookies

DevTools: Fix Network.clearBrowserCookies

Removes ClearCookies method from chrome content browser client.
Changes network handler to directly delete cookies from cookie store.

BUG=672744
R=dgozman

Review-Url: https://codereview.chromium.org/2785943005
Cr-Commit-Position: refs/heads/master@{#461911}
Committed: https://chromium.googlesource.com/chromium/src/+/3f058ec9983877b66fbd014a1253d509dc303172

[phulce, 2017-03-31 16:40:42]

https://codereview.chromium.org/2785943005/diff/1/content/browser/devtools/pr...
File content/browser/devtools/protocol/network_handler.cc (right):

https://codereview.chromium.org/2785943005/diff/1/content/browser/devtools/pr...
content/browser/devtools/protocol/network_handler.cc:195: void
ClearedCookiesOnIO(int num_deleted) {
It'd be nice to communicate this over the protocol, but it looks like that
method is marked as stable and doesn't accept a callback. Is that something we
can change or we'll just have to live with?

[dgozman, 2017-03-31 22:57:55]

Nice patch!

https://codereview.chromium.org/2785943005/diff/1/chrome/browser/chrome_conte...
File chrome/browser/chrome_content_browser_client.cc (left):

https://codereview.chromium.org/2785943005/diff/1/chrome/browser/chrome_conte...
chrome/browser/chrome_content_browser_client.cc:2750: int remove_mask =
ChromeBrowsingDataRemoverDelegate::DATA_TYPE_SITE_DATA;
This looks like more than cookies, doesn't it? Let's double check we do
equivalent things.

https://codereview.chromium.org/2785943005/diff/1/content/browser/devtools/pr...
File content/browser/devtools/protocol/network_handler.cc (right):

https://codereview.chromium.org/2785943005/diff/1/content/browser/devtools/pr...
content/browser/devtools/protocol/network_handler.cc:195: void
ClearedCookiesOnIO(int num_deleted) {
On 2017/03/31 16:40:42, phulce wrote:
> It'd be nice to communicate this over the protocol, but it looks like that
> method is marked as stable and doesn't accept a callback. Is that something we
> can change or we'll just have to live with?

You can mark it "async" in protocol_config.json, in which case there would be a
callback.

[phulce, 2017-03-31 23:54:32]

https://codereview.chromium.org/2785943005/diff/1/chrome/browser/chrome_conte...
File chrome/browser/chrome_content_browser_client.cc (left):

https://codereview.chromium.org/2785943005/diff/1/chrome/browser/chrome_conte...
chrome/browser/chrome_content_browser_client.cc:2750: int remove_mask =
ChromeBrowsingDataRemoverDelegate::DATA_TYPE_SITE_DATA;
On 2017/03/31 22:57:55, dgozman wrote:
> This looks like more than cookies, doesn't it? Let's double check we do
> equivalent things.

Oh yeah forgot to mention this was clearly a bug too :) When you clear cookies
currently in Network panel context menu, it deletes all localStorage/cache/etc

https://codereview.chromium.org/2785943005/diff/1/content/browser/devtools/pr...
File content/browser/devtools/protocol/network_handler.cc (right):

https://codereview.chromium.org/2785943005/diff/1/content/browser/devtools/pr...
content/browser/devtools/protocol/network_handler.cc:195: void
ClearedCookiesOnIO(int num_deleted) {
On 2017/03/31 22:57:55, dgozman wrote:
> On 2017/03/31 16:40:42, phulce wrote:
> > It'd be nice to communicate this over the protocol, but it looks like that
> > method is marked as stable and doesn't accept a callback. Is that something
we
> > can change or we'll just have to live with?
> 
> You can mark it "async" in protocol_config.json, in which case there would be
a
> callback.

done

[pfeldman, 2017-04-01 00:23:22]

pfeldman@chromium.org changed reviewers:
+ pfeldman@chromium.org

[pfeldman, 2017-04-01 00:23:23]

https://codereview.chromium.org/2785943005/diff/20001/content/browser/devtool...
File content/browser/devtools/protocol/network_handler.cc (right):

https://codereview.chromium.org/2785943005/diff/20001/content/browser/devtool...
content/browser/devtools/protocol/network_handler.cc:191:
callback->sendSuccess(num_deleted);
We should get back to UI!

[phulce, 2017-04-03 17:04:04]

https://codereview.chromium.org/2785943005/diff/20001/content/browser/devtool...
File content/browser/devtools/protocol/network_handler.cc (right):

https://codereview.chromium.org/2785943005/diff/20001/content/browser/devtool...
content/browser/devtools/protocol/network_handler.cc:191:
callback->sendSuccess(num_deleted);
On 2017/04/01 00:23:23, pfeldman wrote:
> We should get back to UI!

Like in an alert post-confirmation?

[dgozman, 2017-04-03 17:36:39]

On 2017/04/03 17:04:04, phulce wrote:
>
https://codereview.chromium.org/2785943005/diff/20001/content/browser/devtool...
> File content/browser/devtools/protocol/network_handler.cc (right):
> 
>
https://codereview.chromium.org/2785943005/diff/20001/content/browser/devtool...
> content/browser/devtools/protocol/network_handler.cc:191:
> callback->sendSuccess(num_deleted);
> On 2017/04/01 00:23:23, pfeldman wrote:
> > We should get back to UI!
> 
> Like in an alert post-confirmation?

Pavel means that executing callback on IO thread is a no-no, since it operates
on UI thread.

[phulce, 2017-04-03 17:48:52]

On 2017/04/03 17:36:39, dgozman wrote:
> On 2017/04/03 17:04:04, phulce wrote:
> >
>
https://codereview.chromium.org/2785943005/diff/20001/content/browser/devtool...
> > File content/browser/devtools/protocol/network_handler.cc (right):
> > 
> >
>
https://codereview.chromium.org/2785943005/diff/20001/content/browser/devtool...
> > content/browser/devtools/protocol/network_handler.cc:191:
> > callback->sendSuccess(num_deleted);
> > On 2017/04/01 00:23:23, pfeldman wrote:
> > > We should get back to UI!
> > 
> > Like in an alert post-confirmation?
> 
> Pavel means that executing callback on IO thread is a no-no, since it operates
> on UI thread.

oooooh thanks for the clarification :)

[phulce, 2017-04-04 16:46:41]

PTAL

[dgozman, 2017-04-04 16:53:43]

lgtm modulo protocol change

https://codereview.chromium.org/2785943005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/browser_protocol.json (right):

https://codereview.chromium.org/2785943005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/browser_protocol.json:1279: { "name":
"numberDeleted", "type": "number", "description": "The number of cookies
successfully deleted." }
Do we actually need this? Let's drop.

[phulce, 2017-04-04 18:23:27]

https://codereview.chromium.org/2785943005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/browser_protocol.json (right):

https://codereview.chromium.org/2785943005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/browser_protocol.json:1279: { "name":
"numberDeleted", "type": "number", "description": "The number of cookies
successfully deleted." }
On 2017/04/04 16:53:43, dgozman wrote:
> Do we actually need this? Let's drop.

Done.

[pfeldman, 2017-04-04 19:07:41]

lgtm

[phulce, 2017-04-04 22:39:20]

The CQ bit was checked by phulce@chromium.org

[phulce, 2017-04-04 22:39:20]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2785943005/#ps60001
(title: "undo protocol changes")

[commit-bot: I haz the power, 2017-04-04 22:40:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-05 00:21:43]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1491345560185090,
"parent_rev": "8cb7e8847ef119d5b997c9d9964457c69b622adb", "commit_rev":
"3f058ec9983877b66fbd014a1253d509dc303172"}

[commit-bot: I haz the power, 2017-04-05 00:23:12]

Description was changed from

==========
DevTools: Fix Network.clearBrowserCookies

Removes ClearCookies method from chrome content browser client.
Changes network handler to directly delete cookies from cookie store.

BUG=672744
R=dgozman
==========

to

==========
DevTools: Fix Network.clearBrowserCookies

Removes ClearCookies method from chrome content browser client.
Changes network handler to directly delete cookies from cookie store.

BUG=672744
R=dgozman

Review-Url: https://codereview.chromium.org/2785943005
Cr-Commit-Position: refs/heads/master@{#461911}
Committed:
https://chromium.googlesource.com/chromium/src/+/3f058ec9983877b66fbd014a1253...
==========

[commit-bot: I haz the power, 2017-04-05 00:23:20]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/3f058ec9983877b66fbd014a1253...