Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(28)

Issue 2764613003: Media: Add MEDIALOG_CREATED event

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 months, 1 week ago by sandersd (OOO until July 31)
Modified:
6 months 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 (OOO until July 31)
6 months, 1 week ago (2017-03-21 00:00:35 UTC) #3
sandersd (OOO until July 31)
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 ...
6 months, 1 week 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 ...
6 months, 1 week 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 ...
6 months, 1 week 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. ...
6 months ago (2017-03-21 17:50:14 UTC) #7
wolenetz
6 months 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 b40b6558b