Skip to content
This repository has been archived by the owner on Jun 1, 2022. It is now read-only.

Commit

Permalink
[headless] Ensure devtooled tests call Disable/RemoveObserver.
Browse files Browse the repository at this point in the history
Related to crbug.com/682670, where a missing RemoveObserver() call
caused a segfault. For completeness, this also adds Disable() calls
where they were missing.

BUG=

Review-Url: https://codereview.chromium.org/2642683004
Cr-Commit-Position: refs/heads/master@{#444737}
  • Loading branch information
betasheet authored and Commit bot committed Jan 19, 2017
1 parent 12d9cda commit 6ccdcca
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 1 deletion.
8 changes: 7 additions & 1 deletion headless/lib/embedder_mojo_browsertest.cc
Expand Up @@ -234,14 +234,20 @@ class HttpDisabledByDefaultWhenMojoBindingsUsed : public EmbedderMojoTest,
}

void ReturnTestResult(const std::string& result) override {
FinishAsynchronousTest();
DisableClientAndFinishAsynchronousTest();
FAIL() << "The HTTP page should not have been served and we should not have"
" recieved a mojo callback!";
}

void OnLoadingFailed(const network::LoadingFailedParams& params) override {
// The navigation should fail since HTTP requests are blackholed.
EXPECT_EQ(params.GetErrorText(), "net::ERR_FILE_NOT_FOUND");
DisableClientAndFinishAsynchronousTest();
}

void DisableClientAndFinishAsynchronousTest() {
devtools_client_->GetNetwork()->Disable();
devtools_client_->GetNetwork()->RemoveObserver(this);
FinishAsynchronousTest();
}
};
Expand Down
5 changes: 5 additions & 0 deletions headless/lib/headless_devtools_client_browsertest.cc
Expand Up @@ -65,6 +65,7 @@ class HeadlessDevToolsClientNavigationTest
}

void OnLoadEventFired(const page::LoadEventFiredParams& params) override {
devtools_client_->GetPage()->Disable();
devtools_client_->GetPage()->GetExperimental()->RemoveObserver(this);
FinishAsynchronousTest();
}
Expand Down Expand Up @@ -174,6 +175,7 @@ class HeadlessDevToolsClientObserverTest
&content_type));
EXPECT_EQ("text/html", content_type);

devtools_client_->GetNetwork()->Disable();
devtools_client_->GetNetwork()->RemoveObserver(this);
FinishAsynchronousTest();
}
Expand Down Expand Up @@ -208,6 +210,9 @@ class HeadlessDevToolsClientExperimentalTest

void OnFrameStoppedLoading(
const page::FrameStoppedLoadingParams& params) override {
devtools_client_->GetPage()->Disable();
devtools_client_->GetPage()->GetExperimental()->RemoveObserver(this);

// Check that a non-experimental command which has no return value can be
// called with a void() callback.
devtools_client_->GetPage()->Reload(
Expand Down
1 change: 1 addition & 0 deletions headless/public/util/dom_tree_extractor_browsertest.cc
Expand Up @@ -49,6 +49,7 @@ class DomTreeExtractorBrowserTest : public HeadlessAsyncDevTooledBrowserTest,
}

void OnLoadEventFired(const page::LoadEventFiredParams& params) override {
devtools_client_->GetPage()->Disable();
devtools_client_->GetPage()->RemoveObserver(this);

extractor_.reset(new DomTreeExtractor(devtools_client_.get()));
Expand Down

0 comments on commit 6ccdcca

Please sign in to comment.