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

Issue 940543003: added clock interstitial to chrome://interstitials (Closed)

Created:
5 years, 10 months ago by fahl
Modified:
5 years, 9 months ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

added clock interstitial to chrome://interstitials BUG=452218 TEST=Go to chrome://interstitials and click on "Clock is ahead" or "Clock is behind". The shown interstitial displays the clock error message instead of the common certificate warning. Committed: https://crrev.com/920f3bdfc9fc08cf60da6708a050a9bc82a0d223 Cr-Commit-Position: refs/heads/master@{#318548}

Patch Set 1 #

Total comments: 5

Patch Set 2 : Add the clock interstitial to chrome://interstitials. #

Total comments: 24

Patch Set 3 : added clock interstitial to chrome://interstitials #

Total comments: 6

Patch Set 4 : added clock interstitial to chrome://interstitials #

Total comments: 6

Patch Set 5 : chrome/browser/ssl/ssl_blocking_page.cc #

Total comments: 7

Patch Set 6 : added clock interstitial to chrome://interstitials #

Total comments: 6

Patch Set 7 : added clock interstitial to chrome://interstitials #

Total comments: 4

Patch Set 8 : added clock interstitial to chrome://interstitials #

Unified diffs Side-by-side diffs Delta from patch set Stats (+72 lines, -37 lines) Patch
M chrome/browser/browser_resources.grd View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
A chrome/browser/resources/security_warnings/interstitial_ui.html View 1 2 3 4 5 6 1 chunk +21 lines, -0 lines 0 comments Download
M chrome/browser/ssl/ssl_blocking_page.h View 1 2 3 4 5 6 7 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/ssl/ssl_blocking_page.cc View 1 2 3 4 5 6 7 7 chunks +20 lines, -15 lines 0 comments Download
M chrome/browser/ssl/ssl_error_handler.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/interstitials/interstitial_ui.cc View 1 2 3 4 5 6 7 7 chunks +24 lines, -21 lines 0 comments Download

Messages

