Create only a single LegacyRenderWidgetHostHWND per WebContentsViewAura.

content/browser/renderer_host/legacy_render_widget_host_win.cc

 // Copyright (c) 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/legacy_render_widget_host_win.h"
 
 #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"
 #include "content/public/common/content_switches.h"
 #include "ui/base/touch/touch_enabled.h"
 #include "ui/base/view_prop.h"
 #include "ui/base/win/internal_constants.h"
 #include "ui/base/win/window_event_target.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/win/dpi.h"
 
 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;
   }
   legacy_window_instance->Init();
   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) {
   ::SetParent(hwnd(), parent);
   // If the new parent is the desktop Window, then we disable the child window
   // to ensure that it does not receive any input events. It should not because
   // of WS_EX_TRANSPARENT. This is only for safety.
   if (parent == ::GetDesktopWindow()) {
     ::EnableWindow(hwnd(), FALSE);
   } else {
     ::EnableWindow(hwnd(), TRUE);
   }
 }
 
 HWND LegacyRenderWidgetHostHWND::GetParent() {
   return ::GetParent(hwnd());
 }
 
-void LegacyRenderWidgetHostHWND::OnManagerDeleted(
-    content::BrowserAccessibilityManagerWin* manager) {
-  if (manager_ == manager)
-    manager_ = NULL;
-}
-
 void LegacyRenderWidgetHostHWND::Show() {
   ::ShowWindow(hwnd(), SW_SHOW);
 }
 
 void LegacyRenderWidgetHostHWND::Hide() {
   ::ShowWindow(hwnd(), SW_HIDE);
 }
 
 void LegacyRenderWidgetHostHWND::SetBounds(const gfx::Rect& bounds) {
   gfx::Rect bounds_in_pixel = gfx::win::DIPToScreenRect(bounds);
   ::SetWindowPos(hwnd(), NULL, bounds_in_pixel.x(), bounds_in_pixel.y(),
                  bounds_in_pixel.width(), bounds_in_pixel.height(), 0);
 }
 
 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,
                WS_EX_TRANSPARENT);
 }
 
 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,
       reinterpret_cast<void **>(window_accessible_.Receive()));
   DCHECK(SUCCEEDED(hr));
 
   if (!BrowserAccessibilityState::GetInstance()->IsAccessibleBrowser()) {
     // Attempt to detect screen readers or other clients who want full
     // accessibility support, by seeing if they respond to this event.
     NotifyWinEvent(EVENT_SYSTEM_ALERT, hwnd(), kIdScreenReaderHoneyPot,
@@ skipping 23 matching lines @@
   // because it sometimes gets sign-extended incorrectly (but not always).
   DWORD obj_id = static_cast<DWORD>(static_cast<DWORD_PTR>(l_param));
 
   if (kIdScreenReaderHoneyPot == obj_id) {
     // When an MSAA client has responded to our fake event on this id,
     // enable screen reader support.
     BrowserAccessibilityState::GetInstance()->OnScreenReaderDetected();
     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()));
 }
 
 // We send keyboard/mouse/touch messages to the parent window via SendMessage.
 // While this works, this has the side effect of converting input messages into
 // sent messages which changes their priority and could technically result
 // in these messages starving other messages in the queue. Additionally
 // keyboard/mouse hooks would not see these messages. The alternative approach
 // is to set and release capture as needed on the parent to ensure that it
@@ skipping 188 matching lines @@
   // generate the legacy WM_VSCROLL/WM_HSCROLL messages.
   // We add these styles to ensure that trackpad/trackpoint scrolling
   // work.
   long current_style = ::GetWindowLong(hwnd(), GWL_STYLE);
   ::SetWindowLong(hwnd(), GWL_STYLE,
                   current_style | WS_VSCROLL | WS_HSCROLL);
   return 0;
 }
 
 }  // namespace content