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

Issue 2746443002: Include display names in Quirks URL (Closed)

Created:
3 years, 9 months ago by Greg Levin
Modified:
3 years, 9 months ago
Reviewers:
oshima
CC:
chromium-reviews, kalyank, sadrul, oshima+watch_chromium.org, davemoore+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Include display names in Quirks URL BUG=667416 TEST=Enable VLOGing for Quirks, plug in an external display, check log, verify that Quirks URL includes display name (roughly the same name as seen in display settings for 2nd monitor). Review-Url: https://codereview.chromium.org/2746443002 Cr-Commit-Position: refs/heads/master@{#457551} Committed: https://chromium.googlesource.com/chromium/src/+/2aa9dd13b0ba68a0f493c86f80b1bb0f3ceffc41

Patch Set 1 #

Total comments: 13

Patch Set 2 : Address first review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+28 lines, -10 lines) Patch
M ash/display/display_color_manager_chromeos.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/display/quirks_browsertest.cc View 1 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/policy/device_quirks_policy_browsertest.cc View 2 chunks +4 lines, -2 lines 0 comments Download
M components/quirks/quirks_client.h View 1 2 chunks +4 lines, -0 lines 0 comments Download
M components/quirks/quirks_client.cc View 1 4 chunks +10 lines, -3 lines 0 comments Download
M components/quirks/quirks_manager.h View 1 2 chunks +2 lines, -0 lines 0 comments Download
M components/quirks/quirks_manager.cc View 1 4 chunks +4 lines, -2 lines 0 comments Download

Messages

Total messages: 10 (4 generated)
Greg Levin
Please have a look!
3 years, 9 months ago (2017-03-09 16:46:56 UTC) #2
oshima
https://codereview.chromium.org/2746443002/diff/1/chrome/browser/chromeos/display/quirks_browsertest.cc File chrome/browser/chromeos/display/quirks_browsertest.cc (right): https://codereview.chromium.org/2746443002/diff/1/chrome/browser/chromeos/display/quirks_browsertest.cc#newcode68 chrome/browser/chromeos/display/quirks_browsertest.cc:68: product_id, "", nit: std::string() https://codereview.chromium.org/2746443002/diff/1/components/quirks/quirks_client.h File components/quirks/quirks_client.h (right): https://codereview.chromium.org/2746443002/diff/1/components/quirks/quirks_client.h#newcode29 ...
3 years, 9 months ago (2017-03-14 12:38:54 UTC) #3
Greg Levin
Done... please have another look! https://codereview.chromium.org/2746443002/diff/1/chrome/browser/chromeos/display/quirks_browsertest.cc File chrome/browser/chromeos/display/quirks_browsertest.cc (right): https://codereview.chromium.org/2746443002/diff/1/chrome/browser/chromeos/display/quirks_browsertest.cc#newcode68 chrome/browser/chromeos/display/quirks_browsertest.cc:68: product_id, "", On 2017/03/14 ...
3 years, 9 months ago (2017-03-14 21:36:58 UTC) #4
oshima
lgtm
3 years, 9 months ago (2017-03-16 20:28:20 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2746443002/20001
3 years, 9 months ago (2017-03-16 20:29:51 UTC) #7
commit-bot: I haz the power
3 years, 9 months ago (2017-03-16 21:08:47 UTC) #10
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/2aa9dd13b0ba68a0f493c86f80b1...

Powered by Google App Engine
This is Rietveld 408576698