Material Page Info (Views, 1/3): Rename {ChosenObject, PermissionSelector}View classes to {...}Row.

Material Page Info (Views, 1/3): Rename {ChosenObject, PermissionSelector}View classes to {...}Row.

In the near future, these will not be Views of their own, but classes that
manage individual rows in a GridLayout.

Also adds a missing build file entry for `chosen_object_row_observer.h`.

BUG=512442
Committed: https://crrev.com/8f0f6cbe0c26ac1f11509984db71e8e344b48baa
Cr-Commit-Position: refs/heads/master@{#416874}

[lgarron, 2016-08-27 02:14:08]

Description was changed from

==========
Rename {ChosenObject, PermissionSelector}View classes to  {ChosenObject,
PermissionSelector}Row.

In the near future, these will not be Views of their own, but classes that
manage individual rows in a GridLayout.

Also adds a missing build file entry for `chosen_object_row_observer.h`.

BUG=512442
==========

to

==========
Material Page Info (Views, 1/3): Rename {ChosenObject, PermissionSelector}View
classes to ...Row

In the near future, these will not be Views of their own, but classes that
manage individual rows in a GridLayout.

Also adds a missing build file entry for `chosen_object_row_observer.h`.

BUG=512442
==========

[lgarron, 2016-08-31 20:29:01]

lgarron@chromium.org changed reviewers:
+ felt@chromium.org

[lgarron, 2016-08-31 20:29:01]

felt@, could you review?

I don't think this is significant enough to require a Views review, but CCing
msw@ since I discussed this with him (and this change will affect an upcoming
CL).

[lgarron, 2016-08-31 20:30:38]

Description was changed from

==========
Material Page Info (Views, 1/3): Rename {ChosenObject, PermissionSelector}View
classes to ...Row

In the near future, these will not be Views of their own, but classes that
manage individual rows in a GridLayout.

Also adds a missing build file entry for `chosen_object_row_observer.h`.

BUG=512442
==========

to

==========
Material Page Info (Views, 1/3): Rename {ChosenObject, PermissionSelector}View
classes to {...}Row

In the near future, these will not be Views of their own, but classes that
manage individual rows in a GridLayout.

Also adds a missing build file entry for `chosen_object_row_observer.h`.

BUG=512442
==========

[msw, 2016-09-01 01:42:07]

msw@chromium.org changed reviewers:
+ msw@chromium.org

[msw, 2016-09-01 01:42:08]

It might make more sense to rename after your View->not-a-view change goes
through. Perhaps there's a better word than 'row', like 'item'? but maybe
'ChosenObjectItem' is a bit vague...

https://codereview.chromium.org/2272793007/diff/1/chrome/browser/ui/views/web...
File chrome/browser/ui/views/website_settings/chosen_object_row.cc (right):

https://codereview.chromium.org/2272793007/diff/1/chrome/browser/ui/views/web...
chrome/browser/ui/views/website_settings/chosen_object_row.cc:70: const
ui::Event& event) {
nit: git cl format

https://codereview.chromium.org/2272793007/diff/1/chrome/browser/ui/views/web...
File chrome/browser/ui/views/website_settings/permission_selector_row.h (right):

https://codereview.chromium.org/2272793007/diff/1/chrome/browser/ui/views/web...
chrome/browser/ui/views/website_settings/permission_selector_row.h:36: //
control whether tha access is granted.
nit: s/tha/that/

https://codereview.chromium.org/2272793007/diff/1/chrome/browser/ui/views/web...
chrome/browser/ui/views/website_settings/permission_selector_row.h:40: const
WebsiteSettingsUI::PermissionInfo& permission);
nit: git cl format

https://codereview.chromium.org/2272793007/diff/1/chrome/chrome_browser_ui.gypi
File chrome/chrome_browser_ui.gypi (right):

