Commit 85202fc5 authored by mfomitchev's avatar mfomitchev Committed by Commit bot

Gesture Nav: Protect against the case when navigation completes as soon as it is requested.

If DidStopLoading is called on WebContentsObserver immeditely after navigation
is requested, then window_ in could be released via StopObservingIfDone as soon
as GoForward/GoBack is called

OerscrollNavigationOverlay::OnOverscrollCompleted requests navigaions by calling
GoForward/GoBack on the controller. If these navigations immeditely trigger
DidStopLoading, main_window->SetTransform would segfault.
Protect against this case by setting up the transforms and window positioning
before doing navigation.

BUG=623620

Review-Url: https://codereview.chromium.org/2102283002
Cr-Commit-Position: refs/heads/master@{#403186}
parent baef2ce0
...@@ -223,6 +223,14 @@ void OverscrollNavigationOverlay::OnOverscrollCompleted( ...@@ -223,6 +223,14 @@ void OverscrollNavigationOverlay::OnOverscrollCompleted(
return; return;
} }
main_window->SetTransform(gfx::Transform());
window_ = std::move(window);
// Make sure the window is in its default position.
window_->SetBounds(gfx::Rect(web_contents_window_->bounds().size()));
window_->SetTransform(gfx::Transform());
// Make sure the overlay window is on top.
web_contents_window_->StackChildAtTop(window_.get());
// Make sure we can navigate first, as other factors can trigger a navigation // Make sure we can navigate first, as other factors can trigger a navigation
// during an overscroll gesture and navigating without history produces a // during an overscroll gesture and navigating without history produces a
// crash. // crash.
...@@ -247,13 +255,6 @@ void OverscrollNavigationOverlay::OnOverscrollCompleted( ...@@ -247,13 +255,6 @@ void OverscrollNavigationOverlay::OnOverscrollCompleted(
StartObserving(); StartObserving();
} }
main_window->SetTransform(gfx::Transform());
window_ = std::move(window);
// Make sure the window is in its default position.
window_->SetBounds(gfx::Rect(web_contents_window_->bounds().size()));
window_->SetTransform(gfx::Transform());
// Make sure the overlay window is on top.
web_contents_window_->StackChildAtTop(window_.get());
direction_ = NONE; direction_ = NONE;
StopObservingIfDone(); StopObservingIfDone();
} }
......
...@@ -58,6 +58,8 @@ class CONTENT_EXPORT OverscrollNavigationOverlay ...@@ -58,6 +58,8 @@ class CONTENT_EXPORT OverscrollNavigationOverlay
FRIEND_TEST_ALL_PREFIXES(OverscrollNavigationOverlayTest, OverlayWindowSwap); FRIEND_TEST_ALL_PREFIXES(OverscrollNavigationOverlayTest, OverlayWindowSwap);
FRIEND_TEST_ALL_PREFIXES(OverscrollNavigationOverlayTest, FRIEND_TEST_ALL_PREFIXES(OverscrollNavigationOverlayTest,
CloseDuringAnimation); CloseDuringAnimation);
FRIEND_TEST_ALL_PREFIXES(OverscrollNavigationOverlayTest,
ImmediateLoadOnNavigate);
// Resets state and starts observing |web_contents_| for page load/paint // Resets state and starts observing |web_contents_| for page load/paint
// updates. This function makes sure that the screenshot window is stacked // updates. This function makes sure that the screenshot window is stacked
......
...@@ -31,6 +31,29 @@ ...@@ -31,6 +31,29 @@
namespace content { namespace content {
// Forces web contents to complete web page load as soon as navigation starts.
class ImmediateLoadObserver : WebContentsObserver {
public:
explicit ImmediateLoadObserver(TestWebContents* contents)
: contents_(contents) {
Observe(contents);
}
~ImmediateLoadObserver() override {}
void DidStartNavigationToPendingEntry(
const GURL& url,
NavigationController::ReloadType reload_type) override {
// Simulate immediate web page load.
contents_->TestSetIsLoading(false);
Observe(nullptr);
}
private:
TestWebContents* contents_;
DISALLOW_COPY_AND_ASSIGN(ImmediateLoadObserver);
};
// A subclass of TestWebContents that offers a fake content window. // A subclass of TestWebContents that offers a fake content window.
class OverscrollTestWebContents : public TestWebContents { class OverscrollTestWebContents : public TestWebContents {
public: public:
...@@ -331,6 +354,18 @@ TEST_F(OverscrollNavigationOverlayTest, CloseDuringAnimation) { ...@@ -331,6 +354,18 @@ TEST_F(OverscrollNavigationOverlayTest, CloseDuringAnimation) {
// Ensure a clean close. // Ensure a clean close.
} }
// Tests that we can handle the case when the load completes as soon as the
// navigation is started.
TEST_F(OverscrollNavigationOverlayTest, ImmediateLoadOnNavigate) {
PerformBackNavigationViaSliderCallbacks();
// This observer will force the page load to complete as soon as the
// navigation starts.
ImmediateLoadObserver immediate_nav(contents());
GetOverlay()->owa_->OnOverscrollModeChange(OVERSCROLL_NONE, OVERSCROLL_EAST);
// This will start and immediately complete the navigation.
GetOverlay()->owa_->OnOverscrollComplete(OVERSCROLL_EAST);
EXPECT_FALSE(GetOverlay()->window_.get());
}
// Tests that swapping the overlay window at the end of a gesture caused by the // Tests that swapping the overlay window at the end of a gesture caused by the
// start of a new overscroll does not crash and the events still reach the new // start of a new overscroll does not crash and the events still reach the new
......
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