Rewrite YouTube Flash embeds

chrome/renderer/chrome_content_renderer_client_browsertest.cc

Index: chrome/renderer/chrome_content_renderer_client_browsertest.cc
diff --git a/chrome/renderer/chrome_content_renderer_client_browsertest.cc b/chrome/renderer/chrome_content_renderer_client_browsertest.cc
index 88d9e2b528f3a79be32c9f16adba1b16b928477b..9579c222dae80bc1eed7c71cf7a236986b52d2b5 100644
--- a/chrome/renderer/chrome_content_renderer_client_browsertest.cc
+++ b/chrome/renderer/chrome_content_renderer_client_browsertest.cc
@@ -10,18 +10,27 @@
 #include "base/command_line.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/renderer/chrome_content_renderer_client.h"
 #include "chrome/test/base/chrome_render_view_test.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_view.h"
+#include "content/public/test/browser_test_utils.h"
 #include "content/public/test/mock_render_thread.h"
+#include "content/public/test/test_utils.h"
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_sender.h"
 #include "ipc/ipc_test_sink.h"
+#include "net/dns/mock_host_resolver.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
 #include "third_party/WebKit/public/web/WebPluginParams.h"
@@ -33,6 +42,9 @@ using testing::SetArgPointee;
 
 typedef ChromeRenderViewTest InstantProcessNavigationTest;
 
+const base::FilePath::CharType kDocRoot[] =
+    FILE_PATH_LITERAL("chrome/test/data");
+
 // Tests that renderer-initiated navigations from an Instant render process get
 // bounced back to the browser to be rebucketed into a non-Instant renderer if
 // necessary.
@@ -70,3 +82,103 @@ TEST_F(InstantProcessNavigationTest, ForkForNavigationsToSearchURLs) {
       GetMainFrame(), GURL("http://example.com/"), "GET", false, false,
       &unused));
 }
