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

Issue 2889443002: mash: Add type names to PropertyConverter::Register*Property. (Closed)

Created:
3 years, 7 months ago by msw
Modified:
3 years, 7 months ago
CC:
chromium-reviews, kalyank, sadrul, tfarina, James Cook, Elliot Glaysher
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

mash: Add type names to PropertyConverter::Register*Property. Came up in https://codereview.chromium.org/2878133002/ (calling the wrong function ate up 1hr+ debugging) (this cl also depends on that one) BUG=NONE TEST=NONE R=sky@chromium.org NOTRY=true Review-Url: https://codereview.chromium.org/2889443002 Cr-Commit-Position: refs/heads/master@{#472239} Committed: https://chromium.googlesource.com/chromium/src/+/e6975c7828e90d6d5e60f1dd0ead735772f06386

Patch Set 1 #

Total comments: 3

Patch Set 2 : Alphabetize calls by function name. #

Total comments: 2

Patch Set 3 : Rename to RegisterImageSkiaProperty, update test identifiers. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+90 lines, -88 lines) Patch
M ash/mus/window_manager.cc View 1 1 chunk +9 lines, -9 lines 0 comments Download
M chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc View 1 1 chunk +7 lines, -7 lines 0 comments Download
M ui/aura/mus/property_converter.h View 1 2 2 chunks +12 lines, -13 lines 0 comments Download
M ui/aura/mus/property_converter.cc View 1 2 2 chunks +30 lines, -28 lines 0 comments Download
M ui/aura/mus/property_converter_unittest.cc View 1 2 9 chunks +28 lines, -27 lines 0 comments Download
M ui/aura/mus/window_tree_client_unittest.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M ui/views/mus/mus_client.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 29 (17 generated)
msw
Hey Scott, please take a look; thanks! +CC James and Elliot for FYI.
3 years, 7 months ago (2017-05-15 22:43:12 UTC) #2
Elliot Glaysher
lgtm feel free to ignore this suggestion: https://codereview.chromium.org/2889443002/diff/1/ui/aura/mus/property_converter.cc File ui/aura/mus/property_converter.cc (right): https://codereview.chromium.org/2889443002/diff/1/ui/aura/mus/property_converter.cc#newcode71 ui/aura/mus/property_converter.cc:71: CreateAcceptAnyValueCallback()); Now ...
3 years, 7 months ago (2017-05-15 22:46:05 UTC) #4
msw
https://codereview.chromium.org/2889443002/diff/1/ui/aura/mus/property_converter.cc File ui/aura/mus/property_converter.cc (right): https://codereview.chromium.org/2889443002/diff/1/ui/aura/mus/property_converter.cc#newcode71 ui/aura/mus/property_converter.cc:71: CreateAcceptAnyValueCallback()); On 2017/05/15 22:46:04, Elliot Glaysher wrote: > Now ...
3 years, 7 months ago (2017-05-15 22:52:54 UTC) #5
msw
Done! Please take a look, Scott. https://codereview.chromium.org/2889443002/diff/1/ui/aura/mus/property_converter.cc File ui/aura/mus/property_converter.cc (right): https://codereview.chromium.org/2889443002/diff/1/ui/aura/mus/property_converter.cc#newcode71 ui/aura/mus/property_converter.cc:71: CreateAcceptAnyValueCallback()); On 2017/05/15 ...
3 years, 7 months ago (2017-05-15 23:03:59 UTC) #7
James Cook
lgtm
3 years, 7 months ago (2017-05-16 01:02:27 UTC) #11
sky
LGTM https://codereview.chromium.org/2889443002/diff/10008/ui/aura/mus/property_converter.h File ui/aura/mus/property_converter.h (right): https://codereview.chromium.org/2889443002/diff/10008/ui/aura/mus/property_converter.h#newcode97 ui/aura/mus/property_converter.h:97: void RegisterImageProperty(const WindowProperty<gfx::ImageSkia*>* property, We also have a ...
3 years, 7 months ago (2017-05-16 12:21:34 UTC) #14
msw
Done; landing. https://codereview.chromium.org/2889443002/diff/10008/ui/aura/mus/property_converter.h File ui/aura/mus/property_converter.h (right): https://codereview.chromium.org/2889443002/diff/10008/ui/aura/mus/property_converter.h#newcode97 ui/aura/mus/property_converter.h:97: void RegisterImageProperty(const WindowProperty<gfx::ImageSkia*>* property, On 2017/05/16 12:21:34, ...
3 years, 7 months ago (2017-05-16 16:10:00 UTC) #16
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/2889443002/90001
3 years, 7 months ago (2017-05-16 16:11:00 UTC) #20
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/446157)
3 years, 7 months ago (2017-05-16 22:38:36 UTC) #22
msw
Landing with NOTRY=true, all the try bots actually passed except for 3 PrefHashBrowserTest* fixtures in ...
3 years, 7 months ago (2017-05-16 22:48:24 UTC) #24
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/2889443002/90001
3 years, 7 months ago (2017-05-16 22:50:45 UTC) #26
commit-bot: I haz the power
3 years, 7 months ago (2017-05-16 23:07:26 UTC) #29
Message was sent while issue was closed.
Committed patchset #3 (id:90001) as
https://chromium.googlesource.com/chromium/src/+/e6975c7828e90d6d5e60f1dd0ead...

Powered by Google App Engine
This is Rietveld 408576698