Canvas Capture: chrome browsertest- added frame rate test

chrome/browser/media/chrome_webrtc_canvas_capture_browsertest.cc

Index: chrome/browser/media/chrome_webrtc_canvas_capture_browsertest.cc
diff --git a/chrome/browser/media/chrome_webrtc_canvas_capture_browsertest.cc b/chrome/browser/media/chrome_webrtc_canvas_capture_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e6ba4efce0bb2cb249cee9f92b4acb472b1d186f
--- /dev/null
+++ b/chrome/browser/media/chrome_webrtc_canvas_capture_browsertest.cc
@@ -0,0 +1,79 @@
+// Copyright 2016 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 "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/path_service.h"
+#include "build/build_config.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_tabstrip.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/render_process_host.h"

[mcasas, 2016/03/04 22:30:33]

I'm not sure all these includes are needed. There's 
no reference to RenderProcessHost for example.
Perhaps taken from another test?

[cpaulin (no longer in chrome), 2016/03/10 22:21:24]

I pruned the not needed includes. Done.

+#include "content/public/common/content_switches.h"
+#include "content/public/test/browser_test_utils.h"
+#include "media/base/media_switches.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "testing/perf/perf_test.h"
+#include "ui/gl/gl_switches.h"
+
+static const char kCanvasTestHtmlPage[] = "/media/canvas_capture_test.html";
+
+#if defined(OS_MACOSX)
+// TODO(cpaulin): when http://crbug.com/591529 is fixed on MAC OS, enable this
+// test.
+#define MAYBE_WebRtcCanvasCaptureBrowserTest \
+    DISABLED_WebRtcCanvasCaptureBrowserTest
+#else
+#define MAYBE_WebRtcCanvasCaptureBrowserTest WebRtcCanvasCaptureBrowserTest
+#endif
+
+// Canvas Capture browser test.
+// At this time this browser test verifies that the frame rate of a stream

[mcasas, 2016/03/04 22:30:33]

I'm surprised to read "At this time". Maybe you could 
rephrase as 

// Verifies the frame of a Media Stream created out of a <canvas>.
// TODO(cpaulin): .Extend this test to...

[cpaulin (no longer in chrome), 2016/03/10 22:21:24]

That's fine, this was indeed not necessary info. Done

+// originated from a captured canvas is as expected.
+class MAYBE_WebRtcCanvasCaptureBrowserTest : public InProcessBrowserTest {

[mcasas, 2016/03/04 22:30:33]

The name of the test does not match the file name.
This is also true of other files in this folder, so I 
made a CL to clean that up: http://crrev.com/1762413002

[cpaulin (no longer in chrome), 2016/03/10 22:21:24]

OK, I follow your example CL now. Done

+ public:
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    // The video playback will not work without a GPU, so force its use here.
+    command_line->AppendSwitch(switches::kUseGpuInTests);

[mcasas, 2016/03/04 22:30:33]

Someone else says: 
// This test enables switches::kUseGpuInTests which causes false positives
// with MemorySanitizer.

in
https://code.google.com/p/chromium/codesearch#search/&sq=package:chromium&typ...
which could be the issue.

[cpaulin (no longer in chrome), 2016/03/10 22:21:24]

Thanks for the heads up.
Done.

+
+    command_line->AppendSwitch(
+        switches::kEnableExperimentalWebPlatformFeatures);
+  }
+
+ protected:
+  MAYBE_WebRtcCanvasCaptureBrowserTest() {}
+  ~MAYBE_WebRtcCanvasCaptureBrowserTest() override {};
+
+  std::string ExecuteJavascript(
+      const std::string& javascript,
+      content::WebContents* tab_contents) const {
+    std::string result;
+    EXPECT_TRUE(content::ExecuteScriptAndExtractString(
+        tab_contents, javascript, &result));
+    return result;
+  }

[mcasas, 2016/03/04 22:30:33]

I was thinking this looks strangely indented and rightly so:

  std::string ExecuteJavascript(const std::string& javascript,
                                content::WebContents* tab_contents) const {
    std::string result;
    EXPECT_TRUE(content::ExecuteScriptAndExtractString(tab_contents, javascript,
                                                       &result));
    return result;
  }


is correct.

I strongly suggest you run
git cl format 
before uploading your patch.

[cpaulin (no longer in chrome), 2016/03/10 22:21:24]

Done.

+
+ private:
+   DISALLOW_COPY_AND_ASSIGN(MAYBE_WebRtcCanvasCaptureBrowserTest);

[mcasas, 2016/03/04 22:30:33]

nit: this is one space too far right.

[cpaulin (no longer in chrome), 2016/03/10 22:21:24]

Done.

+};
+
+// Tests that the frame rate of the canvas capture is as expected.
+IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcCanvasCaptureBrowserTest,
+                       VerifyCanvasCaptureFrameRate) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  ui_test_utils::NavigateToURL(
+      browser(), embedded_test_server()->GetURL(kCanvasTestHtmlPage));
+
+  content::WebContents* tab_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  ASSERT_EQ("OK", ExecuteJavascript("testFrameRateOfCanvasCapture();",
+                                    tab_contents));
+}