Skip to content

GitLab

T
tangled
Commit a92266e9 authored by Mounir Lamouri's avatar Mounir Lamouri Committed by Commit Bot
Browse files
Options

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/657277Reviewed-by: default avatarJochen Eisinger <jochen@chromium.org>
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501587}
parent 4560fda0
Hide whitespace changes
Inline Side-by-side
Showing with 20 additions and 123 deletions
chrome/renderer/chrome_content_renderer_client.cc
View file @ a92266e9
......@@ -1638,19 +1638,12 @@ GURL ChromeContentRendererClient::OverrideFlashEmbedWithHTML(const GURL& url) {
}
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();
......
chrome/renderer/chrome_content_renderer_client_browsertest.cc
View file @ a92266e9
......@@ -132,42 +132,18 @@ struct FlashEmbedsTestData {
};
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
......
chrome/renderer/chrome_content_renderer_client_unittest.cc
View file @ a92266e9
......@@ -418,8 +418,7 @@ TEST_F(ChromeContentRendererClientTest,
[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 @@ TEST_F(ChromeContentRendererClientTest, RewriteEmbedCommon) {
// 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 @@ TEST_F(ChromeContentRendererClientTest, RewriteEmbedAndroid) {
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 @@ TEST_F(ChromeContentRendererClientMetricsTest, RewriteEmbedSuccessRewrite) {
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 @@ TEST_F(ChromeContentRendererClientMetricsTest,
}
}
#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
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment