Attempt at fixing tab dragging crash. I believe the crash is happening

Attempt at fixing tab dragging crash. I believe the crash is happening
because there is an existing TabDragController when a new one is
created. Right after we create the new TabDragController it replaces
the old, but if we create the new one before the old is destroyed it
means the old is still set as a delegate on the TabContents so that
when the new restores the delegates it restores the old
TabDragController, which has now been deleted.

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


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114653

[Ben Goodger (Google), 2011-12-15 00:39:28]

LGTM

On Wed, Dec 14, 2011 at 4:36 PM, <sky@chromium.org> wrote:

> Reviewers: Ben Goodger (Google),
>
> Description:
> Attempt at fixing tab dragging crash. I believe the crash is happening
> because there is an existing TabDragController when a new one is
> created. Right after we create the new TabDragController it replaces
> the old, but if we create the new one before the old is destroyed it
> means the old is still set as a delegate on the TabContents so that
> when the new restores the delegates it restores the old
> TabDragController, which has now been deleted.
>
> BUG=107172
> TEST=none
> R=ben@chromium.org
>
>
> Please review this at
http://codereview.chromium.**org/8915026/<http://codereview.chromium.org/8915...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>  M chrome/browser/ui/views/tabs/**base_tab_strip.cc
>
>
> Index: chrome/browser/ui/views/tabs/**base_tab_strip.cc
> diff --git a/chrome/browser/ui/views/**tabs/base_tab_strip.cc
> b/chrome/browser/ui/views/**tabs/base_tab_strip.cc
> index e7e220d4f543bf4b4bbb8cafed805a**c3c2b4d56b..**
> c07c22d433359de11834b782ead81f**e0ec4dbda6 100644
> --- a/chrome/browser/ui/views/**tabs/base_tab_strip.cc
> +++ b/chrome/browser/ui/views/**tabs/base_tab_strip.cc
> @@ -343,6 +343,10 @@ void BaseTabStrip::MaybeStartDrag(
>   TabStripSelectionModel selection_model;
>   if (!original_selection.**IsSelected(model_index))
>     selection_model.Copy(original_**selection);
> +  // Delete the existing DragController before creating a new one. We do
> this as
> +  // creating the DragController remembers the TabContents delegates and
> we need
> +  // to make sure the existing DragController isn't still a delegate.
> +  drag_controller_.reset();
>   drag_controller_.reset(**TabDragController::Create(
>       this, tab, tabs, gfx::Point(x, y), tab->GetMirroredXInView(event.**
> x()),
>       selection_model));
>
>
>

[commit-bot: I haz the power, 2011-12-15 00:40:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sky@chromium.org/8915026/1

[commit-bot: I haz the power, 2011-12-15 02:31:21]

Try job failure for 8915026-1 (retry) on mac_rel for steps "browser_tests,
ui_tests" (clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2011-12-15 15:59:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sky@chromium.org/8915026/1

[commit-bot: I haz the power, 2011-12-15 17:10:06]

Change committed as 114653