Makes View::focusable() return last value supplied to SetFocusable()

As focusable() may effect the preferred size it is important that it
returns the last value passed to SetFocusable() rather than if the
view is focusable right now. Otherwise focusable() could change value
based on enabled or visible state, resulting in the preferred size
bouncing around.

I'm also making focusable() protected as it's really only subclasses
that should care about it. Especially since we also have
IsFocusable().

BUG=330696
TEST=covered by test
R=ben@chromium.org

Review URL: https://codereview.chromium.org/129863002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@244282 0039d316-1c4b-4281-b951-d872f2087c98

ui/views/controls/textfield/native_textfield_views_unittest.cc

@@ -79,6 +79,8 @@ class TestTextfield : public views::Textfield {
 
   void clear() { key_received_ = key_handled_ = false; }
 
+  bool focusable() const { return View::focusable(); }
+
  private:
   bool key_handled_;
   bool key_received_;

ui/views/examples/button_example.cc

@@ -163,7 +163,7 @@ void ButtonExample::LabelButtonPressed(const ui::Event& event) {
     }
   } else if (event.IsShiftDown()) {
     if (event.IsAltDown()) {
-      label_button_->SetFocusable(!label_button_->focusable());
+      label_button_->SetFocusable(!label_button_->IsFocusable());
     } else {
       label_button_->SetStyle(static_cast<Button::ButtonStyle>(
           (label_button_->style() + 1) % Button::STYLE_COUNT));

ui/views/view.h

@@ -773,9 +773,6 @@ class VIEWS_EXPORT View : public ui::LayerDelegate,
   // not get the focus.
   void SetFocusable(bool focusable);
 
-  // Returns true if this view is capable of taking focus.
-  bool focusable() const { return focusable_ && enabled_ && visible_; }
-
   // Returns true if this view is |focusable_|, |enabled_| and drawn.
   virtual bool IsFocusable() const;
 
@@ -1142,6 +1139,10 @@ class VIEWS_EXPORT View : public ui::LayerDelegate,
 
   // Focus ---------------------------------------------------------------------
 
+  // Returns last value passed to SetFocusable(). Use IsFocusable() to determine
+  // if a view can take focus right now.
+  bool focusable() const { return focusable_; }
+
   // Override to be notified when focus has changed either to or from this View.
   virtual void OnFocus();
   virtual void OnBlur();

ui/views/view_unittest.cc

@@ -231,6 +231,8 @@ class TestView : public View {
     views::View::Blur();
   }
 
+  bool focusable() const { return View::focusable(); }
+
   virtual void OnBoundsChanged(const gfx::Rect& previous_bounds) OVERRIDE;
   virtual bool OnMousePressed(const ui::MouseEvent& event) OVERRIDE;
   virtual bool OnMouseDragged(const ui::MouseEvent& event) OVERRIDE;
@@ -3572,4 +3574,20 @@ TEST_F(ViewLayerTest, RecreateLayerZOrderWidgetParent) {
 
 #endif  // USE_AURA
 
+TEST_F(ViewTest, FocusableAssertions) {
+  // View subclasses may change insets based on whether they are focusable,
+  // which effects the preferred size. To avoid preferred size changing around
+  // these Views need to key off the last value set to SetFocusable(), not
+  // whether the View is focusable right now. For this reason it's important
+  // that focusable() return the last value passed to SetFocusable and not
+  // whether the View is focusable right now.
+  TestView view;
+  view.SetFocusable(true);
+  EXPECT_TRUE(view.focusable());
+  view.SetEnabled(false);
+  EXPECT_TRUE(view.focusable());
+  view.SetFocusable(false);
+  EXPECT_FALSE(view.focusable());
+}
+
 }  // namespace views