https://codereview.chromium.org/2272793007/diff/1/chrome/chrome_browser_ui.gy...
chrome/chrome_browser_ui.gypi:499:
'browser/ui/webui/offline/offline_internals_ui.cc',
Looks like a tangential/unintentional change or rebase issue?

[msw, 2016-09-01 01:43:50]

On 2016/09/01 01:42:08, msw wrote:
> It might make more sense to rename after your View->not-a-view change goes
> through. Perhaps there's a better word than 'row', like 'item'? but maybe
> 'ChosenObjectItem' is a bit vague...

To clarify, 'row' is fine, especially since you added good class comments.

[lgarron, 2016-09-02 02:56:49]

msw@, could you take another look?

Yeah, I thought a bit about the name. "Row" is the best I could think of, and it
should stay accurate unless the classes get a major refactor.

I'd like to land this change before the View->not-a-view implementation lands,
since the pure rename doesn't have complicated interactions with the other
commits, and landing it will let me juggle one fewer commit.
This way, the class name also won't have a brief period of inaccuracy. ;-)

https://codereview.chromium.org/2272793007/diff/1/chrome/browser/ui/views/web...
File chrome/browser/ui/views/website_settings/chosen_object_row.cc (right):

https://codereview.chromium.org/2272793007/diff/1/chrome/browser/ui/views/web...
chrome/browser/ui/views/website_settings/chosen_object_row.cc:70: const
ui::Event& event) {
On 2016/09/01 at 01:42:07, msw wrote:
> nit: git cl format

Done.

https://codereview.chromium.org/2272793007/diff/1/chrome/browser/ui/views/web...
File chrome/browser/ui/views/website_settings/permission_selector_row.h (right):

https://codereview.chromium.org/2272793007/diff/1/chrome/browser/ui/views/web...
chrome/browser/ui/views/website_settings/permission_selector_row.h:36: //
control whether tha access is granted.
On 2016/09/01 at 01:42:08, msw wrote:
> nit: s/tha/that/

Done.

https://codereview.chromium.org/2272793007/diff/1/chrome/browser/ui/views/web...
chrome/browser/ui/views/website_settings/permission_selector_row.h:40: const
WebsiteSettingsUI::PermissionInfo& permission);
On 2016/09/01 at 01:42:08, msw wrote:
> nit: git cl format

Done.

https://codereview.chromium.org/2272793007/diff/1/chrome/chrome_browser_ui.gypi
File chrome/chrome_browser_ui.gypi (right):

https://codereview.chromium.org/2272793007/diff/1/chrome/chrome_browser_ui.gy...
chrome/chrome_browser_ui.gypi:499:
'browser/ui/webui/offline/offline_internals_ui.cc',
On 2016/09/01 at 01:42:08, msw wrote:
> Looks like a tangential/unintentional change or rebase issue?

Indeed, a rebase issue. [1]
Fixed in the latest patch.

[1] https://twitter.com/lgarron/status/771092242165399552

[lgarron, 2016-09-02 03:23:04]

Description was changed from

==========
Material Page Info (Views, 1/3): Rename {ChosenObject, PermissionSelector}View
classes to {...}Row

In the near future, these will not be Views of their own, but classes that
manage individual rows in a GridLayout.

Also adds a missing build file entry for `chosen_object_row_observer.h`.

BUG=512442
==========

to

==========
Material Page Info (Views, 1/3): Rename {ChosenObject, PermissionSelector}View
classes to {...}Row.

In the near future, these will not be Views of their own, but classes that
manage individual rows in a GridLayout.

Also adds a missing build file entry for `chosen_object_row_observer.h`.

BUG=512442
==========

[lgarron, 2016-09-02 23:28:05]

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

[commit-bot: I haz the power, 2016-09-02 23:28: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, 2016-09-02 23:39:08]

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

[commit-bot: I haz the power, 2016-09-02 23:39:09]

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

[msw, 2016-09-06 19:04:08]

lgtm

[felt, 2016-09-06 19:56:00]

