Refactor and push window properties up to class properties.

Refactor and push window properties up to class properties.

This is a precondition for being able to use properties on view classes
and not just Aura classes.

Review-Url: https://codereview.chromium.org/2632543003
Cr-Commit-Position: refs/heads/master@{#447039}
Committed: https://chromium.googlesource.com/chromium/src/+/61e45cfa8c57280973848e75d31eae78c27a3e62

[kylix_rd, 2017-01-13 17:20:23]

Description was changed from

==========
Refactor and push window properties up to class properties.

This is a precondition for being able to use properties on view classes
and not just Aura classes.
==========

to

==========
Refactor and push window properties up to class properties.

This is a precondition for being able to use properties on view classes
and not just Aura classes.
==========

[kylix_rd, 2017-01-13 17:20:23]

kylixrd@chromium.org changed reviewers:
+ robliao@chromium.org, sky@chromium.org

[kylix_rd, 2017-01-13 17:21:20]

kylixrd@chromium.org changed reviewers:
+ pkasting@chromium.org

[kylix_rd, 2017-01-13 17:24:34]

This is the preparatory phase for moving Aura window properties into ui/base for
use in both ui/aura and ui/views.

[sky, 2017-01-13 20:28:21]

I only looked at ui/base. Could you also promote tests?

https://codereview.chromium.org/2632543003/diff/1/ui/aura/window.h
File ui/aura/window.h (right):

https://codereview.chromium.org/2632543003/diff/1/ui/aura/window.h#newcode301
ui/aura/window.h:301: //void SetProperty(const WindowProperty<T>* property, T
value);
Is there a reason you are commenting this out and not removing them? In general
we don't comment out code like this.

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h
File ui/base/class_property.h (right):

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h#ne...
ui/base/class_property.h:1: // Copyright (c) 2017 The Chromium Authors. All
rights reserved.
we don't use (c) in new copyrights.

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h#ne...
ui/base/class_property.h:54: typedef void (*PropertyDeallocator)(int64_t value);
using?

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h#ne...
ui/base/class_property.h:185: #define
DECLARE_EXPORTED_CLASS_PROPERTY_TYPE(EXPORT, T)                      \
I tend to feel _CLASS_ is too generic here. How about UI_CLASS ?

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h#ne...
ui/base/class_property.h:189: const ClassProperty<T>* property, T value) {      
                    \
optional: It sure would be nice if there was a way to generate a setter that
takes a unique_ptr for owned properties. I get that doing that at the same time
as the move is likely complex and I don't want to derail the move, so don't feel
you need to do that now.

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h#ne...
ui/base/class_property.h:192: template <>                                       
                        \
I'm assuming this code is, a copy of that from window_property and not reviewing
it any detail.

https://codereview.chromium.org/2632543003/diff/1/ui/base/property_data.h
File ui/base/property_data.h (right):

https://codereview.chromium.org/2632543003/diff/1/ui/base/property_data.h#new...
ui/base/property_data.h:19: 
remove empty line.

[kylix_rd, 2017-01-13 21:44:59]

I waited on the tests in order to ensure I'm heading in the right direction.

https://codereview.chromium.org/2632543003/diff/1/ui/aura/window.h
File ui/aura/window.h (right):

https://codereview.chromium.org/2632543003/diff/1/ui/aura/window.h#newcode301
ui/aura/window.h:301: //void SetProperty(const WindowProperty<T>* property, T
value);
On 2017/01/13 20:28:20, sky wrote:
> Is there a reason you are commenting this out and not removing them? In
general
> we don't comment out code like this.

I'll do a cleanup pass once the substance of these changes have been vetted.

This review is kind of a checkpoint right now to make sure I've not taken a
left-turn and it is what we'd discussed and expected.

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h
File ui/base/class_property.h (right):

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h#ne...
ui/base/class_property.h:185: #define
DECLARE_EXPORTED_CLASS_PROPERTY_TYPE(EXPORT, T)                      \
On 2017/01/13 20:28:20, sky wrote:
> I tend to feel _CLASS_ is too generic here. How about UI_CLASS ?

I agree. UI_CLASS will work.

