Adding Android specific behavior when overriding YouTube Flash embeds.

chrome/renderer/chrome_content_renderer_client_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/renderer/chrome_content_renderer_client.h"
 
 #include <stddef.h>
 
 #include <vector>
 
@@ skipping 393 matching lines @@
   SearchBouncer::GetInstance()->OnSetSearchURLs(
       std::vector<GURL>(), GURL("http://example.com/n"));
   EXPECT_FALSE(client.ShouldSuppressErrorPage(nullptr,
                                               GURL("http://example.com")));
   EXPECT_TRUE(client.ShouldSuppressErrorPage(nullptr,
                                              GURL("http://example.com/n")));
   SearchBouncer::GetInstance()->OnSetSearchURLs(
       std::vector<GURL>(), GURL::EmptyGURL());
 }
 
-TEST_F(ChromeContentRendererClientTest, RewriteYouTubeFlashEmbed) {
+// These are tests that are common for both Android and desktop browsers.
+TEST_F(ChromeContentRendererClientTest, RewriteEmbedCommon) {
   struct TestData {
     std::string original;
     std::string expected;
   } test_data[] = {
       // { original, expected }
       {"youtube.com", ""},
       {"www.youtube.com", ""},
       {"http://www.youtube.com", ""},
       {"https://www.youtube.com", ""},
       {"http://www.foo.youtube.com", ""},
       {"https://www.foo.youtube.com", ""},
       // Non-YouTube domains shouldn't be modified
       {"http://www.plus.google.com", ""},
       // URL isn't using Flash
       {"http://www.youtube.com/embed/deadbeef", ""},
       // URL isn't using Flash, no www
       {"http://youtube.com/embed/deadbeef", ""},
-      // URL isn't using Flash, has JS API enabled
-      {"http://www.youtube.com/embed/deadbeef?enablejsapi=1", ""},
       // URL isn't using Flash, invalid parameter construct
       {"http://www.youtube.com/embed/deadbeef&start=4", ""},
       // URL is using Flash, no www
       {"http://youtube.com/v/deadbeef", "http://youtube.com/embed/deadbeef"},
-      // URL is using Flash, has JS API enabled
-      {"http://www.youtube.com/v/deadbeef?enablejsapi=1", ""},
       // URL is using Flash, is valid, https
       {"https://www.youtube.com/v/deadbeef",
        "https://www.youtube.com/embed/deadbeef"},
       // URL is using Flash, is valid, http
       {"http://www.youtube.com/v/deadbeef",
        "http://www.youtube.com/embed/deadbeef"},
       // URL is using Flash, is valid, not a complete URL, no www or protocol
       {"youtube.com/v/deadbeef", ""},
       // URL is using Flash, is valid, not a complete URL,or protocol
       {"www.youtube.com/v/deadbeef", ""},
@@ skipping 21 matching lines @@
       {"http://www.youtube.com/abcd/v/deadbeef", ""},
       // Invalid parameter construct
       {"http://www.youtube.com/v/abcd/", "http://www.youtube.com/embed/abcd/"},
       // Invalid parameter construct
       {"http://www.youtube.com/v/123/", "http://www.youtube.com/embed/123/"},
       // youtube-nocookie.com
       {"http://www.youtube-nocookie.com/v/123/",
        "http://www.youtube-nocookie.com/embed/123/"},
       // youtube-nocookie.com, isn't using flash
       {"http://www.youtube-nocookie.com/embed/123/", ""},
-      // youtube-nocookie.com, has JS API enabled
-      {"http://www.youtube-nocookie.com/v/123?enablejsapi=1", ""},
       // youtube-nocookie.com, has one parameter
       {"http://www.youtube-nocookie.com/v/123?start=foo",
        "http://www.youtube-nocookie.com/embed/123?start=foo"},
       // youtube-nocookie.com, has multiple parameters
       {"http://www.youtube-nocookie.com/v/123?start=foo&bar=baz",
        "http://www.youtube-nocookie.com/embed/123?start=foo&bar=baz"},
       // youtube-nocookie.com, invalid parameter construct, has one parameter
       {"http://www.youtube-nocookie.com/v/123&start=foo",
        "http://www.youtube-nocookie.com/embed/123?start=foo"},
       // youtube-nocookie.com, invalid parameter construct, has multiple
       // parameters
       {"http://www.youtube-nocookie.com/v/123&start=foo&bar=baz",
        "http://www.youtube-nocookie.com/embed/123?start=foo&bar=baz"},
       // 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
+      {"http://www.youtube.com/v/deadbeef?enablejsapi=1",
+       "http://www.youtube.com/embed/deadbeef?enablejsapi=1"},
+      // youtube-nocookie.com, has JS API enabled
+      {"http://www.youtube-nocookie.com/v/123?enablejsapi=1",
+       "http://www.youtube-nocookie.com/embed/123?enablejsapi=1"},
+      // URL is using Flash, has JS API enabled, invalid parameter construct
+      {"http://www.youtube.com/v/deadbeef&enablejsapi=1",
+       "http://www.youtube.com/embed/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",
+       "http://www.youtube.com/embed/deadbeef?start=4&enablejsapi=1"},
+  };
+
+  ChromeContentRendererClient client;
+
+  for (const auto& data : test_data)
+    EXPECT_EQ(GURL(data.expected),
+              client.OverrideFlashEmbedWithHTML(GURL(data.original)));

[mlamouri (slow - plz ping), 2016/08/09 11:12:38]

style: this requires { }

[kdsilva, 2016/08/09 12:43:55]

Done.

+}
+

[mlamouri (slow - plz ping), 2016/08/09 11:12:39]

nit: you could remove this empty line.

[kdsilva, 2016/08/09 12:43:55]

Done.

+#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)));

[mlamouri (slow - plz ping), 2016/08/09 11:12:38]

style: this requires { }

[kdsilva, 2016/08/09 12:43:55]

Done.

+}
+#endif
+
 class ChromeContentRendererClientMetricsTest : public testing::Test {
  public:
   ChromeContentRendererClientMetricsTest() = default;
 
   std::unique_ptr<base::HistogramSamples> GetHistogramSamples() {
     return histogram_tester_.GetHistogramSamplesSinceCreation(
         internal::kFlashYouTubeRewriteUMA);
   }
 
   void OverrideFlashEmbed(const GURL& gurl) {
@@ skipping 96 matching lines @@
   }
 
   // Invalid parameter construct, not flash
   GURL gurl = GURL("http://www.youtube.com/embed/deadbeef&start=4");
   OverrideFlashEmbed(gurl);
   samples = GetHistogramSamples();
   EXPECT_EQ(total_count, samples->GetCount(internal::SUCCESS_PARAMS_REWRITE));
   EXPECT_EQ(total_count, samples->TotalCount());
 }
 
-TEST_F(ChromeContentRendererClientMetricsTest, RewriteEmbedFailureJSAPI) {
+#if defined(OS_ANDROID)
+TEST_F(ChromeContentRendererClientMetricsTest,
+       RewriteEmbedFailureJSAPIAndroid) {
   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",
+      // Valid parameter construct, has multiple parameters
+      "http://www.youtube.com/v/deadbeef?enablejsapi=1&foo=2",
+      // 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&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::SUCCESS_ENABLEJSAPI));
+    EXPECT_EQ(total_count, samples->TotalCount());
+  }
+}
+

[mlamouri (slow - plz ping), 2016/08/09 11:12:38]

ditto

[kdsilva, 2016/08/09 12:43:55]

Done.

+#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());
   }
 }
+

[mlamouri (slow - plz ping), 2016/08/09 11:12:38]

nit: remove this empty line

[kdsilva, 2016/08/09 12:43:55]

Done.

+#endif