Implement V2 design for quiet safe browsing interstitial

Implement V2 design for quiet safe browsing interstitial

+ Add details link and details content
+ Implement proceed link.

BUG=731721
Review-Url: https://codereview.chromium.org/2930043002
Cr-Commit-Position: refs/heads/master@{#478951}
Committed: https://chromium.googlesource.com/chromium/src/+/bd53cb080c712e419d2a1a3f753576f4de4342a5

[edwardjung, 2017-06-09 14:23:38]

Description was changed from

==========
Implement details for quiet safe browsing interstitial

BUG=731721
==========

to

==========
Implement V2 design for quiet safe browsing interstitial

+ Add details link and details content
+ Implement proceed link.

BUG=731721
==========

[edwardjung, 2017-06-09 14:33:02]

edwardjung@chromium.org changed reviewers:
+ felt@chromium.org

[edwardjung, 2017-06-09 14:33:02]

@felt PTAL. 

I'm not sure if there is anything else required from the WebView side to
implement the proceed action.

[felt, 2017-06-09 16:55:18]

https://codereview.chromium.org/2930043002/diff/1/components/security_interst...
File
components/security_interstitials/core/browser/resources/interstitial_webview_quiet.css
(right):

https://codereview.chromium.org/2930043002/diff/1/components/security_interst...
components/security_interstitials/core/browser/resources/interstitial_webview_quiet.css:13:
font-size: 0.93333em;
ha, very precise :)

https://codereview.chromium.org/2930043002/diff/1/components/security_interst...
File
components/security_interstitials/core/browser/resources/interstitial_webview_quiet.html
(left):

https://codereview.chromium.org/2930043002/diff/1/components/security_interst...
components/security_interstitials/core/browser/resources/interstitial_webview_quiet.html:26:
$i18Raw{explanationParagraph}
oof! has this been broken :0

https://codereview.chromium.org/2930043002/diff/1/components/security_interst...
File components/security_interstitials/core/safe_browsing_quiet_error_ui.cc
(right):

https://codereview.chromium.org/2930043002/diff/1/components/security_interst...
components/security_interstitials/core/safe_browsing_quiet_error_ui.cc:101: }
break; at the end to avoid falling through in the else case

https://codereview.chromium.org/2930043002/diff/1/components/security_interst...
components/security_interstitials/core/safe_browsing_quiet_error_ui.cc:117: case
CMD_TEXT_NOT_FOUND:
NOTREACHED()?

[felt, 2017-06-09 16:56:08]

Nate: It looks to me like the webview version uses
components/security_interstitials/content/security_interstitial_controller_client.h
for the controller. And so it will invoke that class's version of Proceed(). Is
that still correct for the AW case?

[Nate Fischer, 2017-06-10 01:01:05]

On 2017/06/09 16:56:08, felt wrote:
> Nate: It looks to me like the webview version uses
>
components/security_interstitials/content/security_interstitial_controller_client.h
> for the controller. And so it will invoke that class's version of Proceed().
Is
> that still correct for the AW case?

Sorry for the late response.

Yes, we use the same Proceed(). Our "visit-unsafe-page-anyway" behavior is the
same as Chrome's, so I don't anticipate any issues. I can try patching this CL
on Monday to test the change locally if we want to be sure. We've been using
that Proceed() in the loud interstitial with no issues.

[edwardjung, 2017-06-12 13:21:35]

The CQ bit was checked by edwardjung@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-12 13:21:45]

Dry run: CQ is trying da patch.

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

[edwardjung, 2017-06-12 13:22:25]

Address comments and fixed padding for medium sized interstitials according to
the spec.

https://codereview.chromium.org/2930043002/diff/1/components/security_interst...
File
components/security_interstitials/core/browser/resources/interstitial_webview_quiet.html
(left):

https://codereview.chromium.org/2930043002/diff/1/components/security_interst...
components/security_interstitials/core/browser/resources/interstitial_webview_quiet.html:26:
$i18Raw{explanationParagraph}
On 2017/06/09 16:55:18, felt wrote:
> oof! has this been broken :0

Yeah, it was broken. Fortunately we weren't showing the details for V1.

https://codereview.chromium.org/2930043002/diff/1/components/security_interst...
File components/security_interstitials/core/safe_browsing_quiet_error_ui.cc
(right):

https://codereview.chromium.org/2930043002/diff/1/components/security_interst...
components/security_interstitials/core/safe_browsing_quiet_error_ui.cc:101: }
On 2017/06/09 16:55:18, felt wrote:
> break; at the end to avoid falling through in the else case

Done.

https://codereview.chromium.org/2930043002/diff/1/components/security_interst...
components/security_interstitials/core/safe_browsing_quiet_error_ui.cc:117: case
CMD_TEXT_NOT_FOUND:
On 2017/06/09 16:55:18, felt wrote:
> NOTREACHED()?

Done.

[edwardjung, 2017-06-12 13:23:47]

