Commit 3dc13139 authored by Xiaoqian Dai's avatar Xiaoqian Dai Committed by Commit Bot

Tab dragging: Fix the two fingers crash.

If a tab dragging is ended by a second finger tap down, PerformDeferredAttach()
might be called to merge the dragged tab into an overview window under the
current event position. The event position is calculated based on the current
attached tabstrip if it's a touch down event, thus we need to make sure the
current attached tabstrip is not nullptr when GetCursorScreenPoint() is called.

Bug: 877698
Change-Id: Ic2dcf9f2d4e79c00f3fd7ff0b724a8a45ba64781
Reviewed-on: https://chromium-review.googlesource.com/1200526Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588868}
parent bc1bd3c0
......@@ -1591,10 +1591,14 @@ void TabDragController::PerformDeferredAttach() {
SetDeferredTargetTabstrip(nullptr);
deferred_target_tabstrip_observer_.reset();
// GetCursorScreenPoint() needs to be called before Detach() is called as
// GetCursorScreenPoint() may use the current attached tabstrip to get the
// touch event position but Detach() sets attached tabstrip to nullptr.
const gfx::Point current_screen_point = GetCursorScreenPoint();
Detach(DONT_RELEASE_CAPTURE);
// If we're attaching the dragged tabs to an overview window's tabstrip, the
// tabstrip should not have focus.
Attach(target_tabstrip, GetCursorScreenPoint(), /*set_capture=*/false);
Attach(target_tabstrip, current_screen_point, /*set_capture=*/false);
#endif
}
......
......@@ -2950,6 +2950,73 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
EXPECT_EQ(3u, browser_list->size());
}
namespace {
void SecondFingerPressTestStep3(DetachToBrowserTabDragControllerTest* test) {
ASSERT_TRUE(test->ReleaseInput());
}
void SecondFingerPressTestStep2(DetachToBrowserTabDragControllerTest* test,
TabStrip* not_attached_tab_strip,
TabStrip* target_tab_strip) {
ASSERT_TRUE(TabDragController::IsActive());
// And there should be three browser windows, including the newly created one
// for the dragged tab.
EXPECT_EQ(3u, test->browser_list->size());
// Put the window that accociated with |target_tab_strip| in overview.
target_tab_strip->GetWidget()->GetNativeWindow()->SetProperty(
ash::kIsShowingInOverviewKey, true);
// Drag to |target_tab_strip|.
gfx::Point target_point(target_tab_strip->width() / 2,
target_tab_strip->height() / 2);
views::View::ConvertPointToScreen(target_tab_strip, &target_point);
ASSERT_TRUE(test->DragInputTo(target_point));
// Now add a second finger to tap on it.
not_attached_tab_strip->GetWidget()->GetNativeWindow()->env()->set_touch_down(
true);
ASSERT_TRUE(test->PressInput2());
ASSERT_TRUE(test->ReleaseInput2());
ASSERT_TRUE(test->DragInputToNotifyWhenDone(
target_point.x(), target_point.y(),
base::BindOnce(&SecondFingerPressTestStep3, test)));
}
} // namespace
// Tests that when drgging a tab to a browser window that's currently in
// overview, press the second finger should not cause chrome crash.
IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTestTouch,
SecondFingerPressTest) {
TabStrip* tab_strip = GetTabStripForBrowser(browser());
// Add another tab to browser().
AddTabAndResetBrowser(browser());
// Create another browser.
Browser* browser2 = CreateAnotherWindowBrowserAndRelayout();
TabStrip* tab_strip2 = GetTabStripForBrowser(browser2);
// Move to the first tab and drag it enough so that it detaches, but not
// enough that it attaches to browser2.
gfx::Point tab_0_center(GetCenterInScreenCoordinates(tab_strip->tab_at(0)));
ASSERT_TRUE(PressInput(tab_0_center));
ASSERT_TRUE(DragInputToNotifyWhenDone(
tab_0_center.x(), tab_0_center.y() + GetDetachY(tab_strip),
base::BindOnce(&SecondFingerPressTestStep2, this, tab_strip,
tab_strip2)));
QuitWhenNotDragging();
// Test that after dragging there is no crash and the dragged tab should now
// be merged into the target tabstrip.
ASSERT_FALSE(TabDragController::IsActive());
EXPECT_EQ(2u, browser_list->size());
}
#endif // OS_CHROMEOS
#if defined(OS_CHROMEOS)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment