Media: Add MEDIALOG_CREATED event

Media: Add MEDIALOG_CREATED event

This will allow the browser to discard old players reliably, without
creating any truncated logs.

This CL also passes some new metadata at creation time, which will make it easier to identify players in the chrome://media-internals list.

[sandersd (OOO until July 31), 2017-03-21 00:00:13]

Description was changed from

==========
WIP: MEDIALOG_CREATED event

This will allow the browser to discard old players reliably, without
creating any truncated logs.
==========

to

==========
Media: Add MEDIALOG_CREATED event

This will allow the browser to discard old players reliably, without
creating any truncated logs.

This CL also passes some new metadata at creation time, which will make it
easier to identify players in the chrome://media-internals list.
==========

[sandersd (OOO until July 31), 2017-03-21 00:00:35]

sandersd@chromium.org changed reviewers:
+ wolenetz@chromium.org

[sandersd (OOO until July 31), 2017-03-21 00:07:22]

https://codereview.chromium.org/2764613003/diff/1/content/renderer/render_fra...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2764613003/diff/1/content/renderer/render_fra...
content/renderer/render_frame_impl.cc:2830:
media_log->SetStringProperty("ui_description",
frame_->document().url().string().utf8());
Looks like .utf8() can return a string referencing nullptr, I'll add a check for
that in a new PS.

[wolenetz, 2017-03-21 00:21:26]

https://codereview.chromium.org/2764613003/diff/1/content/renderer/media/rend...
File content/renderer/media/render_media_log.cc (right):

https://codereview.chromium.org/2764613003/diff/1/content/renderer/media/rend...
content/renderer/media/render_media_log.cc:54:
*CreateEvent(media::MediaLogEvent::MEDIALOG_CREATED));
In a hypothetical scenario where someone just creates a RenderMediaLog but
delays a long time before doing any ::AddEvent, this MEDIALOG_CREATED event
won't get delivered to the browser until after that long delay. IOW, need to
trigger ipc send pending and the timer, etc (possibly refactoring into helper(s)
l.101-122 might work, or do something one-off here at risk of fragility if the
common case in l.101-122 changes later).

A safer refactor might be to have a private AddEventInternal which is the
current AddEvent, allowed only by the ctor and ::AddEvent, the latter containing
the new DCHECK_NE.

https://codereview.chromium.org/2764613003/diff/1/content/renderer/render_fra...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2764613003/diff/1/content/renderer/render_fra...
content/renderer/render_frame_impl.cc:2829:
media_log->SetStringProperty("ui_title", frame_->document().title().utf8());
Are these two lines integral to the new MEDIALOG_CREATED event? (Do they need to
be in this CL?)

[wolenetz, 2017-03-21 00:29:37]

Also, update render_media_log_unittest.cc please (I suspect the expectations
within it may now fail because there is an initial 1 (or up to 3) extra events
vs previous, and the pending ipc flag may also be set (if 2 or 3). (2-3 are the
ui title and so forth, which the test doesn't add, but the MEDIALOG_CREATED
event is new and likely will hose the expectations).

[wolenetz, 2017-03-21 17:50:14]

Dan and I also chatted yesterday about possibly including the UI title/etc in
RenderMediaLog ctor. Perhaps even have multiple ctors eventually if it's needed.

[wolenetz, 2017-03-21 17:50:15]

Dan and I also chatted yesterday about possibly including the UI title/etc in
RenderMediaLog ctor. Perhaps even have multiple ctors eventually if it's needed.