Relanding 49339

It was unjustly reverted due to flaky unit-test failure. Original review: http://codereview.chromium.org/2358003 TBR=ctguil@chromium.org Review URL: http://codereview.chromium.org/2720003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@49344 0039d316-1c4b-4281-b951-d872f2087c98
author: jcivelli@chromium.org <jcivelli@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> 2010-06-10 00:22:56 +0000
committer: jcivelli@chromium.org <jcivelli@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> 2010-06-10 00:22:56 +0000
commit: f017d8da88460724e9405c3c61e346ecca36a1a2 (patch)
tree: 38f6f689a5099d49ca7c491395c8bb53a7787456
parent: ead48028cab08493a3a5c897bcf779602d1cbe85 (diff)
download: chromium_src-f017d8da88460724e9405c3c61e346ecca36a1a2.zip
chromium_src-f017d8da88460724e9405c3c61e346ecca36a1a2.tar.gz
chromium_src-f017d8da88460724e9405c3c61e346ecca36a1a2.tar.bz2
10 files changed, 126 insertions, 50 deletions
diff --git a/chrome/browser/accessibility_win_browsertest.cc b/chrome/browser/accessibility_win_browsertest.cc
index 580a8fe..2199a07 100644
--- a/chrome/browser/accessibility_win_browsertest.cc
+++ b/chrome/browser/accessibility_win_browsertest.cc
@@ -19,13 +19,6 @@
 namespace {
 
 class AccessibilityWinBrowserTest : public InProcessBrowserTest {
- public:
-  void SetUpCommandLine(CommandLine* command_line) {
-    // Turns on the accessibility in the renderer. Off by default until
-    // http://crbug.com/25564 is fixed.
-    command_line->AppendSwitch(switches::kEnableRendererAccessibility);
-  }
-
  protected:
   IAccessible* GetRenderWidgetHostViewClientAccessible();
 };
@@ -181,6 +174,20 @@ void AccessibleChecker::CheckAccessibleChildren(IAccessible* parent) {
 
 IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
                        TestRendererAccessibilityTree) {
+  // By requesting an accessible chrome will believe a screen reader has been
+  // detected.
+  ScopedComPtr<IAccessible> document_accessible(
+      GetRenderWidgetHostViewClientAccessible());
+
+  // The initial accessible returned should have state STATE_SYSTEM_BUSY while
+  // the accessibility tree is being requested from the renderer.
+  VARIANT var_state;
+  HRESULT hr = document_accessible->
+      get_accState(CreateI4Variant(CHILDID_SELF), &var_state);
+  EXPECT_EQ(hr, S_OK);
+  EXPECT_EQ(V_VT(&var_state), VT_I4);
+  EXPECT_EQ(V_I4(&var_state), STATE_SYSTEM_BUSY);
+
   GURL tree_url(
       "data:text/html,<html><head><title>Accessibility Win Test</title></head>"
       "<body><input type='button' value='push' /><input type='checkbox' />"
@@ -189,8 +196,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
   ui_test_utils::WaitForNotification(
       NotificationType::RENDER_VIEW_HOST_ACCESSIBILITY_TREE_UPDATED);
 
-  ScopedComPtr<IAccessible> document_accessible(
-      GetRenderWidgetHostViewClientAccessible());
+  document_accessible = GetRenderWidgetHostViewClientAccessible();
   ASSERT_NE(document_accessible.get(), reinterpret_cast<IAccessible*>(NULL));
 
   AccessibleChecker button_checker(L"push", ROLE_SYSTEM_PUSHBUTTON);
@@ -208,7 +214,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
 
   // Check that document accessible has a parent accessible.
   ScopedComPtr<IDispatch> parent_dispatch;
-  HRESULT hr = document_accessible->get_accParent(parent_dispatch.Receive());
+  hr = document_accessible->get_accParent(parent_dispatch.Receive());
   EXPECT_EQ(hr, S_OK);
   EXPECT_NE(parent_dispatch, reinterpret_cast<IDispatch*>(NULL));
 
diff --git a/chrome/browser/renderer_host/render_widget_host.cc b/chrome/browser/renderer_host/render_widget_host.cc
index 673a237..125c8a9 100644
--- a/chrome/browser/renderer_host/render_widget_host.cc
+++ b/chrome/browser/renderer_host/render_widget_host.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/renderer_host/render_widget_host.h"
 
 #include "base/auto_reset.h"
+#include "base/command_line.h"
 #include "base/histogram.h"
 #include "base/keyboard_codes.h"
 #include "base/message_loop.h"
@@ -16,6 +17,7 @@
 #include "chrome/browser/renderer_host/render_widget_host_painting_observer.h"
 #include "chrome/browser/renderer_host/render_widget_host_view.h"
 #include "chrome/browser/renderer_host/video_layer.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/render_messages.h"
 #include "webkit/glue/webcursor.h"
@@ -58,6 +60,9 @@ static const int kHungRendererDelayMs = 20000;
 // in trailing scrolls after the user ends their input.
 static const int kMaxTimeBetweenWheelMessagesMs = 250;
 
+// static
+bool RenderWidgetHost::renderer_accessible_ = false;
+
 ///////////////////////////////////////////////////////////////////////////////
 // RenderWidgetHost
 
@@ -1111,6 +1116,35 @@ void RenderWidgetHost::RequestAccessibilityTree() {
   Send(new ViewMsg_GetAccessibilityTree(routing_id()));
 }
 
+void RenderWidgetHost::SetDocumentLoaded(bool document_loaded) {
+  document_loaded_ = document_loaded;
+
+  if (!document_loaded_)
+    requested_accessibility_tree_ = false;
+
+  if (renderer_accessible_ && document_loaded_) {
+    RequestAccessibilityTree();
+    requested_accessibility_tree_ = true;
+  }
+}
+
+void RenderWidgetHost::EnableRendererAccessibility() {
+  if (renderer_accessible_)
+    return;
+
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisableRendererAccessibility)) {
+    return;
+  }
+
+  renderer_accessible_ = true;
+
+  if (document_loaded_ && !requested_accessibility_tree_) {
+    RequestAccessibilityTree();
+    requested_accessibility_tree_ = true;
+  }
+}
+
 void RenderWidgetHost::SetAccessibilityFocus(int acc_obj_id) {
   Send(new ViewMsg_SetAccessibilityFocus(routing_id(), acc_obj_id));
 }
diff --git a/chrome/browser/renderer_host/render_widget_host.h b/chrome/browser/renderer_host/render_widget_host.h
index ab64ef7..27bb5dd 100644
--- a/chrome/browser/renderer_host/render_widget_host.h
+++ b/chrome/browser/renderer_host/render_widget_host.h
@@ -6,6 +6,8 @@
 #define CHROME_BROWSER_RENDERER_HOST_RENDER_WIDGET_HOST_H_
 
 #include <deque>
+#include <string>
+#include <vector>
 
 #include "app/surface/transport_dib.h"
 #include "base/gtest_prod_util.h"
@@ -370,6 +372,14 @@ class RenderWidgetHost : public IPC::Channel::Listener,
   // Requests a snapshot of an accessible DOM tree from the renderer.
   void RequestAccessibilityTree();
 
+  // Aid for determining when an accessibility tree request can be made. Set by
+  // TabContents to true on document load and to false on page nativigation.
+  void SetDocumentLoaded(bool document_loaded);
+
+  // Enable renderer accessibility. This should only be called when a
+  // screenreader is detected.
+  void EnableRendererAccessibility();
+
   // Relays a request from assistive technology to set focus to the
   // node with this accessibility object id.
   void SetAccessibilityFocus(int acc_obj_id);
@@ -530,6 +540,10 @@ class RenderWidgetHost : public IPC::Channel::Listener,
   // input messages to be coalesced.
   void ProcessWheelAck();
 
+  // True if renderer accessibility is enabled. This should only be set when a
+  // screenreader is detected as it can potentially slow down Chrome.
+  static bool renderer_accessible_;
+
   // The View associated with the RenderViewHost. The lifetime of this object
   // is associated with the lifetime of the Render process. If the Renderer
   // crashes, its View is destroyed and this pointer becomes NULL, even though
@@ -667,6 +681,14 @@ class RenderWidgetHost : public IPC::Channel::Listener,
   // changed.
   bool suppress_next_char_events_;
 
+  // Keep track of if we have a loaded document so that we can request an
+  // accessibility tree on demand when renderer accessibility is enabled.
+  bool document_loaded_;
+
+  // Keep track of if we've already requested the accessibility tree so
+  // we don't do it more than once.
+  bool requested_accessibility_tree_;
+
   // Optional video YUV layer for used for out-of-process compositing.
   scoped_ptr<VideoLayer> video_layer_;
 
diff --git a/chrome/browser/renderer_host/render_widget_host_view_win.cc b/chrome/browser/renderer_host/render_widget_host_view_win.cc
index e195317..a6ce128 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_win.cc
+++ b/chrome/browser/renderer_host/render_widget_host_view_win.cc
@@ -252,7 +252,6 @@ void DrawDeemphasized(const gfx::Rect& paint_rect,
                      paint_rect.width(), paint_rect.height());
   canvas.getTopPlatformDevice().drawToHDC(paint_dc, paint_rect.x(),
                                           paint_rect.y(), NULL);
-
 }
 
 }  // namespace
@@ -284,12 +283,15 @@ RenderWidgetHostViewWin::RenderWidgetHostViewWin(RenderWidgetHost* widget)
       is_loading_(false),
       visually_deemphasized_(false) {
   render_widget_host_->set_view(this);
-  renderer_accessible_ =
-      CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableRendererAccessibility);
   registrar_.Add(this,
                  NotificationType::RENDERER_PROCESS_TERMINATED,
                  NotificationService::AllSources());
+
+  BOOL screenreader_running = FALSE;
+  if (SystemParametersInfo(SPI_GETSCREENREADER, 0, &screenreader_running, 0) &&
+      screenreader_running) {
+    render_widget_host_->EnableRendererAccessibility();
+  }
 }
 
 RenderWidgetHostViewWin::~RenderWidgetHostViewWin() {
@@ -1483,6 +1485,9 @@ void RenderWidgetHostViewWin::UpdateAccessibilityTree(
     const webkit_glue::WebAccessibility& tree) {
   browser_accessibility_manager_.reset(
       new BrowserAccessibilityManager(m_hWnd, tree, this));
+
+  ::NotifyWinEvent(
+      IA2_EVENT_DOCUMENT_LOAD_COMPLETE, m_hWnd, OBJID_CLIENT, CHILDID_SELF);
 }
 
 void RenderWidgetHostViewWin::OnAccessibilityFocusChange(int acc_obj_id) {
@@ -1542,18 +1547,46 @@ void RenderWidgetHostViewWin::AccessibilityDoDefaultAction(int acc_obj_id) {
 
 LRESULT RenderWidgetHostViewWin::OnGetObject(UINT message, WPARAM wparam,
                                              LPARAM lparam, BOOL& handled) {
-  // TODO(dmazzoni): http://crbug.com/25564 Disabling accessibility in the
-  // renderer is a temporary work-around until that bug is fixed.
-  if (!renderer_accessible_) {
+  if (lparam != OBJID_CLIENT) {
     handled = false;
     return static_cast<LRESULT>(0L);
   }
 
-  if (lparam == OBJID_CLIENT && browser_accessibility_manager_.get()) {
+  if (!browser_accessibility_manager_.get()) {
+    render_widget_host_->EnableRendererAccessibility();
+
+    if (!loading_accessible_.get()) {
+      // Create IAccessible to return while waiting for the accessibility tree
+      // from the renderer.
+      HRESULT hr = ::CreateStdAccessibleObject(
+          m_hWnd, OBJID_CLIENT, IID_IAccessible,
+          reinterpret_cast<void **>(&loading_accessible_));
+
+      // Annotate with STATE_SYSTEM_BUSY to indicate that the page is loading.
+      // We annotate the HWND, not the loading_accessible IAccessible, but the
+      // IAccessible will reflect the state annotation.
+      ScopedComPtr<IAccPropServices> pAccPropServices;
+      hr = CoCreateInstance(CLSID_AccPropServices, NULL, CLSCTX_SERVER,
+          IID_IAccPropServices, reinterpret_cast<void**>(&pAccPropServices));
+      if (SUCCEEDED(hr)) {
+        VARIANT var;
+        var.vt = VT_I4;
+        var.lVal = STATE_SYSTEM_BUSY;
+        pAccPropServices->SetHwndProp(m_hWnd, OBJID_CLIENT,
+            CHILDID_SELF, PROPID_ACC_STATE, var);
+      }
+    }
+
+    if (loading_accessible_.get()) {
+      return LresultFromObject(
+          IID_IAccessible, wparam,
+          static_cast<IAccessible*>(loading_accessible_));
+    }
+  } else {
     BrowserAccessibility* root = browser_accessibility_manager_->GetRoot();
     if (root) {
       return LresultFromObject(IID_IAccessible, wparam,
-                               static_cast<IAccessible*>(root->NewReference()));
+          static_cast<IAccessible*>(root->NewReference()));
     }
   }
 
diff --git a/chrome/browser/renderer_host/render_widget_host_view_win.h b/chrome/browser/renderer_host/render_widget_host_view_win.h
index c298f58..9580e88 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_win.h
+++ b/chrome/browser/renderer_host/render_widget_host_view_win.h
@@ -10,6 +10,8 @@
 #include <atlcrack.h>
 #include <atlmisc.h>
 
+#include <vector>
+
 #include "base/scoped_comptr_win.h"
 #include "base/scoped_ptr.h"
 #include "base/task.h"
@@ -249,6 +251,9 @@ class RenderWidgetHostViewWin
   // Whether the window should be activated.
   bool IsActivatable() const;
 
+  // MSAA IAccessible returned while page contents is loading.
+  ScopedComPtr<IAccessible> loading_accessible_;
+
   // The associated Model.
   RenderWidgetHost* render_widget_host_;
 
@@ -320,11 +325,6 @@ class RenderWidgetHostViewWin
   // value returns true for is_null() if we are not recording whiteout times.
   base::TimeTicks whiteout_start_time_;
 
-  // Whether the renderer is made accessible.
-  // TODO(jcampan): http://b/issue?id=1432077 This is a temporary work-around
-  // until that bug is fixed.
-  bool renderer_accessible_;
-
   // The time it took after this view was selected for it to be fully painted.
   base::TimeTicks tab_switch_paint_time_;
 
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 2fd0504..e2bbfe8 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -296,7 +296,6 @@ TabContents::TabContents(Profile* profile,
       opener_dom_ui_type_(DOMUIFactory::kNoDOMUI),
       language_state_(&controller_),
       geolocation_settings_state_(profile),
-      requested_accessibility_tree_(false),
       closed_by_user_gesture_(false) {
   ClearBlockedContentSettings();
   renderer_preferences_util::UpdateFromSystemSettings(
@@ -355,10 +354,6 @@ TabContents::TabContents(Profile* profile,
   // Set-up the showing of the omnibox search infobar if applicable.
   if (OmniboxSearchHint::IsEnabled(profile))
     omnibox_search_hint_.reset(new OmniboxSearchHint(this));
-
-  renderer_accessible_ =
-    CommandLine::ForCurrentProcess()->HasSwitch(
-        switches::kEnableRendererAccessibility);
 }
 
 TabContents::~TabContents() {
@@ -2147,10 +2142,8 @@ void TabContents::DidFailProvisionalLoadWithError(
 
 void TabContents::DocumentLoadedInFrame() {
   controller_.DocumentLoadedInFrame();
-  if (renderer_accessible_ && !requested_accessibility_tree_) {
-    render_view_host()->RequestAccessibilityTree();
-    requested_accessibility_tree_ = true;
-  }
+
+  render_view_host()->SetDocumentLoaded(true);
 }
 
 void TabContents::OnContentBlocked(ContentSettingsType type) {
@@ -2320,7 +2313,7 @@ void TabContents::DidNavigate(RenderViewHost* rvh,
                               const ViewHostMsg_FrameNavigate_Params& params) {
   int extra_invalidate_flags = 0;
 
-  requested_accessibility_tree_ = false;
+  render_view_host()->SetDocumentLoaded(false);
 
   if (PageTransition::IsMainFrame(params.transition)) {
     bool was_bookmark_bar_visible = ShouldShowBookmarkBar();
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index 42c409b..62dd896 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -1282,15 +1282,6 @@ class TabContents : public PageNavigator,
   // Manages information about Geolocation API usage in this page.
   GeolocationSettingsState geolocation_settings_state_;
 
-  // Whether the renderer is made accessible.
-  // TODO(dmazzoni): http://crbug.com/25564 This is a temporary work-around
-  // until that bug is fixed.
-  bool renderer_accessible_;
-
-  // Keep track of if we've already requested the accessibility tree so
-  // we don't do it more than once.
-  bool requested_accessibility_tree_;
-
   // See description above setter.
   bool closed_by_user_gesture_;
 
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 4187b10..404c18c 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -186,6 +186,9 @@ const char kDisablePromptOnRepost[]         = "disable-prompt-on-repost";
 // this option is specified or not.
 const char kDisableRemoteFonts[]            = "disable-remote-fonts";
 
+// Turns off the accessibility in the renderer.
+const char kDisableRendererAccessibility[]  = "disable-renderer-accessibility";
+
 // Disable session storage.
 const char kDisableSessionStorage[]         = "disable-session-storage";
 
@@ -339,10 +342,6 @@ const char kEnablePrintPreview[]            = "enable-print-preview";
 // Enable Privacy Blacklists.
 const char kEnablePrivacyBlacklists[]       = "enable-privacy-blacklists";
 
-// Turns on the accessibility in the renderer.  Off by default until
-// http://b/issue?id=1432077 is fixed.
-const char kEnableRendererAccessibility[]   = "enable-renderer-accessibility";
-
 // Enables StatsTable, logging statistics to a global named shared memory table.
 const char kEnableStatsTable[]              = "enable-stats-table";
 
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 4a620e6..e7d3ec2 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -67,6 +67,7 @@ extern const char kDisablePlugins[];
 extern const char kDisablePopupBlocking[];
 extern const char kDisablePromptOnRepost[];
 extern const char kDisableRemoteFonts[];
+extern const char kDisableRendererAccessibility[];
 extern const char kDisableSessionStorage[];
 extern const char kDisableSharedWorkers[];
 extern const char kDisableSiteSpecificQuirks[];
@@ -112,7 +113,6 @@ extern const char kEnablePreparsedJsCaching[];
 extern const char kEnablePreconnect[];
 extern const char kEnablePrintPreview[];
 extern const char kEnablePrivacyBlacklists[];
-extern const char kEnableRendererAccessibility[];
 extern const char kEnableStatsTable[];
 extern const char kEnableSync[];
 extern const char kEnableSyncAutofill[];
diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc
index 0b62298..ee1ed5e 100644
--- a/chrome_frame/chrome_frame_automation.cc
+++ b/chrome_frame/chrome_frame_automation.cc
@@ -296,8 +296,6 @@ void ProxyFactory::CreateProxy(ProxyFactory::ProxyCacheEntry* entry,
   command_line->AppendSwitch(switches::kNoErrorDialogs);
 #endif
 
-  command_line->AppendSwitch(switches::kEnableRendererAccessibility);
-
   // In headless mode runs like reliability test runs we want full crash dumps
   // from chrome.
   if (IsHeadlessMode())
author	jcivelli@chromium.org <jcivelli@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>	2010-06-10 00:22:56 +0000
committer	jcivelli@chromium.org <jcivelli@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>	2010-06-10 00:22:56 +0000
commit	f017d8da88460724e9405c3c61e346ecca36a1a2 (patch)
tree	38f6f689a5099d49ca7c491395c8bb53a7787456
parent	ead48028cab08493a3a5c897bcf779602d1cbe85 (diff)
download	chromium_src-f017d8da88460724e9405c3c61e346ecca36a1a2.zip chromium_src-f017d8da88460724e9405c3c61e346ecca36a1a2.tar.gz chromium_src-f017d8da88460724e9405c3c61e346ecca36a1a2.tar.bz2