Enable client side phishing detection on https sites.

Enable client side phishing detection on https sites.

(Client side malware detection is already enabled on https).

BUG=730895
Review-Url: https://codereview.chromium.org/2927123004
Cr-Commit-Position: refs/heads/master@{#478810}
Committed: https://chromium.googlesource.com/chromium/src/+/837b32b0bc91016102f09448259fb6048516f0a1

[Jialiu Lin, 2017-06-08 22:45:49]

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

[commit-bot: I haz the power, 2017-06-08 22:46:22]

Dry run: CQ is trying da patch.

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

[Jialiu Lin, 2017-06-08 23:07:33]

jialiul@chromium.org changed reviewers:
+ nparker@chromium.org, vakh@chromium.org

[Jialiu Lin, 2017-06-08 23:07:33]

A tiny CL to turn on CSD phishing detection on Https sites.

[vakh (use Gerrit instead), 2017-06-08 23:30:31]

https://codereview.chromium.org/2927123004/diff/1/chrome/browser/safe_browsin...
File chrome/browser/safe_browsing/client_side_detection_host.cc (left):

https://codereview.chromium.org/2927123004/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/client_side_detection_host.cc:113: // For phishing
we only classify HTTP pages.
Any chance this was intended as: this URL is not HTTP (or HTTPS)
but the code only checked for HTTP.

We do this for DB checks also:
https://cs.chromium.org/chromium/src/components/safe_browsing_db/v4_local_dat...

[commit-bot: I haz the power, 2017-06-08 23:45:12]

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

[commit-bot: I haz the power, 2017-06-08 23:45:13]

Dry run: This issue passed the CQ dry run.

[Jialiu Lin, 2017-06-09 00:19:07]

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

[commit-bot: I haz the power, 2017-06-09 00:19:32]

Dry run: CQ is trying da patch.

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

[Jialiu Lin, 2017-06-09 00:26:12]

Thanks vakh@! CL updated based on your feedback.

https://codereview.chromium.org/2927123004/diff/1/chrome/browser/safe_browsin...
File chrome/browser/safe_browsing/client_side_detection_host.cc (left):

https://codereview.chromium.org/2927123004/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/client_side_detection_host.cc:113: // For phishing
we only classify HTTP pages.
On 2017/06/08 23:30:31, vakh (Varun Khaneja) wrote:
> Any chance this was intended as: this URL is not HTTP (or HTTPS)
> but the code only checked for HTTP.
> 
> We do this for DB checks also:
>
https://cs.chromium.org/chromium/src/components/safe_browsing_db/v4_local_dat...

Good catch. You're right. I should be aware of other schemes.

[vakh (use Gerrit instead), 2017-06-10 07:11:31]

lgtm

[Jialiu Lin, 2017-06-12 17:36:00]

ping nparker@ ~

[Nathan Parker, 2017-06-12 21:43:56]

lgtm

https://codereview.chromium.org/2927123004/diff/40001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/client_side_detection_host.cc (right):

https://codereview.chromium.org/2927123004/diff/40001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/client_side_detection_host.cc:170:
NO_CLASSIFY_SCHEME_NOT_SUPPORTED,
nit: add integer values to these to verify they match up with UMA histogram.

[Jialiu Lin, 2017-06-12 21:59:35]

Thanks!

https://codereview.chromium.org/2927123004/diff/40001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/client_side_detection_host.cc (right):

https://codereview.chromium.org/2927123004/diff/40001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/client_side_detection_host.cc:170:
NO_CLASSIFY_SCHEME_NOT_SUPPORTED,
On 2017/06/12 21:43:56, Nathan Parker wrote:
> nit: add integer values to these to verify they match up with UMA histogram.

Done.

[Jialiu Lin, 2017-06-12 21:59:52]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2017-06-12 21:59:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
nparker@chromium.org, vakh@chromium.org
Link to the patchset: https://codereview.chromium.org/2927123004/#ps60001
(title: "address nparker's comments")

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

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 23:08:32]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1497304792750950,
"parent_rev": "05d349a91971b2afc98b41693feaa7fcbec76e4f", "commit_rev":
"837b32b0bc91016102f09448259fb6048516f0a1"}

[commit-bot: I haz the power, 2017-06-12 23:08:45]

Description was changed from

==========
Enable client side phishing detection on https sites.

(Client side malware detection is already enabled on https).

BUG=730895
==========

to

==========
Enable client side phishing detection on https sites.

(Client side malware detection is already enabled on https).

BUG=730895

Review-Url: https://codereview.chromium.org/2927123004
Cr-Commit-Position: refs/heads/master@{#478810}
Committed:
https://chromium.googlesource.com/chromium/src/+/837b32b0bc91016102f09448259f...
==========

[commit-bot: I haz the power, 2017-06-12 23:08:46]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/837b32b0bc91016102f09448259f...

[vakh (use Gerrit instead), 2017-06-15 20:51:26]

On 2017/06/12 23:08:46, commit-bot: I haz the power wrote:
> Committed patchset #4 (id:60001) as
>
https://chromium.googlesource.com/chromium/src/+/837b32b0bc91016102f09448259f...

Do we need to do this here also:
https://cs.chromium.org/chromium/src/chrome/renderer/safe_browsing/phishing_c...
?

I don't know but I suspect.

[Jialiu Lin, 2017-06-15 21:25:26]

Good catch!  I missed this one. I'll reopen this bug and send a follow-up CL
shortly.