Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1143)

Unified Diff: content/browser/presentation/presentation_service_impl.cc

Issue 996173006: [Presentation API] Fix potential callback leaks in PSImpl. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Addressed Anton's comments Created 5 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « content/browser/presentation/presentation_service_impl.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/presentation/presentation_service_impl.cc
diff --git a/content/browser/presentation/presentation_service_impl.cc b/content/browser/presentation/presentation_service_impl.cc
index 2d2a52c5dd309772f8dedb6951f286b3336c35aa..9d4baa282682d883252dca381973bb53d95cff04 100644
--- a/content/browser/presentation/presentation_service_impl.cc
+++ b/content/browser/presentation/presentation_service_impl.cc
@@ -25,6 +25,7 @@ PresentationServiceImpl::PresentationServiceImpl(
: WebContentsObserver(web_contents),
render_frame_host_(render_frame_host),
delegate_(delegate),
+ next_request_session_id_(0),
weak_factory_(this) {
DCHECK(render_frame_host_);
DCHECK(web_contents);
@@ -133,10 +134,7 @@ void PresentationServiceImpl::StartSession(
const NewSessionMojoCallback& callback) {
DVLOG(2) << "StartSession";
if (!delegate_) {
- callback.Run(
- presentation::PresentationSessionInfoPtr(),
- presentation::PresentationError::From(
- PresentationError(PRESENTATION_ERROR_UNKNOWN, "")));
+ InvokeNewSessionMojoCallbackWithError(callback);
return;
}
@@ -152,23 +150,20 @@ void PresentationServiceImpl::JoinSession(
const NewSessionMojoCallback& callback) {
DVLOG(2) << "JoinSession";
if (!delegate_) {
- callback.Run(
- presentation::PresentationSessionInfoPtr(),
- presentation::PresentationError::From(
- PresentationError(PRESENTATION_ERROR_UNKNOWN, "")));
+ InvokeNewSessionMojoCallbackWithError(callback);
return;
}
+ int request_session_id = RegisterNewSessionCallback(callback);
delegate_->JoinSession(
render_frame_host_->GetProcess()->GetID(),
render_frame_host_->GetRoutingID(),
presentation_url,
presentation_id,
- // TODO(imcheng): These callbacks may be dropped. http://crbug.com/468575
base::Bind(&PresentationServiceImpl::OnStartOrJoinSessionSucceeded,
- weak_factory_.GetWeakPtr(), false, callback),
+ weak_factory_.GetWeakPtr(), false, request_session_id),
base::Bind(&PresentationServiceImpl::OnStartOrJoinSessionError,
- weak_factory_.GetWeakPtr(), false, callback));
+ weak_factory_.GetWeakPtr(), false, request_session_id));
}
void PresentationServiceImpl::HandleQueuedStartSessionRequests() {
@@ -183,27 +178,36 @@ void PresentationServiceImpl::HandleQueuedStartSessionRequests() {
}
}
+int PresentationServiceImpl::RegisterNewSessionCallback(
+ const NewSessionMojoCallback& callback) {
+ ++next_request_session_id_;
+ pending_session_cbs_[next_request_session_id_].reset(
+ new NewSessionMojoCallback(callback));
+ return next_request_session_id_;
+}
+
void PresentationServiceImpl::DoStartSession(
const std::string& presentation_url,
const std::string& presentation_id,
const NewSessionMojoCallback& callback) {
+ int request_session_id = RegisterNewSessionCallback(callback);
delegate_->StartSession(
render_frame_host_->GetProcess()->GetID(),
render_frame_host_->GetRoutingID(),
presentation_url,
presentation_id,
- // TODO(imcheng): These callbacks may be dropped. http://crbug.com/468575
base::Bind(&PresentationServiceImpl::OnStartOrJoinSessionSucceeded,
- weak_factory_.GetWeakPtr(), true, callback),
+ weak_factory_.GetWeakPtr(), true, request_session_id),
base::Bind(&PresentationServiceImpl::OnStartOrJoinSessionError,
- weak_factory_.GetWeakPtr(), true, callback));
+ weak_factory_.GetWeakPtr(), true, request_session_id));
}
void PresentationServiceImpl::OnStartOrJoinSessionSucceeded(
bool is_start_session,
- const NewSessionMojoCallback& callback,
+ int request_session_id,
const PresentationSessionInfo& session_info) {
- callback.Run(
+ RunAndEraseNewSessionMojoCallback(
+ request_session_id,
presentation::PresentationSessionInfo::From(session_info),
presentation::PresentationErrorPtr());
if (is_start_session)
@@ -212,15 +216,29 @@ void PresentationServiceImpl::OnStartOrJoinSessionSucceeded(
void PresentationServiceImpl::OnStartOrJoinSessionError(
bool is_start_session,
- const NewSessionMojoCallback& callback,
+ int request_session_id,
const PresentationError& error) {
- callback.Run(
+ RunAndEraseNewSessionMojoCallback(
+ request_session_id,
presentation::PresentationSessionInfoPtr(),
presentation::PresentationError::From(error));
if (is_start_session)
HandleQueuedStartSessionRequests();
}
+void PresentationServiceImpl::RunAndEraseNewSessionMojoCallback(
+ int request_session_id,
+ presentation::PresentationSessionInfoPtr session,
+ presentation::PresentationErrorPtr error) {
+ auto it = pending_session_cbs_.find(request_session_id);
+ if (it == pending_session_cbs_.end())
+ return;
+
+ DCHECK(it->second.get());
+ it->second->Run(session.Pass(), error.Pass());
+ pending_session_cbs_.erase(it);
+}
+
void PresentationServiceImpl::DoSetDefaultPresentationUrl(
const std::string& default_presentation_url,
const std::string& default_presentation_id) {
@@ -327,12 +345,26 @@ void PresentationServiceImpl::Reset() {
default_presentation_url_.clear();
default_presentation_id_.clear();
- for (const auto& context : availability_contexts_) {
- context.second->OnScreenAvailabilityChanged(false);
+ for (const auto& context_entry : availability_contexts_) {
+ context_entry.second->OnScreenAvailabilityChanged(false);
}
availability_contexts_.clear();
- // TODO(imcheng): This may drop callbacks. See http://crbug.com/468575.
+ for (auto& request_ptr : queued_start_session_requests_) {
+ InvokeNewSessionMojoCallbackWithError(request_ptr->callback);
+ }
queued_start_session_requests_.clear();
+ for (auto& pending_entry : pending_session_cbs_) {
+ InvokeNewSessionMojoCallbackWithError(*pending_entry.second);
+ }
+ pending_session_cbs_.clear();
+}
+
+void PresentationServiceImpl::InvokeNewSessionMojoCallbackWithError(
+ const NewSessionMojoCallback& callback) {
+ callback.Run(
+ presentation::PresentationSessionInfoPtr(),
+ presentation::PresentationError::From(
+ PresentationError(PRESENTATION_ERROR_UNKNOWN, "Internal error")));
}
void PresentationServiceImpl::OnDelegateDestroyed() {
« no previous file with comments | « content/browser/presentation/presentation_service_impl.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698