Rotation into Video Layer + Content Transform

content/browser/media/media_browsertest.cc

Index: content/browser/media/media_browsertest.cc
diff --git a/content/browser/media/media_browsertest.cc b/content/browser/media/media_browsertest.cc
index 0ca34d11cb899073242e534eb9a5d19f7daecea7..058a16a52fc9a03fcc926c4ad480b71165e54ff6 100644
--- a/content/browser/media/media_browsertest.cc
+++ b/content/browser/media/media_browsertest.cc
@@ -54,6 +54,7 @@ std::string MediaBrowserTest::RunTest(const GURL& gurl,
   TitleWatcher title_watcher(shell()->web_contents(),
                              base::ASCIIToUTF16(expected_title));
   AddWaitForTitles(&title_watcher);
+  title_watcher.AlsoWaitForTitle(base::ASCIIToUTF16(expected_title));

[scherkus (not reviewing), 2014/08/06 23:06:42]

we already pass this into the TitleWatcher constructor -- is this needed?

[suderman, 2014/08/07 00:31:31]

The title when the video terminates must match the expected title.
In order to terminate for the dimensions title it needs to add the
title to the title_watcher, however as the title varies on the size
of the video it must explicitly have the expected title added to
the title watcher.

[scherkus (not reviewing), 2014/08/07 00:38:48]

I get that part, but on line 56 we already add |expected_title| via the
TitleWatcher() constructor, which pushes it into its vector:
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/tes...

I'm confused why we need to add the same string to TitleWatcher's vector twice

[suderman, 2014/08/07 00:49:36]

Ah, okay. I wasn't aware that the title_watcher automatically added the expected
title. I believe I saw the title timing out and thought it was due to not
watching for the expected title. Removed.

   NavigateToURL(shell(), gurl);
   base::string16 result = title_watcher.WaitAndGetTitle();
   return base::UTF16ToASCII(result);
@@ -97,6 +98,12 @@ class MediaTest : public testing::WithParamInterface<bool>,
     query_params.push_back(std::make_pair(tag, media_file));
     RunMediaTestPage("player.html", query_params, kEnded, http);
   }
+
+  void PlayVideoSize(const char* media_file, bool http, const char* expected) {

[scherkus (not reviewing), 2014/08/06 23:06:42]

nit: PlayVideoSize isn't really the best name

how about something along the lines of RunVideoSizeTest()

also it's a minor detail, but instead of requiring callers to know the format of
the string, it'd be preferable to have them pass in the sizes as int and have
this helper method construct the string seeing as this method is the one picking
the html page to run

[suderman, 2014/08/07 00:31:31]

Done.

+    media::QueryParams query_params;
+    query_params.push_back(std::make_pair("video", media_file));
+    RunMediaTestPage("player.html", query_params, expected, http);
+  }
 };
 
 IN_PROC_BROWSER_TEST_P(MediaTest, VideoBearTheora) {
@@ -142,6 +149,22 @@ IN_PROC_BROWSER_TEST_P(MediaTest, VideoBearMovPcmS16be) {
 IN_PROC_BROWSER_TEST_P(MediaTest, VideoBearMovPcmS24be) {
   PlayVideo("bear_pcm_s24be.mov", GetParam());
 }
+
+IN_PROC_BROWSER_TEST_P(MediaTest, VideoBearRotated0) {
+  PlayVideoSize("bear_rotate_0.mp4", GetParam(), "1280 720");
+}
+
+IN_PROC_BROWSER_TEST_P(MediaTest, VideoBearRotated90) {
+  PlayVideoSize("bear_rotate_90.mp4", GetParam(), "720 1280");
+}
+
+IN_PROC_BROWSER_TEST_P(MediaTest, VideoBearRotated180) {
+  PlayVideoSize("bear_rotate_180.mp4", GetParam(), "1280 720");
+}
+
+IN_PROC_BROWSER_TEST_P(MediaTest, VideoBearRotated270) {
+  PlayVideoSize("bear_rotate_270.mp4", GetParam(), "720 1280");
+}
 #endif  // defined(USE_PROPRIETARY_CODECS)
 
 #if defined(OS_CHROMEOS)