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

Issue 2627463003: Convert MediaRouter mojom apis to intake url::Origin objects instead of strings (Closed)

Created:
3 years, 11 months ago by steimel
Modified:
3 years, 10 months ago
CC:
Aaron Boodman, abarth-chromium, chfremer+watch_chromium.org, chromium-reviews, darin (slow to review), feature-media-reviews_chromium.org, mcasas+watch+vc_chromium.org, media-router+watch_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Convert MediaRouter mojom apis to intake url::Origin objects instead of strings Currently, MediaRouter passes origins around as strings in the mojom api, and as GURLs at a higher level. This CL changes both the strings and the GURLs to be url::Origins instead. BUG=629374 Review-Url: https://codereview.chromium.org/2627463003 Cr-Original-Original-Commit-Position: refs/heads/master@{#446868} Committed: https://chromium.googlesource.com/chromium/src/+/588ccce1960c1ca47d65c75f5edb34e5e17d6deb Review-Url: https://codereview.chromium.org/2627463003 Cr-Original-Commit-Position: refs/heads/master@{#448492} Committed: https://chromium.googlesource.com/chromium/src/+/441f2c5583174d5411ced1d308007b34a055a4fb Review-Url: https://codereview.chromium.org/2627463003 Cr-Commit-Position: refs/heads/master@{#448771} Committed: https://chromium.googlesource.com/chromium/src/+/2537528e220f768c47177abbb4c6b13d4d047117

Patch Set 1 #

Patch Set 2 : Higher level api changes #

Patch Set 3 : Fix android and chromeos mediarouter usage #

Patch Set 4 : Fix chromeos browser tests #

Total comments: 10

Patch Set 5 : Don't use complex ctors in static declarations #

Total comments: 4

Patch Set 6 : return an Origin instead of GURL for GetLastCommittedURLForFrame #

Total comments: 20

Patch Set 7 : mfoltz comments #

Total comments: 2

Patch Set 8 : comment and ternary op #

Total comments: 22

Patch Set 9 : add DCHECK for RFH and other comments #

Patch Set 10 : rebase #

Patch Set 11 : change media_router_bindings to handle Origin<->string conversions #

Total comments: 8

Patch Set 12 : mfoltz comments #

Total comments: 12

Patch Set 13 : throw error instead of returning unique origin #

Total comments: 2

Patch Set 14 : build origin object correctly #

Total comments: 4

Patch Set 15 : Infer port #

Patch Set 16 : rebase #

Patch Set 17 : Add missing dependency for extensions_renderer_resources target #

