Refactor MdFocusRing to make it easier to reuse on other controls.

Refactor MdFocusRing to make it easier to reuse on other controls.

BUG=649815
Committed: https://crrev.com/7758d0c238aefe508c4052d589b7418ab8e158e2
Cr-Commit-Position: refs/heads/master@{#422921}

[Evan Stade, 2016-10-03 23:33:38]

estade@chromium.org changed reviewers:
+ sky@chromium.org

[Evan Stade, 2016-10-03 23:33:38]

changed to just overriding onfocus/onblur everywhere since I'm worried we'll
have many (usually hidden) buttons and other controls instantiated and don't
want them all listening to every focus change.

[Evan Stade, 2016-10-03 23:34:13]

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

[commit-bot: I haz the power, 2016-10-03 23:34:54]

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

[sky, 2016-10-03 23:53:16]

https://codereview.chromium.org/2383243002/diff/60001/ui/views/controls/combo...
File ui/views/controls/combobox/combobox.cc (right):

https://codereview.chromium.org/2383243002/diff/60001/ui/views/controls/combo...
ui/views/controls/combobox/combobox.cc:734: if (UseMd())
Is there a reason not to delegate focus_ring_ here?

https://codereview.chromium.org/2383243002/diff/60001/ui/views/controls/focus...
File ui/views/controls/focus_ring.cc (right):

https://codereview.chromium.org/2383243002/diff/60001/ui/views/controls/focus...
ui/views/controls/focus_ring.cc:26: FocusRing::FocusRing() {
WDYT of making this take the View to add to?

https://codereview.chromium.org/2383243002/diff/60001/ui/views/controls/focus...
File ui/views/controls/focus_ring.h (right):

https://codereview.chromium.org/2383243002/diff/60001/ui/views/controls/focus...
ui/views/controls/focus_ring.h:4: 
Don't forget header guards.

https://codereview.chromium.org/2383243002/diff/60001/ui/views/controls/textf...
File ui/views/controls/textfield/textfield.cc (right):

https://codereview.chromium.org/2383243002/diff/60001/ui/views/controls/textf...
ui/views/controls/textfield/textfield.cc:1019: focus_ring_->SetVisible(false);
Similar comment about deleting here.

[Evan Stade, 2016-10-04 01:04:55]

https://codereview.chromium.org/2383243002/diff/60001/ui/views/controls/combo...
File ui/views/controls/combobox/combobox.cc (right):

https://codereview.chromium.org/2383243002/diff/60001/ui/views/controls/combo...
ui/views/controls/combobox/combobox.cc:734: if (UseMd())
On 2016/10/03 23:53:15, sky wrote:
> Is there a reason not to delegate focus_ring_ here?

No reason.

Made a couple of statics to create and add/remove and destroy rings. (This also
addresses most of your other comments.)

https://codereview.chromium.org/2383243002/diff/60001/ui/views/controls/focus...
File ui/views/controls/focus_ring.h (right):

https://codereview.chromium.org/2383243002/diff/60001/ui/views/controls/focus...
ui/views/controls/focus_ring.h:4: 
On 2016/10/03 23:53:15, sky wrote:
> Don't forget header guards.

Done.

[sky, 2016-10-04 02:18:16]

Nice cleanup! LGTM

[Evan Stade, 2016-10-04 18:44:31]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2016-10-04 18:44:47]

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-10-04 20:50:01]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-10-04 20:53:22]

Description was changed from

==========
Refactor MdFocusRing to make it easier to reuse on other controls.

BUG=649815
==========

to

==========
Refactor MdFocusRing to make it easier to reuse on other controls.

BUG=649815

Committed: https://crrev.com/7758d0c238aefe508c4052d589b7418ab8e158e2
Cr-Commit-Position: refs/heads/master@{#422921}
==========

[commit-bot: I haz the power, 2016-10-04 20:53:23]

Patchset 5 (id:??) landed as
https://crrev.com/7758d0c238aefe508c4052d589b7418ab8e158e2
Cr-Commit-Position: refs/heads/master@{#422921}