Fix Up SingletonHwnd Observer Lifetime Issues

ui/gfx/win/singleton_hwnd_observer.cc

+// Copyright (c) 2015 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 "ui/gfx/win/singleton_hwnd_observer.h"
+
+#include "ui/gfx/win/singleton_hwnd.h"
+
+namespace gfx {
+
+SingletonHwndObserver::SingletonHwndObserver(const WndProc& wnd_proc) :
+    wnd_proc_(wnd_proc) {
+  DCHECK(!wnd_proc.is_null());
+  SingletonHwnd::GetInstance()->AddObserver(this);
+}
+
+SingletonHwndObserver::~SingletonHwndObserver() {
+  ClearWndProc();

[sky, 2015/04/28 13:20:34]

Why do you need the ClearWndProc? Won't evertyhing work fine without it?

[robliao, 2015/04/28 14:11:55]

We need to unregister from the SingletonHwnd in two cases:
1) The SingletonHwnd is tearing down.
2) The owner of this object is tearing down.

This call covers case 2.

[sky, 2015/04/28 16:45:46]

I see it. You're taking is_null() to know when you removed the observer.

+}
+
+void SingletonHwndObserver::ClearWndProc() {
+  if (!wnd_proc_.is_null())
+    SingletonHwnd::GetInstance()->RemoveObserver(this);
+
+  wnd_proc_.Reset();
+}
+
+void SingletonHwndObserver::OnWndProc(HWND hwnd,
+                                      UINT message,
+                                      WPARAM wparam,
+                                      LPARAM lparam) {
+  if (wnd_proc_.is_null())

[sky, 2015/04/28 16:45:46]

Shouldn't this be a DCHECK?

[robliao, 2015/04/28 22:14:10]

Actually, even a DCHECK isn't necessary, as it would only trigger if we're
getting ready to crash on the next line. We would only be here if the
SingletonHwnd is sending us messages after unregistration.

Removed the check.

+    return;
+
+  wnd_proc_.Run(hwnd, message, wparam, lparam);
+
+  if (message == WM_NCDESTROY) {

[sky, 2015/04/28 13:20:34]

nit: no {}

[robliao, 2015/04/28 14:11:55]

Done.

+    ClearWndProc();
+  }
+}
+
+}  // namespace gfx