Remove WebFrame from WebFramesManager before destruction.
WebFrame destructor can lead to calling callbacks that will
try to access the WebFrame via GetFrameWithId.
This leads to reentrancy problem on the std::map.
Remove the WebFrame from the map before destroying it.
Bug: 935000
Change-Id: I458cef209d509dd0c61acdb3b9da01cf301d49c1
Reviewed-on: https://chromium-review.googlesource.com/c/1488732
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Commit-Queue: Olivier Robin <olivierrobin@chromium.org>
Auto-Submit: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636045}
diff --git a/ios/web/web_state/web_frames_manager_impl.mm b/ios/web/web_state/web_frames_manager_impl.mm
index 343a1adf..9b34495 100644
--- a/ios/web/web_state/web_frames_manager_impl.mm
+++ b/ios/web/web_state/web_frames_manager_impl.mm
@@ -70,15 +70,25 @@
DCHECK(web_frames_.count(frame_id) == 0 ||
!web_frames_[frame_id]->IsMainFrame() ||
main_web_frame_ == web_frames_[frame_id].get());
+ if (web_frames_.count(frame_id) == 0) {
+ return;
+ }
if (main_web_frame_ && main_web_frame_->GetFrameId() == frame_id) {
main_web_frame_ = nullptr;
}
+ // The web::WebFrame destructor can call some callbacks that will try to
+ // access the frame via GetFrameWithId. This can lead to a reentrancy issue
+ // on |web_frames_|.
+ // To avoid this issue, keep the frame alive during the map operation and
+ // destroy it after.
+ auto keep_frame_alive = std::move(web_frames_[frame_id]);
web_frames_.erase(frame_id);
}
void WebFramesManagerImpl::RemoveAllWebFrames() {
- main_web_frame_ = nullptr;
- web_frames_.clear();
+ while (web_frames_.size()) {
+ RemoveFrameWithId(web_frames_.begin()->first);
+ }
}
WebFrame* WebFramesManagerImpl::GetFrameWithId(const std::string& frame_id) {