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

Issue 813873005: Add field trial and flag to mark HTTP as non-secure. (Closed)

Created:
6 years ago by palmer
Modified:
5 years, 11 months ago
Reviewers:
egm, felt, sky, Ryan Sleevi, Mark P, jww
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add field trial and flag to mark HTTP as non-secure. BUG=444242 Committed: https://crrev.com/601fe3c76c5ae5a0fab68766a95b9313c1b408ec Cr-Commit-Position: refs/heads/master@{#312920}

Patch Set 1 #

Total comments: 7

Patch Set 2 : de-format, meet field trial style on the field of style trial #

Total comments: 6

Patch Set 3 : Fix histogram stuff; refactor helper function. #

Patch Set 4 : Rebase. #

Patch Set 5 : Only badge if SECURITY_STYLE_UNAUTHENTICATED && unauthenticated network transport. (E.g. not chrome… #

Total comments: 9

Patch Set 6 : Respond to comments. #

Patch Set 7 : Describe when the flag's counter is emitted. #

Total comments: 1

Patch Set 8 : s/counter/action/ #

Total comments: 4

Patch Set 9 : Rename to "Mark Non-Secure As ...", use named constants. #

Total comments: 4

Patch Set 10 : Rebase and fix style nit. #

Patch Set 11 : Fix histogram name. #

Patch Set 12 : Consistently use "mark non secure as". #

Patch Set 13 : Gotta have a default option. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+81 lines, -1 line) Patch
M chrome/app/generated_resources.grd View 1 chunk +15 lines, -0 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +18 lines, -0 lines 0 comments Download
M chrome/browser/ui/toolbar/toolbar_model_impl.cc View 1 2 3 4 5 6 7 8 9 2 chunks +28 lines, -1 line 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 4 5 6 7 8 9 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 4 5 6 7 8 9 1 chunk +6 lines, -0 lines 0 comments Download
M tools/metrics/actions/actions.xml View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +9 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 49 (15 generated)
palmer
Might need more work, but, have fun.
6 years ago (2014-12-20 00:57:50 UTC) #2
jww
Drive by review! https://codereview.chromium.org/813873005/diff/1/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/813873005/diff/1/chrome/browser/about_flags.cc#newcode191 chrome/browser/about_flags.cc:191: const Experiment::Choice kMarkHttpAsNonSecureChoices[] = { I ...
6 years ago (2014-12-20 01:21:57 UTC) #4
Ryan Sleevi
https://codereview.chromium.org/813873005/diff/1/chrome/browser/ui/toolbar/toolbar_model_impl.cc File chrome/browser/ui/toolbar/toolbar_model_impl.cc (right): https://codereview.chromium.org/813873005/diff/1/chrome/browser/ui/toolbar/toolbar_model_impl.cc#newcode97 chrome/browser/ui/toolbar/toolbar_model_impl.cc:97: if (GetSecurityLevelForFieldTrialGroup( On 2014/12/20 01:21:57, jww wrote: > According ...
6 years ago (2014-12-20 01:29:32 UTC) #6
palmer
https://codereview.chromium.org/813873005/diff/1/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/813873005/diff/1/chrome/browser/about_flags.cc#newcode191 chrome/browser/about_flags.cc:191: const Experiment::Choice kMarkHttpAsNonSecureChoices[] = { On 2014/12/20 01:21:57, jww ...
6 years ago (2014-12-20 01:59:53 UTC) #7
jww
https://codereview.chromium.org/813873005/diff/20001/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/813873005/diff/20001/chrome/browser/about_flags.cc#newcode2101 chrome/browser/about_flags.cc:2101: "mark-http-as-non-secure", // FLAGS:RECORD_UMA Somewhere at the top of this ...
6 years ago (2014-12-20 02:07:58 UTC) #8
palmer
https://codereview.chromium.org/813873005/diff/20001/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/813873005/diff/20001/chrome/browser/about_flags.cc#newcode2101 chrome/browser/about_flags.cc:2101: "mark-http-as-non-secure", // FLAGS:RECORD_UMA On 2014/12/20 02:07:58, jww wrote: > ...
6 years ago (2014-12-20 02:32:34 UTC) #9
palmer
msw: OWNER for Chrome UI mpearson: OWNER for UMA/histograms
6 years ago (2014-12-23 00:40:53 UTC) #11
felt
https://codereview.chromium.org/813873005/diff/80001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/813873005/diff/80001/chrome/app/generated_resources.grd#newcode5690 chrome/app/generated_resources.grd:5690: + Do not mark HTTP as non-secure or as ...
6 years ago (2014-12-23 01:34:58 UTC) #12
Mark P
https://codereview.chromium.org/813873005/diff/80001/tools/metrics/actions/actions.xml File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/813873005/diff/80001/tools/metrics/actions/actions.xml#newcode186 tools/metrics/actions/actions.xml:186: <owner>palmer@chromium.org, felt@chromium.org</owner> owner is a repeated field, one e-mail ...
5 years, 11 months ago (2015-01-02 19:14:28 UTC) #14
blelump
5 years, 11 months ago (2015-01-02 20:02:23 UTC) #16
palmer
https://codereview.chromium.org/813873005/diff/80001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/813873005/diff/80001/chrome/app/generated_resources.grd#newcode5690 chrome/app/generated_resources.grd:5690: + Do not mark HTTP as non-secure or as ...
5 years, 11 months ago (2015-01-07 00:59:00 UTC) #17
felt
did you forget to upload the new patchset?
5 years, 11 months ago (2015-01-07 03:31:10 UTC) #18
Mark P
Also, please upload a new patchset to reflect your past changes. --mark https://codereview.chromium.org/813873005/diff/80001/tools/metrics/actions/actions.xml File tools/metrics/actions/actions.xml ...
5 years, 11 months ago (2015-01-07 22:43:58 UTC) #19
palmer
> did you forget to upload the new patchset? I was waiting for it to ...
5 years, 11 months ago (2015-01-07 22:47:42 UTC) #20
palmer
> In that case, you need to revise your comment explain when this action is ...
5 years, 11 months ago (2015-01-07 23:38:57 UTC) #21
Mark P
On 2015/01/07 23:38:57, Chromium Palmer wrote: > > In that case, you need to revise ...
5 years, 11 months ago (2015-01-07 23:52:10 UTC) #22
palmer
> If you're keeping it, please revise the action description as I mentioned above. OK, ...
5 years, 11 months ago (2015-01-08 00:04:38 UTC) #23
Mark P
xml files lgtm baring one minor comment https://codereview.chromium.org/813873005/diff/120001/tools/metrics/actions/actions.xml File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/813873005/diff/120001/tools/metrics/actions/actions.xml#newcode197 tools/metrics/actions/actions.xml:197: Non-Secure. The ...
5 years, 11 months ago (2015-01-08 00:06:54 UTC) #24
felt
lgtm % nit https://codereview.chromium.org/813873005/diff/140001/chrome/browser/ui/toolbar/toolbar_model_impl.cc File chrome/browser/ui/toolbar/toolbar_model_impl.cc (right): https://codereview.chromium.org/813873005/diff/140001/chrome/browser/ui/toolbar/toolbar_model_impl.cc#newcode71 chrome/browser/ui/toolbar/toolbar_model_impl.cc:71: if (choice == "no") nit: I'd ...
5 years, 11 months ago (2015-01-08 00:46:56 UTC) #25
Ryan Sleevi
I'm surprised and shocked that you don't have to trust the website settings UI bubble ...
5 years, 11 months ago (2015-01-12 21:03:52 UTC) #26
palmer
https://codereview.chromium.org/813873005/diff/140001/chrome/browser/ui/toolbar/toolbar_model_impl.cc File chrome/browser/ui/toolbar/toolbar_model_impl.cc (right): https://codereview.chromium.org/813873005/diff/140001/chrome/browser/ui/toolbar/toolbar_model_impl.cc#newcode71 chrome/browser/ui/toolbar/toolbar_model_impl.cc:71: if (choice == "no") On 2015/01/08 00:46:56, felt wrote: ...
5 years, 11 months ago (2015-01-16 23:42:41 UTC) #27
palmer
> I'm surprised and shocked that you don't have to trust the website settings UI ...
5 years, 11 months ago (2015-01-16 23:47:48 UTC) #28
palmer
Since msw will be out, +sky for chrome/browser/ui/toolbar/toolbar_model_impl.cc OWNERS.
5 years, 11 months ago (2015-01-22 21:21:59 UTC) #30
sky
You only need a review of chrome/browser/ui/toolbar/toolbar_model_impl.cc ? Right? https://codereview.chromium.org/813873005/diff/160001/chrome/browser/ui/toolbar/toolbar_model_impl.cc File chrome/browser/ui/toolbar/toolbar_model_impl.cc (right): https://codereview.chromium.org/813873005/diff/160001/chrome/browser/ui/toolbar/toolbar_model_impl.cc#newcode73 chrome/browser/ui/toolbar/toolbar_model_impl.cc:73: ...
5 years, 11 months ago (2015-01-22 21:56:47 UTC) #32
palmer
Yes, just the 1 file. Thanks, sky! https://codereview.chromium.org/813873005/diff/160001/chrome/browser/ui/toolbar/toolbar_model_impl.cc File chrome/browser/ui/toolbar/toolbar_model_impl.cc (right): https://codereview.chromium.org/813873005/diff/160001/chrome/browser/ui/toolbar/toolbar_model_impl.cc#newcode73 chrome/browser/ui/toolbar/toolbar_model_impl.cc:73: else if ...
5 years, 11 months ago (2015-01-22 22:01:01 UTC) #33
sky
LGTM
5 years, 11 months ago (2015-01-23 00:37:30 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/813873005/180001
5 years, 11 months ago (2015-01-23 00:40:44 UTC) #36
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/17421) Try jobs failed on following ...
5 years, 11 months ago (2015-01-23 02:17:57 UTC) #38
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/813873005/200001
5 years, 11 months ago (2015-01-23 02:33:01 UTC) #40
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/813873005/220001
5 years, 11 months ago (2015-01-23 03:19:00 UTC) #43
commit-bot: I haz the power
Try jobs failed on following builders: linux_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/117112) linux_gpu_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_triggered_tests/builds/102880) Try ...
5 years, 11 months ago (2015-01-23 03:57:09 UTC) #45
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/813873005/240001
5 years, 11 months ago (2015-01-23 19:23:00 UTC) #47
commit-bot: I haz the power
Committed patchset #13 (id:240001)
5 years, 11 months ago (2015-01-23 20:43:40 UTC) #48
commit-bot: I haz the power
5 years, 11 months ago (2015-01-23 20:44:34 UTC) #49
Message was sent while issue was closed.
Patchset 13 (id:??) landed as
https://crrev.com/601fe3c76c5ae5a0fab68766a95b9313c1b408ec
Cr-Commit-Position: refs/heads/master@{#312920}

Powered by Google App Engine
This is Rietveld 408576698