Commit 48476970 authored by ananta@chromium.org's avatar ananta@chromium.org

Fix a use after free crasher in the BrowserAccessibilityManagerWinTest.TestAccessibleHWND test.

The crash occurs because a scoped_ptr instance which holds the TestLegacyRenderWidgetHostHWND class
is left with a dangling pointer to the LegacyRenderWidgetHostHWND instance which is destroyed via
DestroyWindow.

Fix is to first reset the scoped_ptr which in turn destroys the legacy window and the instance.
The DestroyWindow call is removed as it is not needed anymore.

I also added a call to the CreateATLModuleIfNeeded helper function in BrowserAccessibility::Create as I was
hitting a crash due to a NULL AtlModule. We do this already in the ctor of the BrowserAccessibilityManagerWin
class. However that is too late as we have base classes which create COM objects.

This crash only happens in the component builds.

BUG=393228

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282759 0039d316-1c4b-4281-b951-d872f2087c98
parent 9136bef4
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include "ui/accessibility/ax_text_utils.h" #include "ui/accessibility/ax_text_utils.h"
#include "ui/base/win/accessibility_ids_win.h" #include "ui/base/win/accessibility_ids_win.h"
#include "ui/base/win/accessibility_misc_utils.h" #include "ui/base/win/accessibility_misc_utils.h"
#include "ui/base/win/atl_module.h"
namespace content { namespace content {
...@@ -179,6 +180,7 @@ STDMETHODIMP BrowserAccessibilityRelation::get_targets(long max_targets, ...@@ -179,6 +180,7 @@ STDMETHODIMP BrowserAccessibilityRelation::get_targets(long max_targets,
// static // static
BrowserAccessibility* BrowserAccessibility::Create() { BrowserAccessibility* BrowserAccessibility::Create() {
ui::win::CreateATLModuleIfNeeded();
CComObject<BrowserAccessibilityWin>* instance; CComObject<BrowserAccessibilityWin>* instance;
HRESULT hr = CComObject<BrowserAccessibilityWin>::CreateInstance(&instance); HRESULT hr = CComObject<BrowserAccessibilityWin>::CreateInstance(&instance);
DCHECK(SUCCEEDED(hr)); DCHECK(SUCCEEDED(hr));
......
...@@ -744,8 +744,10 @@ TEST(BrowserAccessibilityManagerWinTest, TestAccessibleHWND) { ...@@ -744,8 +744,10 @@ TEST(BrowserAccessibilityManagerWinTest, TestAccessibleHWND) {
ASSERT_NE(0, GetClassName(new_parent_hwnd, hwnd_class_name, 256)); ASSERT_NE(0, GetClassName(new_parent_hwnd, hwnd_class_name, 256));
ASSERT_STREQ(L"Chrome_RenderWidgetHostHWND", hwnd_class_name); ASSERT_STREQ(L"Chrome_RenderWidgetHostHWND", hwnd_class_name);
// Destroy the hwnd explicitly; that should trigger clearing parent_hwnd(). // Destroy the TestLegacyRenderWidgetHostHWND instance. That should in turn
DestroyWindow(new_parent_hwnd); // destroy the hwnd, which should clear the parent_hwnd().
accessibility_test.reset(NULL);
ASSERT_EQ(FALSE, ::IsWindow(new_parent_hwnd));
ASSERT_EQ(NULL, manager->parent_hwnd()); ASSERT_EQ(NULL, manager->parent_hwnd());
// Now create it again. // Now create it again.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment