Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(796)

Issue 2386633003: [Media Router] Convert MediaRouter to use GURL for presentation URLs. (Closed)

Created:
4 years, 2 months ago by mark a. foltz
Modified:
4 years, 2 months ago
Reviewers:
imcheng, dcheng, zhaobin
CC:
chromium-reviews, feature-media-reviews_chromium.org, mcasas+watch+vc_chromium.org, media-router+watch_chromium.org, miu+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Converts content/browser for Presentation API and Media Router APIs to use GURL for presentation URLs. Updates many, many unit tests. BUG=632623 Committed: https://crrev.com/7a2c823bd8f66f7ea61f8ade94f5e76f0f32ed74 Cr-Commit-Position: refs/heads/master@{#424037}

Patch Set 1 #

Patch Set 2 : content/ cleanup done, unittests pass #

Patch Set 3 : Revert url::Origin change #

Patch Set 4 : More unittest fixes... #

Patch Set 5 : Update remaining unittests #

Patch Set 6 : Minor cleanups #

Patch Set 7 : Update media_router_e2e_browsertest #

Total comments: 4

Patch Set 8 : Respond to imcheng@ comments #

Total comments: 2

Patch Set 9 : Respond to dcheng@ comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+336 lines, -346 lines) Patch
M chrome/browser/media/router/create_presentation_connection_request.h View 1 2 3 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/media/router/create_presentation_connection_request.cc View 4 5 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/media/router/create_presentation_connection_request_unittest.cc View 1 2 3 4 5 6 7 4 chunks +12 lines, -6 lines 0 comments Download
M chrome/browser/media/router/media_route.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/media/router/media_route_unittest.cc View 1 2 3 4 5 6 7 1 chunk +17 lines, -14 lines 0 comments Download
M chrome/browser/media/router/media_router.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/media/router/media_sink.h View 1 2 3 1 chunk +1 line, -3 lines 0 comments Download
M chrome/browser/media/router/media_sinks_observer_unittest.cc View 1 2 3 4 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/media/router/media_source.h View 1 2 3 1 chunk +6 lines, -1 line 0 comments Download
M chrome/browser/media/router/media_source.cc View 1 2 3 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/media/router/media_source_helper.h View 1 2 3 4 5 6 7 3 chunks +5 lines, -8 lines 0 comments Download
M chrome/browser/media/router/media_source_helper.cc View 1 2 3 4 5 6 7 3 chunks +6 lines, -18 lines 0 comments Download
M chrome/browser/media/router/media_source_helper_unittest.cc View 1 2 3 4 5 6 7 2 chunks +13 lines, -22 lines 0 comments Download
M chrome/browser/media/router/mock_screen_availability_listener.h View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/media/router/mock_screen_availability_listener.cc View 1 2 3 1 chunk +3 lines, -4 lines 0 comments Download
M chrome/browser/media/router/mojo/media_router_mojo_impl_unittest.cc View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/media/router/presentation_media_sinks_observer_unittest.cc View 1 2 3 4 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/media/router/presentation_request.h View 1 2 3 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/media/router/presentation_request.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/media/router/presentation_request_unittest.cc View 1 2 3 4 5 6 7 8 1 chunk +17 lines, -15 lines 0 comments Download
M chrome/browser/media/router/presentation_service_delegate_impl.h View 1 2 chunks +9 lines, -11 lines 0 comments Download
M chrome/browser/media/router/presentation_service_delegate_impl.cc View 1 2 3 4 8 chunks +16 lines, -15 lines 0 comments Download
M chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc View 1 2 3 4 5 6 7 8 19 chunks +44 lines, -39 lines 0 comments Download
M chrome/browser/media/router/test_helper.cc View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/media_router/cast_modes_with_media_sources_unittest.cc View 1 2 3 4 1 chunk +5 lines, -2 lines 0 comments Download
M chrome/browser/ui/webui/media_router/media_router_ui.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/webui/media_router/media_router_ui_unittest.cc View 1 2 3 4 7 chunks +13 lines, -10 lines 0 comments Download
M chrome/browser/ui/webui/media_router/query_result_manager.h View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/media_router/query_result_manager.cc View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/webui/media_router/query_result_manager_unittest.cc View 1 2 3 4 6 chunks +16 lines, -8 lines 0 comments Download
M chrome/test/media_router/media_router_e2e_browsertest.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M content/browser/presentation/presentation_service_impl.h View 1 2 3 2 chunks +6 lines, -7 lines 0 comments Download
M content/browser/presentation/presentation_service_impl.cc View 1 2 3 7 chunks +18 lines, -39 lines 0 comments Download
M content/browser/presentation/presentation_service_impl_unittest.cc View 1 2 3 4 5 6 7 8 28 chunks +81 lines, -86 lines 0 comments Download
M content/browser/presentation/presentation_type_converters.h View 1 2 chunks +2 lines, -4 lines 0 comments Download
M content/browser/presentation/presentation_type_converters_unittest.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M content/public/browser/presentation_screen_availability_listener.h View 1 2 chunks +2 lines, -1 line 0 comments Download
M content/public/browser/presentation_service_delegate.h View 1 4 chunks +5 lines, -6 lines 0 comments Download
M content/public/browser/presentation_session.h View 1 2 chunks +3 lines, -2 lines 0 comments Download
M content/public/browser/presentation_session.cc View 1 2 3 1 chunk +2 lines, -4 lines 0 comments Download

