Remove View::focusable().

Remove View::focusable().

This CL removes View::focusable(). Since Link is the only kind of focusable 
label, the GetInsets() override in Label is removed and moved to Link.

[karandeepb, 2016-04-27 07:21:18]

Description was changed from

==========
Remove View::focusable()
==========

to

==========
Remove View::focusable().

This CL removes View::focusable(). Since Link is the only kind of focusable 
label, the GetInsets() override in Label is removed and moved to Link.
==========

[karandeepb, 2016-04-27 07:39:31]

Patchset #1 (id:1) has been deleted

[karandeepb, 2016-04-27 08:04:40]

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

[karandeepb, 2016-04-27 08:04:40]

PTAL sky@. Thanks.

https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/label.cc
File ui/views/controls/label.cc (right):

https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/label...
ui/views/controls/label.cc:491: focus_bounds.Inset(-GetInsets());
This will be different from the current value if the focused link also has an
explicitly set border. Is this case possible?

https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/label...
File ui/views/controls/label_unittest.cc (left):

https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/label...
ui/views/controls/label_unittest.cc:649: TEST_F(LabelFocusTest, EmptyLabel) {
An empty link is not focusable. Hence this test can be removed.

https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/link.cc
File ui/views/controls/link.cc (right):

https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/link....
ui/views/controls/link.cc:183: if (!text().empty()) {
There are 1-2 instances where the focusability of a link is explicitly set to
NEVER. This will cause even those links to have an inset.

[sky, 2016-04-27 16:34:20]

https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/label.cc
File ui/views/controls/label.cc (left):

https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/label...
ui/views/controls/label.cc:214: gfx::Insets Label::GetInsets() const {
Is there a reason to remove this? Isn't is possible someone is creating a label
and making it focusable?

[karandeepb, 2016-04-27 23:51:14]

On 2016/04/27 16:34:20, sky wrote:
>
https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/label.cc
> File ui/views/controls/label.cc (left):
> 
>
https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/label...
> ui/views/controls/label.cc:214: gfx::Insets Label::GetInsets() const {
> Is there a reason to remove this? Isn't is possible someone is creating a
label
> and making it focusable?

Yeah, though I am not sure if it's a valid use case. There are no instances in
the codebase at least. I was trying to find a way so that we don't have to use
IsFocusable(), since it will change based on whether the view is drawn or not. I
have also tried an alternate approach here-
https://codereview.chromium.org/1915863004/, though I think it's a bit hacky.

[sky, 2016-04-28 00:05:57]

Meta question. My request to remove focusable() is that it is confusing given
use of the enumation you are adding, and easy to misuse. All I'm asking for is
to remove it from view and update label to use the new enum. In other words you
shouldn't have to do all the changes here. Unless I'm missing something?

https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/label.cc
File ui/views/controls/label.cc (right):

https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/label...
ui/views/controls/label.cc:427: // GetContentBounds() does not use GetInsets()
which may be overriden in Label
ick. We should fix GetcontentsBounds to call GetInsets.

[karandeepb, 2016-04-28 00:13:02]

On 2016/04/28 00:05:57, sky wrote:
> Meta question. My request to remove focusable() is that it is confusing given
> use of the enumation you are adding, and easy to misuse. All I'm asking for is
> to remove it from view and update label to use the new enum. In other words
you
> shouldn't have to do all the changes here. Unless I'm missing something?
> 
>
https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/label.cc
> File ui/views/controls/label.cc (right):
> 
>
https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/label...
> ui/views/controls/label.cc:427: // GetContentBounds() does not use GetInsets()
> which may be overriden in Label
> ick. We should fix GetcontentsBounds to call GetInsets.

Oh ok. I misunderstood. I thought you wanted me to remove focusable() w/o
exposing focus_behavior_. Thanks.

[karandeepb, 2016-04-28 00:13:34]

https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/label.cc
File ui/views/controls/label.cc (right):

https://codereview.chromium.org/1920193003/diff/20001/ui/views/controls/label...
ui/views/controls/label.cc:427: // GetContentBounds() does not use GetInsets()
which may be overriden in Label
On 2016/04/28 00:05:56, sky wrote:
> ick. We should fix GetcontentsBounds to call GetInsets.

Will assign a bug to myself and fix this.