Add a ChromeOS implementation of VideoCaptureDevice

content/browser/renderer_host/media/media_stream_manager.cc

Index: content/browser/renderer_host/media/media_stream_manager.cc
diff --git a/content/browser/renderer_host/media/media_stream_manager.cc b/content/browser/renderer_host/media/media_stream_manager.cc
index 3cdb1abffd3cddfffe648d9c74711eb1c7b8f0ae..c0508302d2728226501944a22c02cb18a91de4d4 100644
--- a/content/browser/renderer_host/media/media_stream_manager.cc
+++ b/content/browser/renderer_host/media/media_stream_manager.cc
@@ -39,6 +39,7 @@
 #include "media/base/media_switches.h"
 #include "media/video/capture/fake_video_capture_device_factory.h"
 #include "media/video/capture/file_video_capture_device_factory.h"
+#include "ui/gfx/screen.h"
 #include "url/gurl.h"
 
 #if defined(OS_WIN)
@@ -343,12 +344,14 @@ MediaStreamManager::MediaStreamManager()
     : audio_manager_(NULL),
       monitoring_started_(false),
       io_loop_(NULL),
+      observing_screen_(false),
       use_fake_ui_(false) {}
 
 MediaStreamManager::MediaStreamManager(media::AudioManager* audio_manager)
     : audio_manager_(audio_manager),
       monitoring_started_(false),
       io_loop_(NULL),
+      observing_screen_(false),
       use_fake_ui_(false) {
   DCHECK(audio_manager_);
   memset(active_enumeration_ref_count_, 0,
@@ -382,6 +385,13 @@ MediaStreamManager::~MediaStreamManager() {
   // MediaStreamManager, but it may be NULL in unit tests.
   if (power_monitor)
     power_monitor->RemoveObserver(this);
+
+  if (observing_screen_) {
+    gfx::Screen* screen =
+        gfx::Screen::GetScreenByType(gfx::SCREEN_TYPE_ALTERNATE);
+    if (screen)
+      screen->RemoveObserver(this);
+  }
 }
 
 VideoCaptureManager* MediaStreamManager::video_capture_manager() {
@@ -447,6 +457,16 @@ void MediaStreamManager::GenerateStream(MediaStreamRequester* requester,
     UseFakeUI(scoped_ptr<FakeMediaStreamUIProxy>());
   }
 
+  if (!observing_screen_) {

[vrk (LEFT CHROMIUM), 2014/05/09 01:11:31]

Hmm.. I don't think this is the right place to add the observer. I'm also
wondering if VideoCaptureManager should observe this directly, instead of having
MediaStreamManager observe it... also, this is really only useful with CrOS,
right? So there's possibly a CrOS-specific place where you can add this code.

Unfortunately this goes beyond my familiarity with this code, so I'm deferring
this to tommi@ and perkj@.

[Zachary Kuznia, 2014/05/09 07:42:22]

I will discuss this with them.

+    gfx::Screen* screen =
+        gfx::Screen::GetScreenByType(gfx::SCREEN_TYPE_ALTERNATE);
+    if (screen) {
+      screen->AddObserver(this);
+      OnDisplayBoundsChanged(screen->GetPrimaryDisplay());

[vrk (LEFT CHROMIUM), 2014/05/09 01:11:31]

Generally it's best let observer functions be called solely by the object it's
observing, instead of calling it directly. That makes it easier to reason about
the code.

So modify SetDisplayRotationOnIOThread() to include the "If
display.IsInternal()" check and call that instead of OnDisplayBoundsChange.

[Zachary Kuznia, 2014/05/09 07:42:22]

Done.

+      observing_screen_ = true;
+    }
+  }
+
   DeviceRequest* request = new DeviceRequest(requester,
                                              render_process_id,
                                              render_view_id,
@@ -708,6 +728,7 @@ void MediaStreamManager::OpenDevice(MediaStreamRequester* requester,
   DCHECK(type == MEDIA_DEVICE_AUDIO_CAPTURE ||
          type == MEDIA_DEVICE_VIDEO_CAPTURE);
   DVLOG(1) << "OpenDevice ({page_request_id = " << page_request_id <<  "})";
+
   StreamOptions options;
   if (IsAudioMediaType(type)) {
     options.audio_requested = true;
@@ -1947,4 +1968,26 @@ void MediaStreamManager::OnMediaStreamUIWindowId(MediaStreamType video_type,
   }
 }
 
+void MediaStreamManager::OnDisplayBoundsChanged(const gfx::Display& display) {
+  if (display.IsInternal()) {

[vrk (LEFT CHROMIUM), 2014/05/09 01:11:31]

Move the "if (display.IsInternal())" check to SetDisplayRotationOnIOThread, and
pass |display| instead of |rotation|. See comment above.

[Zachary Kuznia, 2014/05/09 07:42:22]

Done.

+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(&MediaStreamManager::SetDisplayRotationOnIOThread,
+                   base::Unretained(this), display.rotation()));
+  }
+}
+
+void MediaStreamManager::OnDisplayAdded(const gfx::Display& new_display) {

[vrk (LEFT CHROMIUM), 2014/05/09 01:11:31]

Nit: {} on one line, and comment out the unused param, so
void MediaStreamManager::OnDisplayAdded(const gfx::Display& /*new_display*/) {}

(http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...)

[Zachary Kuznia, 2014/05/09 07:42:22]

Done.

+}
+
+void MediaStreamManager::OnDisplayRemoved(const gfx::Display& old_display) {

[vrk (LEFT CHROMIUM), 2014/05/09 01:11:31]

ditto above

[Zachary Kuznia, 2014/05/09 07:42:22]

Done, save that the } doesn't fit on the same line.

+}
+
+void MediaStreamManager::SetDisplayRotationOnIOThread(
+    gfx::Display::Rotation rotation) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  video_capture_manager()->SetDisplayRotation(rotation);
+}
+
 }  // namespace content