Add kAccessibilityFocusFallsbackToWidget property

ui/views/accessibility/ax_aura_obj_cache.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/accessibility/ax_aura_obj_cache.h"
 
 #include "base/memory/ptr_util.h"
 #include "base/memory/singleton.h"
+#include "base/strings/string_util.h"
+#include "ui/aura/client/aura_constants.h"
 #include "ui/aura/client/focus_client.h"
 #include "ui/aura/window.h"
 #include "ui/views/accessibility/ax_aura_obj_wrapper.h"
 #include "ui/views/accessibility/ax_view_obj_wrapper.h"
 #include "ui/views/accessibility/ax_widget_obj_wrapper.h"
 #include "ui/views/accessibility/ax_window_obj_wrapper.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 
 namespace views {
 
+namespace {
+
+const char kExoShellSurfaceWindowName[] = "ExoShellSurface";
+const char kArcProcessNamePrefix[] = "org.chromium.arc.";
+
+// TODO(yawano): chrome/browser/memory/tab_manager_delegate_chromeos.cc is doing
+// the same thing. Move this to some utility function and share the logic with
+// it.
+bool IsArcWindow(aura::Window* window) {

[sky, 2016/12/06 17:23:20]

Views shouldn't need to know about arc. Instead the arc code should set a
property on the aura::Window to indicate this new behavior should be provided.
Something like:

widget->GetNativeView()->SetProperty(kAccessibilityFocusFallsbackToWidget,
false);

[yawano, 2016/12/07 11:10:00]

Thank you! Seems good property for this behavior. Added
kAccessibilityFocusFallsbackToWidget.

+  if (!window || window->GetName() != kExoShellSurfaceWindowName)

[sky, 2016/12/02 17:49:56]

Why do you need to check the name *and* the kAppIdKey? Can't you just check
kAppIdKey?

[sky, 2016/12/02 17:50:40]

Also, can't you set an explicit property rather than having to do string
comparison? I'm thinking a specific property to identify what you need.

[yawano, 2016/12/05 02:16:36]

As far as I tested, checking only kAppIdKey would be enough for the current
version of Chrome OS. But I think checking both name and kAppIdKey is better
since it's more semantic.

We want to consider the window which meets the both 2 conditions as special case
here.

- Window is using ExoShellSurface - Checking name
- Window is for Arc - Checking application id

My concern for only checking application id is that someone might add an arc
window which doesn't use ExoShellSurface in the future.

For specific property, does it mean to add new property or change value of an
existing property? I think there should be a way to do it in the way. But, IMO,
it's better if we don't need to have IsArcWindow branch in this file, i.e. this
shouldn't be handled as an exceptional case.

[reveman, 2016/12/05 16:42:37]

We already do this. exo::ShellSurface::GetMainSurface can be used to determine
if a window is an exo shell surface widget:
https://cs.chromium.org/chromium/src/components/exo/shell_surface.h?l=174

[reveman, 2016/12/05 16:42:37]

exo::ShellSurface::GetMainSurface can be used to more efficiently determine if a
window is an exo shell surface widget:
https://cs.chromium.org/chromium/src/components/exo/shell_surface.h?l=174

It think using that instead of a string compare is what sky@ asked for.

+    return false;
+
+  std::string* application_id = window->GetProperty(aura::client::kAppIdKey);
+  if (!application_id)
+    return false;
+
+  return base::StartsWith(*application_id, kArcProcessNamePrefix,
+                          base::CompareCase::SENSITIVE);
+}
+}  // namespace
+
 // static
 AXAuraObjCache* AXAuraObjCache::GetInstance() {
   return base::Singleton<AXAuraObjCache>::get();
 }
 
 AXAuraObjWrapper* AXAuraObjCache::GetOrCreate(View* view) {
   return CreateInternal<AXViewObjWrapper>(view, view_to_id_map_);
 }
 
 AXAuraObjWrapper* AXAuraObjCache::GetOrCreate(Widget* widget) {
@@ skipping 121 matching lines @@
   }
 
   if (!focused_widget)
     return nullptr;
 
   FocusManager* focus_manager = focused_widget->GetFocusManager();
   if (!focus_manager)
     return nullptr;
 
   View* focused_view = focus_manager->GetFocusedView();
-  if (focused_view)
+
+  // If focused window is arc, don't try to put focus on root view even if
+  // focused_view is nullptr.
+  if (IsArcWindow(focused_window) || focused_view)
     return focused_view;
-  else
-    return focused_widget->GetRootView();
+
+  // If no view is focused, falls back to root view.
+  return focused_widget->GetRootView();
 }
 
 void AXAuraObjCache::OnWindowFocused(aura::Window* gained_focus,
                                      aura::Window* lost_focus) {
   OnFocusedViewChanged();
 }
 
 void AXAuraObjCache::OnWindowDestroying(aura::Window* window) {
   focus_client_ = nullptr;
 }
@@ skipping 36 matching lines @@
     AuraView* aura_view,
     std::map<AuraView*, int32_t>& aura_view_to_id_map) {
   int32_t id = GetID(aura_view);
   if (id == -1)
     return;
   aura_view_to_id_map.erase(aura_view);
   Remove(id);
 }
 
 }  // namespace views