Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2076873003:
[SafeBrowsing] Clarify `danger_level` in safe_browsing/README.md (Closed)
|
Created:
4 years, 6 months ago by asanka Modified:
4 years, 6 months ago CC:
arv+watch_chromium.org, chromium-reviews, Jialiu Lin, vakh (use Gerrit instead) Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[SafeBrowsing] Clarify `danger_level` in safe_browsing/README.md
BUG=none
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/3be09a7aab77f4eb9fa20efd9802187ae857df5a
Cr-Commit-Position: refs/heads/master@{#401036}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Address comments. #Patch Set 3 : Address comments #
Total comments: 2
Patch Set 4 : Fix list of dangerous download verdicts. #Messages
Total messages: 20 (9 generated)
Description was changed from ========== [SafeBrowsing] Clarify `danger_level` in safe_browsing/README.md BUG=none ========== to ========== [SafeBrowsing] Clarify `danger_level` in safe_browsing/README.md BUG=none CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
asanka@chromium.org changed reviewers: + nparker@chromium.org
lgtm Thanks! https://codereview.chromium.org/2076873003/diff/1/chrome/browser/resources/sa... File chrome/browser/resources/safe_browsing/README.md (right): https://codereview.chromium.org/2076873003/diff/1/chrome/browser/resources/sa... chrome/browser/resources/safe_browsing/README.md:71: `SAMPLED_PING` or `NO_PING`, or downloads where the Safe Browsing ping and downloads https://codereview.chromium.org/2076873003/diff/1/chrome/browser/resources/sa... chrome/browser/resources/safe_browsing/README.md:72: either fails or returns an `UNKNOWN` verdict. Exceptions are noted. either fails, is disabled, or returns... How about adding something like: The warning controlled here is a generic "this file type may harm your computer." If the Safe Browsing verdict is UNWANTED or MALWARE, Chrome will show that more severe warning regardless of this setting. (This is hard to describe succinctly!) https://codereview.chromium.org/2076873003/diff/1/chrome/browser/resources/sa... chrome/browser/resources/safe_browsing/README.md:77: Browsing returns a `SAFE` verdict, we still warn the user. (We should probably remove this logic once the backend can mark them UNKNOWN reliably... this sort of mucks up the layering of decisions).
vakh@chromium.org changed reviewers: + vakh@chromium.org
lgtm
nparker: PTAL?
lgtm with one fix https://codereview.chromium.org/2076873003/diff/40001/chrome/browser/resource... File chrome/browser/resources/safe_browsing/README.md (right): https://codereview.chromium.org/2076873003/diff/40001/chrome/browser/resource... chrome/browser/resources/safe_browsing/README.md:76: If the Safe Browsing verdict is `UNKNOWN` or `DANGEROUS`, Chrome will show s/UNKNOWN/UNCOMMON, POTENTIALLY_UNWANTED, DANGEROUS_HOST/
Thanks! https://codereview.chromium.org/2076873003/diff/1/chrome/browser/resources/sa... File chrome/browser/resources/safe_browsing/README.md (right): https://codereview.chromium.org/2076873003/diff/1/chrome/browser/resources/sa... chrome/browser/resources/safe_browsing/README.md:71: `SAMPLED_PING` or `NO_PING`, or downloads where the Safe Browsing ping On 2016/06/17 at 21:44:01, Nathan Parker wrote: > and downloads Done. https://codereview.chromium.org/2076873003/diff/1/chrome/browser/resources/sa... chrome/browser/resources/safe_browsing/README.md:72: either fails or returns an `UNKNOWN` verdict. Exceptions are noted. On 2016/06/17 at 21:44:01, Nathan Parker wrote: > either fails, is disabled, or returns... > > How about adding something like: > > The warning controlled here is a generic "this file type may harm your computer." If the Safe Browsing verdict is UNWANTED or MALWARE, Chrome will show that more severe warning regardless of this setting. > > (This is hard to describe succinctly!) Done and done. :) https://codereview.chromium.org/2076873003/diff/1/chrome/browser/resources/sa... chrome/browser/resources/safe_browsing/README.md:77: Browsing returns a `SAFE` verdict, we still warn the user. On 2016/06/17 at 21:44:01, Nathan Parker wrote: > (We should probably remove this logic once the backend can mark them UNKNOWN reliably... this sort of mucks up the layering of decisions). Acknowledged. https://codereview.chromium.org/2076873003/diff/40001/chrome/browser/resource... File chrome/browser/resources/safe_browsing/README.md (right): https://codereview.chromium.org/2076873003/diff/40001/chrome/browser/resource... chrome/browser/resources/safe_browsing/README.md:76: If the Safe Browsing verdict is `UNKNOWN` or `DANGEROUS`, Chrome will show On 2016/06/21 at 16:55:00, Nathan Parker wrote: > s/UNKNOWN/UNCOMMON, POTENTIALLY_UNWANTED, DANGEROUS_HOST/ D'oh. Done.
The CQ bit was checked by asanka@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from vakh@chromium.org Link to the patchset: https://codereview.chromium.org/2076873003/#ps40001 (title: "Address comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2076873003/40001
The CQ bit was unchecked by asanka@chromium.org
The CQ bit was checked by asanka@chromium.org
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/2076873003/#ps60001 (title: "Fix list of dangerous download verdicts.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2076873003/60001
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== [SafeBrowsing] Clarify `danger_level` in safe_browsing/README.md BUG=none CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== [SafeBrowsing] Clarify `danger_level` in safe_browsing/README.md BUG=none CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/3be09a7aab77f4eb9fa20efd9802187ae857df5a Cr-Commit-Position: refs/heads/master@{#401036} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/3be09a7aab77f4eb9fa20efd9802187ae857df5a Cr-Commit-Position: refs/heads/master@{#401036} |
