New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 757358 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

[Media Router] New Cast Presentation URLs

Project Member Reported by imch...@chromium.org, Aug 21 2017

Issue description

Tracker for updating support code / tests that references "https://google.com/cast..." as a Presentation URL for Cast. They should be updated to use "cast:appId" URLs.


 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee20edad71544b8d5fa41d8b597d54e12eb9163e

commit ee20edad71544b8d5fa41d8b597d54e12eb9163e
Author: Derek Cheng <imcheng@chromium.org>
Date: Fri Aug 25 07:09:29 2017

[Media Router] Update Cast Presentation URL logic.

- Update media_source_helper to consider cast: URLs joinable

Bug:  757358 
Change-Id: I7fc10ac372cd0adf4b6e4a0f9ece2f743cc9787b
Reviewed-on: https://chromium-review.googlesource.com/630982
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497341}
[modify] https://crrev.com/ee20edad71544b8d5fa41d8b597d54e12eb9163e/chrome/common/media_router/media_source_helper.cc
[modify] https://crrev.com/ee20edad71544b8d5fa41d8b597d54e12eb9163e/chrome/common/media_router/media_source_helper_unittest.cc

Why don't we require `cast://appId`?
Cc: mlamouri@chromium.org
That is just what we decided on. However I did a bit more digging on when the 2 forward slashes are used. The answer is this:

From Wikipedia: "When the authority component (explained below) is absent, the path component cannot begin with two slashes." [1]

The new URLs don't have an authority component (as appId / appName isn't considered a true hostname), so slashes are omitted.

[1] https://en.wikipedia.org/wiki/Uniform_Resource_Identifier
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2de666deb1c694182625441131da2f5068523e5f

commit 2de666deb1c694182625441131da2f5068523e5f
Author: Derek Cheng <imcheng@chromium.org>
Date: Wed Sep 13 18:31:49 2017

[Media Router Android] Parse Cast Presentation URLs.

Also added check for valid autojoin policy for legacy and new URL
formats.

Bug:  757358 
Change-Id: If99b46e9f0fd692e2665b5148c6ac7949b2fcb0f
Reviewed-on: https://chromium-review.googlesource.com/644511
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501701}
[modify] https://crrev.com/2de666deb1c694182625441131da2f5068523e5f/chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/MediaSource.java
[modify] https://crrev.com/2de666deb1c694182625441131da2f5068523e5f/chrome/android/junit/src/org/chromium/chrome/browser/media/router/cast/MediaSourceTest.java

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f493473bcfa1d41a29a71e8d05a10903e6dede78

commit f493473bcfa1d41a29a71e8d05a10903e6dede78
Author: Derek Cheng <imcheng@chromium.org>
Date: Mon Sep 25 21:26:23 2017

[Media Router] Update Cast Presentation URL logic - tests only

- Update various tests that uses https://google.com/cast... to Cast URLs

Should be landed only after the extension-side patch lands + sticks.

Bug:  757358 
Change-Id: I58a7635872cb4880fed9a3ba2539c01271d3ba01
Reviewed-on: https://chromium-review.googlesource.com/623040
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Anton Vayvod <avayvod@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504167}
[modify] https://crrev.com/f493473bcfa1d41a29a71e8d05a10903e6dede78/chrome/android/junit/src/org/chromium/chrome/browser/media/router/ChromeMediaRouterTestBase.java
[modify] https://crrev.com/f493473bcfa1d41a29a71e8d05a10903e6dede78/chrome/android/junit/src/org/chromium/chrome/browser/media/router/cast/CastMediaRouteProviderTest.java
[modify] https://crrev.com/f493473bcfa1d41a29a71e8d05a10903e6dede78/chrome/android/junit/src/org/chromium/chrome/browser/media/router/cast/CastMessageHandlerTest.java
[modify] https://crrev.com/f493473bcfa1d41a29a71e8d05a10903e6dede78/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc
[modify] https://crrev.com/f493473bcfa1d41a29a71e8d05a10903e6dede78/chrome/common/media_router/media_source_helper_unittest.cc
[modify] https://crrev.com/f493473bcfa1d41a29a71e8d05a10903e6dede78/chrome/test/media_router/media_router_e2e_browsertest.cc
[modify] https://crrev.com/f493473bcfa1d41a29a71e8d05a10903e6dede78/chrome/test/media_router/resources/common.js
[modify] https://crrev.com/f493473bcfa1d41a29a71e8d05a10903e6dede78/third_party/WebKit/LayoutTests/external/wpt/presentation-api/controlling-ua/common.js

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed57c8e13e3cffab8b3c381e0bb775a3acc82eef

commit ed57c8e13e3cffab8b3c381e0bb775a3acc82eef
Author: Derek Cheng <imcheng@chromium.org>
Date: Sat Oct 14 02:08:03 2017

[PresentationAPI]UMA for dial/legacy Cast Presentation URL usage.

Direct usage of dial: / legacy Cast URL ("https://google.com/cast#...")
in Presentation API were never officially supported.

We would like to remove support for dial: URLs in the future. DIAL
functionality will still be supported via Cast SDK.

Legacy Cast URLs are now migrated to cast: URL format in the Cast SDK.
We expect only stragglers, such as those who hardcode an older copy of
the SDK code, will be affected by the migration.

