Accessibility shortcuts to access PIP window

Make PIP window non activatable by default on ChromeOS. Bug: 910323 Test: Manual. unittest T.B.D. Change-Id: Ibd444a182c731c4e585c428938bbc4cc0d03d9a2 Reviewed-on: https://chromium-review.googlesource.com/c/1354410 Reviewed-by: David Tseng <[email protected]> Reviewed-by: Mounir Lamouri <[email protected]> Reviewed-by: Brian White <[email protected]> Reviewed-by: Eliot Courtney <[email protected]> Commit-Queue: Mitsuru Oshima <[email protected]> Cr-Commit-Position: refs/heads/master@{#612905}
nwjs · Dec 1, 2018 · 7353b9b · 7353b9b
1 parent 7a97b4c
commit 7353b9b
Show file tree

Hide file tree

Showing 13 changed files with 89 additions and 6 deletions.
diff --git a/ash/accelerators/accelerator_controller.cc b/ash/accelerators/accelerator_controller.cc
@@ -64,6 +64,7 @@
 #include "ash/wm/window_state.h"
 #include "ash/wm/window_util.h"
 #include "ash/wm/wm_event.h"
+#include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
 #include "base/optional.h"
@@ -239,6 +240,20 @@ void HandleFocusShelf() {
   Shell::Get()->focus_cycler()->FocusWidget(shelf->shelf_widget());
 }
 
