Chromium Code Reviews
Help | Chromium Project | Sign in
(799)

Issue 12092089: Display an info banner on the clear browsing data dialog if the browsing data was cleared within th… (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 2 months ago by markusheintz_
Modified:
1 year, 2 months ago
Reviewers:
James Hawkins, sky
CC:
chromium-reviews_chromium.org, dbeam+watch-options_chromium.org, arv, Evan Stade
Visibility:
Public.

Description

Display an info banner on the clear browsing data dialog if the browsing data was cleared within the last 24h. The displayed info banner suggest to use the incognito mode instead.


BUG=164600
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=180372

Patch Set 1 #

Patch Set 2 : " #

Patch Set 3 : Fix titel banner padding. #

Total comments: 9

Patch Set 4 : Address comments from James #

Total comments: 2

Patch Set 5 : Change type of kHoursPerDay from uint to int64 to match the FromHours param type. #

Patch Set 6 : Address comments round 2 from James. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+64 lines, -1 line) Lint Patch
M chrome/app/generated_resources.grd View 2 chunks +4 lines, -1 line 0 comments ? errors Download
M chrome/browser/resources/options/clear_browser_data_overlay.css View 1 2 3 1 chunk +10 lines, -0 lines 0 comments ? errors Download
M chrome/browser/resources/options/clear_browser_data_overlay.html View 1 2 3 1 chunk +3 lines, -0 lines 0 comments ? errors Download
M chrome/browser/resources/options/clear_browser_data_overlay.js View 1 1 chunk +5 lines, -0 lines 0 comments ? errors Download
M chrome/browser/ui/browser_ui_prefs.cc View 1 1 chunk +3 lines, -0 lines 0 comments 0 errors Download
M chrome/browser/ui/webui/options/clear_browser_data_handler.h View 1 2 3 1 chunk +3 lines, -0 lines 0 comments ? errors Download
M chrome/browser/ui/webui/options/clear_browser_data_handler.cc View 1 2 3 4 5 4 chunks +33 lines, -0 lines 0 comments ? errors Download
M chrome/common/pref_names.h View 1 1 chunk +1 line, -0 lines 0 comments 0 errors Download
M chrome/common/pref_names.cc View 1 1 chunk +2 lines, -0 lines 0 comments ? errors Download
Commit:

Messages

Total messages: 11
markusheintz_
@Evan: Could you please review this CL? You'll find all necessary screenshots and mocks in ...
1 year, 2 months ago #1
sky
LGTM
1 year, 2 months ago #2
markusheintz_
@Scott: Thanks a lot. Just seeing that Evan is OOO. Replacing Evan with James as ...
1 year, 2 months ago #3
James Hawkins
Nice, clean CL. Just a few comments. https://codereview.chromium.org/12092089/diff/11/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/12092089/diff/11/chrome/app/generated_resources.grd#newcode8099 chrome/app/generated_resources.grd:8099: + Psst! ...
1 year, 2 months ago #4
markusheintz_
https://codereview.chromium.org/12092089/diff/11/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/12092089/diff/11/chrome/app/generated_resources.grd#newcode8099 chrome/app/generated_resources.grd:8099: + Psst! Incognito mode (<ph name="INCOGNITO_MODE_SHORTCUT">$1<ex>Shift+Ctrl+N</ex></ph>) may come in ...
1 year, 2 months ago #5
James Hawkins
LGTM with comment. https://codereview.chromium.org/12092089/diff/3011/chrome/browser/ui/webui/options/clear_browser_data_handler.cc File chrome/browser/ui/webui/options/clear_browser_data_handler.cc (right): https://codereview.chromium.org/12092089/diff/3011/chrome/browser/ui/webui/options/clear_browser_data_handler.cc#newcode28 chrome/browser/ui/webui/options/clear_browser_data_handler.cc:28: const uint kHoursPerDay = 24; Go ...
1 year, 2 months ago #6
markusheintz_
FYI: I change the const type from uint to int64 in order to match the ...
1 year, 2 months ago #7
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/markusheintz@chromium.org/12092089/2002
1 year, 2 months ago #8
I haz the power (commit-bot)
Retried try job too often on win_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=106450
1 year, 2 months ago #9
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/markusheintz@chromium.org/12092089/2002
1 year, 2 months ago #10
I haz the power (commit-bot)
1 year, 2 months ago #11
Message was sent while issue was closed.
Change committed as 180372
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1275:d14800f88434