VR: Add the notion of focusable to UiElement

Bug: 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I57add5c261f28762a3cec1be364c6be8572703ca
Reviewed-on: https://chromium-review.googlesource.com/809606
Commit-Queue: Yash Malik <ymalik@chromium.org>
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522413}

chrome/browser/vr/elements/keyboard.cc

@@ -12,6 +12,7 @@ namespace vr {
 
 Keyboard::Keyboard() {
   set_name(kKeyboard);
+  set_focusable(false);
   SetVisibleImmediately(false);
 }
 
@@ -86,6 +87,10 @@ void Keyboard::Render(UiElementRenderer* renderer,
   delegate_->Draw(camera_model);
 }
 
+void Keyboard::OnSetFocusable() {
+  DCHECK(!focusable());
+}
+
 void Keyboard::UpdateDelegateVisibility() {
   if (!delegate_)
     return;

chrome/browser/vr/elements/keyboard.h

@@ -34,6 +34,7 @@ class Keyboard : public UiElement {
                     const gfx::Vector3dF& head_direction) override;
   void Render(UiElementRenderer* renderer,
               const CameraModel& camera_model) const final;
+  void OnSetFocusable() override;
 
   void UpdateDelegateVisibility();
 

chrome/browser/vr/elements/text_input.cc

@@ -26,6 +26,7 @@ TextInput::TextInput(int maximum_width_pixels,
   text->set_type(kTypeTextInputHint);
   text->set_draw_phase(kPhaseForeground);
   text->set_hit_testable(false);
+  text->set_focusable(false);
   text->set_x_anchoring(LEFT);
   text->set_x_centering(LEFT);
   text->SetSize(1, 1);
@@ -38,6 +39,7 @@ TextInput::TextInput(int maximum_width_pixels,
   text->set_type(kTypeTextInputText);
   text->set_draw_phase(kPhaseForeground);
   text->set_hit_testable(true);
+  text->set_focusable(false);
   text->set_x_anchoring(LEFT);
   text->set_x_centering(LEFT);
   text->set_bubble_events(true);
@@ -53,6 +55,7 @@ TextInput::TextInput(int maximum_width_pixels,
   cursor->set_type(kTypeTextInputCursor);
   cursor->set_draw_phase(kPhaseForeground);
   cursor->set_hit_testable(false);
+  cursor->set_focusable(false);
   cursor->set_x_anchoring(LEFT);
   cursor->set_y_anchoring(BOTTOM);
   cursor->SetColor(SK_ColorBLUE);
@@ -68,10 +71,6 @@ void TextInput::SetTextInputDelegate(TextInputDelegate* text_input_delegate) {
   delegate_ = text_input_delegate;
 }
 
-bool TextInput::IsEditable() {
-  return true;
-}
-
 void TextInput::OnButtonUp(const gfx::PointF& position) {
   RequestFocus();
 }

chrome/browser/vr/elements/text_input.h

@@ -31,7 +31,6 @@ class TextInput : public UiElement {
             OnInputEditedCallback input_edit_callback);
   ~TextInput() override;
 
-  bool IsEditable() override;
   void OnButtonUp(const gfx::PointF& position) override;
   void OnFocusChanged(bool focused) override;
   void OnInputEdited(const TextInputInfo& info) override;

chrome/browser/vr/elements/ui_element.cc

@@ -87,6 +87,13 @@ void UiElement::set_draw_phase(DrawPhase draw_phase) {
 
 void UiElement::OnSetDrawPhase() {}
 
+void UiElement::set_focusable(bool focusable) {
+  focusable_ = focusable;
+  OnSetFocusable();
+}
+
+void UiElement::OnSetFocusable() {}
+
 void UiElement::Render(UiElementRenderer* renderer,
                        const CameraModel& model) const {
   // Elements without an overridden implementation of Render should have their
@@ -190,10 +197,6 @@ bool UiElement::IsHitTestable() const {
   return IsVisible() && hit_testable_;
 }
 
-bool UiElement::IsEditable() {
-  return false;
-}
-
 void UiElement::SetSize(float width, float height) {
   animation_player_.TransitionSizeTo(last_frame_time_, BOUNDS, size_,
                                      gfx::SizeF(width, height));

chrome/browser/vr/elements/ui_element.h

@@ -140,10 +140,6 @@ class UiElement : public cc::AnimationTarget {
   // Indicates whether the element should be tested for cursor input.
   bool IsHitTestable() const;
 
-  // Indicates whether the element is an editable element. Editable elements
-  // such as the input field should override this.
-  virtual bool IsEditable();
-
   virtual void Render(UiElementRenderer* renderer,
                       const CameraModel& model) const;
 
@@ -201,6 +197,10 @@ class UiElement : public cc::AnimationTarget {
   bool hit_testable() const { return hit_testable_; }
   void set_hit_testable(bool hit_testable) { hit_testable_ = hit_testable; }
 
+  bool focusable() const { return focusable_; }
+  void set_focusable(bool focusable);
+  virtual void OnSetFocusable();
+
   bool scrollable() const { return scrollable_; }
   void set_scrollable(bool scrollable) { scrollable_ = scrollable; }
 
@@ -430,6 +430,9 @@ class UiElement : public cc::AnimationTarget {
   // If false, the reticle will not hit the element, even if visible.
   bool hit_testable_ = true;
 
+  // If false, clicking on the element doesn't give it focus.
+  bool focusable_ = true;
+
   // A signal to the input routing machinery that this element accepts scrolls.
   bool scrollable_ = false;
 

chrome/browser/vr/ui_input_manager.cc

@@ -326,7 +326,7 @@ void UiInputManager::SendButtonDown(UiElement* target,
     // Clicking outside of the focused element causes it to lose focus.
     // TODO(ymalik): We will lose focus if we hit an element inside the focused
     // element, which is incorrect behavior.
-    if (target->id() != focused_element_id_ && target->name() != kKeyboard) {
+    if (target->id() != focused_element_id_ && target->focusable()) {
       UnfocusFocusedElement();
     }
   } else {
@@ -433,7 +433,7 @@ void UiInputManager::UnfocusFocusedElement() {
     return;
 
   UiElement* focused = scene_->GetUiElementById(focused_element_id_);
-  if (focused && focused->IsEditable()) {
+  if (focused && focused->focusable()) {
     focused->OnFocusChanged(false);
   }
   focused_element_id_ = -1;
@@ -446,7 +446,7 @@ void UiInputManager::RequestFocus(int element_id) {
   UnfocusFocusedElement();
 
   UiElement* focused = scene_->GetUiElementById(element_id);
-  if (!focused || !focused->IsEditable())
+  if (!focused || !focused->focusable())
     return;
 
   focused_element_id_ = element_id;
@@ -455,15 +455,17 @@ void UiInputManager::RequestFocus(int element_id) {
 
 void UiInputManager::OnInputEdited(const TextInputInfo& info) {
   UiElement* focused = scene_->GetUiElementById(focused_element_id_);
-  if (!focused || !focused->IsEditable())
+  if (!focused)
     return;
+  DCHECK(focused->focusable());
   focused->OnInputEdited(info);
 }
 
 void UiInputManager::OnInputCommitted(const TextInputInfo& info) {
   UiElement* focused = scene_->GetUiElementById(focused_element_id_);
-  if (!focused || !focused->IsEditable())
+  if (!focused || !focused->focusable())
     return;
+  DCHECK(focused->focusable());
   focused->OnInputCommitted(info);
 }
 

chrome/browser/vr/ui_input_manager_unittest.cc

@@ -47,6 +47,9 @@ class MockRect : public Rect {
   MOCK_METHOD1(OnMove, void(const gfx::PointF& position));
   MOCK_METHOD1(OnButtonDown, void(const gfx::PointF& position));
   MOCK_METHOD1(OnButtonUp, void(const gfx::PointF& position));
+  MOCK_METHOD1(OnFocusChanged, void(bool));
+  MOCK_METHOD1(OnInputEdited, void(const TextInputInfo&));
+  MOCK_METHOD1(OnInputCommitted, void(const TextInputInfo&));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockRect);
@@ -76,7 +79,7 @@ class UiInputManagerTest : public testing::Test {
     input_manager_ = base::MakeUnique<UiInputManager>(scene_.get());
   }
 
-  StrictMock<MockRect>* CreateMockElement(float z_position) {
+  StrictMock<MockRect>* CreateAndAddMockElement(float z_position) {
     auto element = base::MakeUnique<StrictMock<MockRect>>();
     StrictMock<MockRect>* p_element = element.get();
     element->SetTranslate(0, 0, z_position);
@@ -86,7 +89,7 @@ class UiInputManagerTest : public testing::Test {
     return p_element;
   }
 
-  StrictMock<MockTextInput>* CreateMockInputElement(float z_position) {
+  StrictMock<MockTextInput>* CreateAndAddMockInputElement(float z_position) {
     auto element = base::MakeUnique<StrictMock<MockTextInput>>();
     StrictMock<MockTextInput>* p_element = element.get();
     element->SetTranslate(0, 0, z_position);
@@ -133,8 +136,8 @@ class UiInputManagerContentTest : public UiTest {
 };
 
 TEST_F(UiInputManagerTest, FocusedElement) {
-  StrictMock<MockTextInput>* p_element1 = CreateMockInputElement(-5.f);
-  StrictMock<MockTextInput>* p_element2 = CreateMockInputElement(-5.f);
+  StrictMock<MockTextInput>* p_element1 = CreateAndAddMockInputElement(-5.f);
+  StrictMock<MockTextInput>* p_element2 = CreateAndAddMockInputElement(-5.f);
   TextInputInfo edit(base::ASCIIToUTF16("asdfg"));
 
   // Focus request triggers OnFocusChanged.
@@ -156,6 +159,54 @@ TEST_F(UiInputManagerTest, FocusedElement) {
   input_manager_->RequestFocus(p_element2->id());
 }
 
+// Verify that a focusable child clears focus off its parent. Note that the
+// child isn't any different from other elements in that it should also steal
+// focus from its parent.
+TEST_F(UiInputManagerTest, FocusableChildStealsFocus) {
+  StrictMock<MockRect>* p_element = CreateAndAddMockElement(-5.f);
+
+  auto child = base::MakeUnique<StrictMock<MockRect>>();
+  auto* p_child = child.get();
+  child->set_hit_testable(true);
+  child->set_focusable(true);
+  p_element->AddChild(std::move(child));
+  scene_->OnBeginFrame(base::TimeTicks(), kForwardVector);
+
+  // Focus element.
+  testing::Sequence s;
+  EXPECT_CALL(*p_element, OnFocusChanged(true)).InSequence(s);
+  input_manager_->RequestFocus(p_element->id());
+
+  // Focus child.
+  EXPECT_CALL(*p_child, OnHoverEnter(_)).InSequence(s);
+  EXPECT_CALL(*p_child, OnButtonDown(_)).InSequence(s);
+  EXPECT_CALL(*p_element, OnFocusChanged(false)).InSequence(s);
+  HandleInput(kForwardVector, kDown);
+}
+
+// Verify that a non-focusable child does not clear focus off its parent.
+TEST_F(UiInputManagerTest, NonFocusableChildDoestNotStealFocus) {
+  StrictMock<MockRect>* p_element = CreateAndAddMockElement(-5.f);
+
+  auto child = base::MakeUnique<StrictMock<MockRect>>();
+  auto* p_child = child.get();
+  child->set_hit_testable(true);
+  child->set_focusable(false);
+  p_element->AddChild(std::move(child));
+  scene_->OnBeginFrame(base::TimeTicks(), kForwardVector);
+
+  // Focus element.
+  testing::Sequence s;
+  EXPECT_CALL(*p_element, OnFocusChanged(true)).InSequence(s);
+  input_manager_->RequestFocus(p_element->id());
+
+  // Focus child.
+  EXPECT_CALL(*p_child, OnHoverEnter(_)).InSequence(s);
+  EXPECT_CALL(*p_child, OnButtonDown(_)).InSequence(s);
+  EXPECT_CALL(*p_element, OnFocusChanged(false)).Times(0).InSequence(s);
+  HandleInput(kForwardVector, kDown);
+}
+
 TEST_F(UiInputManagerTest, ReticleRenderTarget) {
   auto element = base::MakeUnique<Rect>();
   UiElement* p_element = element.get();
@@ -186,7 +237,7 @@ TEST_F(UiInputManagerTest, ReticleRenderTarget) {
 // either directly at (forward) or directly away from (backward) a test element.
 // Verify mock expectations along the way to make failures easier to track.
 TEST_F(UiInputManagerTest, HoverClick) {
-  StrictMock<MockRect>* p_element = CreateMockElement(-5.f);
+  StrictMock<MockRect>* p_element = CreateAndAddMockElement(-5.f);
 
   // Move over the test element.
   EXPECT_CALL(*p_element, OnHoverEnter(_));
@@ -244,8 +295,8 @@ TEST_F(UiInputManagerTest, HoverClick) {
 // releasing the button. Upon release, the previous element should see its click
 // and hover states cleared, and the new element should see a hover.
 TEST_F(UiInputManagerTest, ReleaseButtonOnAnotherElement) {
-  StrictMock<MockRect>* p_front_element = CreateMockElement(-5.f);
-  StrictMock<MockRect>* p_back_element = CreateMockElement(5.f);
+  StrictMock<MockRect>* p_front_element = CreateAndAddMockElement(-5.f);
+  StrictMock<MockRect>* p_back_element = CreateAndAddMockElement(5.f);
 
   // Press on an element, move away, then release.
   EXPECT_CALL(*p_front_element, OnHoverEnter(_));
@@ -262,7 +313,7 @@ TEST_F(UiInputManagerTest, ReleaseButtonOnAnotherElement) {
 
 // Test that input is tolerant of disappearing elements.
 TEST_F(UiInputManagerTest, ElementDeletion) {
-  StrictMock<MockRect>* p_element = CreateMockElement(-5.f);
+  StrictMock<MockRect>* p_element = CreateAndAddMockElement(-5.f);
 
   // Hover on an element.
   EXPECT_CALL(*p_element, OnHoverEnter(_));