+views::Widget* FindPipWidget() {
+  return Shell::Get()->focus_cycler()->FindWidget(
+      base::BindRepeating([](views::Widget* widget) {
+        return wm::GetWindowState(widget->GetNativeWindow())->IsPip();
+      }));
+}
+
+void HandleFocusPip() {
+  base::RecordAction(UserMetricsAction("Accel_Focus_Pip"));
+  auto* widget = FindPipWidget();
+  if (widget)
+    Shell::Get()->focus_cycler()->FocusWidget(widget);
+}
+
 void HandleLaunchAppN(int n) {
   base::RecordAction(UserMetricsAction("Accel_Launch_App"));
   Shelf::LaunchShelfItem(n);
@@ -1333,6 +1348,8 @@ bool AcceleratorController::CanPerformAction(
       return CanHandleWindowSnap();
     case WINDOW_POSITION_CENTER:
       return CanHandlePositionCenter();
+    case FOCUS_PIP:
+      return !!FindPipWidget();
 
     // The following are always enabled.
     case BRIGHTNESS_DOWN:
@@ -1466,6 +1483,9 @@ void AcceleratorController::PerformAction(AcceleratorAction action,
     case FOCUS_SHELF:
       HandleFocusShelf();
       break;
+    case FOCUS_PIP:
+      HandleFocusPip();
+      break;
     case KEYBOARD_BRIGHTNESS_DOWN: {
       KeyboardBrightnessControlDelegate* delegate =
           Shell::Get()->keyboard_brightness_control_delegate();

diff --git a/ash/accelerators/accelerator_table_unittest.cc b/ash/accelerators/accelerator_table_unittest.cc
@@ -17,10 +17,10 @@ namespace ash {
 namespace {
 
 // The number of non-Search-based accelerators.
-constexpr int kNonSearchAcceleratorsNum = 88;
+constexpr int kNonSearchAcceleratorsNum = 89;
 // The hash of non-Search-based accelerators. See HashAcceleratorData().
 constexpr char kNonSearchAcceleratorsHash[] =
-    "66028d49a13708e1c1d8c5791d37a8e4";
+    "569da153a9c19edb92a8b30896bf0ff3";
 
 struct Cmp {
   bool operator()(const AcceleratorData& lhs,

diff --git a/ash/focus_cycler.cc b/ash/focus_cycler.cc
@@ -12,6 +12,7 @@
 #include "ui/views/accessible_pane_view.h"
 #include "ui/views/focus/focus_search.h"
 #include "ui/views/widget/widget.h"
+#include "ui/views/widget/widget_delegate.h"
 #include "ui/wm/public/activation_client.h"
 
 namespace ash {
@@ -104,6 +105,10 @@ void FocusCycler::RotateFocus(Direction direction) {
 }
 
 bool FocusCycler::FocusWidget(views::Widget* widget) {
+  // If the target is PIP window, temporarily make it activatable.
+  if (wm::GetWindowState(widget->GetNativeWindow())->IsPip())
+    widget->widget_delegate()->set_can_activate(true);
+
   // Note: It is not necessary to set the focus directly to the pane since that
   // will be taken care of by the widget activation.
   widget_activating_ = widget;
@@ -112,4 +117,13 @@ bool FocusCycler::FocusWidget(views::Widget* widget) {
   return widget->IsActive();
 }
 
+views::Widget* FocusCycler::FindWidget(
+    base::RepeatingCallback<bool(views::Widget*)> callback) {
+  for (auto* widget : widgets_) {
+    if (callback.Run(widget))
+      return widget;
+  }
+  return nullptr;
+}
+
 }  // namespace ash
diff --git a/ash/focus_cycler.h b/ash/focus_cycler.h
@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "ash/ash_export.h"
+#include "base/callback.h"
 #include "base/macros.h"
 
 namespace views {
@@ -42,6 +43,11 @@ class ASH_EXPORT FocusCycler {
   // Moves focus the specified widget. Returns true if the widget was activated.
   bool FocusWidget(views::Widget* widget);
 
+  // Find a widget that matches the criteria given by |callback|
+  // in the cycle list.
+  views::Widget* FindWidget(
+      base::RepeatingCallback<bool(views::Widget*)> callback);
+
  private:
   std::vector<views::Widget*> widgets_;
 

diff --git a/ash/public/cpp/accelerators.cc b/ash/public/cpp/accelerators.cc
@@ -109,6 +109,7 @@ const AcceleratorData kAcceleratorData[] = {
     {true, ui::VKEY_MEDIA_LAUNCH_APP2, ui::EF_SHIFT_DOWN, TOGGLE_FULLSCREEN},
     {true, ui::VKEY_ESCAPE, ui::EF_SHIFT_DOWN | ui::EF_COMMAND_DOWN, UNPIN},
     {true, ui::VKEY_L, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN, FOCUS_SHELF},
+    {true, ui::VKEY_V, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN, FOCUS_PIP},
     {true, ui::VKEY_HELP, ui::EF_NONE, SHOW_SHORTCUT_VIEWER},
     {true, ui::VKEY_OEM_2, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
      SHOW_SHORTCUT_VIEWER},

diff --git a/ash/public/cpp/accelerators.h b/ash/public/cpp/accelerators.h
@@ -41,6 +41,7 @@ enum AcceleratorAction {
   FOCUS_NEXT_PANE,
   FOCUS_PREVIOUS_PANE,
   FOCUS_SHELF,
+  FOCUS_PIP,
   KEYBOARD_BRIGHTNESS_DOWN,
   KEYBOARD_BRIGHTNESS_UP,
   LAUNCH_APP_0,

diff --git a/ash/wm/mru_window_tracker.cc b/ash/wm/mru_window_tracker.cc
@@ -26,7 +26,8 @@ namespace {
 using CanActivateWindowPredicate = base::Callback<bool(aura::Window*)>;
 
 bool CallCanActivate(aura::Window* window) {
-  return ::wm::CanActivateWindow(window);
+  return ::wm::CanActivateWindow(window) &&
+         !wm::GetWindowState(window)->IsPip();
 }
 
 // Adds the windows that can be cycled through for the specified window id to

diff --git a/ash/wm/window_state.cc b/ash/wm/window_state.cc
@@ -7,6 +7,7 @@
 #include <memory>
 #include <utility>
 
+#include "ash/focus_cycler.h"
 #include "ash/public/cpp/window_animation_types.h"
 #include "ash/public/cpp/window_properties.h"
 #include "ash/public/cpp/window_state_type.h"
@@ -37,6 +38,8 @@
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/views/painter.h"
+#include "ui/views/widget/widget.h"
+#include "ui/views/widget/widget_delegate.h"
 #include "ui/wm/core/coordinate_conversion.h"
 #include "ui/wm/core/ime_util_chromeos.h"
 #include "ui/wm/core/window_util.h"
@@ -480,6 +483,14 @@ void WindowState::OnRevertDrag(const gfx::Point& location) {
     delegate_->OnDragFinished(/*canceled=*/true, location);
 }
 
+void WindowState::OnActivationLost() {
+  if (IsPip()) {
+    views::Widget::GetWidgetForNativeWindow(window())
+        ->widget_delegate()
+        ->set_can_activate(false);
+  }
+}
+
 display::Display WindowState::GetDisplay() {
   return display::Screen::GetScreen()->GetDisplayNearestWindow(window());
 }
@@ -667,10 +678,14 @@ void WindowState::SetBoundsDirectCrossFade(const gfx::Rect& new_bounds,
 }
 
 void WindowState::UpdatePipState(bool was_pip) {
+  auto* widget = views::Widget::GetWidgetForNativeWindow(window());
+
   if (IsPip()) {
+    Shell::Get()->focus_cycler()->AddWidget(widget);
     ::wm::SetWindowVisibilityAnimationType(
         window(), WINDOW_VISIBILITY_ANIMATION_TYPE_FADE_IN_SLIDE_OUT);
   } else if (was_pip) {
+    Shell::Get()->focus_cycler()->RemoveWidget(widget);
     ::wm::SetWindowVisibilityAnimationType(
         window(), ::wm::WINDOW_VISIBILITY_ANIMATION_TYPE_DEFAULT);
   }
@@ -770,6 +785,10 @@ void WindowState::OnWindowAddedToRootWindow(aura::Window* window) {
 
 void WindowState::OnWindowDestroying(aura::Window* window) {
   DCHECK_EQ(window_, window);
+  auto* widget = views::Widget::GetWidgetForNativeWindow(window);
+  if (widget)
+    Shell::Get()->focus_cycler()->RemoveWidget(widget);
+
   immersive_gesture_drag_handler_.reset();
   current_state_->OnWindowDestroying(this);
   delegate_.reset();

diff --git a/ash/wm/window_state.h b/ash/wm/window_state.h
@@ -336,6 +336,9 @@ class ASH_EXPORT WindowState : public aura::WindowObserver {
   void OnCompleteDrag(const gfx::Point& location);
   void OnRevertDrag(const gfx::Point& location);
 
+  // Notifies that the window lost the activation.
+  void OnActivationLost();
+
   // Returns a pointer to DragDetails during drag operations.
   const DragDetails* drag_details() const { return drag_details_.get(); }
   DragDetails* drag_details() { return drag_details_.get(); }

diff --git a/ash/wm/workspace/workspace_layout_manager.cc b/ash/wm/workspace/workspace_layout_manager.cc
@@ -342,6 +342,9 @@ void WorkspaceLayoutManager::OnWindowActivating(ActivationReason reason,
 void WorkspaceLayoutManager::OnWindowActivated(ActivationReason reason,
                                                aura::Window* gained_active,
                                                aura::Window* lost_active) {
+  if (lost_active)
+    wm::GetWindowState(lost_active)->OnActivationLost();
+
   UpdateFullscreenState();
   UpdateShelfVisibility();
 }

diff --git a/chrome/browser/ui/ash/ksv/keyboard_shortcut_viewer_metadata_unittest.cc b/chrome/browser/ui/ash/ksv/keyboard_shortcut_viewer_metadata_unittest.cc
@@ -20,9 +20,9 @@
 namespace {
 
 // The total number of Ash accelerators.
-constexpr int kAshAcceleratorsTotalNum = 100;
+constexpr int kAshAcceleratorsTotalNum = 101;
 // The hash of Ash accelerators.
-constexpr char kAshAcceleratorsHash[] = "58ba78b2c8ec069c0642e3073636a1b3";
+constexpr char kAshAcceleratorsHash[] = "5fb453f20fff60b7c085057ac30f3dd7";
 #if defined(GOOGLE_CHROME_BUILD)
 // Internal builds add an extra accelerator for the Feedback app.
 // The total number of Chrome accelerators (available on Chrome OS).

diff --git a/chrome/browser/ui/views/overlay/overlay_window_views.cc b/chrome/browser/ui/views/overlay/overlay_window_views.cc
@@ -148,6 +148,12 @@ class OverlayWindowWidgetDelegate : public views::WidgetDelegate {
   explicit OverlayWindowWidgetDelegate(views::Widget* widget)
       : widget_(widget) {
     DCHECK(widget_);
+#if defined(OS_CHROMEOS)
+    // PIP windows on ChromeOS are not activatable by default.
+    // It'll be explicitly made activatable when necessary for
+    // accessibility.
+    set_can_activate(false);
+#endif
   }
   ~OverlayWindowWidgetDelegate() override = default;
 
@@ -561,14 +567,16 @@ void OverlayWindowViews::Close() {
 }
 
 void OverlayWindowViews::Show() {
-  views::Widget::Show();
 #if defined(OS_CHROMEOS)
+  views::Widget::ShowInactive();
   // For rounded corners.
   if (ash::features::IsPipRoundedCornersEnabled()) {
     decorator_ = std::make_unique<ash::RoundedCornerDecorator>(
         GetNativeWindow(), GetNativeWindow(), GetRootView()->layer(),
         ash::kPipRoundedCornerRadius);
   }
+#else
+  views::Widget::Show();
 #endif
 
   // If this is not the first time the window is shown, this will be a no-op.

diff --git a/tools/metrics/actions/actions.xml b/tools/metrics/actions/actions.xml
@@ -576,6 +576,13 @@ should be able to be added at any place in this file.
   <description>Please enter the description of this user action.</description>
 </action>
 
+<action name="Accel_Focus_Pip">
+  <owner>[email protected]</owner>
+  <description>
+    Keyboard accelerator to focus picture in picture window.
+  </description>
+</action>
+
 <action name="Accel_Focus_Previous_Pane">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <description>Please enter the description of this user action.</description>