This patch adds UMA to measure the usage (via PresentationAvailability)
of both to confirm that usage  is negliable, which should let us remove
support for them safely.

The metric being recorded (MediaRouter.PresentationUrlType) is also
generalized to other types of URLs (cast, cast-dial, http, https) in
this patch.

Bug:  757358 
Change-Id: I964bd2de0d848227f688cc461960c13e92d291cc
Reviewed-on: https://chromium-review.googlesource.com/714449
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508913}
[modify] https://crrev.com/ed57c8e13e3cffab8b3c381e0bb775a3acc82eef/chrome/browser/media/router/media_router_metrics.cc
[modify] https://crrev.com/ed57c8e13e3cffab8b3c381e0bb775a3acc82eef/chrome/browser/media/router/media_router_metrics.h
[modify] https://crrev.com/ed57c8e13e3cffab8b3c381e0bb775a3acc82eef/chrome/browser/media/router/media_router_metrics_unittest.cc
[modify] https://crrev.com/ed57c8e13e3cffab8b3c381e0bb775a3acc82eef/chrome/browser/media/router/presentation_service_delegate_impl.cc
[modify] https://crrev.com/ed57c8e13e3cffab8b3c381e0bb775a3acc82eef/chrome/common/media_router/media_source_helper.cc
[modify] https://crrev.com/ed57c8e13e3cffab8b3c381e0bb775a3acc82eef/chrome/common/media_router/media_source_helper.h
[modify] https://crrev.com/ed57c8e13e3cffab8b3c381e0bb775a3acc82eef/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/ed57c8e13e3cffab8b3c381e0bb775a3acc82eef/tools/metrics/histograms/histograms.xml

Labels: Merge-Request-63
Please add appropriate OSs.
Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Labels: -Pri-3 Pri-2
Upgrading priority to P2 since getting UMAs in will help us rmeove support for old/unused features in a timely manner. 
Labels: Merge-Approved-63
Labels: -Merge-Request-63
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 17 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d853faeb3ff2b6c71e8c1ef0f781c0cd1eaaeba2

commit d853faeb3ff2b6c71e8c1ef0f781c0cd1eaaeba2
Author: Derek Cheng <imcheng@chromium.org>
Date: Tue Oct 17 01:06:47 2017

[PresentationAPI]UMA for dial/legacy Cast Presentation URL usage.

Direct usage of dial: / legacy Cast URL ("https://google.com/cast#...")
in Presentation API were never officially supported.

We would like to remove support for dial: URLs in the future. DIAL
functionality will still be supported via Cast SDK.

Legacy Cast URLs are now migrated to cast: URL format in the Cast SDK.
We expect only stragglers, such as those who hardcode an older copy of
the SDK code, will be affected by the migration.

This patch adds UMA to measure the usage (via PresentationAvailability)
of both to confirm that usage  is negliable, which should let us remove
support for them safely.

The metric being recorded (MediaRouter.PresentationUrlType) is also
generalized to other types of URLs (cast, cast-dial, http, https) in
this patch.

Bug:  757358 
Change-Id: I964bd2de0d848227f688cc461960c13e92d291cc
Reviewed-on: https://chromium-review.googlesource.com/714449
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#508913}(cherry picked from commit ed57c8e13e3cffab8b3c381e0bb775a3acc82eef)
Reviewed-on: https://chromium-review.googlesource.com/722419
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#23}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/d853faeb3ff2b6c71e8c1ef0f781c0cd1eaaeba2/chrome/browser/media/router/media_router_metrics.cc
[modify] https://crrev.com/d853faeb3ff2b6c71e8c1ef0f781c0cd1eaaeba2/chrome/browser/media/router/media_router_metrics.h
[modify] https://crrev.com/d853faeb3ff2b6c71e8c1ef0f781c0cd1eaaeba2/chrome/browser/media/router/media_router_metrics_unittest.cc
[modify] https://crrev.com/d853faeb3ff2b6c71e8c1ef0f781c0cd1eaaeba2/chrome/browser/media/router/presentation_service_delegate_impl.cc
[modify] https://crrev.com/d853faeb3ff2b6c71e8c1ef0f781c0cd1eaaeba2/chrome/common/media_router/media_source_helper.cc
[modify] https://crrev.com/d853faeb3ff2b6c71e8c1ef0f781c0cd1eaaeba2/chrome/common/media_router/media_source_helper.h
[modify] https://crrev.com/d853faeb3ff2b6c71e8c1ef0f781c0cd1eaaeba2/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/d853faeb3ff2b6c71e8c1ef0f781c0cd1eaaeba2/tools/metrics/histograms/histograms.xml

imcheng@, can this be marked Fixed?
Labels: M-63
Status: Fixed (was: Assigned)
I just looked into the MediaRouter.PresentationRequest.AvailabilityUrlType metric collected over the past week on canary/dev. The breakdown for past 7 days on canary is as follows:

PresentationUrlType,Count,PDF
Cast,6,991,570,58.12%
CastDial,4,899,362,40.73%
CastLegacy,139,158,01.16%

Looking at data from earlier dates it is evident that the percentage for CastLegacy has settled just right above 1% of all PresentationRequest URLs.

(Also, Android is still using legacy Cast URLs) despite being on 63 - seems like we haven't deployed Android cast_sender.js for 63 yet?)

Let's discuss next step for deprecation of legacy URLs next week. We can close this bug.

Sign in to add a comment