Messages

Total messages: 24 (15 generated)
mark a. foltz
4 years, 2 months ago (2016-10-05 20:21:03 UTC) #6
imcheng
lgtm https://codereview.chromium.org/2386633003/diff/120001/chrome/browser/media/router/create_presentation_connection_request_unittest.cc File chrome/browser/media/router/create_presentation_connection_request_unittest.cc (right): https://codereview.chromium.org/2386633003/diff/120001/chrome/browser/media/router/create_presentation_connection_request_unittest.cc#newcode28 chrome/browser/media/router/create_presentation_connection_request_unittest.cc:28: presentation_url_(kPresentationUrl) {} presentation_urls_(kPresentationUrl, 1) or presentation_urls_{kPresentationUrl} then you ...
4 years, 2 months ago (2016-10-07 00:38:36 UTC) #13
mark a. foltz
+dcheng for presentation_type_converters[_unittest].cc https://codereview.chromium.org/2386633003/diff/120001/chrome/browser/media/router/create_presentation_connection_request_unittest.cc File chrome/browser/media/router/create_presentation_connection_request_unittest.cc (right): https://codereview.chromium.org/2386633003/diff/120001/chrome/browser/media/router/create_presentation_connection_request_unittest.cc#newcode28 chrome/browser/media/router/create_presentation_connection_request_unittest.cc:28: presentation_url_(kPresentationUrl) {} On 2016/10/07 at 00:38:36, ...
4 years, 2 months ago (2016-10-07 23:14:10 UTC) #15
dcheng
LGTM, thanks for cleaning this up https://codereview.chromium.org/2386633003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc File chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc (right): https://codereview.chromium.org/2386633003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc#newcode337 chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc:337: std::vector<GURL> request1_urls({presentation_url1_}); Nit: ...
4 years, 2 months ago (2016-10-07 23:30:41 UTC) #16
mark a. foltz
https://codereview.chromium.org/2386633003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc File chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc (right): https://codereview.chromium.org/2386633003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc#newcode337 chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc:337: std::vector<GURL> request1_urls({presentation_url1_}); On 2016/10/07 at 23:30:40, dcheng wrote: > ...
4 years, 2 months ago (2016-10-08 00:11:28 UTC) #17
zhaobin
On 2016/10/08 00:11:28, mark a. foltz wrote: > https://codereview.chromium.org/2386633003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc > File chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc > (right): > ...
4 years, 2 months ago (2016-10-08 00:18:26 UTC) #18
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/2386633003/160001
4 years, 2 months ago (2016-10-08 00:34:27 UTC) #21
commit-bot: I haz the power
Committed patchset #9 (id:160001)
4 years, 2 months ago (2016-10-08 01:35:47 UTC) #22
commit-bot: I haz the power
4 years, 2 months ago (2016-10-08 01:38:45 UTC) #24
Message was sent while issue was closed.
Patchset 9 (id:??) landed as
https://crrev.com/7a2c823bd8f66f7ea61f8ade94f5e76f0f32ed74
Cr-Commit-Position: refs/heads/master@{#424037}

Powered by Google App Engine
This is Rietveld 408576698