Extend components/test_runner's generated WebMediaStream to have a ChromeMock VideoTrack and Source

Extend components/test_runner's generated WebMediaStream to have a ChromeMock VideoTrack and Source

components/test_runner is used by Blink LayoutTest to
generate a MediaStream. But this MS has only Blink-side
objects, whereas some tests (e.g. MediaRecorder) need 
to have Chrome counterparts of those objects. This CL
adds those, for video.

Since components/ has a strict dependency requirement,
the added code goes into a subfolder of 
components/test_runner.

BUG=262211, 532509

[mcasas, 2015-09-29 22:29:01]

Patchset #1 (id:1) has been deleted

[mcasas, 2015-09-29 22:29:09]

Patchset #1 (id:20001) has been deleted

[mcasas, 2015-09-29 22:32:40]

Patchset #1 (id:40001) has been deleted

[mcasas, 2015-09-29 23:26:46]

Patchset #1 (id:60001) has been deleted

[mcasas, 2015-09-29 23:40:29]

mcasas@chromium.org changed reviewers:
+ dmazzoni@chromium.org

[mcasas, 2015-09-29 23:40:29]

dmazzoni@ can you have a first look at components/* ?

(FYI tommyw@ is not available for reviews ATM)

[mcasas, 2015-09-30 15:23:27]

Patchset #1 (id:80001) has been deleted

[dmazzoni, 2015-09-30 15:23:45]

lgtm

test_runner code changes look fine, but I'm not an expert on the components
dependency rules. Perhaps someone in the top-level components OWNERS file?

https://codereview.chromium.org/1371373002/diff/100001/content/public/rendere...
File content/public/renderer/media_stream_api.cc (right):

https://codereview.chromium.org/1371373002/diff/100001/content/public/rendere...
content/public/renderer/media_stream_api.cc:96: LOG(ERROR) << "Stream not
found";
Is this error likely to occur? Should it be a fatal error if it happens in
production code, and if not is the log important?

Consider VLOG or DLOG(ERROR)

https://codereview.chromium.org/1371373002/diff/100001/content/public/rendere...
File content/public/renderer/media_stream_api.h (right):

https://codereview.chromium.org/1371373002/diff/100001/content/public/rendere...
content/public/renderer/media_stream_api.h:25: // These methods create a
WebMediaStreamSource + MediaStreamSource couple with
Nit: perhaps "pair" instead of couple (2x)?

https://codereview.chromium.org/1371373002/diff/100001/content/public/rendere...
content/public/renderer/media_stream_api.h:29: // as |web_stream|.
nit: missing closing paren

[mcasas, 2015-09-30 15:32:58]

mcasas@chromium.org changed reviewers:
+ avi@chromium.org

[mcasas, 2015-09-30 15:32:59]

avi@ can you take a look at the *DEPS* files plz?

[Avi (use Gerrit), 2015-09-30 15:55:45]

On 2015/09/30 15:32:59, mcasas wrote:
> avi@ can you take a look at the *DEPS* files plz?

I'm pretty sure test_runner, by definition, has to rely on content, so putting
it into the DEPS LGTM.

[mcasas, 2015-09-30 16:24:21]

hubbe@ I touched content/public/renderer/media_stream_api.h FYI/PTAL

peter@ FYI/PTAL (WebKit side change is trivial)

[mcasas, 2015-09-30 19:14:58]

https://codereview.chromium.org/1371373002/diff/100001/content/public/rendere...
File content/public/renderer/media_stream_api.cc (right):

https://codereview.chromium.org/1371373002/diff/100001/content/public/rendere...
content/public/renderer/media_stream_api.cc:96: LOG(ERROR) << "Stream not
found";
On 2015/09/30 15:23:45, dmazzoni wrote:
> Is this error likely to occur? Should it be a fatal error if it happens in
> production code, and if not is the log important?
> 
> Consider VLOG or DLOG(ERROR)
> 

DLOG() it is, here and in l.55.

https://codereview.chromium.org/1371373002/diff/100001/content/public/rendere...
File content/public/renderer/media_stream_api.h (right):

https://codereview.chromium.org/1371373002/diff/100001/content/public/rendere...
content/public/renderer/media_stream_api.h:25: // These methods create a
WebMediaStreamSource + MediaStreamSource couple with
On 2015/09/30 15:23:45, dmazzoni wrote:
> Nit: perhaps "pair" instead of couple (2x)?

Done.

https://codereview.chromium.org/1371373002/diff/100001/content/public/rendere...
content/public/renderer/media_stream_api.h:29: // as |web_stream|.
On 2015/09/30 15:23:45, dmazzoni wrote:
> nit: missing closing paren

Done.

[mcasas, 2015-09-30 21:56:47]

Patchset #3 (id:130001) has been deleted

[mcasas, 2015-09-30 22:20:21]

Patchset #2 (id:110001) has been deleted

[mcasas, 2015-09-30 23:33:22]

Patchset #4 (id:190001) has been deleted

[mcasas, 2015-10-01 03:04:13]

Patchset #3 (id:170001) has been deleted

[mcasas, 2015-10-01 03:04:21]

Patchset #3 (id:210001) has been deleted

[mcasas, 2015-10-01 16:37:43]

Patchset #4 (id:250001) has been deleted

[mcasas, 2015-10-01 16:38:02]

Patchset #3 (id:230001) has been deleted

[mcasas, 2015-10-01 16:38:11]

Patchset #2 (id:150001) has been deleted

[mcasas, 2015-10-01 16:50:40]

Patchset #3 (id:290001) has been deleted

[mcasas, 2015-10-01 18:06:06]

Patchset #2 (id:270001) has been deleted

[mcasas, 2015-10-02 01:21:42]

Patchset #3 (id:330001) has been deleted

[mcasas, 2015-10-02 01:58:10]

Patchset #3 (id:350001) has been deleted

[mcasas, 2015-10-02 01:58:17]

Patchset #3 (id:370001) has been deleted

[mcasas, 2015-10-02 01:59:02]

Patchset #2 (id:310001) has been deleted

[mcasas, 2015-10-02 02:22:37]

Patchset #2 (id:390001) has been deleted

[mcasas, 2015-10-05 19:14:19]

Patchset #3 (id:420001) has been deleted

[mcasas, 2015-10-05 19:14:26]

Patchset #3 (id:440001) has been deleted

[mcasas, 2015-10-05 19:14:37]

Patchset #2 (id:370002) has been deleted

[mcasas, 2015-10-05 19:33:08]

Patchset #2 (id:460001) has been deleted

[mcasas, 2015-10-05 22:03:53]

Patchset #3 (id:500001) has been deleted

[mcasas, 2015-10-05 23:04:28]

Patchset #2 (id:480001) has been deleted

[sadrul, 2015-10-06 15:23:27]

sadrul@chromium.org changed reviewers:
+ jochen@chromium.org, sadrul@chromium.org

[sadrul, 2015-10-06 15:23:28]

+jochen@ for advice

test_runner should not depend on content. content depends on, and uses,
test_runner for running layout-tests using content-shell. Similarly,
web_view/test_runner uses test_runner for running layout-tests using
html_viewer. So test_runner cannot depend on content.

https://chromiumcodereview.appspot.com/1371373002/diff/510001/components/test...
File components/test_runner/renderer/BUILD.gn (right):

https://chromiumcodereview.appspot.com/1371373002/diff/510001/components/test...
components/test_runner/renderer/BUILD.gn:17: "//content/public/renderer",
test_runner should not depend on content.

[jochen (gone - plz use gerrit), 2015-10-07 09:38:54]

correct, test_runner must not depend on content (not lgtm)

in this case, I'd probably add a callback to WebTestDelegate that the mock video
capture source can invoke. content shell can then use the new content API you
added to add the stream. the html_viewer would probably just ignore that
callback for now

[mcasas, 2015-10-07 20:42:18]

On 2015/10/07 09:38:54, jochen wrote:
> correct, test_runner must not depend on content (not lgtm)
> 
> in this case, I'd probably add a callback to WebTestDelegate that the mock
video
> capture source can invoke. content shell can then use the new content API you
> added to add the stream. the html_viewer would probably just ignore that
> callback for now

Thanks jochen@, I made https://crrev.com/1391363002
with that idea. Closing this CL then, thanks everyone!