I do want to point out that technically ui::PropertyHandler can be inherited by
any class in order to gain property support. If that is attractive, then all
this could move to base/ instead of ui/base.

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h#ne...
ui/base/class_property.h:189: const ClassProperty<T>* property, T value) {      
                    \
On 2017/01/13 20:28:20, sky wrote:
> optional: It sure would be nice if there was a way to generate a setter that
> takes a unique_ptr for owned properties. I get that doing that at the same
time
> as the move is likely complex and I don't want to derail the move, so don't
feel
> you need to do that now.

Good idea... I'll mull this over for a  later pass.

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h#ne...
ui/base/class_property.h:192: template <>                                       
                        \
On 2017/01/13 20:28:20, sky wrote:
> I'm assuming this code is, a copy of that from window_property and not
reviewing
> it any detail.

Yes, I moved window_property to class_property.

[sky, 2017-01-13 21:51:58]

I think you're heading in the right direction. The only open question is putting
in base rather than ui/base.

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h
File ui/base/class_property.h (right):

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h#ne...
ui/base/class_property.h:185: #define
DECLARE_EXPORTED_CLASS_PROPERTY_TYPE(EXPORT, T)                      \
On 2017/01/13 21:44:59, kylix_rd wrote:
> On 2017/01/13 20:28:20, sky wrote:
> > I tend to feel _CLASS_ is too generic here. How about UI_CLASS ?
> 
> I agree. UI_CLASS will work.
> 
> I do want to point out that technically ui::PropertyHandler can be inherited
by
> any class in order to gain property support. If that is attractive, then all
> this could move to base/ instead of ui/base.

I suggested the prefix based on the current location. Moving to base is
certainly a possibility. To do that you would have to get base owners on board,
and no doubt part of that would entail signing up to replace SupportsUserData
with this. It's up to you which direction you go.

[Peter Kasting, 2017-01-17 20:43:39]

On 2017/01/13 21:51:58, sky wrote:
> I think you're heading in the right direction.

I defer to Scott on this.  I consider myself more of a detail reviewer than a
big-picture views-architecture OWNER.

[kylix_rd, 2017-01-24 15:14:53]

kylixrd@chromium.org changed reviewers:
- pkasting@chromium.org

[kylix_rd, 2017-01-24 15:14:54]

This patch includes unit tests and addresses all the suggestions and comments.

Some files missing from the previous patch have been added. There is one file
"components/exo/surface.cc" in which the owner in the OWNERS files doesn't seem
to exist in chromium.org.

https://codereview.chromium.org/2632543003/diff/1/ui/aura/window.h
File ui/aura/window.h (right):

https://codereview.chromium.org/2632543003/diff/1/ui/aura/window.h#newcode301
ui/aura/window.h:301: //void SetProperty(const WindowProperty<T>* property, T
value);
On 2017/01/13 20:28:20, sky wrote:
> Is there a reason you are commenting this out and not removing them? In
general
> we don't comment out code like this.

Done.

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h
File ui/base/class_property.h (right):

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h#ne...
ui/base/class_property.h:1: // Copyright (c) 2017 The Chromium Authors. All
rights reserved.
On 2017/01/13 20:28:20, sky wrote:
> we don't use (c) in new copyrights.

Done.

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h#ne...
ui/base/class_property.h:54: typedef void (*PropertyDeallocator)(int64_t value);
On 2017/01/13 20:28:20, sky wrote:
> using?

Done.

https://codereview.chromium.org/2632543003/diff/1/ui/base/class_property.h#ne...
ui/base/class_property.h:91: std::set<const void*> GetAllPropertKeys() const;
I presume this function name is a typo... GetAll*Propert*Keys() should be
GetAllPropertyKeys(). This typo was preserved since this file was copied from
window_property.h.

I've fixed this and will be in the next patch.

[sky, 2017-01-24 18:45:38]

You can blame me for the misspelling of GetAllPropertyKeys.

As you'll need an owner of exo you may as well update the OWNERS file there too.

https://codereview.chromium.org/2632543003/diff/30001/ui/aura/client/window_p...
File ui/aura/client/window_parenting_client.cc (right):