> Yes, we use the same Proceed(). Our "visit-unsafe-page-anyway" behavior is the
> same as Chrome's, so I don't anticipate any issues. I can try patching this CL
> on Monday to test the change locally if we want to be sure. We've been using
> that Proceed() in the loud interstitial with no issues.

Thanks Nate, if you could patch the latest patchset to check this works properly
in WV, that would great.

[commit-bot: I haz the power, 2017-06-12 14:14:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-12 14:14:21]

Dry run: This issue passed the CQ dry run.

[felt, 2017-06-12 16:15:39]

lgtm % optional nit

https://codereview.chromium.org/2930043002/diff/20001/components/security_int...
File components/security_interstitials/core/safe_browsing_quiet_error_ui.cc
(right):

https://codereview.chromium.org/2930043002/diff/20001/components/security_int...
components/security_interstitials/core/safe_browsing_quiet_error_ui.cc:100:
break;
nit: You don't need this earlier break anymore (although it doesn't hurt)

[Nate Fischer, 2017-06-12 17:54:13]

On 2017/06/12 13:23:47, edwardjung wrote:
> > Yes, we use the same Proceed(). Our "visit-unsafe-page-anyway" behavior is
the
> > same as Chrome's, so I don't anticipate any issues. I can try patching this
CL
> > on Monday to test the change locally if we want to be sure. We've been using
> > that Proceed() in the loud interstitial with no issues.
> 
> Thanks Nate, if you could patch the latest patchset to check this works
properly
> in WV, that would great.

Yup, looks good! Here's a link to some screenshots:

https://drive.google.com/drive/folders/0B6dOsuQZDwrMQ0VFYWJybVFvWDQ?usp=sharing

[edwardjung, 2017-06-12 18:54:18]

On 2017/06/12 17:54:13, Nate Fischer wrote:
> On 2017/06/12 13:23:47, edwardjung wrote:
> > > Yes, we use the same Proceed(). Our "visit-unsafe-page-anyway" behavior is
> the
> > > same as Chrome's, so I don't anticipate any issues. I can try patching
this
> CL
> > > on Monday to test the change locally if we want to be sure. We've been
using
> > > that Proceed() in the loud interstitial with no issues.
> > 
> > Thanks Nate, if you could patch the latest patchset to check this works
> properly
> > in WV, that would great.
> 
> Yup, looks good! Here's a link to some screenshots:
> 
>
https://drive.google.com/drive/folders/0B6dOsuQZDwrMQ0VFYWJybVFvWDQ?usp=sharing

Thanks Nate!

[edwardjung, 2017-06-12 18:54:53]

https://codereview.chromium.org/2930043002/diff/20001/components/security_int...
File components/security_interstitials/core/safe_browsing_quiet_error_ui.cc
(right):

https://codereview.chromium.org/2930043002/diff/20001/components/security_int...
components/security_interstitials/core/safe_browsing_quiet_error_ui.cc:100:
break;
On 2017/06/12 16:15:39, felt wrote:
> nit: You don't need this earlier break anymore (although it doesn't hurt)

Good point!

[edwardjung, 2017-06-12 18:59:45]

The CQ bit was checked by edwardjung@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-12 19:00:39]

Dry run: 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-06-12 20:21:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-12 20:21:29]

Dry run: This issue passed the CQ dry run.

[edwardjung, 2017-06-13 09:24:32]

The CQ bit was checked by edwardjung@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-13 09:24:48]

Dry run: 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-06-13 09:28:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-13 09:28:51]

Dry run: This issue passed the CQ dry run.

[edwardjung, 2017-06-13 09:35:10]

The CQ bit was checked by edwardjung@chromium.org

[edwardjung, 2017-06-13 09:35:11]

The patchset sent to the CQ was uploaded after l-g-t-m from felt@chromium.org
Link to the patchset: https://codereview.chromium.org/2930043002/#ps40001
(title: "Remove extra break")

[commit-bot: I haz the power, 2017-06-13 09:35:22]

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-06-13 09:39:17]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1497346510851420,
"parent_rev": "440815ff0f34bdda27891d41ba73ea97bbb51b03", "commit_rev":
"bd53cb080c712e419d2a1a3f753576f4de4342a5"}

[commit-bot: I haz the power, 2017-06-13 09:39:28]

Description was changed from

==========
Implement V2 design for quiet safe browsing interstitial

+ Add details link and details content
+ Implement proceed link.

BUG=731721
==========

to

==========
Implement V2 design for quiet safe browsing interstitial

+ Add details link and details content
+ Implement proceed link.

BUG=731721

Review-Url: https://codereview.chromium.org/2930043002
Cr-Commit-Position: refs/heads/master@{#478951}
Committed:
https://chromium.googlesource.com/chromium/src/+/bd53cb080c712e419d2a1a3f7535...
==========

[commit-bot: I haz the power, 2017-06-13 09:39:29]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/bd53cb080c712e419d2a1a3f7535...