Media Remoting: Get permission when requested to start.
Get permission to start remoting when harmony cast dialog is enabled.
Bug: 849020
Change-Id: I98e306c7ffaa6f53dc825a418f2c0b48ea7a9ee2
Reviewed-on: https://chromium-review.googlesource.com/1139173
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Commit-Queue: Xiangjun Zhang <xjz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579226}diff --git a/chrome/browser/media/DEPS b/chrome/browser/media/DEPS
index 6c2e8e0..4ba0b3c8 100644
--- a/chrome/browser/media/DEPS
+++ b/chrome/browser/media/DEPS
@@ -11,3 +11,9 @@
"+services/data_decoder/public",
"+services/device/public/mojom",
]
+
+specific_include_rules = {
+ "cast_remoting_connector\.cc": [
+ "+chrome/browser/ui/views/media_router/media_remoting_dialog_view.h",
+ ],
+}
diff --git a/chrome/browser/media/cast_remoting_connector.cc b/chrome/browser/media/cast_remoting_connector.cc
index 70b6cb7..3682fdf0 100644
--- a/chrome/browser/media/cast_remoting_connector.cc
+++ b/chrome/browser/media/cast_remoting_connector.cc
@@ -5,6 +5,7 @@
#include "chrome/browser/media/cast_remoting_connector.h"
#include <memory>
+#include <utility>
#include "base/bind.h"
#include "base/bind_helpers.h"
@@ -24,6 +25,11 @@
#include "content/public/browser/web_contents.h"
#include "mojo/public/cpp/bindings/strong_binding.h"
+#if defined(TOOLKIT_VIEWS) && \
+ (!defined(OS_MACOSX) || defined(MAC_VIEWS_BROWSER))
+#include "chrome/browser/ui/views/media_router/media_remoting_dialog_view.h"
+#endif
+
using content::BrowserThread;
using media::mojom::RemotingStartFailReason;
using media::mojom::RemotingStopReason;
@@ -92,7 +98,7 @@
}
void Stop(RemotingStopReason reason) final {
if (connector_)
- connector_->StopRemoting(this, reason);
+ connector_->StopRemoting(this, reason, true);
}
void SendMessageToSink(const std::vector<uint8_t>& message) final {
if (connector_)
@@ -137,7 +143,30 @@
connector = new CastRemotingConnector(
media_router::MediaRouterFactory::GetApiForBrowserContext(
contents->GetBrowserContext()),
- tab_id);
+ tab_id,
+#if defined(TOOLKIT_VIEWS) && \
+ (!defined(OS_MACOSX) || defined(MAC_VIEWS_BROWSER))
+ base::BindRepeating(
+ [](content::WebContents* contents,
+ PermissionResultCallback result_callback) {
+ if (media_router::ShouldUseViewsDialog()) {
+ media_router::MediaRemotingDialogView::GetPermission(
+ contents, std::move(result_callback));
+ return base::BindOnce(
+ &media_router::MediaRemotingDialogView::HideDialog);
+ } else {
+ std::move(result_callback).Run(true);
+ return CancelPermissionRequestCallback();
+ }
+ },
+ contents)
+#else
+ base::BindRepeating([](PermissionResultCallback result_callback) {
+ std::move(result_callback).Run(true);
+ return CancelPermissionRequestCallback();
+ })
+#endif
+ );
contents->SetUserData(kUserDataKey, base::WrapUnique(connector));
}
return connector;
@@ -158,14 +187,18 @@
connector->CreateBridge(std::move(source), std::move(request));
}
-CastRemotingConnector::CastRemotingConnector(media_router::MediaRouter* router,
- SessionID tab_id)
+CastRemotingConnector::CastRemotingConnector(
+ media_router::MediaRouter* router,
+ SessionID tab_id,
+ PermissionRequestCallback permission_request_callback)
: media_router_(router),
tab_id_(tab_id),
+ permission_request_callback_(std::move(permission_request_callback)),
active_bridge_(nullptr),
binding_(this),
weak_factory_(this) {
VLOG(2) << "Register CastRemotingConnector for tab_id = " << tab_id_;
+ DCHECK(permission_request_callback_);
media_router_->RegisterRemotingSource(tab_id_, this);
}
@@ -174,7 +207,7 @@
// it's possible the owning WebContents will be destroyed before the Mojo
// message pipes to the RemotingBridges have been closed.
if (active_bridge_)
- StopRemoting(active_bridge_, RemotingStopReason::ROUTE_TERMINATED);
+ StopRemoting(active_bridge_, RemotingStopReason::ROUTE_TERMINATED, false);
for (RemotingBridge* notifyee : bridges_) {
notifyee->OnSinkGone();
notifyee->OnCastRemotingConnectorDestroyed();
@@ -213,7 +246,7 @@
sink_metadata_ = RemotingSinkMetadata();
if (active_bridge_)
- StopRemoting(active_bridge_, RemotingStopReason::SERVICE_GONE);
+ StopRemoting(active_bridge_, RemotingStopReason::SERVICE_GONE, false);
for (RemotingBridge* notifyee : bridges_)
notifyee->OnSinkGone();
}
@@ -239,9 +272,9 @@
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK(bridges_.find(bridge) != bridges_.end());
- if (bridge == active_bridge_)
- StopRemoting(bridge, reason);
bridges_.erase(bridge);
+ if (bridge == active_bridge_)
+ StopRemoting(bridge, reason, true);
}
void CastRemotingConnector::StartRemoting(RemotingBridge* bridge) {
@@ -282,14 +315,15 @@
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (!connector)
return;
+ connector->permission_request_cancel_callback_.Reset();
connector->remoting_allowed_ = is_allowed;
connector->StartRemotingIfPermitted();
},
weak_factory_.GetWeakPtr()));
- // TODO(http://crbug.com/849020): Show the remoting dialog to get user's
- // permission.
- std::move(dialog_result_callback).Run(true);
+ DCHECK(!permission_request_cancel_callback_);
+ permission_request_cancel_callback_ =
+ permission_request_callback_.Run(std::move(dialog_result_callback));
}
}
@@ -331,7 +365,7 @@
if ((!audio_pipe.is_valid() && !video_pipe.is_valid()) ||
(audio_pipe.is_valid() && !audio_sender_request.is_pending()) ||
(video_pipe.is_valid() && !video_sender_request.is_pending())) {
- StopRemoting(active_bridge_, RemotingStopReason::DATA_SEND_FAILED);
+ StopRemoting(active_bridge_, RemotingStopReason::DATA_SEND_FAILED, false);
return;
}
@@ -377,7 +411,8 @@
}
void CastRemotingConnector::StopRemoting(RemotingBridge* bridge,
- RemotingStopReason reason) {
+ RemotingStopReason reason,
+ bool is_initiated_by_source) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
VLOG(2) << __func__ << ": reason = " << reason;
@@ -389,6 +424,19 @@
// Cancel all outstanding callbacks related to the remoting session.
weak_factory_.InvalidateWeakPtrs();
+ if (permission_request_cancel_callback_) {
+ std::move(permission_request_cancel_callback_).Run();
+ if (is_initiated_by_source && remoter_) {
+ // The source requested remoting be stopped before the permission request
+ // was resolved. This means the |remoter_| was never started, and remains
+ // in the available state, still all ready to go. Thus, notify the sources
+ // that the sink is available once again.
+ for (RemotingBridge* notifyee : bridges_)
+ notifyee->OnSinkAvailable(sink_metadata_);
+ }
+ return; // Early returns since the |remoter_| was never started.
+ }
+
// Reset |sink_metadata_|. Remoting can only be started after
// OnSinkAvailable() is called again.
sink_metadata_ = RemotingSinkMetadata();
@@ -411,7 +459,7 @@
if (active_bridge_) {
// This call will reset |sink_metadata_| and notify the source that sink is
// gone.
- StopRemoting(active_bridge_, reason);
+ StopRemoting(active_bridge_, reason, false);
} else if (reason == RemotingStopReason::USER_DISABLED) {
// Notify all the sources that the sink is gone. Remoting can only be
// started after OnSinkAvailable() is called again.
@@ -480,7 +528,7 @@
VLOG(2) << __func__;
if (active_bridge_)
- StopRemoting(active_bridge_, RemotingStopReason::UNEXPECTED_FAILURE);
+ StopRemoting(active_bridge_, RemotingStopReason::UNEXPECTED_FAILURE, false);
}
void CastRemotingConnector::OnDataSendFailed() {
@@ -490,5 +538,5 @@
// A single data send failure is treated as fatal to an active remoting
// session.
if (active_bridge_)
- StopRemoting(active_bridge_, RemotingStopReason::DATA_SEND_FAILED);
+ StopRemoting(active_bridge_, RemotingStopReason::DATA_SEND_FAILED, false);
}
diff --git a/chrome/browser/media/cast_remoting_connector.h b/chrome/browser/media/cast_remoting_connector.h
index 397c2be..1878779 100644
--- a/chrome/browser/media/cast_remoting_connector.h
+++ b/chrome/browser/media/cast_remoting_connector.h
@@ -6,6 +6,7 @@
#define CHROME_BROWSER_MEDIA_CAST_REMOTING_CONNECTOR_H_
#include <set>
+#include <vector>
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
@@ -112,7 +113,15 @@
// Main constructor. |tab_id| refers to any remoted content managed
// by this instance (i.e., any remoted content from one tab/WebContents).
- CastRemotingConnector(media_router::MediaRouter* router, SessionID tab_id);
+ using CancelPermissionRequestCallback = base::OnceClosure;
+ // Called with true to mean "allowed", false to mean "not allowed".
+ using PermissionResultCallback = base::OnceCallback<void(bool)>;
+ using PermissionRequestCallback =
+ base::RepeatingCallback<CancelPermissionRequestCallback(
+ PermissionResultCallback)>;
+ CastRemotingConnector(media_router::MediaRouter* router,
+ SessionID tab_id,
+ PermissionRequestCallback request_callback);
// Creates a RemotingBridge that implements the requested Remoter service, and
// binds it to the interface |request|.
@@ -144,7 +153,8 @@
media::mojom::RemotingDataStreamSenderRequest audio_sender_request,
media::mojom::RemotingDataStreamSenderRequest video_sender_request);
void StopRemoting(RemotingBridge* bridge,
- media::mojom::RemotingStopReason reason);
+ media::mojom::RemotingStopReason reason,
+ bool is_initiated_by_source);
void SendMessageToSink(RemotingBridge* bridge,
const std::vector<uint8_t>& message);
void EstimateTransmissionCapacity(
@@ -174,6 +184,9 @@
const SessionID tab_id_;
+ // The callback to get permission.
+ const PermissionRequestCallback permission_request_callback_;
+
// Describes the remoting sink's metadata and its enabled features. The sink's
// metadata is updated by the mirror service calling OnSinkAvailable() and
// cleared when remoting stops.
@@ -195,6 +208,10 @@
// casting session.
base::Optional<bool> remoting_allowed_;
+ // This callback is non-null when a dialog is showing to get user's
+ // permission, and is reset when the dialog closes.
+ CancelPermissionRequestCallback permission_request_cancel_callback_;
+
// Produces weak pointers that are only valid for the current remoting
// session. This is used to cancel any outstanding callbacks when a remoting
// session is stopped.
diff --git a/chrome/browser/media/cast_remoting_connector_unittest.cc b/chrome/browser/media/cast_remoting_connector_unittest.cc
index 2e15e81..b129cc2 100644
--- a/chrome/browser/media/cast_remoting_connector_unittest.cc
+++ b/chrome/browser/media/cast_remoting_connector_unittest.cc
@@ -11,6 +11,7 @@
#include "base/callback.h"
#include "base/memory/weak_ptr.h"
#include "base/run_loop.h"
+#include "build/build_config.h"
#include "chrome/browser/media/router/test/mock_media_router.h"
#include "chrome/common/media_router/media_route.h"
#include "chrome/common/media_router/media_source.h"
@@ -164,9 +165,7 @@
class CastRemotingConnectorTest : public ::testing::Test {
public:
- CastRemotingConnectorTest() : connector_(&media_router_, kRemotingTabId) {
- EXPECT_EQ(kRemotingTabId, media_router_.tab_id());
- }
+ CastRemotingConnectorTest() { CreateConnector(true); }
void TearDown() final {
// Allow any pending Mojo operations to complete before destruction. For
@@ -180,8 +179,8 @@
RemotingSourcePtr source_ptr;
source->Bind(mojo::MakeRequest(&source_ptr));
RemoterPtr remoter_ptr;
- connector_.CreateBridge(std::move(source_ptr),
- mojo::MakeRequest(&remoter_ptr));
+ connector_->CreateBridge(std::move(source_ptr),
+ mojo::MakeRequest(&remoter_ptr));
return remoter_ptr;
}
@@ -190,14 +189,29 @@
}
void DisableRemoting() {
- connector_.OnStopped(RemotingStopReason::USER_DISABLED);
+ connector_->OnStopped(RemotingStopReason::USER_DISABLED);
+ }
+
+ void CreateConnector(bool remoting_allowed) {
+ connector_.reset(); // Call dtor first if there is one created.
+ connector_.reset(new CastRemotingConnector(
+ &media_router_, kRemotingTabId,
+ base::BindRepeating(
+ [](bool remoting_allowed,
+ CastRemotingConnector::PermissionResultCallback
+ result_callback) {
+ std::move(result_callback).Run(remoting_allowed);
+ return CastRemotingConnector::CancelPermissionRequestCallback();
+ },
+ remoting_allowed)));
+ EXPECT_EQ(kRemotingTabId, media_router_.tab_id());
}
FakeMediaRouter media_router_;
private:
content::TestBrowserThreadBundle browser_thread_bundle_;
- CastRemotingConnector connector_;
+ std::unique_ptr<CastRemotingConnector> connector_;
};
TEST_F(CastRemotingConnectorTest, NeverNotifiesThatSinkIsAvailable) {
@@ -309,6 +323,20 @@
RunUntilIdle();
}
+TEST_F(CastRemotingConnectorTest, NoPermissionToStart) {
+ CreateConnector(false);
+ MockRemotingSource source;
+ RemoterPtr remoter = CreateRemoter(&source);
+ std::unique_ptr<MockMediaRemoter> media_remoter =
+ std::make_unique<MockMediaRemoter>(&media_router_);
+
+ EXPECT_CALL(source, OnStartFailed(RemotingStartFailReason::ROUTE_TERMINATED))
+ .Times(1);
+ EXPECT_CALL(source, OnSinkGone()).Times(AtLeast(1));
+ remoter->Start();
+ RunUntilIdle();
+}
+
namespace {
// The possible ways a remoting session may be terminated in the "full
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc
index 786ddc7..29a6274 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -3138,7 +3138,8 @@
void WebMediaPlayerImpl::SwitchToLocalRenderer(
MediaObserverClient::ReasonToSwitchToLocal reason) {
DCHECK(main_task_runner_->BelongsToCurrentThread());
- DCHECK(disable_pipeline_auto_suspend_);
+ if (!disable_pipeline_auto_suspend_)
+ return; // Is currently with local renderer.
disable_pipeline_auto_suspend_ = false;
// Capabilities reporting may resume now that playback is local.
