Adds yet more debugging in hopes of tracking 91396. The latest data

indicates FindPreference is returning NULL and we're in shutdown
because the last window is being closed. I suspect that for some
reason on some machines this triggers us to get a WM_CLOSE after we've
removed the property used to lookup the profile (removed in
WM_DESTROY) so that we fall back to the local state prefs which don't
have a property for window position and we crash. I'm adding debugging
code to determine whether that is in fact happening.

BUG=91396
TEST=none
R=ben@chromium.org

Review URL: http://codereview.chromium.org/7727001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@98180 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/views/chrome_views_delegate.cc

@@ -8,6 +8,7 @@
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/browser_shutdown.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -32,13 +33,16 @@ namespace {
 // been initialized.
 // TODO(mirandac): This function will also separate windows by profile in a
 // multi-profile environment.
-PrefService* GetPrefsForWindow(const views::Widget* window) {
+PrefService* GetPrefsForWindow(const views::Widget* window,
+                               bool* using_local_state) {
   Profile* profile = reinterpret_cast<Profile*>(
       window->GetNativeWindowProperty(Profile::kProfileKey));
   if (!profile) {
     // Use local state for windows that have no explicit profile.
+    *using_local_state = true;
     return g_browser_process->local_state();
   }
+  *using_local_state = false;
   return profile->GetPrefs();
 }
 
@@ -62,11 +66,14 @@ void ChromeViewsDelegate::SaveWindowPlacement(const views::Widget* window,
                                               const std::wstring& window_name,
                                               const gfx::Rect& bounds,
                                               bool maximized) {
-  PrefService* prefs = GetPrefsForWindow(window);
+  bool using_local_state = false;
+  PrefService* prefs = GetPrefsForWindow(window, &using_local_state);
   if (!prefs)
     return;
 
-  DCHECK(prefs->FindPreference(WideToUTF8(window_name).c_str()));
+  CHECK(prefs->FindPreference(WideToUTF8(window_name).c_str())) << " " <<
+      browser_shutdown::GetShutdownType() << " " << using_local_state << " " <<
+      window->destroy_state();
   DictionaryPrefUpdate update(prefs, WideToUTF8(window_name).c_str());
   DictionaryValue* window_preferences = update.Get();
   window_preferences->SetInteger("left", bounds.x());

views/widget/widget.cc

@@ -153,10 +153,13 @@ Widget::Widget()
       saved_maximized_state_(false),
       minimum_size_(100, 100),
       focus_on_creation_(true),
-      is_top_level_(false) {
+      is_top_level_(false),
+      destroy_state_(DESTROY_STATE_NONE) {
 }
 
 Widget::~Widget() {
+  destroy_state_ = DESTROY_STATE_DELETED;
+
   while (!event_stack_.empty()) {
     event_stack_.top()->reset();
     event_stack_.pop();
@@ -862,12 +865,17 @@ void Widget::OnNativeWidgetCreated() {
 
 void Widget::OnNativeWidgetDestroying() {
   FOR_EACH_OBSERVER(Observer, observers_, OnWidgetClosing(this));
+  if (destroy_state_ == DESTROY_STATE_NONE)
+    destroy_state_ = DESTROY_STATE_IN_DESTROYING;
   if (non_client_view_)
     non_client_view_->WindowClosing();
   widget_delegate_->WindowClosing();
 }
 
 void Widget::OnNativeWidgetDestroyed() {
+  if (destroy_state_ == DESTROY_STATE_IN_DESTROYING ||
+      destroy_state_ == DESTROY_STATE_NONE)
+    destroy_state_ = DESTROY_STATE_DESTROYED;
   widget_delegate_->DeleteDelegate();
   widget_delegate_ = NULL;
 }

views/widget/widget.h

@@ -101,6 +101,20 @@ class VIEWS_EXPORT Widget : public internal::NativeWidgetDelegate,
 
   typedef std::set<Widget*> Widgets;
 
+  enum DestroyState {
+    // The default, everything is good and alive.
+    DESTROY_STATE_NONE = 1,
+
+    // Set once OnNativeWidgetDestroying has been invoked.
+    DESTROY_STATE_IN_DESTROYING,
+
+    // Set once OnNativeWidgetDestroyed has been invoked.
+    DESTROY_STATE_DESTROYED,
+
+    // Set once the destructor is invoked.
+    DESTROY_STATE_DELETED,
+  };
+
   enum FrameType {
     FRAME_TYPE_DEFAULT,         // Use whatever the default would be.
     FRAME_TYPE_FORCE_CUSTOM,    // Force the custom frame.
@@ -564,6 +578,8 @@ class VIEWS_EXPORT Widget : public internal::NativeWidgetDelegate,
   // TYPE_CONTROL and TYPE_TOOLTIP is not considered top level.
   bool is_top_level() const { return is_top_level_; }
 
+  DestroyState destroy_state() const { return destroy_state_; }
+
   // Overridden from NativeWidgetDelegate:
   virtual bool IsModal() const OVERRIDE;
   virtual bool IsDialogBox() const OVERRIDE;
@@ -727,6 +743,10 @@ class VIEWS_EXPORT Widget : public internal::NativeWidgetDelegate,
   // Factory used to create Compositors. Settable by tests.
   static ui::Compositor*(*compositor_factory_)();
 
+  // Tracks destroy state.
+  // TODO(sky): remove this, used in tracking 91396.
+  DestroyState destroy_state_;
+
   DISALLOW_COPY_AND_ASSIGN(Widget);
 };