Unified diffs Side-by-side diffs Delta from patch set Stats (+345 lines, -320 lines) Patch
M chrome/browser/media/android/router/media_router_android.h View 1 2 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/media/android/router/media_router_android.cc View 1 2 3 4 5 6 7 7 chunks +9 lines, -23 lines 0 comments Download
M chrome/browser/media/android/router/media_router_dialog_controller_android.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/media/router/BUILD.gn View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/media/router/create_presentation_connection_request.h View 1 2 3 4 5 2 chunks +7 lines, -4 lines 0 comments Download
M chrome/browser/media/router/create_presentation_connection_request.cc View 1 2 3 4 5 2 chunks +5 lines, -3 lines 0 comments Download
M chrome/browser/media/router/create_presentation_connection_request_unittest.cc View 1 2 3 4 5 3 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/media/router/media_router.h View 1 2 3 4 5 4 chunks +7 lines, -3 lines 0 comments Download
M chrome/browser/media/router/media_router_dialog_controller_unittest.cc View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/media/router/media_sinks_observer.h View 1 4 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/media/router/media_sinks_observer.cc View 1 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/media/router/media_sinks_observer_unittest.cc View 1 2 3 4 5 6 7 8 2 chunks +4 lines, -5 lines 0 comments Download
M chrome/browser/media/router/mock_media_router.h View 1 2 3 4 5 4 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/media/router/mojo/media_router.mojom View 1 2 3 4 5 6 7 8 9 5 chunks +5 lines, -4 lines 0 comments Download
M chrome/browser/media/router/mojo/media_router_mojo_impl.h View 1 6 chunks +8 lines, -8 lines 0 comments Download
M chrome/browser/media/router/mojo/media_router_mojo_impl.cc View 1 2 3 4 5 6 7 8 10 chunks +19 lines, -61 lines 0 comments Download
M chrome/browser/media/router/mojo/media_router_mojo_impl_unittest.cc View 1 2 3 4 5 6 7 8 9 27 chunks +87 lines, -68 lines 0 comments Download
M chrome/browser/media/router/mojo/media_router_mojo_test.h View 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/media/router/presentation_media_sinks_observer.h View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/media/router/presentation_media_sinks_observer.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/media/router/presentation_media_sinks_observer_unittest.cc View 1 2 3 4 5 6 7 8 2 chunks +5 lines, -1 line 0 comments Download
M chrome/browser/media/router/presentation_request.h View 1 2 3 4 5 6 4 chunks +5 lines, -7 lines 0 comments Download
M chrome/browser/media/router/presentation_request.cc View 1 2 3 4 5 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/media/router/presentation_request_unittest.cc View 1 2 3 4 5 2 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/media/router/presentation_service_delegate_impl.cc View 1 2 3 4 5 6 7 8 9 10 5 chunks +11 lines, -12 lines 0 comments Download
M chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc View 1 2 3 4 5 6 7 8 9 5 chunks +7 lines, -6 lines 0 comments Download
M chrome/browser/media/router/test_helper.h View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/media/router/test_helper.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/ash/cast_config_client_media_router.cc View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/ash/system_tray_tray_cast_browsertest_media_router_chromeos.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +7 lines, -5 lines 0 comments Download
M chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl_unittest.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/webui/media_router/media_router_ui.h View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/webui/media_router/media_router_ui.cc View 1 2 3 4 5 6 7 8 9 8 chunks +11 lines, -10 lines 0 comments Download
M chrome/browser/ui/webui/media_router/media_router_ui_unittest.cc View 1 2 3 4 5 8 chunks +12 lines, -10 lines 0 comments Download
M chrome/browser/ui/webui/media_router/query_result_manager.h View 1 2 3 4 5 6 7 8 4 chunks +6 lines, -4 lines 0 comments Download
M chrome/browser/ui/webui/media_router/query_result_manager.cc View 1 4 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/ui/webui/media_router/query_result_manager_unittest.cc View 1 2 3 4 15 chunks +23 lines, -24 lines 0 comments Download
M chrome/test/media_router/media_router_e2e_browsertest.h View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/media_router/media_router_e2e_browsertest.cc View 1 2 3 4 5 6 7 8 9 5 chunks +6 lines, -7 lines 0 comments Download
M chrome/test/media_router/test_media_sinks_observer.h View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/media_router/test_media_sinks_observer.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M extensions/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -0 lines 0 comments Download
M extensions/renderer/dispatcher.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
M extensions/renderer/resources/extensions_renderer_resources.grd View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
M extensions/renderer/resources/media_router_bindings.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 9 chunks +47 lines, -8 lines 0 comments Download

Messages

