Attempt at fixing crash during tab dragging. The crash dumps seem to

indicate the tab being dragged is getting deleted between the time we
detach from the model and update the DraggedTabView. This could happen
if the animation were canceled (as could happen if a layout
happened). I don't see how that could happen though. None-the-less
it's easy to deal with the tab going away as it isn't needed when
dragging. This may end up shifting the crash around though, we'll see.

I'm also removing some dead code in the DraggedTabView that is no
longer needed as we only used DraggedTabView for the detached state.

BUG=48365
TEST=make sure tab dragging on windows still works.

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

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

4 files changed, 31 insertions, 85 deletions

diff --git a/chrome/browser/views/tabs/dragged_tab_controller.cc b/chrome/browser/views/tabs/dragged_tab_controller.cc
index 18297dd..d6514bf 100644
--- a/chrome/browser/views/tabs/dragged_tab_controller.cc
+++ b/chrome/browser/views/tabs/dragged_tab_controller.cc
@@ -340,7 +340,6 @@ DraggedTabController::DraggedTabController(BaseTab* source_tab,
       attached_tab_(NULL),
       offset_to_width_ratio_(0),
       old_focused_view_(NULL),
-      in_destructor_(false),
       last_move_screen_loc_(0),
       mini_(source_tab->data().mini),
       pinned_(source_tabstrip->IsTabPinned(source_tab)),
@@ -357,7 +356,6 @@ DraggedTabController::~DraggedTabController() {
   if (instance_ == this)
     instance_ = NULL;
 
-  in_destructor_ = true;
   MessageLoopForUI::current()->RemoveObserver(this);
   // Need to delete the view here manually _before_ we reset the dragged
   // contents to NULL, otherwise if the view is animating to its destination
@@ -897,7 +895,10 @@ void DraggedTabController::Detach() {
   DCHECK(index != -1);
   // Hide the tab so that the user doesn't see it animate closed.
   attached_tab_->SetVisible(false);
+  int attached_tab_width = attached_tab_->width();
   attached_model->DetachTabContentsAt(index);
+  // Detaching may end up deleting the tab, drop references to it.
+  attached_tab_ = NULL;
 
   // If we've removed the last Tab from the TabStrip, hide the frame now.
   if (!attached_model->HasNonPhantomTabs())
@@ -912,14 +913,12 @@ void DraggedTabController::Detach() {
 
   // Create the dragged view.
   EnsureDraggedView(tab_data);
-  view_->Attach(attached_tab_->width());
-  view_->Detach(photobooth_.get());
+  view_->SetTabWidthAndUpdate(attached_tab_width, photobooth_.get());
 
   // Detaching resets the delegate, but we still want to be the delegate.
   dragged_contents_->set_delegate(this);
 
   attached_tabstrip_ = NULL;
-  attached_tab_ = NULL;
 }
 
 int DraggedTabController::GetInsertionIndexForDraggedBounds(
diff --git a/chrome/browser/views/tabs/dragged_tab_controller.h b/chrome/browser/views/tabs/dragged_tab_controller.h
index a860a18..66f4ced 100644
--- a/chrome/browser/views/tabs/dragged_tab_controller.h
+++ b/chrome/browser/views/tabs/dragged_tab_controller.h
@@ -303,8 +303,6 @@ class DraggedTabController : public TabContentsDelegate,
   // ends within this same window.
   views::View* old_focused_view_;
 
-  bool in_destructor_;
-
   // The position along the major axis of the mouse cursor in screen coordinates
   // at the time of the last re-order event.
   int last_move_screen_loc_;
diff --git a/chrome/browser/views/tabs/dragged_tab_view.cc b/chrome/browser/views/tabs/dragged_tab_view.cc
index e0b13a1..5ee2626 100644
--- a/chrome/browser/views/tabs/dragged_tab_view.cc
+++ b/chrome/browser/views/tabs/dragged_tab_view.cc
@@ -30,10 +30,9 @@ DraggedTabView::DraggedTabView(views::View* renderer,
                                const gfx::Size& contents_size,
                                const gfx::Size& min_size)
     : renderer_(renderer),
-      attached_(false),
       show_contents_on_drag_(true),
       mouse_tab_offset_(mouse_tab_offset),
-      attached_tab_size_(min_size),
+      tab_size_(min_size),
       photobooth_(NULL),
       contents_size_(contents_size) {
   set_parent_owned(false);
@@ -69,7 +68,7 @@ DraggedTabView::~DraggedTabView() {
 
 void DraggedTabView::MoveTo(const gfx::Point& screen_point) {
   int x;
-  if (base::i18n::IsRTL() && !attached_) {
+  if (base::i18n::IsRTL()) {
     // On RTL locales, a dragged tab (when it is not attached to a tab strip)
     // is rendered using a right-to-left orientation so we should calculate the
     // window position differently.
@@ -98,23 +97,9 @@ void DraggedTabView::MoveTo(const gfx::Point& screen_point) {
 #endif
 }
 
-void DraggedTabView::Attach(int selected_width) {
-  attached_ = true;
-  photobooth_ = NULL;
-#if defined(OS_WIN)
-  container_->SetOpacity(kOpaqueAlpha);
-#endif
-  Resize(selected_width);
-}
-
-void DraggedTabView::Resize(int width) {
-  attached_tab_size_.set_width(width);
-  ResizeContainer();
-  Update();
-}
-
-void DraggedTabView::Detach(NativeViewPhotobooth* photobooth) {
-  attached_ = false;
+void DraggedTabView::SetTabWidthAndUpdate(int width,
+                                          NativeViewPhotobooth* photobooth) {
+  tab_size_.set_width(width);
   photobooth_ = photobooth;
 #if defined(OS_WIN)
   container_->SetOpacity(kTransparentAlpha);
@@ -138,40 +123,26 @@ void DraggedTabView::Update() {
 // DraggedTabView, views::View overrides:
 
 void DraggedTabView::Paint(gfx::Canvas* canvas) {
-  if (attached_) {
-    PaintAttachedTab(canvas);
-  } else {
-    if (show_contents_on_drag_) {
-      PaintDetachedView(canvas);
-    } else {
-      PaintFocusRect(canvas);
-    }
-  }
+  if (show_contents_on_drag_)
+    PaintDetachedView(canvas);
+  else
+    PaintFocusRect(canvas);
 }
 
 void DraggedTabView::Layout() {
-  if (attached_) {
-    gfx::Size prefsize = GetPreferredSize();
-    renderer_->SetBounds(0, 0, prefsize.width(), prefsize.height());
-  } else {
-    int left = 0;
-    if (base::i18n::IsRTL())
-      left = GetPreferredSize().width() - attached_tab_size_.width();
-    // The renderer_'s width should be attached_tab_size_.width() in both LTR
-    // and RTL locales. Wrong width will cause the wrong positioning of the tab
-    // view in dragging. Please refer to http://crbug.com/6223 for details.
-    renderer_->SetBounds(left, 0, attached_tab_size_.width(),
-                         attached_tab_size_.height());
-  }
+  int left = 0;
+  if (base::i18n::IsRTL())
+    left = GetPreferredSize().width() - tab_size_.width();
+  // The renderer_'s width should be tab_size_.width() in both LTR and RTL
+  // locales. Wrong width will cause the wrong positioning of the tab view in
+  // dragging. Please refer to http://crbug.com/6223 for details.
+  renderer_->SetBounds(left, 0, tab_size_.width(), tab_size_.height());
 }
 
 gfx::Size DraggedTabView::GetPreferredSize() {
-  if (attached_)
-    return attached_tab_size_;
-
-  int width = std::max(attached_tab_size_.width(), contents_size_.width()) +
+  int width = std::max(tab_size_.width(), contents_size_.width()) +
       kTwiceDragFrameBorderSize;
-  int height = attached_tab_size_.height() + kDragFrameBorderSize +
+  int height = tab_size_.height() + kDragFrameBorderSize +
       contents_size_.height();
   return gfx::Size(width, height);
 }
@@ -179,10 +150,6 @@ gfx::Size DraggedTabView::GetPreferredSize() {
 ////////////////////////////////////////////////////////////////////////////////
 // DraggedTabView, private:
 
-void DraggedTabView::PaintAttachedTab(gfx::Canvas* canvas) {
-  renderer_->ProcessPaint(canvas);
-}
-
 void DraggedTabView::PaintDetachedView(gfx::Canvas* canvas) {
   gfx::Size ps = GetPreferredSize();
   gfx::CanvasSkia scale_canvas(ps.width(), ps.height(), false);
@@ -191,13 +158,13 @@ void DraggedTabView::PaintDetachedView(gfx::Canvas* canvas) {
   bitmap_device.eraseARGB(0, 0, 0, 0);
 
   scale_canvas.FillRectInt(kDraggedTabBorderColor, 0,
-      attached_tab_size_.height() - kDragFrameBorderSize,
-      ps.width(), ps.height() - attached_tab_size_.height());
+      tab_size_.height() - kDragFrameBorderSize,
+      ps.width(), ps.height() - tab_size_.height());
   int image_x = kDragFrameBorderSize;
-  int image_y = attached_tab_size_.height();
+  int image_y = tab_size_.height();
   int image_w = ps.width() - kTwiceDragFrameBorderSize;
   int image_h =
-      ps.height() - kTwiceDragFrameBorderSize - attached_tab_size_.height();
+      ps.height() - kTwiceDragFrameBorderSize - tab_size_.height();
   scale_canvas.FillRectInt(SK_ColorBLACK, image_x, image_y, image_w, image_h);
   photobooth_->PaintScreenshotIntoCanvas(
       &scale_canvas,
@@ -252,5 +219,5 @@ void DraggedTabView::ResizeContainer() {
 }
 
 int DraggedTabView::ScaleValue(int value) {
-  return attached_ ? value : static_cast<int>(value * kScalingFactor);
+  return static_cast<int>(value * kScalingFactor);
 }
diff --git a/chrome/browser/views/tabs/dragged_tab_view.h b/chrome/browser/views/tabs/dragged_tab_view.h
index cda1daa..f2ded5a 100644
--- a/chrome/browser/views/tabs/dragged_tab_view.h
+++ b/chrome/browser/views/tabs/dragged_tab_view.h
@@ -43,31 +43,18 @@ class DraggedTabView : public views::View {
     mouse_tab_offset_ = offset;
   }
 
-  // Notifies the DraggedTabView that it has become attached to a TabStrip.
-  void Attach(int selected_width);
-
-  // Resizes the dragged tab to a width of |width|.
-  void Resize(int width);
-
-  // Notifies the DraggedTabView that it has been detached from a TabStrip.
-  void Detach(NativeViewPhotobooth* photobooth);
+  // Sets the width of the dragged tab and updates the dragged image.
+  void SetTabWidthAndUpdate(int width, NativeViewPhotobooth* photobooth);
 
   // Notifies the DraggedTabView that it should update itself.
   void Update();
 
-  // Returns the size of the DraggedTabView. Used when attaching to a TabStrip
-  // to determine where to place the Tab in the attached TabStrip.
-  const gfx::Size& attached_tab_size() const { return attached_tab_size_; }
-
  private:
   // Overridden from views::View:
   virtual void Paint(gfx::Canvas* canvas);
   virtual void Layout();
   virtual gfx::Size GetPreferredSize();
 
-  // Paint the view, when it's attached to a TabStrip.
-  void PaintAttachedTab(gfx::Canvas* canvas);
-
   // Paint the view, when it's not attached to any TabStrip.
   void PaintDetachedView(gfx::Canvas* canvas);
 
@@ -90,10 +77,6 @@ class DraggedTabView : public views::View {
   // The renderer that paints the Tab shape.
   scoped_ptr<views::View> renderer_;
 
-  // True if the view is currently attached to a TabStrip. Controls rendering
-  // and sizing modes.
-  bool attached_;
-
   // True if "Show window contents while dragging" is enabled.
   bool show_contents_on_drag_;
 
@@ -103,9 +86,8 @@ class DraggedTabView : public views::View {
   // position of detached windows.
   gfx::Point mouse_tab_offset_;
 
-  // The size of the tab renderer when the dragged tab is attached to a
-  // tabstrip.
-  gfx::Size attached_tab_size_;
+  // The size of the tab renderer.
+  gfx::Size tab_size_;
 
   // A handle to the DIB containing the current screenshot of the TabContents
   // we are dragging.