Commit 9b953806 authored by jiayl@chromium.org's avatar jiayl@chromium.org

Fix for closing the desktop sharing notification bar when the shared window is closed.

Previously MediaStreamManager is not notified when a video capturing device has stopped due to error (e.g. the shared window is closed), so the notification UI is still shown when the stream already stopped.
This change fixes it by populating the state to MediaStreamManager through VideoCaptureHost --> VidoeCaptureManager --> MediaStreamProviderListner.

BUG=360181

Review URL: https://codereview.chromium.org/248113003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267045 0039d316-1c4b-4281-b951-d872f2087c98
parent 673b8465
......@@ -33,7 +33,7 @@ class MockAudioInputDeviceManagerListener
MOCK_METHOD2(Closed, void(MediaStreamType, const int));
MOCK_METHOD2(DevicesEnumerated, void(MediaStreamType,
const StreamDeviceInfoArray&));
MOCK_METHOD3(Error, void(MediaStreamType, int, MediaStreamProviderError));
MOCK_METHOD2(Aborted, void(MediaStreamType, int));
StreamDeviceInfoArray devices_;
......
......@@ -1620,6 +1620,14 @@ void MediaStreamManager::DevicesEnumerated(
DCHECK_GE(active_enumeration_ref_count_[stream_type], 0);
}
void MediaStreamManager::Aborted(MediaStreamType stream_type,
int capture_session_id) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
DVLOG(1) << "Aborted({stream_type = " << stream_type << "} "
<< "{capture_session_id = " << capture_session_id << "})";
StopDevice(stream_type, capture_session_id);
}
// static
void MediaStreamManager::SendMessageToNativeLog(const std::string& message) {
BrowserThread::PostTask(
......
......@@ -159,6 +159,8 @@ class CONTENT_EXPORT MediaStreamManager
int capture_session_id) OVERRIDE;
virtual void DevicesEnumerated(MediaStreamType stream_type,
const StreamDeviceInfoArray& devices) OVERRIDE;
virtual void Aborted(MediaStreamType stream_type,
int capture_session_id) OVERRIDE;
// Implements base::SystemMonitor::DevicesChangedObserver.
virtual void OnDevicesChanged(
......
......@@ -50,6 +50,10 @@ class CONTENT_EXPORT MediaStreamProviderListener {
virtual void DevicesEnumerated(MediaStreamType stream_type,
const StreamDeviceInfoArray& devices) = 0;
// Called by a MediaStreamProvider when the device has been aborted due to
// device error.
virtual void Aborted(MediaStreamType stream_type, int capture_session_id) = 0;
protected:
virtual ~MediaStreamProviderListener() {}
};
......
......@@ -27,7 +27,7 @@ void VideoCaptureHost::OnChannelClosing() {
if (controller) {
VideoCaptureControllerID controller_id(it->first);
media_stream_manager_->video_capture_manager()->StopCaptureForClient(
controller.get(), controller_id, this);
controller.get(), controller_id, this, false);
++it;
} else {
// Remove the entry for this controller_id so that when the controller
......@@ -177,7 +177,7 @@ void VideoCaptureHost::DoHandleErrorOnIOThread(
Send(new VideoCaptureMsg_StateChanged(controller_id.device_id,
VIDEO_CAPTURE_STATE_ERROR));
DeleteVideoCaptureControllerOnIOThread(controller_id);
DeleteVideoCaptureControllerOnIOThread(controller_id, true);
}
void VideoCaptureHost::DoEndedOnIOThread(
......@@ -189,7 +189,7 @@ void VideoCaptureHost::DoEndedOnIOThread(
Send(new VideoCaptureMsg_StateChanged(controller_id.device_id,
VIDEO_CAPTURE_STATE_ENDED));
DeleteVideoCaptureControllerOnIOThread(controller_id);
DeleteVideoCaptureControllerOnIOThread(controller_id, false);
}
///////////////////////////////////////////////////////////////////////////////
......@@ -261,7 +261,7 @@ void VideoCaptureHost::DoControllerAddedOnIOThread(
if (it == entries_.end()) {
if (controller) {
media_stream_manager_->video_capture_manager()->StopCaptureForClient(
controller.get(), controller_id, this);
controller.get(), controller_id, this, false);
}
return;
}
......@@ -285,7 +285,7 @@ void VideoCaptureHost::OnStopCapture(int device_id) {
Send(new VideoCaptureMsg_StateChanged(device_id,
VIDEO_CAPTURE_STATE_STOPPED));
DeleteVideoCaptureControllerOnIOThread(controller_id);
DeleteVideoCaptureControllerOnIOThread(controller_id, false);
}
void VideoCaptureHost::OnPauseCapture(int device_id) {
......@@ -344,7 +344,7 @@ void VideoCaptureHost::OnGetDeviceFormatsInUse(
}
void VideoCaptureHost::DeleteVideoCaptureControllerOnIOThread(
const VideoCaptureControllerID& controller_id) {
const VideoCaptureControllerID& controller_id, bool on_error) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
EntryMap::iterator it = entries_.find(controller_id);
......@@ -353,7 +353,7 @@ void VideoCaptureHost::DeleteVideoCaptureControllerOnIOThread(
if (it->second) {
media_stream_manager_->video_capture_manager()->StopCaptureForClient(
it->second.get(), controller_id, this);
it->second.get(), controller_id, this, on_error);
}
entries_.erase(it);
}
......
......@@ -140,7 +140,7 @@ class CONTENT_EXPORT VideoCaptureHost
int device_id,
media::VideoCaptureSessionId capture_session_id);
// Send a newly created buffer to the VideoCaptureMessageFilter.
// Sends a newly created buffer to the VideoCaptureMessageFilter.
void DoSendNewBufferOnIOThread(
const VideoCaptureControllerID& controller_id,
base::SharedMemoryHandle handle,
......@@ -151,14 +151,14 @@ class CONTENT_EXPORT VideoCaptureHost
const VideoCaptureControllerID& controller_id,
int buffer_id);
// Send a filled buffer to the VideoCaptureMessageFilter.
// Sends a filled buffer to the VideoCaptureMessageFilter.
void DoSendFilledBufferOnIOThread(
const VideoCaptureControllerID& controller_id,
int buffer_id,
const media::VideoCaptureFormat& format,
base::TimeTicks timestamp);
// Send a filled texture mailbox buffer to the VideoCaptureMessageFilter.
// Sends a filled texture mailbox buffer to the VideoCaptureMessageFilter.
void DoSendFilledMailboxBufferOnIOThread(
const VideoCaptureControllerID& controller_id,
int buffer_id,
......@@ -166,13 +166,15 @@ class CONTENT_EXPORT VideoCaptureHost
const media::VideoCaptureFormat& format,
base::TimeTicks timestamp);
// Handle error coming from VideoCaptureDevice.
// Handles error coming from VideoCaptureDevice.
void DoHandleErrorOnIOThread(const VideoCaptureControllerID& controller_id);
void DoEndedOnIOThread(const VideoCaptureControllerID& controller_id);
// Deletes the controller and notifies the VideoCaptureManager. |on_error| is
// true if this is triggered by VideoCaptureControllerEventHandler::OnError.
void DeleteVideoCaptureControllerOnIOThread(
const VideoCaptureControllerID& controller_id);
const VideoCaptureControllerID& controller_id, bool on_error);
MediaStreamManager* media_stream_manager_;
......
......@@ -158,8 +158,7 @@ void VideoCaptureManager::Close(int capture_session_id) {
DCHECK(listener_);
DVLOG(1) << "VideoCaptureManager::Close, id " << capture_session_id;
std::map<media::VideoCaptureSessionId, MediaStreamDevice>::iterator
session_it = sessions_.find(capture_session_id);
SessionMap::iterator session_it = sessions_.find(capture_session_id);
if (session_it == sessions_.end()) {
NOTREACHED();
return;
......@@ -289,7 +288,8 @@ void VideoCaptureManager::StartCaptureForClient(
void VideoCaptureManager::StopCaptureForClient(
VideoCaptureController* controller,
VideoCaptureControllerID client_id,
VideoCaptureControllerEventHandler* client_handler) {
VideoCaptureControllerEventHandler* client_handler,
bool aborted_due_to_error) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
DCHECK(controller);
DCHECK(client_handler);
......@@ -299,6 +299,16 @@ void VideoCaptureManager::StopCaptureForClient(
NOTREACHED();
return;
}
if (aborted_due_to_error) {
SessionMap::iterator it;
for (it = sessions_.begin(); it != sessions_.end(); ++it) {
if (it->second.type == entry->stream_type &&
it->second.id == entry->id) {
listener_->Aborted(it->second.type, it->first);
break;
}
}
}
// Detach client from controller.
media::VideoCaptureSessionId session_id =
......@@ -316,8 +326,7 @@ bool VideoCaptureManager::GetDeviceSupportedFormats(
DCHECK_CURRENTLY_ON(BrowserThread::IO);
DCHECK(supported_formats->empty());
std::map<media::VideoCaptureSessionId, MediaStreamDevice>::iterator it =
sessions_.find(capture_session_id);
SessionMap::iterator it = sessions_.find(capture_session_id);
if (it == sessions_.end())
return false;
DVLOG(1) << "GetDeviceSupportedFormats for device: " << it->second.name;
......@@ -336,8 +345,7 @@ bool VideoCaptureManager::GetDeviceFormatsInUse(
DCHECK_CURRENTLY_ON(BrowserThread::IO);
DCHECK(formats_in_use->empty());
std::map<media::VideoCaptureSessionId, MediaStreamDevice>::iterator it =
sessions_.find(capture_session_id);
SessionMap::iterator it = sessions_.find(capture_session_id);
if (it == sessions_.end())
return false;
DVLOG(1) << "GetDeviceFormatsInUse for device: " << it->second.name;
......@@ -357,8 +365,7 @@ void VideoCaptureManager::SetDesktopCaptureWindowId(
media::VideoCaptureSessionId session_id,
gfx::NativeViewId window_id) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
std::map<media::VideoCaptureSessionId, MediaStreamDevice>::iterator
session_it = sessions_.find(session_id);
SessionMap::iterator session_it = sessions_.find(session_id);
if (session_it == sessions_.end()) {
device_task_runner_->PostTask(
FROM_HERE,
......@@ -520,9 +527,9 @@ VideoCaptureManager::GetDeviceEntryForMediaStreamDevice(
VideoCaptureManager::DeviceEntry*
VideoCaptureManager::GetDeviceEntryForController(
const VideoCaptureController* controller) {
const VideoCaptureController* controller) const {
// Look up |controller| in |devices_|.
for (DeviceEntries::iterator it = devices_.begin();
for (DeviceEntries::const_iterator it = devices_.begin();
it != devices_.end(); ++it) {
if ((*it)->video_capture_controller.get() == controller) {
return *it;
......@@ -555,8 +562,7 @@ VideoCaptureManager::DeviceEntry* VideoCaptureManager::GetOrCreateDeviceEntry(
media::VideoCaptureSessionId capture_session_id) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
std::map<media::VideoCaptureSessionId, MediaStreamDevice>::iterator
session_it = sessions_.find(capture_session_id);
SessionMap::iterator session_it = sessions_.find(capture_session_id);
if (session_it == sessions_.end()) {
return NULL;
}
......
......@@ -79,7 +79,8 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
// function.
void StopCaptureForClient(VideoCaptureController* controller,
VideoCaptureControllerID client_id,
VideoCaptureControllerEventHandler* client_handler);
VideoCaptureControllerEventHandler* client_handler,
bool aborted_due_to_error);
// Retrieves all capture supported formats for a particular device. Returns
// false if the |capture_session_id| is not found. The supported formats are
......@@ -147,7 +148,7 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
// Find the DeviceEntry that owns a particular controller pointer.
DeviceEntry* GetDeviceEntryForController(
const VideoCaptureController* controller);
const VideoCaptureController* controller) const;
bool IsOnDeviceThread() const;
......@@ -189,11 +190,12 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
MediaStreamProviderListener* listener_;
media::VideoCaptureSessionId new_capture_session_id_;
typedef std::map<media::VideoCaptureSessionId, MediaStreamDevice> SessionMap;
// An entry is kept in this map for every session that has been created via
// the Open() entry point. The keys are session_id's. This map is used to
// determine which device to use when StartCaptureForClient() occurs. Used
// only on the IO thread.
std::map<media::VideoCaptureSessionId, MediaStreamDevice> sessions_;
SessionMap sessions_;
// An entry, kept in a map, that owns a VideoCaptureDevice and its associated
// VideoCaptureController. VideoCaptureManager owns all VideoCaptureDevices
......
......@@ -38,8 +38,7 @@ class MockMediaStreamProviderListener : public MediaStreamProviderListener {
MOCK_METHOD2(Closed, void(MediaStreamType, int));
MOCK_METHOD2(DevicesEnumerated, void(MediaStreamType,
const StreamDeviceInfoArray&));
MOCK_METHOD3(Error, void(MediaStreamType, int,
MediaStreamProviderError));
MOCK_METHOD2(Aborted, void(MediaStreamType, int));
}; // class MockMediaStreamProviderListener
// Needed as an input argument to StartCaptureForClient().
......@@ -131,7 +130,7 @@ class VideoCaptureManagerTest : public testing::Test {
void StopClient(VideoCaptureControllerID client_id) {
ASSERT_TRUE(1 == controllers_.count(client_id));
vcm_->StopCaptureForClient(controllers_[client_id], client_id,
frame_observer_.get());
frame_observer_.get(), false);
controllers_.erase(client_id);
}
......@@ -176,6 +175,35 @@ TEST_F(VideoCaptureManagerTest, CreateAndClose) {
vcm_->Unregister();
}
// Try to open, start, and abort a device.
TEST_F(VideoCaptureManagerTest, CreateAndAbort) {
StreamDeviceInfoArray devices;
InSequence s;
EXPECT_CALL(*listener_, DevicesEnumerated(MEDIA_DEVICE_VIDEO_CAPTURE, _))
.WillOnce(SaveArg<1>(&devices));
EXPECT_CALL(*listener_, Opened(MEDIA_DEVICE_VIDEO_CAPTURE, _));
EXPECT_CALL(*listener_, Aborted(MEDIA_DEVICE_VIDEO_CAPTURE, _));
vcm_->EnumerateDevices(MEDIA_DEVICE_VIDEO_CAPTURE);
// Wait to get device callback.
message_loop_->RunUntilIdle();
int video_session_id = vcm_->Open(devices.front());
VideoCaptureControllerID client_id = StartClient(video_session_id, true);
// Wait for device opened.
message_loop_->RunUntilIdle();
vcm_->StopCaptureForClient(controllers_[client_id], client_id,
frame_observer_.get(), true);
// Wait to check callbacks before removing the listener.
message_loop_->RunUntilIdle();
vcm_->Unregister();
}
// Open the same device twice.
TEST_F(VideoCaptureManagerTest, OpenTwice) {
StreamDeviceInfoArray devices;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment