Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2861793005:
[MD settings] UMA and RevocationReporter in site settings (Closed)
|
Created:
3 years, 7 months ago by dschuyler Modified:
3 years, 7 months ago Reviewers:
Dan Beam CC:
chromium-reviews, michaelpg+watch-md-settings_chromium.org, dbeam+watch-settings_chromium.org, stevenjb+watch-md-settings_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[MD settings] UMA and RevocationReporter in site settings
This CL adds UMA and Revocation reporting from site settings when a site
exception is added or removed. The goal is to match the behavior of the
old options.
BUG=718544
Review-Url: https://codereview.chromium.org/2861793005
Cr-Commit-Position: refs/heads/master@{#469561}
Committed: https://chromium.googlesource.com/chromium/src/+/b1c163ea6bf56b21576bed89fcf4162eebb2ab91
Patch Set 1 #
Total comments: 2
Patch Set 2 : leave secondary pattern in place #Messages
Total messages: 19 (12 generated)
The CQ bit was checked by dschuyler@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
dschuyler@chromium.org changed reviewers: + dbeam@chromium.org
https://codereview.chromium.org/2861793005/diff/1/chrome/browser/ui/webui/set... File chrome/browser/ui/webui/settings/site_settings_handler.cc (right): https://codereview.chromium.org/2861793005/diff/1/chrome/browser/ui/webui/set... chrome/browser/ui/webui/settings/site_settings_handler.cc:505: // TODO(dschuyler): |secondary_pattern_unused| is not used, remove it. why did the old code use it but the new one doesn't need to?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by dschuyler@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2861793005/diff/1/chrome/browser/ui/webui/set... File chrome/browser/ui/webui/settings/site_settings_handler.cc (right): https://codereview.chromium.org/2861793005/diff/1/chrome/browser/ui/webui/set... chrome/browser/ui/webui/settings/site_settings_handler.cc:505: // TODO(dschuyler): |secondary_pattern_unused| is not used, remove it. On 2017/05/04 19:37:45, Dan Beam wrote: > why did the old code use it but the new one doesn't need to? The reasoning was based on the JS sending an empty string for this parameter. Though this doesn't need to be changed as part of this patch, so I've replaced the use of secondary pattern here and changed the todo.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm but worth writing tests to ensure UMA is logged?
On 2017/05/04 22:48:00, Dan Beam wrote: > lgtm but worth writing tests to ensure UMA is logged? Looking into it.
The CQ bit was checked by dschuyler@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493946642178330,
"parent_rev": "a11f72c868235d4673cd96c7eea24ad9e7eb4b55", "commit_rev":
"b1c163ea6bf56b21576bed89fcf4162eebb2ab91"}
Message was sent while issue was closed.
Description was changed from ========== [MD settings] UMA and RevocationReporter in site settings This CL adds UMA and Revocation reporting from site settings when a site exception is added or removed. The goal is to match the behavior of the old options. BUG=718544 ========== to ========== [MD settings] UMA and RevocationReporter in site settings This CL adds UMA and Revocation reporting from site settings when a site exception is added or removed. The goal is to match the behavior of the old options. BUG=718544 Review-Url: https://codereview.chromium.org/2861793005 Cr-Commit-Position: refs/heads/master@{#469561} Committed: https://chromium.googlesource.com/chromium/src/+/b1c163ea6bf56b21576bed89fcf4... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/b1c163ea6bf56b21576bed89fcf4... |
