Create only a single LegacyRenderWidgetHostHWND per WebContentsViewAura.

content/browser/renderer_host/legacy_render_widget_host_win.cc

Index: content/browser/renderer_host/legacy_render_widget_host_win.cc
diff --git a/content/browser/renderer_host/legacy_render_widget_host_win.cc b/content/browser/renderer_host/legacy_render_widget_host_win.cc
index db8ca80c86957e292ce9188142f342b0fc8ad42b..4c179ba1053ad9511e472a771e547fd45dde140c 100644
--- a/content/browser/renderer_host/legacy_render_widget_host_win.cc
+++ b/content/browser/renderer_host/legacy_render_widget_host_win.cc
@@ -7,7 +7,7 @@
 #include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/win/windows_version.h"
-#include "content/browser/accessibility/browser_accessibility_manager_win.h"
+#include "content/browser/accessibility/browser_accessibility_manager.h"
 #include "content/browser/accessibility/browser_accessibility_win.h"
 #include "content/browser/renderer_host/render_widget_host_view_aura.h"
 #include "content/public/browser/browser_accessibility_state.h"
@@ -21,26 +21,49 @@
 
 namespace content {
 
+namespace {
+
+class SimpleDelegate : public LegacyRenderWidgetHostHWNDDelegate {
+ public:
+  explicit SimpleDelegate(BrowserAccessibilityManager* manager)
+      : manager_(manager) {}
+
+  virtual BrowserAccessibilityManager* GetRootBrowserAccessibilityManager()
+      OVERRIDE {
+    return manager_;
+  }
+
+  virtual void OnLegacyHwndDestroyed(LegacyRenderWidgetHostHWND* owner)
+      OVERRIDE {
+    delete owner;
+    delete this;
+  }
+
+ private:
+  BrowserAccessibilityManager* manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(SimpleDelegate);
+};
+
 // A custom MSAA object id used to determine if a screen reader or some
 // other client is listening on MSAA events - if so, we enable full web
 // accessibility support.
 const int kIdScreenReaderHoneyPot = 1;
 
+}  // namespace
+
 // static
 LegacyRenderWidgetHostHWND* LegacyRenderWidgetHostHWND::Create(
-    HWND parent) {
-  // content_unittests passes in the desktop window as the parent. We allow
-  // the LegacyRenderWidgetHostHWND instance to be created in this case for
-  // these tests to pass.
+    HWND parent,
+    LegacyRenderWidgetHostHWNDDelegate* delegate) {
   if (CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kDisableLegacyIntermediateWindow) ||
-      (!GetWindowEventTarget(parent) && parent != ::GetDesktopWindow()))
+          switches::kDisableLegacyIntermediateWindow)) {
     return NULL;
+  }
 
   LegacyRenderWidgetHostHWND* legacy_window_instance =
-      new LegacyRenderWidgetHostHWND(parent);
-  // If we failed to create the child, or if the switch to disable the legacy
-  // window is passed in, then return NULL.
+      new LegacyRenderWidgetHostHWND(parent, delegate);
+  // If we failed to create the child, return NULL.
   if (!::IsWindow(legacy_window_instance->hwnd())) {
     delete legacy_window_instance;
     return NULL;
@@ -49,9 +72,16 @@ LegacyRenderWidgetHostHWND* LegacyRenderWidgetHostHWND::Create(
   return legacy_window_instance;
 }
 
-void LegacyRenderWidgetHostHWND::Destroy() {
-  if (::IsWindow(hwnd()))
-    ::DestroyWindow(hwnd());
+// static
+LegacyRenderWidgetHostHWND* LegacyRenderWidgetHostHWND::CreateForTesting(
+    BrowserAccessibilityManager* manager) {
+  LegacyRenderWidgetHostHWND* legacy_window_instance =
+      new LegacyRenderWidgetHostHWND(::GetDesktopWindow(),
+                                     new SimpleDelegate(manager));
+  CHECK(legacy_window_instance);
+  CHECK(::IsWindow(legacy_window_instance->hwnd()));
+  legacy_window_instance->Init();
+  return legacy_window_instance;
 }
 
 void LegacyRenderWidgetHostHWND::UpdateParent(HWND parent) {
@@ -70,12 +100,6 @@ HWND LegacyRenderWidgetHostHWND::GetParent() {
   return ::GetParent(hwnd());
 }
 
-void LegacyRenderWidgetHostHWND::OnManagerDeleted(
-    content::BrowserAccessibilityManagerWin* manager) {
-  if (manager_ == manager)
-    manager_ = NULL;
-}
-
 void LegacyRenderWidgetHostHWND::Show() {
   ::ShowWindow(hwnd(), SW_SHOW);
 }
@@ -91,19 +115,14 @@ void LegacyRenderWidgetHostHWND::SetBounds(const gfx::Rect& bounds) {
 }
 
 void LegacyRenderWidgetHostHWND::OnFinalMessage(HWND hwnd) {
-  if (manager_)
-    manager_->OnAccessibleHwndDeleted();
-  if (host_) {
-    host_->OnLegacyWindowDestroyed();
-    host_ = NULL;
-  }
-  delete this;
+  delegate_->OnLegacyHwndDestroyed(this);

[ananta, 2014/07/14 18:51:08]

Please validate whether all shutdown scenarios are addressed correctly. 

For e.g. Destroying the main window frame which some browser tests do. Windows
will
destroy the child implicitly. The other case is the delayed close which occurs
when 
closing the browser window.

NPAPI plugins like flash need to be tested as well. Specifically windowless
flash

[dmazzoni, 2014/07/29 23:03:05]

OK, the "Simplify access to LegacyRenderWidgetHostHWND" change
(https://codereview.chromium.org/415633002/) made this much cleaner and simpler.
Nobody keeps a copy of the HWND around anymore, they just query
LegacyRenderWidgetHostHWND directly every time. And now
LegacyRenderWidgetHostHWND doesn't keep the BrowserAccessibilityManager, it just
queries the IAccessible* it should use from its delegate.

Lifetime issues should be totally automatic now. WebContentsViewAura owns
LegacyRenderWidgetHostHWND and will thus outlive any other object that requests
the LegacyRenderWidgetHostHWND.
I tested 301548 (Video controllers are not working on justin.TV with NPAPI
flash) and it worked fine.

 }
 
-LegacyRenderWidgetHostHWND::LegacyRenderWidgetHostHWND(HWND parent)
-    : manager_(NULL),
-      mouse_tracking_enabled_(false),
-      host_(NULL) {
+LegacyRenderWidgetHostHWND::LegacyRenderWidgetHostHWND(
+    HWND parent,
+    LegacyRenderWidgetHostHWNDDelegate* delegate)
+    : mouse_tracking_enabled_(false),
+      delegate_(delegate) {
   RECT rect = {0};
   Base::Create(parent, rect, L"Chrome Legacy Window",
                WS_CHILDWINDOW | WS_CLIPCHILDREN | WS_CLIPSIBLINGS,
@@ -111,13 +130,15 @@ LegacyRenderWidgetHostHWND::LegacyRenderWidgetHostHWND(HWND parent)
 }
 
 LegacyRenderWidgetHostHWND::~LegacyRenderWidgetHostHWND() {
-  DCHECK(!::IsWindow(hwnd()));
+  if (::IsWindow(hwnd()))
+    ::DestroyWindow(hwnd());
 }
 
 bool LegacyRenderWidgetHostHWND::Init() {
   if (base::win::GetVersion() >= base::win::VERSION_WIN7 &&
-      ui::AreTouchEventsEnabled())
+      ui::AreTouchEventsEnabled()) {
     RegisterTouchWindow(hwnd(), TWF_WANTPALM);
+  }
 
   HRESULT hr = ::CreateStdAccessibleObject(
       hwnd(), OBJID_WINDOW, IID_IAccessible,
@@ -161,11 +182,14 @@ LRESULT LegacyRenderWidgetHostHWND::OnGetObject(UINT message,
     return static_cast<LRESULT>(0L);
   }
 
-  if (OBJID_CLIENT != obj_id || !manager_)
+  DCHECK(delegate_);
+  BrowserAccessibilityManager* manager =
+      delegate_->GetRootBrowserAccessibilityManager();
+  if (OBJID_CLIENT != obj_id || !manager)
     return static_cast<LRESULT>(0L);
 
   base::win::ScopedComPtr<IAccessible> root(
-      manager_->GetRoot()->ToBrowserAccessibilityWin());
+      manager->GetRoot()->ToBrowserAccessibilityWin());
   return LresultFromObject(IID_IAccessible, w_param,
       static_cast<IAccessible*>(root.Detach()));
 }