diff options
author | sky@chromium.org <sky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-11-01 16:57:13 +0000 |
---|---|---|
committer | sky@chromium.org <sky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-11-01 16:57:13 +0000 |
commit | 7c9d829192359328576965fe8a4a52a5ad6c7da3 (patch) | |
tree | 6f06ef0a1bd4f797ad2c9f2968950b625867136c | |
parent | 9d3fcf8bc99dfdfef66b983af69802ddbe76514d (diff) | |
download | chromium_src-7c9d829192359328576965fe8a4a52a5ad6c7da3.zip chromium_src-7c9d829192359328576965fe8a4a52a5ad6c7da3.tar.gz chromium_src-7c9d829192359328576965fe8a4a52a5ad6c7da3.tar.bz2 |
Attempt at fixing leaks from SetProp. Apparently there is a finite
amount of memory reserved for properties and Windows doesn't always
automatically free up the memory if the window is deleted without an
explicit remove. For the time being I've made it easier to track
SetProp leaks, but we may want to move to using something other than
SetProp in the future that isn't as fragile.
I didn't fix a couple of places that were trickier. I'm going to file
separate bugs on them.
BUG=44991
TEST=none
Review URL: http://codereview.chromium.org/4195003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@64619 0039d316-1c4b-4281-b951-d872f2087c98
28 files changed, 212 insertions, 62 deletions
diff --git a/app/app_base.gypi b/app/app_base.gypi index e19c8c6..12e1701 100644 --- a/app/app_base.gypi +++ b/app/app_base.gypi @@ -225,6 +225,8 @@ 'win/drop_target.h', 'win/iat_patch_function.cc', 'win/iat_patch_function.h', + 'win/scoped_prop.cc', + 'win/scoped_prop.h', 'x11_util.cc', 'x11_util.h', 'x11_util_internal.h', diff --git a/app/win/scoped_prop.cc b/app/win/scoped_prop.cc new file mode 100644 index 0000000..e2d623e --- /dev/null +++ b/app/win/scoped_prop.cc @@ -0,0 +1,32 @@ +// Copyright (c) 2010 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 "app/win/scoped_prop.h" + +#include "base/win_util.h" + +namespace app { + +namespace win { + +ScopedProp::ScopedProp(HWND hwnd, const std::wstring& key, HANDLE data) + : hwnd_(hwnd), + key_(key) { + BOOL result = SetProp(hwnd, key.c_str(), data); + // Failure to set a propery on a window is typically fatal. It means someone + // is going to ask for the property and get NULL. So, rather than crash later + // on when someone expects a non-NULL value we crash here in hopes of + // diagnosing the failure. + CHECK(result) << win_util::FormatLastWin32Error(); +} + +ScopedProp::~ScopedProp() { + DCHECK(IsWindow(hwnd_)); + RemoveProp(hwnd_, key_.c_str()); +} + + +} // namespace win + +} // namespace app diff --git a/app/win/scoped_prop.h b/app/win/scoped_prop.h new file mode 100644 index 0000000..14e6a6a --- /dev/null +++ b/app/win/scoped_prop.h @@ -0,0 +1,40 @@ +// Copyright (c) 2010 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. + +#ifndef APP_WIN_SCOPED_PROP_H_ +#define APP_WIN_SCOPED_PROP_H_ +#pragma once + +#include <windows.h> + +#include "base/logging.h" + +namespace app { +namespace win { + +// ScopedProp is a wrapper around SetProp. Use ScopedProp rather than SetProp as +// it handles errors conditions for you and forces you to think about +// cleanup. ScopedProp must be destroyed before the window is destroyed, else +// you're going to leak a property, which could lead to failure to set a +// property later on. +class ScopedProp { + public: + // Registers the key value pair for the specified window. ScopedProp does not + // maintain the value, just the key/value pair. + ScopedProp(HWND hwnd, const std::wstring& key, HANDLE data); + ~ScopedProp(); + + const std::wstring& key() const { return key_; } + + private: + HWND hwnd_; + const std::wstring key_; + + DISALLOW_COPY_AND_ASSIGN(ScopedProp); +}; + +} // namespace win +} // namespace app + +#endif // APP_WIN_SCOPED_PROP_H_ diff --git a/chrome/browser/external_tab_container_win.cc b/chrome/browser/external_tab_container_win.cc index 7f90316..6496971 100644 --- a/chrome/browser/external_tab_container_win.cc +++ b/chrome/browser/external_tab_container_win.cc @@ -7,6 +7,7 @@ #include <string> #include "app/l10n_util.h" +#include "app/win/scoped_prop.h" #include "base/debug/trace_event.h" #include "base/i18n/rtl.h" #include "base/logging.h" @@ -129,9 +130,8 @@ bool ExternalTabContainer::Init(Profile* profile, // TODO(jcampan): limit focus traversal to contents. - // We don't ever remove the prop because the lifetime of this object - // is the same as the lifetime of the window - SetProp(GetNativeView(), kWindowObjectKey, this); + prop_.reset( + new app::win::ScopedProp(GetNativeView(), kWindowObjectKey, this)); if (existing_contents) { tab_contents_ = existing_contents; @@ -793,6 +793,7 @@ LRESULT ExternalTabContainer::OnCreate(LPCREATESTRUCT create_struct) { } void ExternalTabContainer::OnDestroy() { + prop_.reset(); Uninitialize(); WidgetWin::OnDestroy(); if (browser_.get()) { diff --git a/chrome/browser/external_tab_container_win.h b/chrome/browser/external_tab_container_win.h index f1168f2..226c01d 100644 --- a/chrome/browser/external_tab_container_win.h +++ b/chrome/browser/external_tab_container_win.h @@ -11,6 +11,7 @@ #include <vector> #include "base/lazy_instance.h" +#include "base/scoped_ptr.h" #include "chrome/browser/automation/automation_resource_message_filter.h" #include "chrome/browser/browser.h" #include "chrome/browser/net/chrome_url_request_context.h" @@ -29,6 +30,12 @@ class Profile; class TabContentsContainer; class RenderViewContextMenuViews; +namespace app { +namespace win { +class ScopedProp; +} +} + namespace IPC { struct NavigationInfo; } @@ -332,6 +339,8 @@ class ExternalTabContainer : public TabContentsDelegate, // page without chrome frame. bool route_all_top_level_navigations_; + scoped_ptr<app::win::ScopedProp> prop_; + DISALLOW_COPY_AND_ASSIGN(ExternalTabContainer); }; diff --git a/chrome/browser/hang_monitor/hung_plugin_action.cc b/chrome/browser/hang_monitor/hung_plugin_action.cc index 5fdcfd1..8399464 100644 --- a/chrome/browser/hang_monitor/hung_plugin_action.cc +++ b/chrome/browser/hang_monitor/hung_plugin_action.cc @@ -80,6 +80,7 @@ bool HungPluginAction::OnHungWindowDetected(HWND hung_window, if (child_window_message_timeout) { child_window_message_timeout *= 2; #pragma warning(disable:4312) + // TODO: this leaks. SetProp(hung_window, HungWindowDetector::kHungChildWindowTimeout, reinterpret_cast<HANDLE>(child_window_message_timeout)); #pragma warning(default:4312) diff --git a/chrome/browser/hang_monitor/hung_window_detector.cc b/chrome/browser/hang_monitor/hung_window_detector.cc index 73b84a8..a9bdead 100644 --- a/chrome/browser/hang_monitor/hung_window_detector.cc +++ b/chrome/browser/hang_monitor/hung_window_detector.cc @@ -101,6 +101,7 @@ bool HungWindowDetector::CheckChildWindow(HWND child_window) { HungWindowNotification::ActionOnHungWindow action = HungWindowNotification::HUNG_WINDOW_IGNORE; #pragma warning(disable:4312) + // TODO: this leaks. SetProp(child_window, kHungChildWindowTimeout, reinterpret_cast<HANDLE>(child_window_message_timeout)); #pragma warning(default:4312) 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 0b66f0c..c9151e7 100644 --- a/chrome/browser/renderer_host/render_widget_host_view_win.cc +++ b/chrome/browser/renderer_host/render_widget_host_view_win.cc @@ -7,6 +7,7 @@ #include "app/l10n_util.h" #include "app/l10n_util_win.h" #include "app/resource_bundle.h" +#include "app/win/scoped_prop.h" #include "base/command_line.h" #include "base/i18n/rtl.h" #include "base/metrics/histogram.h" @@ -310,9 +311,10 @@ void RenderWidgetHostViewWin::CreateWnd(HWND parent) { // this window. Used by the GPU process to validate window handles it // receives from renderer processes. int renderer_id = render_widget_host_->process()->id(); - SetProp(m_hWnd, - chrome::kChromiumRendererIdProperty, - reinterpret_cast<HANDLE>(renderer_id)); + props_.push_back( + new app::win::ScopedProp(m_hWnd, + chrome::kChromiumRendererIdProperty, + reinterpret_cast<HANDLE>(renderer_id))); // Uncommenting this will enable experimental out-of-process painting. // Contact brettw for more, @@ -840,12 +842,15 @@ LRESULT RenderWidgetHostViewWin::OnCreate(CREATESTRUCT* create_struct) { OnInputLangChange(0, 0); // Marks that window as supporting mouse-wheel messages rerouting so it is // scrolled when under the mouse pointer even if inactive. - views::SetWindowSupportsRerouteMouseWheel(m_hWnd); + props_.push_back(views::SetWindowSupportsRerouteMouseWheel(m_hWnd)); // Save away our HWND in the parent window as a property so that the // accessibility code can find it. - ::SetProp(GetParent(), kViewsNativeHostPropForAccessibility, m_hWnd); - ::SetProp(m_hWnd, kRenderWidgetHostViewKey, - static_cast<RenderWidgetHostView*>(this)); + props_.push_back(new app::win::ScopedProp( + GetParent(), kViewsNativeHostPropForAccessibility, + m_hWnd)); + props_.push_back(new app::win::ScopedProp( + m_hWnd, kRenderWidgetHostViewKey, + static_cast<RenderWidgetHostView*>(this))); return 0; } @@ -876,7 +881,7 @@ void RenderWidgetHostViewWin::OnDestroy() { // sequence as part of the usual cleanup when the plugin instance goes away. EnumChildWindows(m_hWnd, DetachPluginWindowsCallback, NULL); - ::RemoveProp(m_hWnd, kRenderWidgetHostViewKey); + props_.reset(); ResetTooltip(); TrackMouseLeave(false); 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 9fbaf11..e20a364 100644 --- a/chrome/browser/renderer_host/render_widget_host_view_win.h +++ b/chrome/browser/renderer_host/render_widget_host_view_win.h @@ -15,6 +15,7 @@ #include "base/scoped_comptr_win.h" #include "base/scoped_ptr.h" +#include "base/scoped_vector.h" #include "base/task.h" #include "chrome/browser/accessibility/browser_accessibility_manager.h" #include "chrome/browser/ime_input.h" @@ -23,6 +24,12 @@ #include "chrome/common/notification_registrar.h" #include "webkit/glue/webcursor.h" +namespace app { +namespace win { +class ScopedProp; +} +} + namespace gfx { class Size; class Rect; @@ -343,6 +350,8 @@ class RenderWidgetHostViewWin // method. WebKit::WebTextInputType text_input_type_; + ScopedVector<app::win::ScopedProp> props_; + DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostViewWin); }; diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc index bc77707..6a8df4a 100644 --- a/chrome/browser/views/frame/browser_view.cc +++ b/chrome/browser/views/frame/browser_view.cc @@ -1849,7 +1849,7 @@ void BrowserView::Init() { // Stow a pointer to this object onto the window handle so that we can get // at it later when all we have is a native view. #if defined(OS_WIN) - SetProp(GetWidget()->GetNativeView(), kBrowserViewKey, this); + GetWidget()->SetNativeWindowProperty(kBrowserViewKey, this); #else g_object_set_data(G_OBJECT(GetWidget()->GetNativeView()), kBrowserViewKey, this); diff --git a/chrome/browser/views/generic_info_view_unittest.cc b/chrome/browser/views/generic_info_view_unittest.cc index 2fa0e9d..6c26603 100644 --- a/chrome/browser/views/generic_info_view_unittest.cc +++ b/chrome/browser/views/generic_info_view_unittest.cc @@ -11,10 +11,11 @@ #include "views/controls/label.h" #include "views/controls/textfield/textfield.h" #include "views/widget/root_view.h" +#include "views/window/window.h" + #if defined(OS_WIN) #include "views/widget/widget_win.h" #endif -#include "views/window/window.h" // This class is only used on windows for now. #if defined(OS_WIN) @@ -59,5 +60,6 @@ TEST_F(GenericInfoViewTest, GenericInfoView) { string16 product_desc = l10n_util::GetString(IDS_PRODUCT_DESCRIPTION); EXPECT_EQ(product_name, view2->name_views_[0]->GetText()); EXPECT_EQ(product_desc, view2->name_views_[1]->GetText()); + window->CloseNow(); } #endif // OS_WIN diff --git a/chrome/browser/views/tab_contents/tab_contents_view_win.cc b/chrome/browser/views/tab_contents/tab_contents_view_win.cc index ce6bf8ed..f3126d1 100644 --- a/chrome/browser/views/tab_contents/tab_contents_view_win.cc +++ b/chrome/browser/views/tab_contents/tab_contents_view_win.cc @@ -145,6 +145,8 @@ void TabContentsViewWin::OnDestroy() { RevokeDragDrop(GetNativeView()); drop_target_ = NULL; } + + WidgetWin::OnDestroy(); } void TabContentsViewWin::SetPageTitle(const std::wstring& title) { diff --git a/gpu/gpu_plugin/gpu_plugin.cc b/gpu/gpu_plugin/gpu_plugin.cc index d1dd9ae..89be438 100644 --- a/gpu/gpu_plugin/gpu_plugin.cc +++ b/gpu/gpu_plugin/gpu_plugin.cc @@ -27,6 +27,7 @@ class PluginObject { private: HWND hwnd_; + DISALLOW_COPY_AND_ASSIGN(PluginObject); }; @@ -41,6 +42,7 @@ PluginObject::~PluginObject() { void PluginObject::SetWindow(HWND hwnd) { hwnd_ = hwnd; if (hwnd_) { + // TODO: convert this to using app::win::ScopedProp. // Store plugin object in window property. SetProp(hwnd_, kPluginObject, reinterpret_cast<HANDLE>(this)); diff --git a/o3d/plugin/cross/o3d_glue.cc b/o3d/plugin/cross/o3d_glue.cc index cce9f2a..aa0b54d 100644 --- a/o3d/plugin/cross/o3d_glue.cc +++ b/o3d/plugin/cross/o3d_glue.cc @@ -834,6 +834,7 @@ PluginObject *PluginObject::GetPluginProperty(HWND hWnd) { void PluginObject::ClearPluginProperty(HWND hWnd) { if (hWnd) { + // TODO: convert to using app::win::ScopedProp. RemoveProp(hWnd, kWindowPropertyName); ::DragAcceptFiles(hWnd, false); } diff --git a/o3d/plugin/win/main_win.cc b/o3d/plugin/win/main_win.cc index 225125b..1cbceb8 100644 --- a/o3d/plugin/win/main_win.cc +++ b/o3d/plugin/win/main_win.cc @@ -700,6 +700,7 @@ void CleanupAllWindows(PluginObject *obj) { GetProp(obj->GetPluginHWnd(), kOrigWndProcName)); DCHECK(origWndProc != NULL); + // TODO: this leaks. RemoveProp(obj->GetPluginHWnd(), kOrigWndProcName); SetWindowLongPtr(obj->GetPluginHWnd(), GWLP_WNDPROC, origWndProc); diff --git a/views/controls/menu/native_menu_win.cc b/views/controls/menu/native_menu_win.cc index 6236913..9993d4e 100644 --- a/views/controls/menu/native_menu_win.cc +++ b/views/controls/menu/native_menu_win.cc @@ -9,6 +9,7 @@ #include "app/l10n_util_win.h" #include "base/logging.h" #include "base/stl_util-inl.h" +#include "base/win_util.h" #include "gfx/canvas_skia.h" #include "gfx/font.h" #include "third_party/skia/include/core/SkBitmap.h" @@ -55,18 +56,16 @@ class NativeMenuWin::MenuHostWindow { RegisterClass(); hwnd_ = CreateWindowEx(l10n_util::GetExtendedStyles(), kWindowClassName, L"", 0, 0, 0, 0, 0, HWND_MESSAGE, NULL, NULL, NULL); - SetProp(hwnd_, kMenuHostWindowKey, this); + win_util::SetWindowUserData(hwnd_, this); } ~MenuHostWindow() { - RemoveProp(hwnd_, kMenuHostWindowKey); DestroyWindow(hwnd_); } HWND hwnd() const { return hwnd_; } private: - static const wchar_t* kMenuHostWindowKey; static const wchar_t* kWindowClassName; void RegisterClass() { @@ -276,7 +275,8 @@ class NativeMenuWin::MenuHostWindow { WPARAM w_param, LPARAM l_param) { MenuHostWindow* host = - reinterpret_cast<MenuHostWindow*>(GetProp(window, kMenuHostWindowKey)); + reinterpret_cast<MenuHostWindow*>(win_util::GetWindowUserData(window)); + // host is null during initial construction. LRESULT l_result = 0; if (!host || !host->ProcessWindowMessage(window, message, w_param, l_param, &l_result)) { @@ -294,10 +294,6 @@ class NativeMenuWin::MenuHostWindow { const wchar_t* NativeMenuWin::MenuHostWindow::kWindowClassName = L"ViewsMenuHostWindow"; -const wchar_t* NativeMenuWin::MenuHostWindow::kMenuHostWindowKey = - L"__MENU_HOST_WINDOW__"; - - //////////////////////////////////////////////////////////////////////////////// // NativeMenuWin, public: diff --git a/views/controls/native_control.cc b/views/controls/native_control.cc index a0ff1b0..ef719c9 100644 --- a/views/controls/native_control.cc +++ b/views/controls/native_control.cc @@ -13,7 +13,9 @@ #include "app/keyboard_code_conversion_win.h" #include "app/keyboard_codes.h" #include "app/l10n_util_win.h" +#include "app/win/scoped_prop.h" #include "base/logging.h" +#include "base/scoped_ptr.h" #include "base/win_util.h" #include "gfx/native_theme_win.h" #include "views/background.h" @@ -24,11 +26,6 @@ namespace views { -// Maps to the original WNDPROC for the controller window before we subclassed -// it. -static const wchar_t* const kHandlerKey = - L"__CONTROL_ORIGINAL_MESSAGE_HANDLER__"; - // Maps to the NativeControl. static const wchar_t* const kNativeControlKey = L"__NATIVE_CONTROL__"; @@ -37,10 +34,14 @@ class NativeControlContainer : public CWindowImpl<NativeControlContainer, CWinTraits<WS_CHILD | WS_CLIPSIBLINGS | WS_CLIPCHILDREN>> { public: + explicit NativeControlContainer(NativeControl* parent) + : parent_(parent), + control_(NULL), + original_handler_(NULL) { + } - explicit NativeControlContainer(NativeControl* parent) : parent_(parent), - control_(NULL) { - Create(parent->GetWidget()->GetNativeView()); + void Init() { + Create(parent_->GetWidget()->GetNativeView()); ::ShowWindow(m_hWnd, SW_SHOW); } @@ -78,17 +79,18 @@ class NativeControlContainer : public CWindowImpl<NativeControlContainer, parent_->NativeControlDestroyed(); delete this; } + private: + friend class NativeControl; LRESULT OnCreate(LPCREATESTRUCT create_struct) { control_ = parent_->CreateNativeControl(m_hWnd); // We subclass the control hwnd so we get the WM_KEYDOWN messages. - WNDPROC original_handler = - win_util::SetWindowProc(control_, - &NativeControl::NativeControlWndProc); - SetProp(control_, kHandlerKey, original_handler); - SetProp(control_, kNativeControlKey , parent_); + original_handler_ = win_util::SetWindowProc( + control_, &NativeControl::NativeControlWndProc); + prop_.reset( + new app::win::ScopedProp(control_, kNativeControlKey , parent_)); ::ShowWindow(control_, SW_SHOW); return 1; @@ -160,6 +162,12 @@ class NativeControlContainer : public CWindowImpl<NativeControlContainer, NativeControl* parent_; HWND control_; + + // Message handler that was set before we reset it. + WNDPROC original_handler_; + + scoped_ptr<app::win::ScopedProp> prop_; + DISALLOW_COPY_AND_ASSIGN(NativeControlContainer); }; @@ -188,6 +196,7 @@ void NativeControl::ValidateNativeControl() { if (!container_ && IsVisible()) { container_ = new NativeControlContainer(this); + container_->Init(); hwnd_view_->Attach(*container_); if (!enabled_) EnableWindow(GetNativeControlHWND(), enabled_); @@ -350,14 +359,15 @@ DWORD NativeControl::GetAdditionalRTLStyle() const { } // static -LRESULT CALLBACK NativeControl::NativeControlWndProc(HWND window, UINT message, +LRESULT CALLBACK NativeControl::NativeControlWndProc(HWND window, + UINT message, WPARAM w_param, LPARAM l_param) { - HANDLE original_handler = GetProp(window, kHandlerKey); - DCHECK(original_handler); NativeControl* native_control = static_cast<NativeControl*>(GetProp(window, kNativeControlKey)); DCHECK(native_control); + WNDPROC original_handler = native_control->container_->original_handler_; + DCHECK(original_handler); if (message == WM_KEYDOWN && native_control->OnKeyDown(app::KeyboardCodeForWindowsKeyCode(w_param))) { @@ -373,8 +383,7 @@ LRESULT CALLBACK NativeControl::NativeControlWndProc(HWND window, UINT message, } else if (message == WM_DESTROY) { win_util::SetWindowProc(window, reinterpret_cast<WNDPROC>(original_handler)); - RemoveProp(window, kHandlerKey); - RemoveProp(window, kNativeControlKey); + native_control->container_->prop_.reset(); } return CallWindowProc(reinterpret_cast<WNDPROC>(original_handler), window, diff --git a/views/controls/native_control.h b/views/controls/native_control.h index 5a180d2..75a2a49 100644 --- a/views/controls/native_control.h +++ b/views/controls/native_control.h @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -42,6 +42,7 @@ class NativeControl : public View { // Overridden to do nothing. virtual void Paint(gfx::Canvas* canvas); + protected: friend class NativeControlContainer; diff --git a/views/controls/native_control_win.cc b/views/controls/native_control_win.cc index ba27f85..0c6d257 100644 --- a/views/controls/native_control_win.cc +++ b/views/controls/native_control_win.cc @@ -7,6 +7,7 @@ #include <windowsx.h> #include "app/l10n_util_win.h" +#include "app/win/scoped_prop.h" #include "base/logging.h" #include "base/win_util.h" #include "views/focus/focus_manager.h" @@ -129,8 +130,8 @@ void NativeControlWin::ShowContextMenu(const gfx::Point& location) { void NativeControlWin::NativeControlCreated(HWND native_control) { // Associate this object with the control's HWND so that WidgetWin can find // this object when it receives messages from it. - // Note that we never unset this property. We don't have to. - SetProp(native_control, kNativeControlWinKey, this); + prop_.reset( + new app::win::ScopedProp(native_control, kNativeControlWinKey, this)); // Subclass so we get WM_KEYDOWN and WM_SETFOCUS messages. original_wndproc_ = @@ -210,6 +211,7 @@ LRESULT NativeControlWin::NativeControlWndProc(HWND window, NOTREACHED(); } } else if (message == WM_DESTROY) { + native_control->prop_.reset(); win_util::SetWindowProc(window, native_control->original_wndproc_); } diff --git a/views/controls/native_control_win.h b/views/controls/native_control_win.h index 1b0273b..6d0fdf0 100644 --- a/views/controls/native_control_win.h +++ b/views/controls/native_control_win.h @@ -6,9 +6,16 @@ #define VIEWS_CONTROLS_NATIVE_CONTROL_WIN_H_ #pragma once +#include "base/scoped_ptr.h" #include "views/controls/combobox/combobox.h" #include "views/controls/native/native_view_host.h" +namespace app { +namespace win { +class ScopedProp; +} +} + namespace views { // A View that hosts a native Windows control. @@ -86,6 +93,8 @@ class NativeControlWin : public NativeViewHost { // The window procedure before we subclassed. WNDPROC original_wndproc_; + scoped_ptr<app::win::ScopedProp> prop_; + DISALLOW_COPY_AND_ASSIGN(NativeControlWin); }; diff --git a/views/focus/focus_util_win.cc b/views/focus/focus_util_win.cc index 381d02b..266a891 100644 --- a/views/focus/focus_util_win.cc +++ b/views/focus/focus_util_win.cc @@ -6,6 +6,7 @@ #include <windowsx.h> +#include "app/win/scoped_prop.h" #include "base/auto_reset.h" #include "base/win_util.h" @@ -52,9 +53,9 @@ static bool CanRedirectMouseWheelFrom(HWND window) { return true; } -void SetWindowSupportsRerouteMouseWheel(HWND hwnd) { - SetProp(hwnd, kHWNDSupportMouseWheelRerouting, - reinterpret_cast<HANDLE>(true)); +app::win::ScopedProp* SetWindowSupportsRerouteMouseWheel(HWND hwnd) { + return new app::win::ScopedProp(hwnd, kHWNDSupportMouseWheelRerouting, + reinterpret_cast<HANDLE>(true)); } bool RerouteMouseWheel(HWND window, WPARAM w_param, LPARAM l_param) { diff --git a/views/focus/focus_util_win.h b/views/focus/focus_util_win.h index 7a85720..2c53c68 100644 --- a/views/focus/focus_util_win.h +++ b/views/focus/focus_util_win.h @@ -8,12 +8,19 @@ #include <windows.h> +namespace app { +namespace win { +class ScopedProp; +} +} + namespace views { // Marks the passed |hwnd| as supporting mouse-wheel message rerouting. // We reroute the mouse wheel messages to such HWND when they are under the -// mouse pointer (but are not the active window) -void SetWindowSupportsRerouteMouseWheel(HWND hwnd); +// mouse pointer (but are not the active window). Callers must delete the +// returned object before the window is destroyed (see ScopedProp for details). +app::win::ScopedProp* SetWindowSupportsRerouteMouseWheel(HWND hwnd); // Forwards mouse wheel messages to the window under it. // Windows sends mouse wheel messages to the currently active window. diff --git a/views/widget/widget_win.cc b/views/widget/widget_win.cc index 102b48d..bb6ca8d 100644 --- a/views/widget/widget_win.cc +++ b/views/widget/widget_win.cc @@ -8,6 +8,7 @@ #include "app/l10n_util_win.h" #include "app/system_monitor.h" #include "app/win_util.h" +#include "app/win/scoped_prop.h" #include "base/string_util.h" #include "base/win_util.h" #include "gfx/canvas_skia.h" @@ -36,10 +37,6 @@ bool WidgetWin::screen_reader_active_ = false; // listening for MSAA events. #define OBJID_CUSTOM 1 -bool SetRootViewForHWND(HWND hwnd, RootView* root_view) { - return SetProp(hwnd, kRootViewWindowProperty, root_view) ? true : false; -} - RootView* GetRootViewForHWND(HWND hwnd) { return reinterpret_cast<RootView*>(::GetProp(hwnd, kRootViewWindowProperty)); } @@ -168,7 +165,7 @@ void WidgetWin::Init(gfx::NativeView parent, const gfx::Rect& bounds) { default_theme_provider_.reset(new DefaultThemeProvider()); - SetWindowSupportsRerouteMouseWheel(hwnd()); + props_.push_back(SetWindowSupportsRerouteMouseWheel(hwnd())); drop_target_ = new DropTargetWin(root_view_.get()); @@ -183,7 +180,7 @@ void WidgetWin::Init(gfx::NativeView parent, const gfx::Rect& bounds) { } // Sets the RootView as a property, so the automation can introspect windows. - SetRootViewForHWND(hwnd(), root_view_.get()); + SetNativeWindowProperty(kRootViewWindowProperty, root_view_.get()); MessageLoopForUI::current()->AddObserver(this); @@ -421,12 +418,17 @@ const Window* WidgetWin::GetWindow() const { return GetWindowImpl(hwnd()); } -void WidgetWin::SetNativeWindowProperty(const std::wstring& name, - void* value) { +void WidgetWin::SetNativeWindowProperty(const std::wstring& name, void* value) { + // Remove the existing property (if any). + for (ScopedProps::iterator i = props_.begin(); i != props_.end(); ++i) { + if ((*i)->key() == name) { + props_.erase(i); + break; + } + } + if (value) - SetProp(hwnd(), name.c_str(), value); - else - RemoveProp(hwnd(), name.c_str()); + props_.push_back(new app::win::ScopedProp(hwnd(), name, value)); } void* WidgetWin::GetNativeWindowProperty(const std::wstring& name) { @@ -583,14 +585,12 @@ LRESULT WidgetWin::OnCreate(CREATESTRUCT* create_struct) { } void WidgetWin::OnDestroy() { - SetNativeWindowProperty(kWidgetKey, NULL); - if (drop_target_.get()) { RevokeDragDrop(hwnd()); drop_target_ = NULL; } - RemoveProp(hwnd(), kRootViewWindowProperty); + props_.reset(); } void WidgetWin::OnDisplayChange(UINT bits_per_pixel, CSize screen_size) { diff --git a/views/widget/widget_win.h b/views/widget/widget_win.h index 029f34e..bfb9137 100644 --- a/views/widget/widget_win.h +++ b/views/widget/widget_win.h @@ -11,15 +11,23 @@ #include <atlcrack.h> #include <atlmisc.h> +#include <string> #include <vector> #include "base/message_loop.h" #include "base/scoped_comptr_win.h" +#include "base/scoped_vector.h" #include "gfx/window_impl.h" #include "views/focus/focus_manager.h" #include "views/layout_manager.h" #include "views/widget/widget.h" +namespace app { +namespace win { +class ScopedProp; +} +} + namespace gfx { class CanvasSkia; class Rect; @@ -34,7 +42,6 @@ class RootView; class TooltipManagerWin; class Window; -bool SetRootViewForHWND(HWND hwnd, RootView* root_view); RootView* GetRootViewForHWND(HWND hwnd); // A Windows message reflected from other windows. This message is sent @@ -480,6 +487,8 @@ class WidgetWin : public gfx::WindowImpl, bool is_window_; private: + typedef ScopedVector<app::win::ScopedProp> ScopedProps; + // Implementation of GetWindow. Ascends the parents of |hwnd| returning the // first ancestor that is a Window. static Window* GetWindowImpl(HWND hwnd); @@ -585,6 +594,10 @@ class WidgetWin : public gfx::WindowImpl, // The current position of the view events vector. When incrementing, // we always mod this value with the max view events above . int accessibility_view_events_index_; + + ScopedProps props_; + + DISALLOW_COPY_AND_ASSIGN(WidgetWin); }; } // namespace views diff --git a/webkit/glue/plugins/test/plugin_create_instance_in_paint.cc b/webkit/glue/plugins/test/plugin_create_instance_in_paint.cc index 0bea703..305e86f 100644 --- a/webkit/glue/plugins/test/plugin_create_instance_in_paint.cc +++ b/webkit/glue/plugins/test/plugin_create_instance_in_paint.cc @@ -46,6 +46,7 @@ NPError CreateInstanceInPaintTest::SetWindow(NPWindow* pNPWindow) { WS_CHILD | WS_CLIPCHILDREN | WS_CLIPSIBLINGS | WS_VISIBLE , 0, 0, 100, 100, parent, 0, GetModuleHandle(NULL), 0); DCHECK(window_); + // TODO: this property leaks. ::SetProp(window_, L"Plugin_Instance", this); } } else if (test_id() == "2") { diff --git a/webkit/glue/plugins/test/plugin_get_javascript_url_test.cc b/webkit/glue/plugins/test/plugin_get_javascript_url_test.cc index cc6bc19..344cae3 100644 --- a/webkit/glue/plugins/test/plugin_get_javascript_url_test.cc +++ b/webkit/glue/plugins/test/plugin_get_javascript_url_test.cc @@ -45,6 +45,7 @@ NPError ExecuteGetJavascriptUrlTest::SetWindow(NPWindow* pNPWindow) { #ifdef OS_WIN HWND window_handle = reinterpret_cast<HWND>(pNPWindow->window); if (!::GetProp(window_handle, L"Plugin_Instance")) { + // TODO: this propery leaks. ::SetProp(window_handle, L"Plugin_Instance", this); // We attempt to retreive the NPObject for the plugin instance identified // by the NPObjectLifetimeTestInstance2 class as it may not have been diff --git a/webkit/glue/plugins/test/plugin_npobject_lifetime_test.cc b/webkit/glue/plugins/test/plugin_npobject_lifetime_test.cc index 10b3239..7277211 100644 --- a/webkit/glue/plugins/test/plugin_npobject_lifetime_test.cc +++ b/webkit/glue/plugins/test/plugin_npobject_lifetime_test.cc @@ -27,6 +27,7 @@ NPError NPObjectLifetimeTest::SetWindow(NPWindow* pNPWindow) { HWND window_handle = reinterpret_cast<HWND>(pNPWindow->window); if (!::GetProp(window_handle, L"Plugin_Instance")) { + // TODO: this propery leaks. ::SetProp(window_handle, L"Plugin_Instance", this); // We attempt to retreive the NPObject for the plugin instance identified // by the NPObjectLifetimeTestInstance2 class as it may not have been diff --git a/webkit/glue/plugins/test/plugin_windowed_test.cc b/webkit/glue/plugins/test/plugin_windowed_test.cc index 461fc20..0c46d68 100644 --- a/webkit/glue/plugins/test/plugin_windowed_test.cc +++ b/webkit/glue/plugins/test/plugin_windowed_test.cc @@ -62,6 +62,7 @@ NPError WindowedPluginTest::SetWindow(NPWindow* pNPWindow) { WS_CHILD | WS_CLIPCHILDREN | WS_CLIPSIBLINGS | WS_VISIBLE , 0, 0, 100, 100, parent, 0, GetModuleHandle(NULL), 0); DCHECK(window_); + // TODO: this propery leaks. ::SetProp(window_, L"Plugin_Instance", this); } |