YouTube Flash Embeds: rewrite all embeds, even enablejsapi=1.
YouTube has deprecated Flash embeds so it will provide a better
experience for our users to have them inlined with possible breakage
instead of having to use Flash and navigate to YouTube.
Bug: 763322
Change-Id: I3243e3048f499ec51e3220ebd18201a94e3eca46
Reviewed-on: https://chromium-review.googlesource.com/657277
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501587}diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
index d826061..36fdaa7 100644
--- a/chrome/renderer/chrome_content_renderer_client.cc
+++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -1638,19 +1638,12 @@
}
GURL corrected_url = GURL(url_str);
- // Unless we're on an Android device, we don't modify any URLs that contain
- // the enablejsapi=1 parameter since the page may be interacting with the
- // YouTube Flash player in Javascript and we don't want to break working
- // content. If we're on an Android device and the URL contains the
- // enablejsapi=1 parameter, we do override the URL.
- if (corrected_url.query().find("enablejsapi=1") != std::string::npos) {
-#if defined(OS_ANDROID)
+ // Chrome used to only rewrite embeds with enablejsapi=1 on mobile for
+ // backward compatibility but with Flash embeds deprecated by YouTube, they
+ // are rewritten on all platforms. However, a different result is used in
+ // order to keep track of how popular they are.
+ if (corrected_url.query().find("enablejsapi=1") != std::string::npos)
result = internal::SUCCESS_ENABLEJSAPI;
-#else
- RecordYouTubeRewriteUMA(internal::FAILURE_ENABLEJSAPI);
- return GURL();
-#endif
- }
// Change the path to use the YouTube HTML5 API
std::string path = corrected_url.path();
diff --git a/chrome/renderer/chrome_content_renderer_client_browsertest.cc b/chrome/renderer/chrome_content_renderer_client_browsertest.cc
index 934c1d7e..e8a4d65 100644
--- a/chrome/renderer/chrome_content_renderer_client_browsertest.cc
+++ b/chrome/renderer/chrome_content_renderer_client_browsertest.cc
@@ -132,42 +132,18 @@
};
const FlashEmbedsTestData kFlashEmbedsTestData[] = {
- {
- "Valid URL, no parameters",
- "www.youtube.com",
- "/v/deadbeef",
- "application/x-shockwave-flash",
- "/embed/deadbeef"
- },
- {
- "Valid URL, no parameters, subdomain",
- "www.foo.youtube.com",
- "/v/deadbeef",
- "application/x-shockwave-flash",
- "/embed/deadbeef"
- },
- {
- "Valid URL, many parameters",
- "www.youtube.com",
- "/v/deadbeef?start=4&fs=1",
- "application/x-shockwave-flash",
- "/embed/deadbeef?start=4&fs=1"
- },
- {
- "Invalid parameter construct, many parameters",
- "www.youtube.com",
- "/v/deadbeef&bar=4&foo=6",
- "application/x-shockwave-flash",
- "/embed/deadbeef?bar=4&foo=6"
- },
- {
- "Valid URL, enablejsapi=1",
- "www.youtube.com",
- "/v/deadbeef?enablejsapi=1",
- "",
- "/v/deadbeef?enablejsapi=1"
- }
-};
+ {"Valid URL, no parameters", "www.youtube.com", "/v/deadbeef",
+ "application/x-shockwave-flash", "/embed/deadbeef"},
+ {"Valid URL, no parameters, subdomain", "www.foo.youtube.com",
+ "/v/deadbeef", "application/x-shockwave-flash", "/embed/deadbeef"},
+ {"Valid URL, many parameters", "www.youtube.com",
+ "/v/deadbeef?start=4&fs=1", "application/x-shockwave-flash",
+ "/embed/deadbeef?start=4&fs=1"},
+ {"Invalid parameter construct, many parameters", "www.youtube.com",
+ "/v/deadbeef&bar=4&foo=6", "application/x-shockwave-flash",
+ "/embed/deadbeef?bar=4&foo=6"},
+ {"Valid URL, enablejsapi=1", "www.youtube.com", "/v/deadbeef?enablejsapi=1",
+ "application/x-shockwave-flash", "/embed/deadbeef?enablejsapi=1"}};
} // namespace
diff --git a/chrome/renderer/chrome_content_renderer_client_unittest.cc b/chrome/renderer/chrome_content_renderer_client_unittest.cc
index 7e4e8565..50d8c94b 100644
--- a/chrome/renderer/chrome_content_renderer_client_unittest.cc
+++ b/chrome/renderer/chrome_content_renderer_client_unittest.cc
@@ -418,8 +418,7 @@
[data_reduction_proxy::chrome_proxy_content_transform_header()]);
}
-// These are tests that are common for both Android and desktop browsers.
-TEST_F(ChromeContentRendererClientTest, RewriteEmbedCommon) {
+TEST_F(ChromeContentRendererClientTest, RewriteEmbed) {
struct TestData {
std::string original;
std::string expected;
@@ -498,21 +497,6 @@
// youtube-nocookie.com, https
{"https://www.youtube-nocookie.com/v/123/",
"https://www.youtube-nocookie.com/embed/123/"},
- };
-
- ChromeContentRendererClient client;
-
- for (const auto& data : test_data)
- EXPECT_EQ(GURL(data.expected),
- client.OverrideFlashEmbedWithHTML(GURL(data.original)));
-}
-
-#if defined(OS_ANDROID)
-TEST_F(ChromeContentRendererClientTest, RewriteEmbedAndroid) {
- struct TestData {
- std::string original;
- std::string expected;
- } test_data[] = {
// URL isn't using Flash, has JS API enabled
{"http://www.youtube.com/embed/deadbeef?enablejsapi=1", ""},
// URL is using Flash, has JS API enabled
@@ -532,38 +516,11 @@
ChromeContentRendererClient client;
- for (const auto& data : test_data) {
+ for (const auto& data : test_data)
EXPECT_EQ(GURL(data.expected),
client.OverrideFlashEmbedWithHTML(GURL(data.original)));
- }
}
-#else
-TEST_F(ChromeContentRendererClientTest, RewriteEmbedDesktop) {
- struct TestData {
- std::string original;
- std::string expected;
- } test_data[] = {
- // URL isn't using Flash, has JS API enabled
- {"http://www.youtube.com/embed/deadbeef?enablejsapi=1", ""},
- // URL is using Flash, has JS API enabled
- {"http://www.youtube.com/v/deadbeef?enablejsapi=1", ""},
- // youtube-nocookie.com, has JS API enabled
- {"http://www.youtube-nocookie.com/v/123?enablejsapi=1", ""},
- // URL is using Flash, has JS API enabled, invalid parameter construct
- {"http://www.youtube.com/v/deadbeef&enablejsapi=1", ""},
- // URL is using Flash, has JS API enabled, invalid parameter construct,
- // has multiple parameters
- {"http://www.youtube.com/v/deadbeef&start=4&enablejsapi=1", ""},
- };
- ChromeContentRendererClient client;
-
- for (const auto& data : test_data) {
- EXPECT_EQ(GURL(data.expected),
- client.OverrideFlashEmbedWithHTML(GURL(data.original)));
- }
-}
-#endif
class ChromeContentRendererClientMetricsTest : public testing::Test {
public:
ChromeContentRendererClientMetricsTest() = default;
@@ -680,9 +637,7 @@
EXPECT_EQ(total_count, samples->TotalCount());
}
-#if defined(OS_ANDROID)
-TEST_F(ChromeContentRendererClientMetricsTest,
- RewriteEmbedFailureJSAPIAndroid) {
+TEST_F(ChromeContentRendererClientMetricsTest, RewriteEmbedJSAPI) {
ChromeContentRendererClient client;
std::unique_ptr<base::HistogramSamples> samples = GetHistogramSamples();
@@ -709,30 +664,3 @@
}
}
-#else
-TEST_F(ChromeContentRendererClientMetricsTest,
- RewriteEmbedFailureJSAPIDesktop) {
- ChromeContentRendererClient client;
-
- std::unique_ptr<base::HistogramSamples> samples = GetHistogramSamples();
- auto total_count = 0;
- EXPECT_EQ(total_count, samples->TotalCount());
-
- const std::string test_data[] = {
- // Valid parameter construct, one parameter
- "http://www.youtube.com/v/deadbeef?enablejsapi=1",
- // Invalid parameter construct, one parameter
- "http://www.youtube.com/v/deadbeef&enablejsapi=1",
- // Invalid parameter construct, has multiple parameters
- "http://www.youtube.com/v/deadbeef&start=4&enablejsapi=1?foo=2"};
-
- for (const auto& data : test_data) {
- ++total_count;
- GURL gurl = GURL(data);
- OverrideFlashEmbed(gurl);
- samples = GetHistogramSamples();
- EXPECT_EQ(total_count, samples->GetCount(internal::FAILURE_ENABLEJSAPI));
- EXPECT_EQ(total_count, samples->TotalCount());
- }
-}
-#endif
