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

Unified Diff: chrome/browser/ui/webui/media_router/media_router_ui.cc

Issue 1224093004: [Media Router] 2nd take on fix route response callback lifetime in UI. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 5 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
Index: chrome/browser/ui/webui/media_router/media_router_ui.cc
diff --git a/chrome/browser/ui/webui/media_router/media_router_ui.cc b/chrome/browser/ui/webui/media_router/media_router_ui.cc
index 658a368924e85d1cad18e8533f08ae8e0763b4aa..cafd5fd161b3dea7d5c41edf98ad03310cae85c9 100644
--- a/chrome/browser/ui/webui/media_router/media_router_ui.cc
+++ b/chrome/browser/ui/webui/media_router/media_router_ui.cc
@@ -7,7 +7,6 @@
#include <string>
#include "base/strings/string_util.h"
-#include "chrome/browser/media/router/create_session_request.h"
#include "chrome/browser/media/router/issue.h"
#include "chrome/browser/media/router/issues_observer.h"
#include "chrome/browser/media/router/media_route.h"
@@ -22,6 +21,7 @@
#include "chrome/browser/media/router/presentation_service_delegate_impl.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/sessions/session_tab_helper.h"
+#include "chrome/browser/ui/webui/media_router/media_router_dialog_controller.h"
#include "chrome/browser/ui/webui/media_router/media_router_localized_strings_provider.h"
#include "chrome/browser/ui/webui/media_router/media_router_resources_provider.h"
#include "chrome/browser/ui/webui/media_router/media_router_webui_message_handler.h"
@@ -91,6 +91,7 @@ MediaRouterUI::MediaRouterUI(content::WebUI* web_ui)
ui_initialized_(false),
has_pending_route_request_(false),
requesting_route_for_default_source_(false),
+ dialog_callbacks_(nullptr),
initiator_(nullptr),
router_(nullptr),
weak_factory_(this) {
@@ -121,37 +122,50 @@ MediaRouterUI::~MediaRouterUI() {
query_result_manager_->RemoveObserver(this);
if (presentation_service_delegate_.get())
presentation_service_delegate_->RemoveDefaultMediaSourceObserver(this);
+ if (dialog_callbacks_)
+ dialog_callbacks_->OnDialogDestroyed(requesting_route_for_default_source_);
}
void MediaRouterUI::InitWithDefaultMediaSource(
- PresentationServiceDelegateImpl* delegate) {
+ const base::WeakPtr<PresentationServiceDelegateImpl>& delegate,
+ MediaRouterDialogCallbacks* dialog_callbacks) {
DCHECK(delegate);
+ DCHECK(dialog_callbacks);
DCHECK(!presentation_service_delegate_);
DCHECK(!query_result_manager_.get());
- presentation_service_delegate_ = delegate->GetWeakPtr();
+ presentation_service_delegate_ = delegate;
presentation_service_delegate_->AddDefaultMediaSourceObserver(this);
InitCommon(presentation_service_delegate_->web_contents(),
presentation_service_delegate_->default_source(),
- presentation_service_delegate_->default_frame_url());
+ presentation_service_delegate_->default_frame_url(),
+ dialog_callbacks);
}
void MediaRouterUI::InitWithPresentationSessionRequest(
const content::WebContents* initiator,
- scoped_ptr<CreateSessionRequest> request) {
- DCHECK(request.get());
- DCHECK(!presentation_session_request_.get());
+ MediaRouterDialogCallbacks* dialog_callbacks) {
+ DCHECK(initiator);
+ DCHECK(dialog_callbacks);
DCHECK(!query_result_manager_.get());
- presentation_session_request_ = request.Pass();
- InitCommon(initiator, presentation_session_request_->GetMediaSource(),
- presentation_session_request_->frame_url());
+ const CreateSessionRequest* presentation_request =
+ dialog_callbacks->presentation_request();
+ DCHECK(presentation_request);
+ InitCommon(initiator, presentation_request->GetMediaSource(),
+ presentation_request->frame_url(), dialog_callbacks);
}
void MediaRouterUI::InitCommon(const content::WebContents* initiator,
const MediaSource& default_source,
- const GURL& default_frame_url) {
+ const GURL& default_frame_url,
+ MediaRouterDialogCallbacks* dialog_callbacks) {
DCHECK(initiator);
+ DCHECK(dialog_callbacks);
+
+ dialog_callbacks_ = dialog_callbacks;
+ dialog_callbacks_->set_route_response_cb(base::Bind(
+ &MediaRouterUI::OnRouteResponseReceived, weak_factory_.GetWeakPtr()));
// Register for Issue and MediaRoute updates.
issues_observer_.reset(new UIIssuesObserver(router_, this));
@@ -186,20 +200,6 @@ void MediaRouterUI::OnDefaultMediaSourceChanged(const MediaSource& source,
UpdateSourceHostAndCastModes(frame_url);
}
-void MediaRouterUI::HandleRouteResponseForPresentation(
- const MediaRoute* route,
- const std::string& error) {
- if (!route) {
- presentation_session_request_->MaybeInvokeErrorCallback(
- content::PresentationError(content::PRESENTATION_ERROR_UNKNOWN, error));
- } else {
- // TODO(imcheng): Presentation ID should come from the response
- // as the IDs might not be the same.
- presentation_session_request_->MaybeInvokeSuccessCallback(
- route->media_route_id());
- }
-}
-
void MediaRouterUI::UpdateSourceHostAndCastModes(const GURL& frame_url) {
DCHECK(query_result_manager_);
frame_url_ = frame_url;
@@ -209,13 +209,6 @@ void MediaRouterUI::UpdateSourceHostAndCastModes(const GURL& frame_url) {
}
void MediaRouterUI::Close() {
- if (presentation_session_request_.get()) {
- presentation_session_request_->MaybeInvokeErrorCallback(
- content::PresentationError(
- content::PRESENTATION_ERROR_SESSION_REQUEST_CANCELLED,
- "Dialog closed."));
- }
-
ConstrainedWebDialogDelegate* delegate = GetConstrainedDelegate();
if (delegate) {
delegate->GetWebDialogDelegate()->OnDialogClosed(std::string());
@@ -273,7 +266,7 @@ void MediaRouterUI::OnRoutesUpdated(const std::vector<MediaRoute>& routes) {
handler_->UpdateRoutes(routes_);
}
-void MediaRouterUI::OnRouteResponseReceived(scoped_ptr<MediaRoute> route,
+void MediaRouterUI::OnRouteResponseReceived(const MediaRoute* route,
const std::string& error) {
DVLOG(1) << "OnRouteResponseReceived";
// TODO(imcheng): Display error in UI. (crbug.com/490372)
@@ -282,18 +275,6 @@ void MediaRouterUI::OnRouteResponseReceived(scoped_ptr<MediaRoute> route,
else
handler_->AddRoute(*route);
- if (requesting_route_for_default_source_) {
- if (presentation_session_request_.get()) {
- HandleRouteResponseForPresentation(route.get(), error);
- } else {
- // Dialog initiated via browser action. Let
- // PresentationServiceDelegateImpl perform the match against the default
- // presentation URL.
- if (route && presentation_service_delegate_.get())
- presentation_service_delegate_->OnRouteCreated(*route);
- }
- }
-
has_pending_route_request_ = false;
requesting_route_for_default_source_ = false;
}
@@ -301,6 +282,7 @@ void MediaRouterUI::OnRouteResponseReceived(scoped_ptr<MediaRoute> route,
bool MediaRouterUI::DoCreateRoute(const MediaSink::Id& sink_id,
MediaCastMode cast_mode) {
DCHECK(query_result_manager_.get());
+ DCHECK(dialog_callbacks_);
DCHECK(initiator_);
// Note that there is a rarely-encountered bug, where the MediaCastMode to
@@ -329,10 +311,11 @@ bool MediaRouterUI::DoCreateRoute(const MediaSink::Id& sink_id,
DVLOG(1) << "DoCreateRoute: origin: " << origin;
- router_->CreateRoute(source.id(), sink_id, origin,
- SessionTabHelper::IdForTab(initiator_),
- base::Bind(&MediaRouterUI::OnRouteResponseReceived,
- weak_factory_.GetWeakPtr()));
+ router_->CreateRoute(
+ source.id(), sink_id, origin, SessionTabHelper::IdForTab(initiator_),
+ base::Bind(&MediaRouterDialogCallbacks::OnRouteResponseReceived,
+ dialog_callbacks_->GetWeakPtr(),
+ requesting_route_for_default_source_));
return true;
}

Powered by Google App Engine
This is Rietveld 408576698