Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(2)

Issue 2892953002: Remove the ClearCookies and ClearCache methods from ContentBrowserClient (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 days, 23 hours ago by msramek
Modified:
2 days, 2 hours ago
Reviewers:
dgozman, boliu
CC:
chromium-reviews, jam, darin-cc_chromium.org, android-webview-reviews_chromium.org, pfeldman, devtools-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove the ClearCookies and ClearCache methods from ContentBrowserClient These methods were used by content/ to request the deletion of cookies and cache from embedder. This should no longer be necessary, since BrowsingDataRemover has been moved to content/ and can be used directly. Embedders that need to execute additional deletions can still subclass BrowsingDataRemoverDelegate and implement them there. There were only two ContentBrowserClient subclasses that used the methods: 1. ChromeContentBrowserClient::ClearCache() was used by dev_tools/. This was replaced by an equivalent call to BrowsingDataRemover. 2. In android_webview, ClearCookies() and ClearCache() were defined, but not called. ClearCache() used android_webview::RemoveHttpDiskCache for cache deletion, which is in fact a subset of functionality that already exists in BrowsingDataRemover, and can be replaced by it. BUG=668114 Review-Url: https://codereview.chromium.org/2892953002 Cr-Commit-Position: refs/heads/master@{#473434} Committed: https://chromium.googlesource.com/chromium/src/+/507bbc4d6eaf6ebd7b5ed1e63b611169e0d13308

Patch Set 1 #

Patch Set 2 : Rebase. #

Total comments: 4

Patch Set 3 : Use BrowsingDataRemover to delete cache. #

Patch Set 4 : Remove unused includes #

Patch Set 5 : Merge conflict in BUILD.gn -> Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -131 lines) Patch
M android_webview/BUILD.gn View 1 2 3 4 1 chunk +0 lines, -2 lines 0 comments Download
M android_webview/browser/aw_content_browser_client.h View 1 2 3 4 1 chunk +0 lines, -2 lines 0 comments Download
M android_webview/browser/aw_content_browser_client.cc View 1 2 3 4 2 chunks +0 lines, -10 lines 0 comments Download
M android_webview/browser/aw_contents.cc View 1 2 3 4 3 chunks +11 lines, -3 lines 0 comments Download
D android_webview/browser/net_disk_cache_remover.h View 1 2 1 chunk +0 lines, -22 lines 0 comments Download
D android_webview/browser/net_disk_cache_remover.cc View 1 2 1 chunk +0 lines, -72 lines 0 comments Download
M chrome/browser/chrome_content_browser_client.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/chrome_content_browser_client.cc View 1 1 chunk +0 lines, -9 lines 0 comments Download
M content/browser/devtools/protocol/network_handler.cc View 1 2 chunks +10 lines, -2 lines 0 comments Download
M content/public/browser/content_browser_client.h View 1 1 chunk +0 lines, -8 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 53 (31 generated)
msramek
Hi Dmitry and Bo, Please have a look! Dmitry: devtools/ and content/public/ Bo: android_webview/ - ...
2 days, 22 hours ago (2017-05-19 16:44:48 UTC) #8
boliu
this CL lgtm https://codereview.chromium.org/2892953002/diff/20001/android_webview/browser/aw_contents.cc File android_webview/browser/aw_contents.cc (right): https://codereview.chromium.org/2892953002/diff/20001/android_webview/browser/aw_contents.cc#newcode716 android_webview/browser/aw_contents.cc:716: // BrowsingDataRemover would do for REMOVE_CACHE ...
2 days, 22 hours ago (2017-05-19 17:31:31 UTC) #9
msramek
Thanks! https://codereview.chromium.org/2892953002/diff/20001/android_webview/browser/aw_contents.cc File android_webview/browser/aw_contents.cc (right): https://codereview.chromium.org/2892953002/diff/20001/android_webview/browser/aw_contents.cc#newcode716 android_webview/browser/aw_contents.cc:716: // BrowsingDataRemover would do for REMOVE_CACHE mask. Consider ...
2 days, 22 hours ago (2017-05-19 17:35:29 UTC) #10
boliu
https://codereview.chromium.org/2892953002/diff/20001/android_webview/browser/aw_contents.cc File android_webview/browser/aw_contents.cc (right): https://codereview.chromium.org/2892953002/diff/20001/android_webview/browser/aw_contents.cc#newcode716 android_webview/browser/aw_contents.cc:716: // BrowsingDataRemover would do for REMOVE_CACHE mask. Consider calling ...
2 days, 21 hours ago (2017-05-19 17:38:36 UTC) #11
msramek
https://codereview.chromium.org/2892953002/diff/20001/android_webview/browser/aw_contents.cc File android_webview/browser/aw_contents.cc (right): https://codereview.chromium.org/2892953002/diff/20001/android_webview/browser/aw_contents.cc#newcode716 android_webview/browser/aw_contents.cc:716: // BrowsingDataRemover would do for REMOVE_CACHE mask. Consider calling ...
2 days, 21 hours ago (2017-05-19 18:14:59 UTC) #14
boliu
On 2017/05/19 18:14:59, msramek wrote: > https://codereview.chromium.org/2892953002/diff/20001/android_webview/browser/aw_contents.cc > File android_webview/browser/aw_contents.cc (right): > > https://codereview.chromium.org/2892953002/diff/20001/android_webview/browser/aw_contents.cc#newcode716 > ...
2 days, 21 hours ago (2017-05-19 18:18:35 UTC) #15
msramek
On 2017/05/19 18:18:35, boliu wrote: > On 2017/05/19 18:14:59, msramek wrote: > > > https://codereview.chromium.org/2892953002/diff/20001/android_webview/browser/aw_contents.cc ...
2 days, 20 hours ago (2017-05-19 18:41:21 UTC) #22
boliu
Can remove net_disk_cache_remover.cc/h as well I think?
2 days, 20 hours ago (2017-05-19 18:49:18 UTC) #23
boliu
On 2017/05/19 18:49:18, boliu wrote: > Can remove net_disk_cache_remover.cc/h as well I think? oh wait, ...
2 days, 20 hours ago (2017-05-19 18:49:43 UTC) #24
msramek
Thanks! But you did make me double-check and notice that I didn't remove their #include-s ...
2 days, 20 hours ago (2017-05-19 18:53:37 UTC) #25
dgozman
lgtm
2 days, 20 hours ago (2017-05-19 18:58:40 UTC) #28
msramek
Thanks! Landing this now.
2 days, 20 hours ago (2017-05-19 19:05:10 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2892953002/80001
2 days, 20 hours ago (2017-05-19 19:06:06 UTC) #33
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/450316)
2 days, 18 hours ago (2017-05-19 21:06:00 UTC) #35
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2892953002/80001
2 days, 17 hours ago (2017-05-19 21:45:45 UTC) #37
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/442927)
2 days, 17 hours ago (2017-05-19 21:54:38 UTC) #39
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2892953002/100001
2 days, 17 hours ago (2017-05-19 22:06:44 UTC) #42
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/298663)
2 days, 16 hours ago (2017-05-19 23:31:03 UTC) #44
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2892953002/100001
2 days, 15 hours ago (2017-05-19 23:53:27 UTC) #46
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/298784)
2 days, 12 hours ago (2017-05-20 03:26:00 UTC) #48
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2892953002/100001
2 days, 3 hours ago (2017-05-20 11:48:55 UTC) #50
commit-bot: I haz the power
2 days, 2 hours ago (2017-05-20 12:45:49 UTC) #53
Message was sent while issue was closed.
Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/507bbc4d6eaf6ebd7b5ed1e63b61...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06