Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(368)

Issue 19026: Fixes tab contents crash. This changes a number of things in dragged... (Closed)

Created:
11 years, 11 months ago by sky
Modified:
9 years, 7 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Fixes tab contents crash. This changes a number of things in dragged tab controller: . DraggedTabController now installs itself as the delegate of the TabContents immediately. It needs to do this so that it can stay in sync if the selected TabContents of the NavigationController changes. . DraggedTabController forwards changes to the selected TabContents to the original delegate. This needs to be done so that the original delegate can do any cleanup it needs to do. For example, Browser needs to know if the tab contents changes so that it can remove entries from its internal mapping of what TabContents need to be updated. Similarly the TabStripModel needs to install a listener on the new TabContents type. . If the tab is destroyed while dragging we shouldn't set the attached_tabstrip_ to NULL. Doing so results in invoking source_tabstrip_->DestroyDraggedSourceTab(source_tab_), which is not what should happen. These changes are subtle, so give them a good think. BUG=6369 TEST=this is covered by chrome bot tests. But be sure and exercise tab dragging in as many permutations as you can to make sure this doesn't break anything. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=8731

Patch Set 1 #

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -13 lines) Patch
M chrome/browser/views/tabs/dragged_tab_controller.cc View 1 6 chunks +40 lines, -13 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
sky
11 years, 11 months ago (2009-01-27 18:26:26 UTC) #1
Ben Goodger (Google)
11 years, 11 months ago (2009-01-27 18:38:05 UTC) #2
o rly

LGTM

Powered by Google App Engine
This is Rietveld 408576698