https://codereview.chromium.org/2632543003/diff/30001/ui/aura/client/window_p...
ui/aura/client/window_parenting_client.cc:10: #include
"ui/base/class_property.h"
Given that you changed window.h to include class_property.h, it seems if a file
includes window then it doesn't need to include class_property.

https://codereview.chromium.org/2632543003/diff/30001/ui/aura/window.cc
File ui/aura/window.cc (right):

https://codereview.chromium.org/2632543003/diff/30001/ui/aura/window.cc#newco...
ui/aura/window.cc:126: // Clear properties.
Remove comment (it's obvious given the function you are calling).

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property.cc
File ui/base/class_property.cc (right):

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property....
ui/base/class_property.cc:12: PropertyHandler::PropertyHandler() : prop_map_()
{}
Generally we only use member initializer for members that need it.

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property.h
File ui/base/class_property.h (right):

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property....
ui/base/class_property.h:14: #include "ui/base/property_data.h"
Can you forward declare PropertyData?

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property_...
File ui/base/class_property_unittest.cc (right):

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property_...
ui/base/class_property_unittest.cc:30: static TestProperty* last_deleted() {
return last_deleted_; }
Returning a pointer to an object that was deleted seems to encourage mis-use. As
the value is only used for comparison could you return a void* to make it clear
consumers shouldn't try to use the returned value? Alternative assign an ever
increasing int value to each TestProperty and compare that.

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property_...
ui/base/class_property_unittest.cc:48: class PropertyTest : public testing::Test
{
To write gtests you don't have to subclass Test. Only subclass Test if you need
to do interesting Setup or to share code. As you aren't doing either you
shouldn't need the subclass. Instead use TEST(PropertyTest, ...).

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property_...
ui/base/class_property_unittest.cc:62: std::unique_ptr<PropertyHandler> h =
base::MakeUnique<PropertyHandler>();
As you never explicitly delete 'h'. It seems like you could define
PropertyHandler on the stack here and not use the unique_ptr.

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property_...
ui/base/class_property_unittest.cc:65: EXPECT_EQ(-2, h->GetProperty(kIntKey));
These assertions would be less mysterious if you used constants for the default
values.

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property_...
ui/base/class_property_unittest.cc:77: h->SetProperty(kStringKey,
static_cast<const char*>(NULL));
NULL -> nullptr. Maybe that means you don't need the static_cast either?

https://codereview.chromium.org/2632543003/diff/30001/ui/base/property_data.h
File ui/base/property_data.h (right):

https://codereview.chromium.org/2632543003/diff/30001/ui/base/property_data.h...
ui/base/property_data.h:12: struct UI_BASE_EXPORT PropertyData {
Document this class.

[kylix_rd, 2017-01-24 21:19:59]

kylixrd@chromium.org changed reviewers:
+ reveman@chromium.org

[kylix_rd, 2017-01-24 21:20:00]

There are a couple of places which have wholesale copied the content and
behavior of ui/aura/window_property.h.

components/exo/surface_property.h
ash/common/wm_window_property.h

Both of these seem to already have +ui in their DEPS, so I propose that these
also refactored to use the ui/base/class_property.h.

Added reveman@ to review the current and proposed components/exo changes.

https://codereview.chromium.org/2632543003/diff/30001/ui/aura/client/window_p...
File ui/aura/client/window_parenting_client.cc (right):

https://codereview.chromium.org/2632543003/diff/30001/ui/aura/client/window_p...
ui/aura/client/window_parenting_client.cc:10: #include
"ui/base/class_property.h"
On 2017/01/24 18:45:37, sky wrote:
> Given that you changed window.h to include class_property.h, it seems if a
file
> includes window then it doesn't need to include class_property.

I was under the impression the idea was that if you referenced something from a
given header, it should be explicitly referenced. Relying on second order
includes seems a little dangerous.

https://codereview.chromium.org/2632543003/diff/30001/ui/aura/window.cc
File ui/aura/window.cc (right):

https://codereview.chromium.org/2632543003/diff/30001/ui/aura/window.cc#newco...
ui/aura/window.cc:126: // Clear properties.
On 2017/01/24 18:45:37, sky wrote:
> Remove comment (it's obvious given the function you are calling).

Done.

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property.cc
File ui/base/class_property.cc (right):

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property....
ui/base/class_property.cc:12: PropertyHandler::PropertyHandler() : prop_map_()
{}
On 2017/01/24 18:45:38, sky wrote:
> Generally we only use member initializer for members that need it.

Done.

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property.h
File ui/base/class_property.h (right):

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property....
ui/base/class_property.h:14: #include "ui/base/property_data.h"
On 2017/01/24 18:45:38, sky wrote:
> Can you forward declare PropertyData?

Not really. The complete definition is needed for the std::unique_ptr<>.

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property_...
File ui/base/class_property_unittest.cc (right):

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property_...
ui/base/class_property_unittest.cc:30: static TestProperty* last_deleted() {
return last_deleted_; }
On 2017/01/24 18:45:38, sky wrote:
> Returning a pointer to an object that was deleted seems to encourage mis-use.
As
> the value is only used for comparison could you return a void* to make it
clear
> consumers shouldn't try to use the returned value? Alternative assign an ever
> increasing int value to each TestProperty and compare that.

These tests were lifted, nearly verbatim from window_unittest.cc. Some cleanup
of the tests over there is probably also warranted.

Given that, I agree that these tests would probably be written a little
differently if done more recently. I'll work on that.

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property_...
ui/base/class_property_unittest.cc:48: class PropertyTest : public testing::Test
{
On 2017/01/24 18:45:38, sky wrote:
> To write gtests you don't have to subclass Test. Only subclass Test if you
need
> to do interesting Setup or to share code. As you aren't doing either you
> shouldn't need the subclass. Instead use TEST(PropertyTest, ...).

Done.

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property_...
ui/base/class_property_unittest.cc:62: std::unique_ptr<PropertyHandler> h =
base::MakeUnique<PropertyHandler>();
On 2017/01/24 18:45:38, sky wrote:
> As you never explicitly delete 'h'. It seems like you could define
> PropertyHandler on the stack here and not use the unique_ptr.

Done.

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property_...
ui/base/class_property_unittest.cc:65: EXPECT_EQ(-2, h->GetProperty(kIntKey));
On 2017/01/24 18:45:38, sky wrote:
> These assertions would be less mysterious if you used constants for the
default
> values.

Done.

https://codereview.chromium.org/2632543003/diff/30001/ui/base/class_property_...
ui/base/class_property_unittest.cc:77: h->SetProperty(kStringKey,
static_cast<const char*>(NULL));
On 2017/01/24 18:45:38, sky wrote:
> NULL -> nullptr. Maybe that means you don't need the static_cast either?

NULL->nullptr - Done.

Either the static_cast is needed or the template method must be instantiated
with the explicit type.

https://codereview.chromium.org/2632543003/diff/30001/ui/base/property_data.h
File ui/base/property_data.h (right):

https://codereview.chromium.org/2632543003/diff/30001/ui/base/property_data.h...
ui/base/property_data.h:12: struct UI_BASE_EXPORT PropertyData {
On 2017/01/24 18:45:38, sky wrote:
> Document this class.

Done.

[kylix_rd, 2017-01-24 21:35:21]

On 2017/01/24 21:20:00, kylix_rd wrote:
> ash/common/wm_window_property.h

Oops. Wrong file.

[sky, 2017-01-24 22:36:13]

LGTM

https://codereview.chromium.org/2632543003/diff/30001/ui/aura/client/window_p...
File ui/aura/client/window_parenting_client.cc (right):

https://codereview.chromium.org/2632543003/diff/30001/ui/aura/client/window_p...
ui/aura/client/window_parenting_client.cc:10: #include
"ui/base/class_property.h"
On 2017/01/24 21:20:00, kylix_rd wrote:
> On 2017/01/24 18:45:37, sky wrote:
> > Given that you changed window.h to include class_property.h, it seems if a
> file
> > includes window then it doesn't need to include class_property.
> 
> I was under the impression the idea was that if you referenced something from
a
> given header, it should be explicitly referenced. Relying on second order
> includes seems a little dangerous.

Agreed. I think this case is a bit of a gray area because class_property is a
superclass.

https://codereview.chromium.org/2632543003/diff/50001/ui/base/property_data.h
File ui/base/property_data.h (right):

https://codereview.chromium.org/2632543003/diff/50001/ui/base/property_data.h...
ui/base/property_data.h:13: // when overriding BeforePropertyChange. This
instance is then passed to
Generally we use () in documentation when referring to function calls, e.g.
BeforePropertyChange().

[reveman, 2017-01-27 18:17:49]

components/exo lgtm

[kylix_rd, 2017-01-27 18:27:15]

The CQ bit was checked by kylixrd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-27 18:27:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-27 18:30:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-27 18:30:31]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[kylix_rd, 2017-01-27 21:09:20]

The CQ bit was checked by kylixrd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-27 21:10:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-27 21:26:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-27 21:26:40]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[kylix_rd, 2017-01-27 21:35:01]

The CQ bit was checked by kylixrd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-27 21:35:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-27 21:54:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-27 21:54:35]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[kylix_rd, 2017-01-30 16:52:12]

The CQ bit was checked by kylixrd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-30 16:52:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-30 17:06:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-30 17:06:50]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[kylix_rd, 2017-01-30 17:31:25]

Patchset #7 (id:110001) has been deleted

[kylix_rd, 2017-01-30 17:32:50]

The CQ bit was checked by kylixrd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-30 17:33:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-30 17:52:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-30 17:52:27]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[kylix_rd, 2017-01-30 18:22:50]

Patchset #7 (id:130001) has been deleted

[kylix_rd, 2017-01-30 18:24:45]

The CQ bit was checked by kylixrd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-30 18:25:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-30 19:27:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-30 19:27:20]