+
+class ChromeContentRendererClientBrowserTest: public InProcessBrowserTest {
+  public:
+    std::unique_ptr<net::test_server::HttpResponse> HandleRequest(
+        const net::test_server::HttpRequest& request) {
+      // We're only interested in YouTube video embeds
+      if (request.relative_url.find("embed") != std::string::npos) {
+        EXPECT_EQ(request.relative_url, expected_url_);
+        content::BrowserThread::PostTask(
+            content::BrowserThread::UI,
+            FROM_HERE,
+            base::Bind(&ChromeContentRendererClientBrowserTest::QuitMessageRunner,
+              base::Unretained(this)));
+      }
+
+      std::unique_ptr<net::test_server::BasicHttpResponse> http_response(
+          new net::test_server::BasicHttpResponse());
+      http_response->set_code(net::HTTP_OK);
+      return std::move(http_response);
+    }
+
+    void QuitMessageRunner() {
+      message_runner_->Quit();
+    }
+
+    void RunMessageRunner() {

[mlamouri (slow - plz ping), 2016/07/28 13:05:13]

nit: maybe rename "WaitForYouTubeRequest()"? It will make the test easier to
read.

[kdsilva, 2016/07/28 15:17:17]

Done.

+      message_runner_ = new content::MessageLoopRunner();
+      message_runner_->Run();
+    }
+
+    void SetExpectedURL(std::string given) {

[mlamouri (slow - plz ping), 2016/07/28 13:05:12]

style: set_expected_url()

[kdsilva, 2016/07/28 15:17:17]

Done.

+      expected_url_ = given;
+    }
+
+  private:
+    std::string expected_url_;
+    scoped_refptr<content::MessageLoopRunner> message_runner_;
+};
+
+// These tests examine Youtube requests that use the Flash API and ensure that
+// the requests have been modified to instead use HTML5.
+IN_PROC_BROWSER_TEST_F(ChromeContentRendererClientBrowserTest, RewriteYouTubeFlashEmbedTest) {

[mlamouri (slow - plz ping), 2016/07/28 13:05:13]

No need to suffix with Test.

[kdsilva, 2016/07/28 15:17:17]

Done.

+  net::EmbeddedTestServer https_server(net::EmbeddedTestServer::TYPE_HTTP);

[mlamouri (slow - plz ping), 2016/07/28 13:05:12]

Can you simply use embedded_test_server() ?

[kdsilva, 2016/07/28 15:17:17]

Done.

+  ASSERT_TRUE(https_server.Start());
+
+  host_resolver()->AddRule("*", "127.0.0.1");
+
+  https_server.ServeFilesFromSourceDirectory(base::FilePath(kDocRoot));
+  https_server.RegisterRequestHandler(base::Bind(

[mlamouri (slow - plz ping), 2016/07/28 13:05:12]

Can you try to use the monitor function? We don't need to handle any request
here so if monitor works, it would be better.

[kdsilva, 2016/07/28 15:17:17]

Done.

+      &ChromeContentRendererClientBrowserTest::HandleRequest, this));
+
+  GURL url(https_server.GetURL("/flash_embeds.html"));
+
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  content::WebContents* web_contents = browser()->tab_strip_model()->GetActiveWebContents();
+  std::string port = std::to_string(https_server.port());
+
+  // Valid URL, no parameters
+  std::string video_url = "http://www.youtube.com:" + port + "/v/cW44BpXpjYw";
+  SetExpectedURL("/embed/cW44BpXpjYw");
+  EXPECT_TRUE(ExecuteScript(web_contents, "appendToDOM('" + video_url + "');"));
+  RunMessageRunner();
+
+  // Valid URL, no parameters, subdomain
+  video_url = "http://www.foo.youtube.com:" + port + "/v/cW44BpXpjYw";
+  SetExpectedURL("/embed/cW44BpXpjYw");
+  EXPECT_TRUE(ExecuteScript(web_contents, "appendToDOM('" + video_url + "');"));
+  RunMessageRunner();
+
+  // Valid URL, one parameter
+  video_url = "http://www.youtube.com:" + port + "/v/cW44BpXpjYw?start=4";
+  SetExpectedURL("/embed/cW44BpXpjYw?start=4");
+  EXPECT_TRUE(ExecuteScript(web_contents, "appendToDOM('" + video_url + "');"));
+  RunMessageRunner();
+
+  // Valid URL, many parameters
+  video_url = "http://www.youtube.com:" + port + "/v/cW44BpXpjYw?start=4&fs=1";
+  SetExpectedURL("/embed/cW44BpXpjYw?start=4&fs=1");
+  EXPECT_TRUE(ExecuteScript(web_contents, "appendToDOM('" + video_url + "');"));
+  RunMessageRunner();
+
+  // Invalid URL, one parameter
+  video_url = "http://www.youtube.com:" + port + "/v/cW44BpXpjYw&foo=4";
+  SetExpectedURL("/embed/cW44BpXpjYw?foo=4");
+  EXPECT_TRUE(ExecuteScript(web_contents, "appendToDOM('" + video_url + "');"));
+  RunMessageRunner();
+
+  // Invalid URL, many parameters
+  video_url = "http://www.youtube.com:" + port + "/v/cW44BpXpjYw&bar=4&foo=6";
+  SetExpectedURL("/embed/cW44BpXpjYw?bar=4&foo=6");
+  EXPECT_TRUE(ExecuteScript(web_contents, "appendToDOM('" + video_url + "');"));
+  RunMessageRunner();
+
+  // Invalid URL, many parameters

[mlamouri (slow - plz ping), 2016/07/28 13:05:12]

I think you have too many tests here. Browsertests take time to run, we should
just make sure that the basics are working. The details and edge cases should be
left to the unit tests.

I would suggest to test:
- normal case
- subdomain
- multiple parameters
- multiple parameters with ? in the wrong place
- enablejsapi=1

[kdsilva, 2016/07/28 15:17:17]

Done.

+  video_url = "http://www.youtube.com:" + port + "/v/cW44BpXpjYw&bar=4&foo=6?baz=1";
+  SetExpectedURL("/embed/cW44BpXpjYw?bar=4&foo=6&baz=1");
+  EXPECT_TRUE(ExecuteScript(web_contents, "appendToDOM('" + video_url + "');"));
+  RunMessageRunner();
+}