Fix crash bugs of FramePainter.

Sometimes FramePainter is deleted without its window's OnWindowDestroying, so removing the kSoloWindowFramePainterKey property explicitly at the destructor.
Also this CL adds the checks of painter's frame_ and its non_client_view() since they may be NULL on window destroying timing on test.

BUG=155634
TEST=ash_unittests passed with the new test


Review URL: https://chromiumcodereview.appspot.com/11194015

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

ash/wm/frame_painter.cc

@@ -147,8 +147,15 @@ FramePainter::FramePainter()
 
 FramePainter::~FramePainter() {
   // Sometimes we are destroyed before the window closes, so ensure we clean up.
-  if (window_)
+  if (window_) {
+    aura::RootWindow* root = window_->GetRootWindow();
+    if (root &&
+        root->GetProperty(internal::kSoloWindowFramePainterKey) == this) {
+      root->SetProperty(internal::kSoloWindowFramePainterKey,
+                        static_cast<FramePainter*>(NULL));
+    }
     window_->RemoveObserver(this);
+  }
   instances_->erase(this);
 }
 
@@ -751,12 +758,16 @@ void FramePainter::UpdateSoloWindowFramePainter(
       internal::kSoloWindowFramePainterKey);
   FramePainter* new_solo_painter = GetSoloPainterInRoot(ignorable_window);
   if (old_solo_painter != new_solo_painter) {
-    if (old_solo_painter)
+    if (old_solo_painter && old_solo_painter->frame_ &&
+        old_solo_painter->frame_->non_client_view()) {
       old_solo_painter->frame_->non_client_view()->SchedulePaint();
+    }
     window_->GetRootWindow()->SetProperty(
         internal::kSoloWindowFramePainterKey, new_solo_painter);
-    if (new_solo_painter)
+    if (new_solo_painter && new_solo_painter->frame_ &&
+        new_solo_painter->frame_->non_client_view()) {
       new_solo_painter->frame_->non_client_view()->SchedulePaint();
+    }
   }
 }
 

ash/wm/frame_painter.h

@@ -134,6 +134,7 @@ class ASH_EXPORT FramePainter : public aura::WindowObserver,
 
  private:
   FRIEND_TEST_ALL_PREFIXES(FramePainterTest, Basics);
+  FRIEND_TEST_ALL_PREFIXES(FramePainterTest, CreateAndDeleteSingleWindow);
   FRIEND_TEST_ALL_PREFIXES(FramePainterTest, UseSoloWindowHeader);
   FRIEND_TEST_ALL_PREFIXES(FramePainterTest, UseSoloWindowHeaderMultiDisplay);
   FRIEND_TEST_ALL_PREFIXES(FramePainterTest, GetHeaderOpacity);

ash/wm/frame_painter_unittest.cc

@@ -8,6 +8,7 @@
 #include "ash/shell_window_ids.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/wm/property_util.h"
+#include "ash/wm/window_properties.h"
 #include "ash/wm/window_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "grit/ash_resources.h"
@@ -123,6 +124,40 @@ TEST_F(FramePainterTest, Basics) {
   EXPECT_EQ(0u, FramePainter::instances_->size());
 }
 
+TEST_F(FramePainterTest, CreateAndDeleteSingleWindow) {
+  // Ensure that creating/deleting a window works well and doesn't cause
+  // crashes.  See crbug.com/155634
+  aura::RootWindow* root = Shell::GetActiveRootWindow();
+
+  scoped_ptr<Widget> widget(CreateTestWidget());
+  scoped_ptr<FramePainter> painter(new FramePainter);
+  ImageButton size(NULL);
+  ImageButton close(NULL);
+  painter->Init(
+      widget.get(), NULL, &size, &close, FramePainter::SIZE_BUTTON_MAXIMIZES);
+  widget->Show();
+
+  // We only have one window, so it should use a solo header.
+  EXPECT_TRUE(painter->UseSoloWindowHeader());
+  EXPECT_EQ(painter.get(),
+            root->GetProperty(internal::kSoloWindowFramePainterKey));
+
+  // Close the window.
+  widget.reset();
+  EXPECT_EQ(NULL, root->GetProperty(internal::kSoloWindowFramePainterKey));
+
+  // Recreate another window again.
+  painter.reset(new FramePainter);
+  widget.reset(CreateTestWidget());
+
+  painter->Init(
+      widget.get(), NULL, &size, &close, FramePainter::SIZE_BUTTON_MAXIMIZES);
+  widget->Show();
+  EXPECT_TRUE(painter->UseSoloWindowHeader());
+  EXPECT_EQ(painter.get(),
+            root->GetProperty(internal::kSoloWindowFramePainterKey));
+}
+
 TEST_F(FramePainterTest, UseSoloWindowHeader) {
   // Create a widget and a painter for it.
   scoped_ptr<Widget> w1(CreateTestWidget());

ash/wm/window_properties.cc

@@ -15,7 +15,7 @@
 // defined in aura.
 DECLARE_WINDOW_PROPERTY_TYPE(ash::internal::AlwaysOnTopController*);
 DECLARE_WINDOW_PROPERTY_TYPE(ash::internal::ShadowType);
-DECLARE_WINDOW_PROPERTY_TYPE(ash::FramePainter*);
+DECLARE_EXPORTED_WINDOW_PROPERTY_TYPE(ASH_EXPORT, ash::FramePainter*);
 DECLARE_WINDOW_PROPERTY_TYPE(ash::WindowPersistsAcrossAllWorkspacesType)
 DECLARE_WINDOW_PROPERTY_TYPE(ui_controls::UIControlsAura*)
 DECLARE_WINDOW_PROPERTY_TYPE(ash::internal::RootWindowController*);

ash/wm/window_properties.h

@@ -9,6 +9,7 @@
 #include "ash/wm/property_util.h"
 #include "ash/wm/shadow_types.h"
 #include "ui/aura/window.h"
+#include "ui/aura/window_property.h"
 #include "ui/base/ui_base_types.h"
 
 namespace ui_controls {
@@ -53,7 +54,7 @@ extern const aura::WindowProperty<ShadowType>* const kShadowTypeKey;
 
 // A property key to remember the frame painter for the solo-window in the root
 // window. It is only available for root windows.
-extern const aura::WindowProperty<ash::FramePainter*>* const
+ASH_EXPORT extern const aura::WindowProperty<ash::FramePainter*>* const
     kSoloWindowFramePainterKey;
 
 // If this is set to true, the window stays in the same root window