Focus following for the non-editable controls on web page in magnifier mode.

chrome/browser/chromeos/accessibility/magnification_manager.cc

Index: chrome/browser/chromeos/accessibility/magnification_manager.cc
diff --git a/chrome/browser/chromeos/accessibility/magnification_manager.cc b/chrome/browser/chromeos/accessibility/magnification_manager.cc
index e1231b9bab1a1c19d90460fa383b8d10b5acd4d9..347275c739c9df0d72de532d2830abb1bd4b92d0 100644
--- a/chrome/browser/chromeos/accessibility/magnification_manager.cc
+++ b/chrome/browser/chromeos/accessibility/magnification_manager.cc
@@ -27,6 +27,7 @@
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
+#include "content/public/browser/render_view_host.h"
 
 namespace chromeos {
 
@@ -47,7 +48,8 @@ class MagnificationManagerImpl : public MagnificationManager,
         magnifier_scale_pref_handler_(
             prefs::kAccessibilityScreenMagnifierScale),
         type_(ui::kDefaultMagnifierType),
-        enabled_(false) {
+        enabled_(false),
+        monitor_focus_change_in_page_(false) {
     registrar_.Add(this,
                    chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE,
                    content::NotificationService::AllSources());
@@ -155,6 +157,7 @@ class MagnificationManagerImpl : public MagnificationManager,
     if (type_ == ui::MAGNIFIER_FULL) {
       ash::Shell::GetInstance()->magnification_controller()->SetEnabled(
           enabled_);
+      MonitorFocusInPageChange();
     } else {
       ash::Shell::GetInstance()->partial_magnification_controller()->SetEnabled(
           enabled_);
@@ -213,6 +216,18 @@ class MagnificationManagerImpl : public MagnificationManager,
 #endif
   }
 
+  void MonitorFocusInPageChange() {
+    if (enabled_ && !monitor_focus_change_in_page_) {
+      registrar_.Add(this, content::NOTIFICATION_FOCUS_CHANGED_IN_PAGE,
+                     content::NotificationService::AllSources());
+      monitor_focus_change_in_page_ = true;
+    } else if (!enabled_ && monitor_focus_change_in_page_) {
+      registrar_.Remove(this, content::NOTIFICATION_FOCUS_CHANGED_IN_PAGE,
+                        content::NotificationService::AllSources());
+      monitor_focus_change_in_page_ = false;
+    }

[sadrul, 2014/12/12 17:13:49]

Can you use a RenderViewObserver instead of using notification?

[jennyz, 2014/12/12 23:14:45]

Is there a strong reason you prefer using RenderViewObserver instead of
notification? Since MagnificationManager has already been a
NotificationObserver, I thought it is just simple to register one more
notification here. 

[sadrul, 2014/12/15 18:37:16]

We should avoid using NotificationService when there's an alternate way of doing
things (e.g. an observer interface). (see various relevant threads on
chromium-dev@). From a quick look, it looks like only test code currently uses
NOTIFICATION_FOCUS_CHANGED_IN_PAGE, so it would be nice to not add its use in
chrome.

However, looks like RenderViewObserver is actually for renderer process code,
and so it can't be used here for UI code.

[jennyz, 2014/12/15 23:24:55]

Talked to jam@, he is ok to use the existing notification in this case.

+  }
+
   // content::NotificationObserver implementation:
   virtual void Observe(int type,
                        const content::NotificationSource& source,
@@ -241,6 +256,15 @@ class MagnificationManagerImpl : public MagnificationManager,
           SetProfile(NULL);
         break;
       }
+      case content::NOTIFICATION_FOCUS_CHANGED_IN_PAGE: {
+        content::FocusedNodeDetails* node_details =
+            content::Details<content::FocusedNodeDetails>(details).ptr();
+        ash::Shell::GetInstance()
+            ->magnification_controller()
+            ->HandleFocusedNodeChanged(node_details->is_editable_node,
+                                       node_details->node_bounds_in_screen);
+        break;
+      }
     }
   }
 
@@ -253,6 +277,7 @@ class MagnificationManagerImpl : public MagnificationManager,
 
   ui::MagnifierType type_;
   bool enabled_;
+  bool monitor_focus_change_in_page_;

[oshima, 2014/12/12 00:48:45]

optional: observing_focus_change_in_page_ maybe better, but you can keep it if
you want.

[jennyz, 2014/12/12 23:14:45]

Done.

 
   content::NotificationRegistrar registrar_;
   scoped_ptr<PrefChangeRegistrar> pref_change_registrar_;