Total messages: 135 (84 generated)
steimel
I'm currently fixing something I missed on the chromeos build and doing the fixes for ...
3 years, 11 months ago (2017-01-12 18:49:51 UTC) #11
steimel
PTAL
3 years, 11 months ago (2017-01-13 01:12:51 UTC) #17
Wez
Over and above simply changing the type we use in the Mojo APIs, this CL ...
3 years, 11 months ago (2017-01-14 00:09:16 UTC) #20
mark a. foltz
On 2017/01/14 at 00:09:16, wez wrote: > Over and above simply changing the type we ...
3 years, 11 months ago (2017-01-17 21:26:11 UTC) #21
mark a. foltz
wez@, origins are validated by Mojo here: https://cs.chromium.org/chromium/src/url/mojo/origin_struct_traits.h Can you add a validation to MRMI ...
3 years, 11 months ago (2017-01-17 21:30:46 UTC) #22
steimel
https://codereview.chromium.org/2627463003/diff/60001/chrome/browser/media/router/presentation_media_sinks_observer_unittest.cc File chrome/browser/media/router/presentation_media_sinks_observer_unittest.cc (right): https://codereview.chromium.org/2627463003/diff/60001/chrome/browser/media/router/presentation_media_sinks_observer_unittest.cc#newcode24 chrome/browser/media/router/presentation_media_sinks_observer_unittest.cc:24: const url::Origin kOrigin = url::Origin((GURL("https://google.com"))); On 2017/01/17 21:30:45, mark ...
3 years, 11 months ago (2017-01-18 01:52:35 UTC) #23
mark a. foltz
Overall looks good, I missed a few things the first time through though. Sorry about ...
3 years, 11 months ago (2017-01-20 18:26:40 UTC) #28
steimel
PTAL https://codereview.chromium.org/2627463003/diff/80001/chrome/browser/media/router/mojo/media_router_mojo_impl_unittest.cc File chrome/browser/media/router/mojo/media_router_mojo_impl_unittest.cc (right): https://codereview.chromium.org/2627463003/diff/80001/chrome/browser/media/router/mojo/media_router_mojo_impl_unittest.cc#newcode82 chrome/browser/media/router/mojo/media_router_mojo_impl_unittest.cc:82: const url::Origin kOrigin = url::Origin(GURL("http://origin/")); On 2017/01/20 18:26:40, ...
3 years, 11 months ago (2017-01-23 19:24:59 UTC) #29
mark a. foltz
LGTM % a few final nits. Thanks for doing this; it was a big cleanup ...
3 years, 11 months ago (2017-01-24 04:04:18 UTC) #34
steimel
PTAL https://codereview.chromium.org/2627463003/diff/100001/chrome/browser/media/android/router/media_router_android.cc File chrome/browser/media/android/router/media_router_android.cc (right): https://codereview.chromium.org/2627463003/diff/100001/chrome/browser/media/android/router/media_router_android.cc#newcode107 chrome/browser/media/android/router/media_router_android.cc:107: base::android::ConvertUTF8ToJavaString(env, origin.GetURL().spec()); On 2017/01/24 04:04:17, mark a. foltz ...
3 years, 11 months ago (2017-01-25 19:01:11 UTC) #36
steimel
adding sky@ for ash/ OWNERS review
3 years, 11 months ago (2017-01-25 19:11:05 UTC) #38
jdufault
chrome/browser/ui/ash/* lgtm; I'm not listed in OWNERs files so I would recommend ccing stevenjb@ for ...
3 years, 11 months ago (2017-01-25 19:11:57 UTC) #40
jdufault
On 2017/01/25 19:11:57, jdufault wrote: > chrome/browser/ui/ash/* lgtm; I'm not listed in OWNERs files so ...
3 years, 11 months ago (2017-01-25 19:12:20 UTC) #41
mark a. foltz
Still LGTM. Thanks. https://codereview.chromium.org/2627463003/diff/100001/chrome/browser/media/android/router/media_router_android.cc File chrome/browser/media/android/router/media_router_android.cc (right): https://codereview.chromium.org/2627463003/diff/100001/chrome/browser/media/android/router/media_router_android.cc#newcode107 chrome/browser/media/android/router/media_router_android.cc:107: base::android::ConvertUTF8ToJavaString(env, origin.GetURL().spec()); On 2017/01/25 at 19:01:10, ...
3 years, 11 months ago (2017-01-25 21:13:04 UTC) #46
sky
LGTM
3 years, 11 months ago (2017-01-25 22:04:10 UTC) #48
steimel
https://codereview.chromium.org/2627463003/diff/100001/chrome/browser/media/android/router/media_router_android.cc File chrome/browser/media/android/router/media_router_android.cc (right): https://codereview.chromium.org/2627463003/diff/100001/chrome/browser/media/android/router/media_router_android.cc#newcode107 chrome/browser/media/android/router/media_router_android.cc:107: base::android::ConvertUTF8ToJavaString(env, origin.GetURL().spec()); On 2017/01/25 21:13:04, mark a. foltz wrote: ...
3 years, 11 months ago (2017-01-25 22:25:26 UTC) #49
dcheng
https://codereview.chromium.org/2627463003/diff/100001/chrome/browser/media/router/media_sinks_observer_unittest.cc File chrome/browser/media/router/media_sinks_observer_unittest.cc (right): https://codereview.chromium.org/2627463003/diff/100001/chrome/browser/media/router/media_sinks_observer_unittest.cc#newcode17 chrome/browser/media/router/media_sinks_observer_unittest.cc:17: url::Origin origin((GURL("https://origin.com"))); Nit: Here and elsewhere write: url::Origin origin{GURL("https://origin.com")}; ...
3 years, 11 months ago (2017-01-26 04:16:32 UTC) #58
dcheng
Btw, it's fine to ignore the nits about MakeUnique and using constexpr base::TimeDelta and do ...
3 years, 11 months ago (2017-01-26 04:17:34 UTC) #59
steimel
PTAL https://codereview.chromium.org/2627463003/diff/100001/chrome/browser/media/router/media_sinks_observer_unittest.cc File chrome/browser/media/router/media_sinks_observer_unittest.cc (right): https://codereview.chromium.org/2627463003/diff/100001/chrome/browser/media/router/media_sinks_observer_unittest.cc#newcode17 chrome/browser/media/router/media_sinks_observer_unittest.cc:17: url::Origin origin((GURL("https://origin.com"))); On 2017/01/26 04:16:32, dcheng wrote: > ...
3 years, 11 months ago (2017-01-27 00:28:59 UTC) #60
imcheng
https://codereview.chromium.org/2627463003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl.cc File chrome/browser/media/router/presentation_service_delegate_impl.cc (right): https://codereview.chromium.org/2627463003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl.cc#newcode64 chrome/browser/media/router/presentation_service_delegate_impl.cc:64: return render_frame_host ? render_frame_host->GetLastCommittedOrigin() On 2017/01/27 00:28:59, steimel wrote: ...
3 years, 11 months ago (2017-01-27 00:55:06 UTC) #64
imcheng
https://codereview.chromium.org/2627463003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl.cc File chrome/browser/media/router/presentation_service_delegate_impl.cc (right): https://codereview.chromium.org/2627463003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl.cc#newcode64 chrome/browser/media/router/presentation_service_delegate_impl.cc:64: return render_frame_host ? render_frame_host->GetLastCommittedOrigin() On 2017/01/27 00:55:06, imcheng wrote: ...
3 years, 11 months ago (2017-01-27 01:04:37 UTC) #65
dcheng
https://codereview.chromium.org/2627463003/diff/140001/chrome/browser/media/router/presentation_media_sinks_observer_unittest.cc File chrome/browser/media/router/presentation_media_sinks_observer_unittest.cc (right): https://codereview.chromium.org/2627463003/diff/140001/chrome/browser/media/router/presentation_media_sinks_observer_unittest.cc#newcode24 chrome/browser/media/router/presentation_media_sinks_observer_unittest.cc:24: const char kOrigin[] = "https://google.com"; On 2017/01/27 00:28:59, steimel ...
3 years, 10 months ago (2017-01-27 09:11:11 UTC) #68
steimel
https://codereview.chromium.org/2627463003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl.cc File chrome/browser/media/router/presentation_service_delegate_impl.cc (right): https://codereview.chromium.org/2627463003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl.cc#newcode64 chrome/browser/media/router/presentation_service_delegate_impl.cc:64: return render_frame_host ? render_frame_host->GetLastCommittedOrigin() On 2017/01/27 09:11:11, dcheng wrote: ...
3 years, 10 months ago (2017-01-27 18:55:19 UTC) #69
imcheng
https://codereview.chromium.org/2627463003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl.cc File chrome/browser/media/router/presentation_service_delegate_impl.cc (right): https://codereview.chromium.org/2627463003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl.cc#newcode64 chrome/browser/media/router/presentation_service_delegate_impl.cc:64: return render_frame_host ? render_frame_host->GetLastCommittedOrigin() On 2017/01/27 18:55:19, steimel wrote: ...
3 years, 10 months ago (2017-01-27 19:02:50 UTC) #70
dcheng
On 2017/01/27 19:02:50, imcheng wrote: > https://codereview.chromium.org/2627463003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl.cc > File chrome/browser/media/router/presentation_service_delegate_impl.cc (right): > > https://codereview.chromium.org/2627463003/diff/140001/chrome/browser/media/router/presentation_service_delegate_impl.cc#newcode64 > ...
3 years, 10 months ago (2017-01-27 19:05:09 UTC) #71
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/2627463003/160001
3 years, 10 months ago (2017-01-27 19:31:46 UTC) #74
commit-bot: I haz the power
Failed to apply patch for chrome/test/media_router/media_router_e2e_browsertest.cc: While running git apply --index -p1; error: patch failed: ...
3 years, 10 months ago (2017-01-27 19:40:24 UTC) #76
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/2627463003/180001
3 years, 10 months ago (2017-01-27 23:52:02 UTC) #79
commit-bot: I haz the power
Committed patchset #10 (id:180001) as https://chromium.googlesource.com/chromium/src/+/588ccce1960c1ca47d65c75f5edb34e5e17d6deb
3 years, 10 months ago (2017-01-28 01:24:19 UTC) #82
imcheng
A revert of this CL (patchset #10 id:180001) has been created in https://codereview.chromium.org/2666733002/ by imcheng@chromium.org. ...
3 years, 10 months ago (2017-01-30 21:26:40 UTC) #83
steimel
PTAL
3 years, 10 months ago (2017-02-02 22:22:00 UTC) #85
mark a. foltz
Minor comments on the bindings. https://codereview.chromium.org/2627463003/diff/200001/extensions/renderer/resources/media_router_bindings.js File extensions/renderer/resources/media_router_bindings.js (right): https://codereview.chromium.org/2627463003/diff/200001/extensions/renderer/resources/media_router_bindings.js#newcode153 extensions/renderer/resources/media_router_bindings.js:153: function mojoOriginToString_(origin) { Please ...
3 years, 10 months ago (2017-02-02 23:05:08 UTC) #88
steimel
PTAL https://codereview.chromium.org/2627463003/diff/200001/extensions/renderer/resources/media_router_bindings.js File extensions/renderer/resources/media_router_bindings.js (right): https://codereview.chromium.org/2627463003/diff/200001/extensions/renderer/resources/media_router_bindings.js#newcode153 extensions/renderer/resources/media_router_bindings.js:153: function mojoOriginToString_(origin) { On 2017/02/02 23:05:08, mark a. ...
3 years, 10 months ago (2017-02-03 00:17:31 UTC) #91
dcheng
Does this need another look from me? It's hard to tell what changed (perhaps include ...
3 years, 10 months ago (2017-02-03 01:33:59 UTC) #96
steimel
oh my bad. PTAL mfoltz@ and imcheng@
3 years, 10 months ago (2017-02-03 17:33:22 UTC) #97
mark a. foltz
LGTM for media_router_bindings.js, with one open question for Derek that should be resolved. https://codereview.chromium.org/2627463003/diff/220001/extensions/renderer/resources/media_router_bindings.js File ...
3 years, 10 months ago (2017-02-03 17:52:39 UTC) #98
imcheng
LGTM. Please test the conversion logic in media_router_bindings.js by running MediaRouter integration tests. Thanks! https://codereview.chromium.org/2627463003/diff/220001/extensions/renderer/resources/media_router_bindings.js ...
3 years, 10 months ago (2017-02-03 18:58:02 UTC) #99
mark a. foltz
https://codereview.chromium.org/2627463003/diff/220001/extensions/renderer/resources/media_router_bindings.js File extensions/renderer/resources/media_router_bindings.js (right): https://codereview.chromium.org/2627463003/diff/220001/extensions/renderer/resources/media_router_bindings.js#newcode168 extensions/renderer/resources/media_router_bindings.js:168: return new originMojom.Origin({ unique: true }); On 2017/02/03 at ...
3 years, 10 months ago (2017-02-03 19:05:13 UTC) #100
steimel
mfoltz@, imcheng@, PTAL. Adding benwells@ for OWNERS review of extensions/renderer/* https://codereview.chromium.org/2627463003/diff/220001/extensions/renderer/resources/media_router_bindings.js File extensions/renderer/resources/media_router_bindings.js (right): https://codereview.chromium.org/2627463003/diff/220001/extensions/renderer/resources/media_router_bindings.js#newcode152 ...
3 years, 10 months ago (2017-02-03 22:38:37 UTC) #102
benwells
extensions/renderer lgtm
3 years, 10 months ago (2017-02-04 01:15:21 UTC) #107
imcheng
still lgtm
3 years, 10 months ago (2017-02-04 01:19:31 UTC) #108
mark a. foltz
Still LGTM with one comment. https://codereview.chromium.org/2627463003/diff/240001/extensions/renderer/resources/media_router_bindings.js File extensions/renderer/resources/media_router_bindings.js (right): https://codereview.chromium.org/2627463003/diff/240001/extensions/renderer/resources/media_router_bindings.js#newcode163 extensions/renderer/resources/media_router_bindings.js:163: * @param {string} string ...
3 years, 10 months ago (2017-02-04 01:44:29 UTC) #109
dcheng
https://codereview.chromium.org/2627463003/diff/260001/chrome/browser/media/router/presentation_request.cc File chrome/browser/media/router/presentation_request.cc (right): https://codereview.chromium.org/2627463003/diff/260001/chrome/browser/media/router/presentation_request.cc#newcode29 chrome/browser/media/router/presentation_request.cc:29: ((frame_origin_.unique() && other.frame_origin_.unique()) || FWIW, I'm not sure about ...
3 years, 10 months ago (2017-02-05 08:48:07 UTC) #110
steimel
imcheng@, ptal https://codereview.chromium.org/2627463003/diff/240001/extensions/renderer/resources/media_router_bindings.js File extensions/renderer/resources/media_router_bindings.js (right): https://codereview.chromium.org/2627463003/diff/240001/extensions/renderer/resources/media_router_bindings.js#newcode163 extensions/renderer/resources/media_router_bindings.js:163: * @param {string} string origin On 2017/02/04 ...
3 years, 10 months ago (2017-02-06 23:01:37 UTC) #111
imcheng
lgtm
3 years, 10 months ago (2017-02-06 23:43:52 UTC) #116
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/2627463003/300001
3 years, 10 months ago (2017-02-07 00:04:59 UTC) #119
commit-bot: I haz the power
Committed patchset #16 (id:300001) as https://chromium.googlesource.com/chromium/src/+/441f2c5583174d5411ced1d308007b34a055a4fb
3 years, 10 months ago (2017-02-07 01:23:44 UTC) #122
shimazu
A revert of this CL (patchset #16 id:300001) has been created in https://codereview.chromium.org/2678123003/ by shimazu@chromium.org. ...
3 years, 10 months ago (2017-02-07 01:58:57 UTC) #123
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/2627463003/320001
3 years, 10 months ago (2017-02-07 22:59:49 UTC) #131
commit-bot: I haz the power
Committed patchset #17 (id:320001) as https://chromium.googlesource.com/chromium/src/+/2537528e220f768c47177abbb4c6b13d4d047117
3 years, 10 months ago (2017-02-07 23:10:11 UTC) #134
dcheng
3 years, 10 months ago (2017-02-08 00:02:38 UTC) #135
Message was sent while issue was closed.
Note: even if it's just a minor build fix, please tbr the latest diffs to
someone at least.

in any case, the diff between PS16 and PS17 lgtm

Powered by Google App Engine
This is Rietveld 408576698