It isn't compiling because of the chosen_object_view.cc name change, can you
please fix?

[lgarron, 2016-09-06 20:01:33]

On 2016/09/06 at 19:56:00, felt wrote:
> It isn't compiling because of the chosen_object_view.cc name change, can you
please fix?

Yeah, I'll rebase and fix.

[felt, 2016-09-06 20:03:53]

lgtm % compile error and tiny nits

https://codereview.chromium.org/2272793007/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/website_settings/permission_selector_row.cc
(right):

https://codereview.chromium.org/2272793007/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/website_settings/permission_selector_row.cc:359:
FOR_EACH_OBSERVER(PermissionSelectorRowObserver, observer_list_,
(is this change just a result of running git cl format?)

https://codereview.chromium.org/2272793007/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/website_settings/permission_selector_row.h (right):

https://codereview.chromium.org/2272793007/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/website_settings/permission_selector_row.h:64:
views::ImageView* icon_;                       // Owned by the views hierachy.
tiny nit: can you pls be consistent here with alignment? either go with two
spaces before the comment for all three, or line all three up

[lgarron, 2016-09-07 06:31:49]

Patchset #4 (id:60001) has been deleted

[lgarron, 2016-09-07 06:34:20]

Patchset #4 (id:80001) has been deleted

[lgarron, 2016-09-07 06:37:18]

The CQ bit was checked by lgarron@chromium.org

[lgarron, 2016-09-07 06:37:19]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org,
felt@chromium.org
Link to the patchset: https://codereview.chromium.org/2272793007/#ps100001
(title: "Fix rebase-induced formatting throughout permission_selector_row.")

[commit-bot: I haz the power, 2016-09-07 06:37:31]

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

[lgarron, 2016-09-07 06:55:26]

https://codereview.chromium.org/2272793007/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/website_settings/permission_selector_row.cc
(right):

https://codereview.chromium.org/2272793007/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/website_settings/permission_selector_row.cc:359:
FOR_EACH_OBSERVER(PermissionSelectorRowObserver, observer_list_,
On 2016/09/06 at 20:03:53, felt wrote:
> (is this change just a result of running git cl format?)

(Ditto.)

https://codereview.chromium.org/2272793007/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/website_settings/permission_selector_row.h (right):

https://codereview.chromium.org/2272793007/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/website_settings/permission_selector_row.h:64:
views::ImageView* icon_;                       // Owned by the views hierachy.
On 2016/09/06 at 20:03:53, felt wrote:
> tiny nit: can you pls be consistent here with alignment? either go with two
spaces before the comment for all three, or line all three up

It seems there were a bunch of rebased-induced formatting changes in just the
permission_selector_row.{h, cc} files. I've fixed them.

[commit-bot: I haz the power, 2016-09-07 07:45:54]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-09-07 07:48:03]

Description was changed from

==========
Material Page Info (Views, 1/3): Rename {ChosenObject, PermissionSelector}View
classes to {...}Row.

In the near future, these will not be Views of their own, but classes that
manage individual rows in a GridLayout.

Also adds a missing build file entry for `chosen_object_row_observer.h`.

BUG=512442
==========

to

==========
Material Page Info (Views, 1/3): Rename {ChosenObject, PermissionSelector}View
classes to {...}Row.

In the near future, these will not be Views of their own, but classes that
manage individual rows in a GridLayout.

Also adds a missing build file entry for `chosen_object_row_observer.h`.

BUG=512442

Committed: https://crrev.com/8f0f6cbe0c26ac1f11509984db71e8e344b48baa
Cr-Commit-Position: refs/heads/master@{#416874}
==========

[commit-bot: I haz the power, 2016-09-07 07:48:04]

Patchset 4 (id:??) landed as
https://crrev.com/8f0f6cbe0c26ac1f11509984db71e8e344b48baa
Cr-Commit-Position: refs/heads/master@{#416874}