Dry run: This issue passed the CQ dry run.

[kylix_rd, 2017-01-30 19:44:19]

The CQ bit was checked by kylixrd@chromium.org

[kylix_rd, 2017-01-30 19:44:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
reveman@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2632543003/#ps150001
(title: "More build fixes")

[commit-bot: I haz the power, 2017-01-30 19:44:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-30 19:52:58]

CQ is committing da patch.

Bot data: {"patchset_id": 150001, "attempt_start_ts": 1485805459576620,
"parent_rev": "20284591559eced2cc3ec539b580cfdca461447a", "commit_rev":
"61e45cfa8c57280973848e75d31eae78c27a3e62"}

[commit-bot: I haz the power, 2017-01-30 19:53:32]

Description was changed from

==========
Refactor and push window properties up to class properties.

This is a precondition for being able to use properties on view classes
and not just Aura classes.
==========

to

==========
Refactor and push window properties up to class properties.

This is a precondition for being able to use properties on view classes
and not just Aura classes.

Review-Url: https://codereview.chromium.org/2632543003
Cr-Commit-Position: refs/heads/master@{#447039}
Committed:
https://chromium.googlesource.com/chromium/src/+/61e45cfa8c57280973848e75d31e...
==========

[commit-bot: I haz the power, 2017-01-30 19:53:34]

Committed patchset #7 (id:150001) as
https://chromium.googlesource.com/chromium/src/+/61e45cfa8c57280973848e75d31e...

[sky, 2017-02-01 00:54:03]

https://codereview.chromium.org/2632543003/diff/150001/ui/base/class_property.h
File ui/base/class_property.h (right):

https://codereview.chromium.org/2632543003/diff/150001/ui/base/class_property...
ui/base/class_property.h:91: std::set<const void*> GetAllPropertyKeys() const;
Thanks for fixing the spelling here!

[kylix_rd, 2017-02-01 18:43:14]

https://codereview.chromium.org/2632543003/diff/150001/ui/base/class_property.h
File ui/base/class_property.h (right):

https://codereview.chromium.org/2632543003/diff/150001/ui/base/class_property...
ui/base/class_property.h:91: std::set<const void*> GetAllPropertyKeys() const;
On 2017/02/01 00:54:02, sky wrote:
> Thanks for fixing the spelling here!

Not a problem. It happens.