Skip to content

Commit

Permalink
Fix chromedriver crash caused my multiple Target.attachedToTarget eve…
Browse files Browse the repository at this point in the history
…nts.

R=johnchen@chromium.org

Change-Id: I8f21d99596c78ff7890432a790242c3876d4ba23
Reviewed-on: https://chromium-review.googlesource.com/c/1352772
Reviewed-by: John Chen <johnchen@chromium.org>
Commit-Queue: John Chen <johnchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613004}
  • Loading branch information
pnoffke-provar authored and Commit Bot committed Dec 3, 2018
1 parent 828f706 commit 5a79045
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 6 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -1030,3 +1030,4 @@ Venture 3 Systems LLC <*@venture3systems.com>
Vewd Software AS <*@vewd.com>
Vivaldi Technologies AS <*@vivaldi.com>
Yandex LLC <*@yandex-team.ru>
Make Positive Provar Limited <*@provartesting.com>
24 changes: 18 additions & 6 deletions chrome/test/chromedriver/chrome/frame_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,24 @@ Status FrameTracker::OnEvent(DevToolsClient* client,
if (!params.GetString("sessionId", &session_id))
return Status(kUnknownError,
"missing session ID in Target.attachedToTarget event");
std::unique_ptr<WebViewImpl> child(
static_cast<WebViewImpl*>(web_view_)->CreateChild(session_id,
target_id));
WebViewImplHolder child_holder(child.get());
frame_to_target_map_[target_id] = std::move(child);
frame_to_target_map_[target_id]->ConnectIfNecessary();
if (frame_to_target_map_.count(target_id) > 0) {
// Since chrome 70 we are seeing multiple Target.attachedToTarget events
// for the same target_id. This is causing crashes because:
// - replacing the value in frame_to_target_map_ is causing the
// pre-existing one to be disposed
// - if there are in-flight requests for the disposed DevToolsClient
// then chromedriver is crashing in the ProcessNextMessage processing
// - the in-flight messages observed were DOM.getDocument requests from
// DomTracker.
// The fix is to not replace an pre-existing frame_to_target_map_ entry.
} else {
std::unique_ptr<WebViewImpl> child(
static_cast<WebViewImpl*>(web_view_)->CreateChild(session_id,
target_id));
WebViewImplHolder child_holder(child.get());
frame_to_target_map_[target_id] = std::move(child);
frame_to_target_map_[target_id]->ConnectIfNecessary();
}
}
} else if (method == "Target.detachedFromTarget") {
std::string target_id;
Expand Down

0 comments on commit 5a79045

Please sign in to comment.