Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(19)

Issue 10502015: Adding "Clear hosted app data" checkbox to browsing data removal UI. (Closed)

Created:
8 years, 6 months ago by Mike West
Modified:
8 years, 6 months ago
CC:
chromium-reviews, Raghu Simha, ncarter (slow), arv (Not doing code reviews), akalin, tim (not reviewing), Mihai Parparita -not on Chrome, jeffreyc, battre
Visibility:
Public.

Description

Adding "Clear hosted app data" checkbox to browsing data removal UI. Original patch from battre@chromium.org, I'm just sheparding it along as he's out on vacation. BUG=116372 TBR=rsimha@chromium.org, timsteele@chromium.org TEST=Install a hosted application, then clear browsing data with the new checkbox checked: the hosted application's data should be cleared. Without the checkbox, the data should remain untouched. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=141646

Patch Set 1 : Fixing? #

Total comments: 13

Patch Set 2 : Feedback. #

Total comments: 16

Patch Set 3 : Feedback. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+81 lines, -12 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/resources/options2/clear_browser_data_overlay.html View 1 chunk +7 lines, -0 lines 0 comments Download
M chrome/browser/resources/options2/clear_browser_data_overlay.js View 1 2 3 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/sync/test/integration/two_client_preferences_sync_test.cc View 1 2 3 chunks +3 lines, -0 lines 0 comments Download
M chrome/browser/ui/browser.cc View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/options2/clear_browser_data_handler2.h View 1 2 2 chunks +13 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc View 1 2 4 chunks +46 lines, -11 lines 0 comments Download
M chrome/common/pref_names.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/pref_names.cc View 1 2 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 20 (0 generated)
Mike West
Hello, lovely reviewers! This CL adds a checkbox to the browsing data removal panel to ...
8 years, 6 months ago (2012-06-04 14:19:39 UTC) #1
Mike West
On 2012/06/04 14:19:39, Mike West (chromium) wrote: > Hello, lovely reviewers! This CL adds a ...
8 years, 6 months ago (2012-06-04 14:21:33 UTC) #2
sky
browser.cc LGTM
8 years, 6 months ago (2012-06-04 14:29:51 UTC) #3
markusheintz_
LGTM with nits prefs stuff https://chromiumcodereview.appspot.com/10502015/diff/3001/chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc File chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc (right): https://chromiumcodereview.appspot.com/10502015/diff/3001/chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc#newcode160 chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc:160: remover_ = new BrowsingDataRemover(profile, ...
8 years, 6 months ago (2012-06-04 14:50:51 UTC) #4
Evan Stade
https://chromiumcodereview.appspot.com/10502015/diff/3001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://chromiumcodereview.appspot.com/10502015/diff/3001/chrome/app/generated_resources.grd#newcode6622 chrome/app/generated_resources.grd:6622: + Clear data from hosted apps I don't have ...
8 years, 6 months ago (2012-06-04 22:03:40 UTC) #5
Mike West
Thanks Evan and Markus. Feedback addressed, and rebased onto ToT. http://codereview.chromium.org/10502015/diff/3001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): http://codereview.chromium.org/10502015/diff/3001/chrome/app/generated_resources.grd#newcode6622 ...
8 years, 6 months ago (2012-06-05 08:55:02 UTC) #6
Evan Stade
http://codereview.chromium.org/10502015/diff/10001/chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc File chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc (right): http://codereview.chromium.org/10502015/diff/10001/chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc#newcode102 chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc:102: void ClearBrowserDataHandler::ClearBrowserData(const ListValue* value) { this one actually does ...
8 years, 6 months ago (2012-06-05 18:07:44 UTC) #7
Evan Stade
also, I just noticed that the cancel button in the overlay is still active when ...
8 years, 6 months ago (2012-06-05 18:10:15 UTC) #8
battre
Just some quick replies. Mike, please continue driving this. Thanks, Dominic http://codereview.chromium.org/10502015/diff/10001/chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc File chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc (right): ...
8 years, 6 months ago (2012-06-07 21:17:24 UTC) #9
Evan Stade
http://codereview.chromium.org/10502015/diff/10001/chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc File chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc (right): http://codereview.chromium.org/10502015/diff/10001/chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc#newcode146 chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc:146: remover_->Remove(remove_mask, BrowsingDataHelper::UNPROTECTED_WEB); On 2012/06/07 21:17:24, battre wrote: > On ...
8 years, 6 months ago (2012-06-07 21:50:20 UTC) #10
Mike West
Thanks for your feedback, Evan and Dominic. I spent some time Friday trying to get ...
8 years, 6 months ago (2012-06-11 09:35:44 UTC) #11
battre
lgtm
8 years, 6 months ago (2012-06-11 13:16:37 UTC) #12
Evan Stade
lgtm
8 years, 6 months ago (2012-06-11 19:15:01 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mkwst@chromium.org/10502015/17003
8 years, 6 months ago (2012-06-11 19:22:27 UTC) #14
commit-bot: I haz the power
Presubmit check for 10502015-17003 failed and returned exit status 1. Running presubmit commit checks ...
8 years, 6 months ago (2012-06-11 19:22:36 UTC) #15
battre
+rsimha for OWNERS approval of changes in chrome/browser/sync
8 years, 6 months ago (2012-06-11 19:27:46 UTC) #16
battre
I think you should just TBR rsimha. The change is really trivial and the CL ...
8 years, 6 months ago (2012-06-12 10:49:11 UTC) #17
Mike West
On 2012/06/12 10:49:11, battre wrote: > I think you should just TBR rsimha. The change ...
8 years, 6 months ago (2012-06-12 10:51:59 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mkwst@chromium.org/10502015/17003
8 years, 6 months ago (2012-06-12 10:52:36 UTC) #19
commit-bot: I haz the power
8 years, 6 months ago (2012-06-12 12:02:45 UTC) #20
Change committed as 141646

Powered by Google App Engine
This is Rietveld 408576698