Move MdFocusRing to be a property of FocusManager.

Move MdFocusRing to be a property of FocusManager.

This allows greater reuse across different Views types, and compared
to alternatives like https://codereview.chromium.org/2383243002 it
reduces the number of views, layers, and focus change listeners
required.

BUG=649815

[Evan Stade, 2016-09-30 21:35:08]

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

[commit-bot: I haz the power, 2016-09-30 21:35:57]

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

[Evan Stade, 2016-09-30 21:38:21]

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

[Evan Stade, 2016-09-30 21:38:21]

wdyt of this approach? You can see the leading alternative in the CL that I
linked in the description --- that has the advantage of keeping all the code in
one special class instead of reaching into views::View and views::FocusManager,
but overall this patch feels more efficient.

[commit-bot: I haz the power, 2016-09-30 21:47:00]

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

[commit-bot: I haz the power, 2016-09-30 21:47:01]

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

[sky, 2016-10-03 16:04:55]

It's not uncommon for views to override layout and expect a specific number of
children, or do processing when a view is added/removed, or the LayoutManager to
want to interact with a newly added view. We shouldn't add child Views to other
Views like this. It breaks too many assumptions.

[Evan Stade, 2016-10-03 19:48:38]

On 2016/10/03 16:04:55, sky wrote:
> It's not uncommon for views to override layout and expect a specific number of
> children, or do processing when a view is added/removed, or the LayoutManager
to
> want to interact with a newly added view. We shouldn't add child Views to
other
> Views like this. It breaks too many assumptions.

yea, that's a good point. But it only happens for the few views that
specifically opt into this.

Did you prefer https://codereview.chromium.org/2383243002 or have another
suggestion for how to accomplish this?

[sky, 2016-10-03 21:38:01]

On 2016/10/03 19:48:38, Evan Stade wrote:
> On 2016/10/03 16:04:55, sky wrote:
> > It's not uncommon for views to override layout and expect a specific number
of
> > children, or do processing when a view is added/removed, or the
LayoutManager
> to
> > want to interact with a newly added view. We shouldn't add child Views to
> other
> > Views like this. It breaks too many assumptions.
> 
> yea, that's a good point. But it only happens for the few views that
> specifically opt into this.

I missed the should_use_md_focus_ring(). I would prefer not to add more
functions like that to views, it doesn't scale well. Same comment about baking
functionality into the focusmanager.

I'm ok with the other patch, or alternatively making subclasses explicitly
override OnFocus/OnBlur and create/destroy the focus ring. That pattern forces
users to understand what is happening and doesn't suffer the extra overhead you
mention.

> 
> Did you prefer https://codereview.chromium.org/2383243002 or have another
> suggestion for how to accomplish this?