Chromium Code Reviews
Help | Chromium Project | Sign in
(1)

Issue 2764613003: Media: Add MEDIALOG_CREATED event

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 days, 17 hours ago by sandersd
Modified:
1 day, 22 hours ago
Reviewers:
wolenetz
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, creis+watch_chromium.org, posciak+watch_chromium.org, chfremer+watch_chromium.org, nasko+codewatch_chromium.org, jam, feature-media-reviews_chromium.org, darin-cc_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

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.

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -0 lines) Patch
M content/renderer/media/render_media_log.cc View 2 chunks +4 lines, -0 lines 1 comment Download
M content/renderer/render_frame_impl.cc View 1 chunk +2 lines, -0 lines 2 comments Download
M media/base/media_log.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M media/base/media_log_event.h View 1 chunk +11 lines, -0 lines 0 comments Download
Trybot results: Sign in to try more bots
Commit queue not available (can’t edit this change).

Messages

Total messages: 8 (2 generated)
sandersd
2 days, 16 hours ago (2017-03-21 00:00:35 UTC) #3
sandersd
https://codereview.chromium.org/2764613003/diff/1/content/renderer/render_frame_impl.cc File content/renderer/render_frame_impl.cc (right): https://codereview.chromium.org/2764613003/diff/1/content/renderer/render_frame_impl.cc#newcode2830 content/renderer/render_frame_impl.cc:2830: media_log->SetStringProperty("ui_description", frame_->document().url().string().utf8()); Looks like .utf8() can return a string ...
2 days, 16 hours ago (2017-03-21 00:07:22 UTC) #4
wolenetz
https://codereview.chromium.org/2764613003/diff/1/content/renderer/media/render_media_log.cc File content/renderer/media/render_media_log.cc (right): https://codereview.chromium.org/2764613003/diff/1/content/renderer/media/render_media_log.cc#newcode54 content/renderer/media/render_media_log.cc:54: *CreateEvent(media::MediaLogEvent::MEDIALOG_CREATED)); In a hypothetical scenario where someone just creates ...
2 days, 16 hours ago (2017-03-21 00:21:26 UTC) #5
wolenetz
Also, update render_media_log_unittest.cc please (I suspect the expectations within it may now fail because there ...
2 days, 15 hours ago (2017-03-21 00:29:37 UTC) #6
wolenetz
Dan and I also chatted yesterday about possibly including the UI title/etc in RenderMediaLog ctor. ...
1 day, 22 hours ago (2017-03-21 17:50:14 UTC) #7
wolenetz
1 day, 22 hours ago (2017-03-21 17:50:15 UTC) #8
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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld d1a128a62