Total messages: 53 (13 generated)
fahl
hey, can you review this for me?
5 years, 10 months ago (2015-02-18 21:38:19 UTC) #2
felt
Hey, I have an idea. Lucas, you are learning to review code. Can you do ...
5 years, 10 months ago (2015-02-19 07:26:21 UTC) #4
lgarron
https://codereview.chromium.org/940543003/diff/1/chrome/browser/ssl/ssl_blocking_page.cc File chrome/browser/ssl/ssl_blocking_page.cc (right): https://codereview.chromium.org/940543003/diff/1/chrome/browser/ssl/ssl_blocking_page.cc#newcode208 chrome/browser/ssl/ssl_blocking_page.cc:208: Nit: I try to avoid introducing extra whitespace, just ...
5 years, 10 months ago (2015-02-19 22:57:06 UTC) #5
felt
https://codereview.chromium.org/940543003/diff/1/chrome/browser/ssl/ssl_blocking_page.cc File chrome/browser/ssl/ssl_blocking_page.cc (right): https://codereview.chromium.org/940543003/diff/1/chrome/browser/ssl/ssl_blocking_page.cc#newcode238 chrome/browser/ssl/ssl_blocking_page.cc:238: std::string testHost("yourclockiswrong.com"); On 2015/02/19 22:57:06, lgarron wrote: > If ...
5 years, 10 months ago (2015-02-19 23:16:07 UTC) #6
fahl
On 2015/02/19 23:16:07, felt wrote: > https://codereview.chromium.org/940543003/diff/1/chrome/browser/ssl/ssl_blocking_page.cc > File chrome/browser/ssl/ssl_blocking_page.cc (right): > > https://codereview.chromium.org/940543003/diff/1/chrome/browser/ssl/ssl_blocking_page.cc#newcode238 > ...
5 years, 10 months ago (2015-02-19 23:26:09 UTC) #7
felt
On 2015/02/19 23:26:09, fahl wrote: > On 2015/02/19 23:16:07, felt wrote: > > > https://codereview.chromium.org/940543003/diff/1/chrome/browser/ssl/ssl_blocking_page.cc ...
5 years, 10 months ago (2015-02-19 23:27:32 UTC) #8
fahl
On 2015/02/19 23:27:32, felt wrote: > On 2015/02/19 23:26:09, fahl wrote: > > On 2015/02/19 ...
5 years, 10 months ago (2015-02-19 23:37:29 UTC) #9
fahl
On 2015/02/19 23:37:29, fahl wrote: > On 2015/02/19 23:27:32, felt wrote: > > On 2015/02/19 ...
5 years, 10 months ago (2015-02-20 03:35:28 UTC) #10
felt
looking good, just a few small changes https://codereview.chromium.org/940543003/diff/1/chrome/browser/ssl/ssl_blocking_page.cc File chrome/browser/ssl/ssl_blocking_page.cc (right): https://codereview.chromium.org/940543003/diff/1/chrome/browser/ssl/ssl_blocking_page.cc#newcode208 chrome/browser/ssl/ssl_blocking_page.cc:208: On 2015/02/19 ...
5 years, 10 months ago (2015-02-20 16:01:53 UTC) #11
lgarron
Looks pretty good overall! Adding a bunch of nitpicks on top of felt@'s. ;-) https://codereview.chromium.org/940543003/diff/20001/chrome/browser/ssl/ssl_blocking_page.cc ...
5 years, 10 months ago (2015-02-20 22:12:46 UTC) #12
fahl
Could you check my comments? https://codereview.chromium.org/940543003/diff/1/chrome/browser/ssl/ssl_blocking_page.cc File chrome/browser/ssl/ssl_blocking_page.cc (right): https://codereview.chromium.org/940543003/diff/1/chrome/browser/ssl/ssl_blocking_page.cc#newcode208 chrome/browser/ssl/ssl_blocking_page.cc:208: On 2015/02/20 16:01:53, felt ...
5 years, 10 months ago (2015-02-23 23:47:49 UTC) #13
lgarron
LGTM - but in typical Chromium dev style, with a few remaining style nitpicks. ;-) ...
5 years, 10 months ago (2015-02-24 02:28:37 UTC) #14
lgarron
By the way, make sure to update the TEST string on this CR (in the ...
5 years, 10 months ago (2015-02-24 20:06:09 UTC) #18
felt
On 2015/02/24 20:06:09, lgarron wrote: > By the way, make sure to update the TEST ...
5 years, 10 months ago (2015-02-24 21:15:39 UTC) #19
fahl
On 2015/02/24 21:15:39, felt wrote: > On 2015/02/24 20:06:09, lgarron wrote: > > By the ...
5 years, 10 months ago (2015-02-24 21:19:18 UTC) #20
felt
On 2015/02/24 21:19:18, fahl wrote: > On 2015/02/24 21:15:39, felt wrote: > > On 2015/02/24 ...
5 years, 10 months ago (2015-02-24 21:22:14 UTC) #21
fahl
https://codereview.chromium.org/940543003/diff/40001/chrome/browser/ssl/ssl_blocking_page.cc File chrome/browser/ssl/ssl_blocking_page.cc (right): https://codereview.chromium.org/940543003/diff/40001/chrome/browser/ssl/ssl_blocking_page.cc#newcode236 chrome/browser/ssl/ssl_blocking_page.cc:236: time_triggered_(base_time) { On 2015/02/24 02:28:37, lgarron wrote: > I ...
5 years, 10 months ago (2015-02-24 21:23:24 UTC) #22
felt
lgtm
5 years, 10 months ago (2015-02-25 16:28:22 UTC) #23
felt
bauerb@chromium.org: Please review changes in chrome/browser/ui/webui/interstitials/
5 years, 10 months ago (2015-02-25 16:29:25 UTC) #25
Bernhard Bauer
https://codereview.chromium.org/940543003/diff/120001/chrome/browser/ui/webui/interstitials/interstitial_ui.cc File chrome/browser/ui/webui/interstitials/interstitial_ui.cc (right): https://codereview.chromium.org/940543003/diff/120001/chrome/browser/ui/webui/interstitials/interstitial_ui.cc#newcode81 chrome/browser/ui/webui/interstitials/interstitial_ui.cc:81: base::TimeDelta::FromDays(365 * atoi(clock_manipulation_param.c_str())); There is base::StringToInt (from base/strings/string_number_conversions.h). https://codereview.chromium.org/940543003/diff/120001/chrome/browser/ui/webui/interstitials/interstitial_ui.cc#newcode196 ...
5 years, 10 months ago (2015-02-25 16:41:09 UTC) #26
fahl
https://codereview.chromium.org/940543003/diff/120001/chrome/browser/ui/webui/interstitials/interstitial_ui.cc File chrome/browser/ui/webui/interstitials/interstitial_ui.cc (right): https://codereview.chromium.org/940543003/diff/120001/chrome/browser/ui/webui/interstitials/interstitial_ui.cc#newcode196 chrome/browser/ui/webui/interstitials/interstitial_ui.cc:196: html = "<html><head><title>Interstitials</title></head>" On 2015/02/25 16:41:08, Bernhard (back on ...
5 years, 10 months ago (2015-02-26 03:00:26 UTC) #27
felt
https://codereview.chromium.org/940543003/diff/120001/chrome/browser/ui/webui/interstitials/interstitial_ui.cc File chrome/browser/ui/webui/interstitials/interstitial_ui.cc (right): https://codereview.chromium.org/940543003/diff/120001/chrome/browser/ui/webui/interstitials/interstitial_ui.cc#newcode196 chrome/browser/ui/webui/interstitials/interstitial_ui.cc:196: html = "<html><head><title>Interstitials</title></head>" On 2015/02/26 03:00:26, fahl wrote: > ...
5 years, 10 months ago (2015-02-26 03:14:00 UTC) #28
fahl
https://codereview.chromium.org/940543003/diff/120001/chrome/browser/ui/webui/interstitials/interstitial_ui.cc File chrome/browser/ui/webui/interstitials/interstitial_ui.cc (right): https://codereview.chromium.org/940543003/diff/120001/chrome/browser/ui/webui/interstitials/interstitial_ui.cc#newcode81 chrome/browser/ui/webui/interstitials/interstitial_ui.cc:81: base::TimeDelta::FromDays(365 * atoi(clock_manipulation_param.c_str())); On 2015/02/25 16:41:08, Bernhard (back on ...
5 years, 10 months ago (2015-02-26 04:52:41 UTC) #31
felt
https://codereview.chromium.org/940543003/diff/180001/chrome/browser/resources/security_warnings/interstitial_ui.html File chrome/browser/resources/security_warnings/interstitial_ui.html (right): https://codereview.chromium.org/940543003/diff/180001/chrome/browser/resources/security_warnings/interstitial_ui.html#newcode2 chrome/browser/resources/security_warnings/interstitial_ui.html:2: <head> nit: can you pls reformat this so it ...
5 years, 10 months ago (2015-02-26 05:26:13 UTC) #32
Bernhard Bauer
LGTM w/ nits: https://codereview.chromium.org/940543003/diff/180001/chrome/browser/resources/security_warnings/interstitial_ui.html File chrome/browser/resources/security_warnings/interstitial_ui.html (right): https://codereview.chromium.org/940543003/diff/180001/chrome/browser/resources/security_warnings/interstitial_ui.html#newcode2 chrome/browser/resources/security_warnings/interstitial_ui.html:2: <head> On 2015/02/26 05:26:13, felt wrote: ...
5 years, 10 months ago (2015-02-26 14:21:44 UTC) #33
fahl
https://codereview.chromium.org/940543003/diff/180001/chrome/browser/resources/security_warnings/interstitial_ui.html File chrome/browser/resources/security_warnings/interstitial_ui.html (right): https://codereview.chromium.org/940543003/diff/180001/chrome/browser/resources/security_warnings/interstitial_ui.html#newcode2 chrome/browser/resources/security_warnings/interstitial_ui.html:2: <head> On 2015/02/26 05:26:13, felt wrote: > nit: can ...
5 years, 10 months ago (2015-02-26 17:33:19 UTC) #34
felt
On 2015/02/26 17:33:19, fahl wrote: > https://codereview.chromium.org/940543003/diff/180001/chrome/browser/resources/security_warnings/interstitial_ui.html > File chrome/browser/resources/security_warnings/interstitial_ui.html (right): > > https://codereview.chromium.org/940543003/diff/180001/chrome/browser/resources/security_warnings/interstitial_ui.html#newcode2 > ...
5 years, 10 months ago (2015-02-26 18:08:56 UTC) #35
fahl
@sleevi, could you please review my change to chrome/browser/browser_resources.grd?
5 years, 10 months ago (2015-02-26 18:31:18 UTC) #37
Bernhard Bauer
https://codereview.chromium.org/940543003/diff/200001/chrome/browser/resources/security_warnings/interstitial_ui.html File chrome/browser/resources/security_warnings/interstitial_ui.html (right): https://codereview.chromium.org/940543003/diff/200001/chrome/browser/resources/security_warnings/interstitial_ui.html#newcode2 chrome/browser/resources/security_warnings/interstitial_ui.html:2: <head> Unindent this as well if you unindent the ...
5 years, 10 months ago (2015-02-26 18:35:43 UTC) #38
fahl
https://codereview.chromium.org/940543003/diff/200001/chrome/browser/resources/security_warnings/interstitial_ui.html File chrome/browser/resources/security_warnings/interstitial_ui.html (right): https://codereview.chromium.org/940543003/diff/200001/chrome/browser/resources/security_warnings/interstitial_ui.html#newcode2 chrome/browser/resources/security_warnings/interstitial_ui.html:2: <head> On 2015/02/26 18:35:43, Bernhard (back on Feb 26) ...
5 years, 10 months ago (2015-02-26 18:50:50 UTC) #39
felt
https://codereview.chromium.org/940543003/diff/200001/chrome/browser/resources/security_warnings/interstitial_ui.html File chrome/browser/resources/security_warnings/interstitial_ui.html (right): https://codereview.chromium.org/940543003/diff/200001/chrome/browser/resources/security_warnings/interstitial_ui.html#newcode2 chrome/browser/resources/security_warnings/interstitial_ui.html:2: <head> On 2015/02/26 18:50:49, fahl wrote: > On 2015/02/26 ...
5 years, 10 months ago (2015-02-26 18:56:27 UTC) #40
Ryan Sleevi
I'm not an owner for that file but... I gave you some nits instead :) ...
5 years, 10 months ago (2015-02-27 00:26:39 UTC) #41
fahl
On 2015/02/27 00:26:39, Ryan Sleevi wrote: > I'm not an owner for that file > ...
5 years, 9 months ago (2015-02-27 21:15:01 UTC) #43
fahl
+thestig for browser_resources.grd https://codereview.chromium.org/940543003/diff/220001/chrome/browser/ssl/ssl_blocking_page.h File chrome/browser/ssl/ssl_blocking_page.h (right): https://codereview.chromium.org/940543003/diff/220001/chrome/browser/ssl/ssl_blocking_page.h#newcode74 chrome/browser/ssl/ssl_blocking_page.h:74: const base::Time& time_triggered); On 2015/02/27 00:26:39, ...
5 years, 9 months ago (2015-02-27 21:16:17 UTC) #45
Lei Zhang
lgtm
5 years, 9 months ago (2015-02-27 21:19:20 UTC) #46
felt
Now if you check the box next to "Commit" it will send it to the ...
5 years, 9 months ago (2015-02-27 21:27:44 UTC) #47
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/940543003/260001
5 years, 9 months ago (2015-02-27 22:34:32 UTC) #50
commit-bot: I haz the power
Committed patchset #8 (id:260001)
5 years, 9 months ago (2015-02-27 23:46:10 UTC) #51
commit-bot: I haz the power
Patchset 8 (id:??) landed as https://crrev.com/920f3bdfc9fc08cf60da6708a050a9bc82a0d223 Cr-Commit-Position: refs/heads/master@{#318548}
5 years, 9 months ago (2015-02-27 23:47:01 UTC) #52
felt
5 years, 9 months ago (2015-02-28 00:36:06 UTC) #53
Message was sent while issue was closed.
On 2015/02/27 23:47:01, I haz the power (commit-bot) wrote:
> Patchset 8 (id:??) landed as
> https://crrev.com/920f3bdfc9fc08cf60da6708a050a9bc82a0d223
> Cr-Commit-Position: refs/heads/master@{#318548}

congrats!! first CL! :)

Powered by Google App Engine
This is Rietveld 408576698