Skip to content

Commit

Permalink
headless: Fix handling of replies without any parameters
Browse files Browse the repository at this point in the history
Fix handling of replies to DevTools commands which don't include any
parameters. Previously we used th presence of the reply dictionary to
decide between a callback with or without parameters, but this
dictionary can be present in the reply also when there are no actual
parameters in it.

BUG=595353

Review URL: https://codereview.chromium.org/1898633006

Cr-Commit-Position: refs/heads/master@{#388543}
  • Loading branch information
skyostil authored and Commit bot committed Apr 20, 2016
1 parent 05e4802 commit 7161641
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 3 deletions.
10 changes: 7 additions & 3 deletions headless/lib/browser/headless_devtools_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,14 @@ void HeadlessDevToolsClientImpl::DispatchProtocolMessage(
NOTREACHED() << "Unexpected reply";
return;
}
base::DictionaryValue* result_dict;
if (message_dict->GetDictionary("result", &result_dict)) {
if (!it->second.callback_with_result.is_null()) {
base::DictionaryValue* result_dict;
if (!message_dict->GetDictionary("result", &result_dict)) {
NOTREACHED() << "Badly formed reply result";
return;
}
it->second.callback_with_result.Run(*result_dict);
} else {
} else if (!it->second.callback.is_null()) {
it->second.callback.Run();
}
pending_messages_.erase(it);
Expand Down
35 changes: 35 additions & 0 deletions headless/lib/headless_devtools_client_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,39 @@ class HeadlessDevToolsClientEvalTest : public HeadlessDevToolsClientTest {

DEVTOOLS_CLIENT_TEST_F(HeadlessDevToolsClientEvalTest);

class HeadlessDevToolsClientCallbackTest : public HeadlessDevToolsClientTest {
public:
HeadlessDevToolsClientCallbackTest() : first_result_received_(false) {}

void RunDevToolsClientTest() override {
// Null callback without parameters.
devtools_client_->GetRuntime()->Run();
// Null callback with parameters.
devtools_client_->GetRuntime()->Evaluate("true");
// Non-null callback without parameters.
devtools_client_->GetRuntime()->Disable(
base::Bind(&HeadlessDevToolsClientCallbackTest::OnFirstResult,
base::Unretained(this)));
// Non-null callback with parameters.
devtools_client_->GetRuntime()->Evaluate(
"true", base::Bind(&HeadlessDevToolsClientCallbackTest::OnSecondResult,
base::Unretained(this)));
}

void OnFirstResult() {
EXPECT_FALSE(first_result_received_);
first_result_received_ = true;
}

void OnSecondResult(std::unique_ptr<runtime::EvaluateResult> result) {
EXPECT_TRUE(first_result_received_);
FinishAsynchronousTest();
}

private:
bool first_result_received_;
};

DEVTOOLS_CLIENT_TEST_F(HeadlessDevToolsClientCallbackTest);

} // namespace headless

0 comments on commit 7161641

Please sign in to comment.