TempParent causing error with ClassRegistrar UnregisterClass

[Marshall Greenblatt]

Hi John,

The TabContentsViewWin class uses a global TempParent object (content/browser/tab_contents/tab_contents_view_win.cc) as the initial parent for all new TabContents. The TempParent class extends ui::WindowImpl which causes a new class type to be registered by ClassRegistrar in ui/base/win/window_impl.cc. This is problematic because the TempParent object is never destroyed. When the ClassRegistrar is destroyed by AtExitManager the call to UnregisterClass fails with error code 1412 (ERROR_CLASS_HAS_WINDOWS).

According to the MSDN documentation [1] "all window classes that an application registers are unregistered when it terminates." It therefore seems safe to leak the class registration in this case. However, WindowImpl doesn't currently provide a mechanism for leaking class registrations. Should we add this mechanism, or should we create new temporary TempParent object for each TabContents and then destroy it when a different parent is set? What do you think?

Thanks,
Marshall

[1] http://msdn.microsoft.com/en-us/library/windows/desktop/ms644899%28v=vs.85%29.aspx

[John Abd-El-Malek]

This pattern was copied from chrome. How is that case handled? http://code.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/ui/views/tab_contents/native_tab_contents_view_win.cc&exact_package=chromium&q=tab_contents_view_win.cc&type=cs&l=20

[Marshall Greenblatt]

On Tue, Feb 7, 2012 at 12:10 PM, John Abd-El-Malek <j...@chromium.org> wrote:

This pattern was copied from chrome. How is that case handled? http://code.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/ui/views/tab_contents/native_tab_contents_view_win.cc&exact_package=chromium&q=tab_contents_view_win.cc&type=cs&l=20

The same error condition is reached here as well. HiddenTabHostWindow extends views::Widget which creates a NativeWidgetWin internally. NativeWidgetWin is also a child of ui::WindowImpl. You can set a break point at window_impl.cc line 54 or view the error output in chrome_debug.log.

[John Abd-El-Malek]

On Tue, Feb 7, 2012 at 11:21 AM, Marshall Greenblatt <magree...@gmail.com> wrote:

On Tue, Feb 7, 2012 at 12:10 PM, John Abd-El-Malek <j...@chromium.org> wrote:

This pattern was copied from chrome. How is that case handled? http://code.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/ui/views/tab_contents/native_tab_contents_view_win.cc&exact_package=chromium&q=tab_contents_view_win.cc&type=cs&l=20

The same error condition is reached here as well. HiddenTabHostWindow extends views::Widget which creates a NativeWidgetWin internally. NativeWidgetWin is also a child of ui::WindowImpl. You can set a break point at window_impl.cc line 54 or view the error output in chrome_debug.log.

I see. I would go through the code history to find out who wrote this original pattern and check with them. I'm not sure which is the best solution as I'm not that familiar with this area.

[Marshall Greenblatt]

Hi Robert,

You added the LOG(ERROR) output for UnregisterClass in revision 118320. Can you comment on the below issue with HiddenTabHostWindow and TempParent?

Thanks,
Marshall