Fix DCHECK error when invalid video constraints are set on gUM. If a video constraint can not be me…

Fix DCHECK error when invalid video constraints are set on gUM. If a video constraint can not be met, VideoCaptureDelegate::StopDeliver is called even if StartDeliver has not been called. This is as expected. The problem is that the messagelooproxy has not been set when VideoCaptureDelegate::StopSourceImpl is called unless StartDeliver has been called. This sets the proxy when the VideoCaptureDelegate is created instead.

A content browser test is added to test what happen if constraints can not be met.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=257450

[perkj_chrome, 2014-03-12 19:15:47]

Hej
Can you please review?

[mcasas, 2014-03-13 08:07:45]

Drive-by.

https://codereview.chromium.org/197213004/diff/1/content/browser/media/webrtc...
File content/browser/media/webrtc_getusermedia_browsertest.cc (right):

https://codereview.chromium.org/197213004/diff/1/content/browser/media/webrtc...
content/browser/media/webrtc_getusermedia_browsertest.cc:352: int large_value =
99999;
I'd recommend splitting this test in more navigate-check
for error parts, requesting a valid but not supported 
resolution, first, and then an unsupported frame rate --
this test runs on FakeVCD which will most likely never
support beyond 180p, 240p, 360p, 480p and 720p, so what
about 800x600? FakeVCD runs at 20fps (_not_ 30fps).

[no longer working on chromium, 2014-03-13 09:29:11]

I defer the test case to Miguel. lgtm with one nit

https://codereview.chromium.org/197213004/diff/1/content/renderer/media/media...
File content/renderer/media/media_stream_video_capturer_source.cc (right):

https://codereview.chromium.org/197213004/diff/1/content/renderer/media/media...
content/renderer/media/media_stream_video_capturer_source.cc:47:
message_loop_proxy_ = base::MessageLoopProxy::current();
move it up to the initializer.

[phoglund_chromium, 2014-03-13 09:43:43]

lgtm.

I wonder if this is the same bug as the crash I saw yesterday where
unsatisfiable constraints led to a crash. I'll apply your patch and test that;
if it still happens I'll file a new bug.

[phoglund_chromium, 2014-03-13 15:42:50]

On 2014/03/13 09:43:43, phoglund wrote:
> lgtm.
> 
> I wonder if this is the same bug as the crash I saw yesterday where
> unsatisfiable constraints led to a crash. I'll apply your patch and test that;
> if it still happens I'll file a new bug.

Yup, I verified it and your patch fixes the crash I was seeing as well.

[perkj_chrome, 2014-03-14 08:29:43]

The CQ bit was checked by perkj@chromium.org

[perkj_chrome, 2014-03-14 08:29:52]

https://codereview.chromium.org/197213004/diff/1/content/browser/media/webrtc...
File content/browser/media/webrtc_getusermedia_browsertest.cc (right):

https://codereview.chromium.org/197213004/diff/1/content/browser/media/webrtc...
content/browser/media/webrtc_getusermedia_browsertest.cc:352: int large_value =
99999;
On 2014/03/13 08:07:46, mcasas wrote:
> I'd recommend splitting this test in more navigate-check
> for error parts, requesting a valid but not supported 
> resolution, first, and then an unsupported frame rate --
> this test runs on FakeVCD which will most likely never
> support beyond 180p, 240p, 360p, 480p and 720p, so what
> about 800x600? FakeVCD runs at 20fps (_not_ 30fps).

Discussed of line. This test is to test what happens if the constraints are
invalid. Leave as is.

[commit-bot: I haz the power, 2014-03-14 08:29:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/perkj@chromium.org/197213004/1

[commit-bot: I haz the power, 2014-03-14 09:48:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-14 09:48:35]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[perkj_chrome, 2014-03-14 15:22:37]

Patrik can you please take another look?

[phoglund_chromium, 2014-03-14 15:26:10]

lgtm

https://codereview.chromium.org/197213004/diff/20001/content/test/data/media/...
File content/test/data/media/getusermedia.html (left):

https://codereview.chromium.org/197213004/diff/20001/content/test/data/media/...
content/test/data/media/getusermedia.html:33: // Requests getusermedia and
expects it to fail.
Nit: comment that it returns the error name.

[perkj_chrome, 2014-03-14 15:30:40]

The CQ bit was checked by perkj@chromium.org

[commit-bot: I haz the power, 2014-03-14 15:31:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/perkj@chromium.org/197213004/40001

[commit-bot: I haz the power, 2014-03-14 15:32:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-14 15:32:31]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_clang_dbg

[perkj_chrome, 2014-03-17 09:31:43]

The CQ bit was checked by perkj@chromium.org

[commit-bot: I haz the power, 2014-03-17 09:31:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/perkj@chromium.org/197213004/60001

[commit-bot: I haz the power, 2014-03-17 16:46:40]

Change committed as 257450