Fix case where BrowserAccessibilityManagerWin is deleted before AccessibleHWND

Also adds a unit test and enables unit tests that should have been running
on Aura previously.

BUG=327354
TBR=jam

Review URL: https://codereview.chromium.org/103243004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@240502 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 68 insertions, 6 deletions

diff --git a/content/browser/accessibility/browser_accessibility_manager_win.cc b/content/browser/accessibility/browser_accessibility_manager_win.cc
index d4dbe6d..4e5f8dc 100644
--- a/content/browser/accessibility/browser_accessibility_manager_win.cc
+++ b/content/browser/accessibility/browser_accessibility_manager_win.cc
@@ -64,9 +64,14 @@ class AccessibleHWND
 
   IAccessible* window_accessible() { return window_accessible_; }
 
+  void OnManagerDeleted() {
+    manager_ = NULL;
+  }
+
  protected:
   virtual void OnFinalMessage(HWND hwnd) OVERRIDE {
-    manager_->OnAccessibleHwndDeleted();
+    if (manager_)
+      manager_->OnAccessibleHwndDeleted();
     delete this;
   }
 
@@ -74,7 +79,7 @@ class AccessibleHWND
   LRESULT OnGetObject(UINT message,
                       WPARAM w_param,
                       LPARAM l_param) {
-    if (OBJID_CLIENT != l_param)
+    if (OBJID_CLIENT != l_param || !manager_)
       return static_cast<LRESULT>(0L);
 
     base::win::ScopedComPtr<IAccessible> root(
@@ -124,6 +129,8 @@ BrowserAccessibilityManagerWin::~BrowserAccessibilityManagerWin() {
     tracked_scroll_object_->Release();
     tracked_scroll_object_ = NULL;
   }
+  if (accessible_hwnd_)
+    accessible_hwnd_->OnManagerDeleted();
 }
 
 // static
@@ -305,7 +312,14 @@ BrowserAccessibilityWin* BrowserAccessibilityManagerWin::GetFromUniqueIdWin(
 }
 
 void BrowserAccessibilityManagerWin::OnAccessibleHwndDeleted() {
+  // If the AccessibleHWND is deleted, |parent_hwnd_| and
+  // |parent_iaccessible_| are no longer valid either, since they were
+  // derived from AccessibleHWND. We don't have to restore them to
+  // previous values, though, because this should only happen
+  // during the destruct sequence for this window.
   accessible_hwnd_ = NULL;
+  parent_hwnd_ = NULL;
+  parent_iaccessible_ = NULL;
 }
 
 }  // namespace content
diff --git a/content/browser/accessibility/browser_accessibility_win_unittest.cc b/content/browser/accessibility/browser_accessibility_win_unittest.cc
index c1159e6..408fb4df 100644
--- a/content/browser/accessibility/browser_accessibility_win_unittest.cc
+++ b/content/browser/accessibility/browser_accessibility_win_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/win/scoped_variant.h"
 #include "content/browser/accessibility/browser_accessibility_manager.h"
 #include "content/browser/accessibility/browser_accessibility_manager_win.h"
+#include "content/browser/accessibility/browser_accessibility_state_impl.h"
 #include "content/browser/accessibility/browser_accessibility_win.h"
 #include "content/common/accessibility_messages.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -680,4 +681,54 @@ TEST_F(BrowserAccessibilityTest, TestCreateEmptyDocument) {
   ASSERT_EQ(0, CountedBrowserAccessibility::num_instances());
 }
 
+#if defined(USE_AURA)
+TEST(BrowserAccessibilityManagerWinTest, TestAccessibleHWND) {
+  HWND desktop_hwnd = GetDesktopWindow();
+  base::win::ScopedComPtr<IAccessible> desktop_hwnd_iaccessible;
+  ASSERT_EQ(S_OK, AccessibleObjectFromWindow(
+      desktop_hwnd, OBJID_CLIENT,
+      IID_IAccessible,
+      reinterpret_cast<void**>(desktop_hwnd_iaccessible.Receive())));
+
+  scoped_ptr<BrowserAccessibilityManagerWin> manager(
+      new BrowserAccessibilityManagerWin(
+          desktop_hwnd,
+          desktop_hwnd_iaccessible,
+          BrowserAccessibilityManagerWin::GetEmptyDocument(),
+          NULL));
+  ASSERT_EQ(desktop_hwnd, manager->parent_hwnd());
+
+  // Enabling screen reader support and calling MaybeCallNotifyWinEvent
+  // should trigger creating the AccessibleHWND, and we should now get a
+  // new parent_hwnd with the right window class to fool older screen
+  // readers.
+  BrowserAccessibilityStateImpl::GetInstance()->OnScreenReaderDetected();
+  manager->MaybeCallNotifyWinEvent(0, 0);
+  HWND new_parent_hwnd = manager->parent_hwnd();
+  ASSERT_NE(desktop_hwnd, new_parent_hwnd);
+  WCHAR hwnd_class_name[256];
+  ASSERT_NE(0, GetClassName(new_parent_hwnd, hwnd_class_name, 256));
+  ASSERT_STREQ(L"Chrome_RenderWidgetHostHWND", hwnd_class_name);
+
+  // Destroy the hwnd explicitly; that should trigger clearing parent_hwnd().
+  DestroyWindow(new_parent_hwnd);
+  ASSERT_EQ(NULL, manager->parent_hwnd());
+
+  // Now create it again.
+  manager.reset(
+      new BrowserAccessibilityManagerWin(
+          desktop_hwnd,
+          desktop_hwnd_iaccessible,
+          BrowserAccessibilityManagerWin::GetEmptyDocument(),
+          NULL));
+  manager->MaybeCallNotifyWinEvent(0, 0);
+  new_parent_hwnd = manager->parent_hwnd();
+  ASSERT_FALSE(NULL == new_parent_hwnd);
+
+  // This time, destroy the manager first, make sure the AccessibleHWND doesn't
+  // crash on destruction (to be caught by SyzyASAN or other tools).
+  manager.reset(NULL);
+}
+#endif
+
 }  // namespace content
diff --git a/content/content_tests.gypi b/content/content_tests.gypi
index e1e9e5d0..2bdfe0b 100644
--- a/content/content_tests.gypi
+++ b/content/content_tests.gypi
@@ -771,10 +771,7 @@
         ['use_aura==1', {
           'dependencies': [
             '../ui/aura/aura.gyp:aura',
-          ],
-          'sources!': [
-            'browser/accessibility/browser_accessibility_win_unittest.cc',
-          ],
+          ]
         }],
         ['use_aura==1 or toolkit_views==1', {
           'dependencies': [