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

Issue 2327993002: [PresentationAPI] Use KURL and WebURL in place of string types. (Closed)

Created:
4 years, 3 months ago by mark a. foltz
Modified:
4 years, 3 months ago
Reviewers:
imcheng, dcheng
CC:
blink-reviews, blink-reviews-api_chromium.org, chromium-reviews, darin-cc_chromium.org, dglazkov+blink, haraken, jam, media-router+watch_chromium.org, mlamouri+watch-content_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Converts Presentation API to use KURL/WebURL in place of string types for availability and presentation URLs. Removes several conversions. BUG=632623 Committed: https://crrev.com/551008391f0723c846b2277048ef7ce9c61e805b Cr-Commit-Position: refs/heads/master@{#418767}

Patch Set 1 #

Patch Set 2 : Revert browser test changes #

Patch Set 3 : Finish KURL #

Patch Set 4 : Simplify presentation_dispatcher #

Patch Set 5 : Fix bug #

Patch Set 6 : Rebase. Fix PresentationReceiverTest #

Total comments: 9

Patch Set 7 : Address imcheng@ comments #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+89 lines, -87 lines) Patch
M content/renderer/presentation/presentation_connection_client.h View 1 chunk +2 lines, -2 lines 0 comments Download
M content/renderer/presentation/presentation_connection_client.cc View 1 2 3 4 5 6 1 chunk +4 lines, -3 lines 0 comments Download
M content/renderer/presentation/presentation_dispatcher.h View 1 2 3 4 chunks +17 lines, -14 lines 0 comments Download
M content/renderer/presentation/presentation_dispatcher.cc View 1 2 3 4 5 6 17 chunks +40 lines, -45 lines 0 comments Download
M third_party/WebKit/Source/modules/presentation/PresentationConnection.h View 1 2 3 4 5 3 chunks +3 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp View 1 2 3 4 5 2 chunks +2 lines, -2 lines 1 comment Download
M third_party/WebKit/Source/modules/presentation/PresentationController.cpp View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/presentation/PresentationReceiverTest.cpp View 1 2 3 4 5 2 chunks +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/presentation/PresentationRequest.cpp View 3 chunks +5 lines, -5 lines 0 comments Download
M third_party/WebKit/public/platform/modules/presentation/WebPresentationClient.h View 4 chunks +10 lines, -10 lines 0 comments Download
M third_party/WebKit/public/platform/modules/presentation/WebPresentationConnectionClient.h View 2 chunks +2 lines, -1 line 0 comments Download

Messages

Total messages: 26 (16 generated)
mark a. foltz
4 years, 3 months ago (2016-09-13 17:42:27 UTC) #11
imcheng
lgtm https://codereview.chromium.org/2327993002/diff/100001/content/renderer/presentation/presentation_connection_client.cc File content/renderer/presentation/presentation_connection_client.cc (right): https://codereview.chromium.org/2327993002/diff/100001/content/renderer/presentation/presentation_connection_client.cc#newcode15 content/renderer/presentation/presentation_connection_client.cc:15: : url_(blink::WebURL(session_info->url)), nit: can this url_(session_info->url) ? https://codereview.chromium.org/2327993002/diff/100001/content/renderer/presentation/presentation_connection_client.cc#newcode21 ...
4 years, 3 months ago (2016-09-14 00:39:14 UTC) #12
mark a. foltz
https://codereview.chromium.org/2327993002/diff/100001/content/renderer/presentation/presentation_connection_client.cc File content/renderer/presentation/presentation_connection_client.cc (right): https://codereview.chromium.org/2327993002/diff/100001/content/renderer/presentation/presentation_connection_client.cc#newcode15 content/renderer/presentation/presentation_connection_client.cc:15: : url_(blink::WebURL(session_info->url)), On 2016/09/14 at 00:39:13, imcheng wrote: > ...
4 years, 3 months ago (2016-09-14 20:02:42 UTC) #13
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/2327993002/120001
4 years, 3 months ago (2016-09-14 20:03:39 UTC) #16
dcheng
LGTM https://codereview.chromium.org/2327993002/diff/120001/third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp File third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp (right): https://codereview.chromium.org/2327993002/diff/120001/third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp#newcode409 third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp:409: return client && m_url == KURL(client->getUrl()) && m_id ...
4 years, 3 months ago (2016-09-14 20:07:27 UTC) #18
mark a. foltz
On 2016/09/14 at 20:07:27, dcheng wrote: > LGTM > > https://codereview.chromium.org/2327993002/diff/120001/third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp > File third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp (right): ...
4 years, 3 months ago (2016-09-14 20:11:33 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/297813)
4 years, 3 months ago (2016-09-14 21:00:12 UTC) #21
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/2327993002/120001
4 years, 3 months ago (2016-09-15 01:43:38 UTC) #23
commit-bot: I haz the power
Committed patchset #7 (id:120001)
4 years, 3 months ago (2016-09-15 02:49:13 UTC) #24
commit-bot: I haz the power
4 years, 3 months ago (2016-09-15 02:51:12 UTC) #26
Message was sent while issue was closed.
Patchset 7 (id:??) landed as
https://crrev.com/551008391f0723c846b2277048ef7ce9c61e805b
Cr-Commit-Position: refs/heads/master@{#418767}

Powered by Google App Engine
This is Rietveld 408576698