Chromium Code Reviews
Help | Chromium Project | Sign in
(496)

Issue 11369042: Caches the display names in MultiDisplayManager. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 5 months ago by Jun Mukai
Modified:
1 year, 5 months ago
CC:
chromium-reviews_chromium.org, stevenjb+watch_chromium.org, sadrul, oshima+watch_chromium.org, ben+watch_chromium.org
Visibility:
Public.

Description

Caches the display names in MultiDisplayManager.


BUG=159034
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=165775

Patch Set 1 #

Total comments: 6

Patch Set 2 : #

Total comments: 2

Patch Set 3 : #

Total comments: 9

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+80 lines, -54 lines) Lint Patch
M ash/ash_strings.grd View 1 2 3 1 chunk +3 lines, -0 lines 0 comments ? errors Download
M ash/display/multi_display_manager.h View 1 2 3 2 chunks +6 lines, -0 lines 0 comments 1 errors Download
M ash/display/multi_display_manager.cc View 1 2 3 5 chunks +41 lines, -23 lines 0 comments ? errors Download
M ash/system/chromeos/tray_display.cc View 1 2 3 4 chunks +30 lines, -31 lines 0 comments ? errors Download
Commit:

Messages

Total messages: 11
Jun Mukai
1 year, 5 months ago #1
oshima (OOO April 11 - 21)
http://codereview.chromium.org/11369042/diff/1/ash/display/multi_display_manager.cc File ash/display/multi_display_manager.cc (right): http://codereview.chromium.org/11369042/diff/1/ash/display/multi_display_manager.cc#newcode556 ash/display/multi_display_manager.cc:556: std::string result; this isn't used? http://codereview.chromium.org/11369042/diff/1/ash/display/multi_display_manager.cc#newcode567 ash/display/multi_display_manager.cc:567: if (!HasInternalDisplay() ...
1 year, 5 months ago #2
Jun Mukai
https://codereview.chromium.org/11369042/diff/1/ash/display/multi_display_manager.cc File ash/display/multi_display_manager.cc (right): https://codereview.chromium.org/11369042/diff/1/ash/display/multi_display_manager.cc#newcode556 ash/display/multi_display_manager.cc:556: std::string result; On 2012/11/02 06:33:56, oshima wrote: > this ...
1 year, 5 months ago #3
oshima (OOO April 11 - 21)
lgtm with a nit https://codereview.chromium.org/11369042/diff/5001/ash/display/multi_display_manager.cc File ash/display/multi_display_manager.cc (right): https://codereview.chromium.org/11369042/diff/5001/ash/display/multi_display_manager.cc#newcode561 ash/display/multi_display_manager.cc:561: if (HasInternalDisplay() && IsInternalDisplayId(display.id())) { ...
1 year, 5 months ago #4
Jun Mukai
https://codereview.chromium.org/11369042/diff/5001/ash/display/multi_display_manager.cc File ash/display/multi_display_manager.cc (right): https://codereview.chromium.org/11369042/diff/5001/ash/display/multi_display_manager.cc#newcode561 ash/display/multi_display_manager.cc:561: if (HasInternalDisplay() && IsInternalDisplayId(display.id())) { On 2012/11/02 16:35:11, oshima ...
1 year, 5 months ago #5
Jun Mukai
derat, can you review this?
1 year, 5 months ago #6
Daniel Erat
https://codereview.chromium.org/11369042/diff/10001/ash/ash_strings.grd File ash/ash_strings.grd (right): https://codereview.chromium.org/11369042/diff/10001/ash/ash_strings.grd#newcode352 ash/ash_strings.grd:352: <message name="IDS_ASH_STATUS_TRAY_UNKNOWN_DISPLAY_NAME" desc="The name of the display used in ...
1 year, 5 months ago #7
Jun Mukai
https://codereview.chromium.org/11369042/diff/10001/ash/ash_strings.grd File ash/ash_strings.grd (right): https://codereview.chromium.org/11369042/diff/10001/ash/ash_strings.grd#newcode352 ash/ash_strings.grd:352: <message name="IDS_ASH_STATUS_TRAY_UNKNOWN_DISPLAY_NAME" desc="The name of the display used in ...
1 year, 5 months ago #8
Daniel Erat
lgtm https://codereview.chromium.org/11369042/diff/10001/ash/display/multi_display_manager.cc File ash/display/multi_display_manager.cc (right): https://codereview.chromium.org/11369042/diff/10001/ash/display/multi_display_manager.cc#newcode387 ash/display/multi_display_manager.cc:387: std::string MultiDisplayManager::GetDisplayNameFor( On 2012/11/02 18:04:35, Jun Mukai wrote: ...
1 year, 5 months ago #9
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mukai@chromium.org/11369042/5
1 year, 5 months ago #10
I haz the power (commit-bot)
1 year, 5 months ago #11
Change committed as 165775
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6