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

Issue 2662393005: [Media Router] (device discovery) Create MediaSinkService interface (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months, 2 weeks ago by zhaobin
Modified:
8 months ago
Reviewers:
mark a. foltz, imcheng
CC:
chfremer+watch_chromium.org, chromium-reviews, feature-media-reviews_chromium.org, mcasas+watch+vc_chromium.org, media-router+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Media Router] (device discovery) Create MediaSinkService interface Created MediaSinkService interface according to device discovery design doc. BUG=687356 Review-Url: https://codereview.chromium.org/2662393005 Cr-Commit-Position: refs/heads/master@{#451048} Committed: https://chromium.googlesource.com/chromium/src/+/8a4c43002d54cbf4f577a5e00b87fddc5e9b6269

Patch Set 1 #

Total comments: 10

Patch Set 2 : resolve code review comments from Derek #

Total comments: 6

Patch Set 3 : resolve code review comments from Derek #

Total comments: 17

Patch Set 4 : resolve code review comments from Mark #

Total comments: 1

Patch Set 5 : resolve code review comments from Mark #

Unified diffs Side-by-side diffs Delta from patch set Stats (+75 lines, -0 lines) Patch
M chrome/browser/media/router/BUILD.gn View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
A chrome/browser/media/router/media_sink_service.h View 1 2 3 4 1 chunk +58 lines, -0 lines 0 comments Download
A chrome/browser/media/router/media_sink_service.cc View 1 2 3 1 chunk +15 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 23 (13 generated)
zhaobin
8 months, 2 weeks ago (2017-02-01 19:54:32 UTC) #2
imcheng
https://codereview.chromium.org/2662393005/diff/1/chrome/browser/media/router/media_sink_service.h File chrome/browser/media/router/media_sink_service.h (right): https://codereview.chromium.org/2662393005/diff/1/chrome/browser/media/router/media_sink_service.h#newcode10 chrome/browser/media/router/media_sink_service.h:10: #include "base/callback_list.h" Did you mean to include base/callback.h ? ...
8 months, 2 weeks ago (2017-02-07 00:48:14 UTC) #3
zhaobin
https://codereview.chromium.org/2662393005/diff/1/chrome/browser/media/router/media_sink_service.h File chrome/browser/media/router/media_sink_service.h (right): https://codereview.chromium.org/2662393005/diff/1/chrome/browser/media/router/media_sink_service.h#newcode10 chrome/browser/media/router/media_sink_service.h:10: #include "base/callback_list.h" On 2017/02/07 00:48:14, imcheng wrote: > Did ...
8 months, 2 weeks ago (2017-02-07 02:38:50 UTC) #4
imcheng
lgtm. One more question. https://codereview.chromium.org/2662393005/diff/20001/chrome/browser/media/router/media_sink_service.h File chrome/browser/media/router/media_sink_service.h (right): https://codereview.chromium.org/2662393005/diff/20001/chrome/browser/media/router/media_sink_service.h#newcode8 chrome/browser/media/router/media_sink_service.h:8: #include <memory> #include <vector> https://codereview.chromium.org/2662393005/diff/20001/chrome/browser/media/router/media_sink_service.h#newcode19 ...
8 months, 2 weeks ago (2017-02-07 19:25:32 UTC) #5
zhaobin
https://codereview.chromium.org/2662393005/diff/20001/chrome/browser/media/router/media_sink_service.h File chrome/browser/media/router/media_sink_service.h (right): https://codereview.chromium.org/2662393005/diff/20001/chrome/browser/media/router/media_sink_service.h#newcode8 chrome/browser/media/router/media_sink_service.h:8: #include <memory> On 2017/02/07 19:25:31, imcheng wrote: > #include ...
8 months, 2 weeks ago (2017-02-07 22:58:46 UTC) #6
mark a. foltz
Looks good, a few questions below. https://codereview.chromium.org/2662393005/diff/40001/chrome/browser/media/router/media_sink_service.h File chrome/browser/media/router/media_sink_service.h (right): https://codereview.chromium.org/2662393005/diff/40001/chrome/browser/media/router/media_sink_service.h#newcode18 chrome/browser/media/router/media_sink_service.h:18: // A service ...
8 months, 1 week ago (2017-02-09 23:47:52 UTC) #7
zhaobin
https://codereview.chromium.org/2662393005/diff/40001/chrome/browser/media/router/media_sink_service.h File chrome/browser/media/router/media_sink_service.h (right): https://codereview.chromium.org/2662393005/diff/40001/chrome/browser/media/router/media_sink_service.h#newcode18 chrome/browser/media/router/media_sink_service.h:18: // A service which can be used to start ...
8 months, 1 week ago (2017-02-14 00:42:17 UTC) #8
mark a. foltz
lgtm with one comment. https://codereview.chromium.org/2662393005/diff/40001/chrome/browser/media/router/media_sink_service.h File chrome/browser/media/router/media_sink_service.h (right): https://codereview.chromium.org/2662393005/diff/40001/chrome/browser/media/router/media_sink_service.h#newcode37 chrome/browser/media/router/media_sink_service.h:37: // Only one observer can ...
8 months, 1 week ago (2017-02-15 01:01:02 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2662393005/80001
8 months ago (2017-02-16 18:36:32 UTC) #20
commit-bot: I haz the power
8 months ago (2017-02-16 19:26:44 UTC) #23
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/8a4c43002d54cbf4f577a5e00b87...
Sign in to reply to this message.

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