diff options
| -rw-r--r-- | chrome_frame/chrome_frame_automation.cc | 26 | ||||
| -rw-r--r-- | chrome_frame/urlmon_url_request.cc | 17 |
2 files changed, 33 insertions, 10 deletions
diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc index 54e114d..1a4b763 100644 --- a/chrome_frame/chrome_frame_automation.cc +++ b/chrome_frame/chrome_frame_automation.cc @@ -1018,8 +1018,13 @@ bool ChromeFrameAutomationClient::ReadRequest( void ChromeFrameAutomationClient::RemoveRequest(PluginUrlRequest* request) { DCHECK_EQ(PlatformThread::CurrentId(), ui_thread_id_); - DCHECK(request_map_.end() != request_map_.find(request->id())); - request_map_.erase(request->id()); + + // We check if the request pointer passed in is actually present in the map + // before going ahead and deleting it. This avoids the issue where we would + // incorrectly delete a different request with the same request id. + if (IsValidRequest(request)) { + request_map_.erase(request->id()); + } } void ChromeFrameAutomationClient::RemoveRequest(int request_id, bool abort) { @@ -1081,17 +1086,23 @@ void ChromeFrameAutomationClient::CleanupRequests() { void ChromeFrameAutomationClient::CleanupAsyncRequests() { DCHECK_EQ(PlatformThread::CurrentId(), ui_thread_id_); + + std::vector<scoped_refptr<PluginUrlRequest> > request_list; + // We copy the pending requests into a temporary vector as the Stop + // function in the request could also try to delete the request from + // the request map and the iterator could end up being invalid. RequestMap::iterator index = request_map_.begin(); while (index != request_map_.end()) { PluginUrlRequest* request = (*index).second; - if (request) { - int request_id = request->id(); - request->Stop(); - } + DCHECK(request != NULL); + request_list.push_back(request); index++; } - request_map_.clear(); + + for (unsigned int index = 0; index < request_list.size(); ++index) { + request_list[index]->Stop(); + } } bool ChromeFrameAutomationClient::Reinitialize( @@ -1108,6 +1119,7 @@ bool ChromeFrameAutomationClient::Reinitialize( return false; } + CleanupAsyncRequests(); chrome_frame_delegate_ = delegate; SetParentWindow(NULL); return true; diff --git a/chrome_frame/urlmon_url_request.cc b/chrome_frame/urlmon_url_request.cc index 1c71edb..560f4b8 100644 --- a/chrome_frame/urlmon_url_request.cc +++ b/chrome_frame/urlmon_url_request.cc @@ -46,13 +46,14 @@ bool UrlmonUrlRequest::Start() { return false; } - worker_thread_->message_loop()->PostTask( - FROM_HERE, NewRunnableMethod(this, &UrlmonUrlRequest::StartAsync)); - // Take a self reference to maintain COM lifetime. This will be released // in EndRequest AddRef(); request_handler()->AddRequest(this); + + worker_thread_->message_loop()->PostTask( + FROM_HERE, NewRunnableMethod(this, &UrlmonUrlRequest::StartAsync)); + return true; } @@ -64,6 +65,11 @@ void UrlmonUrlRequest::Stop() { return; } + // We can remove the request from the map safely here if it is still valid. + // There is an additional reference on the UrlmonUrlRequest instance which + // is released when the task scheduled by the EndRequest function executes. + request_handler()->RemoveRequest(this); + worker_thread_->message_loop()->PostTask( FROM_HERE, NewRunnableMethod(this, &UrlmonUrlRequest::StopAsync)); } @@ -205,6 +211,8 @@ STDMETHODIMP UrlmonUrlRequest::OnStopBinding(HRESULT result, LPCWSTR error) { status_.set_os_error(0); } + DLOG(INFO) << "OnStopBinding received for request id: " << id(); + // Release these variables after reporting EndRequest since we might need to // access their state. binding_ = NULL; @@ -605,6 +613,9 @@ void UrlmonUrlRequest::EndRequest() { } void UrlmonUrlRequest::EndRequestInternal() { + // The request object could have been removed from the map in the + // OnRequestEnd callback which executes on receiving the + // AutomationMsg_RequestEnd IPC from Chrome. request_handler()->RemoveRequest(this); // Release the outstanding reference in the context of the UI thread to // ensure that our instance gets deleted in the same thread which created it. |