Add tests for the video player in Files.app.

Add tests for the video player in Files.app.

This patch add basic tests for the video player. It opens an OGV video in both: drive and downloads and checks if the video is loaded properly and the window size is set to the video size.

TEST=browser_tests
BUG=244146
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202829

[mtomasz, 2013-05-28 05:28:36]

@hirono: PTAL at JS.
@hashimoto: PTAL at C++ (and JS).

This CL depends on: https://codereview.chromium.org/16092002/

Thanks.

[hashimoto, 2013-05-28 07:54:12]

https://codereview.chromium.org/15984003/diff/2001/chrome/browser/chromeos/ex...
File chrome/browser/chromeos/extensions/file_manager/file_manager_browsertest.cc
(right):

https://codereview.chromium.org/15984003/diff/2001/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/file_manager/file_manager_browsertest.cc:658:
StartTest("audioOpen" + volume->GetName());
Can't these method inlined in the test bodies?
If we really want to eliminate duplication of "ResultCatcher cathcer;" and
"ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();", these lines should
be included in StartTest() instead. (currently this is not possible because of
DoTestFileDisplay(), though.)
Also, "audioOpenDrive" is more reader-friendly than "audioOpen" +
volume->GetName().

[hirono, 2013-05-28 07:59:46]

https://codereview.chromium.org/15984003/diff/2001/chrome/test/data/extension...
File chrome/test/data/extensions/api_test/file_manager_browsertest/test_cases.js
(right):

https://codereview.chromium.org/15984003/diff/2001/chrome/test/data/extension...
chrome/test/data/extensions/api_test/file_manager_browsertest/test_cases.js:327:
function(appId) {
Please use 'inAppId'.

https://codereview.chromium.org/15984003/diff/2001/chrome/test/data/extension...
chrome/test/data/extensions/api_test/file_manager_browsertest/test_cases.js:345:
chrome.test.succeed);
It seems that chrome.test.succeed just records success log.
Even if it is not called, the test will succeed.
We can use chrome.test.callbackPass to ensure the callback function is called.

[mtomasz, 2013-05-28 10:26:37]

https://codereview.chromium.org/15984003/diff/2001/chrome/browser/chromeos/ex...
File chrome/browser/chromeos/extensions/file_manager/file_manager_browsertest.cc
(right):

https://codereview.chromium.org/15984003/diff/2001/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/file_manager/file_manager_browsertest.cc:658:
StartTest("audioOpen" + volume->GetName());
On 2013/05/28 07:54:12, hashimoto wrote:
> Can't these method inlined in the test bodies?
> If we really want to eliminate duplication of "ResultCatcher cathcer;" and
> "ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();", these lines
should
> be included in StartTest() instead. (currently this is not possible because of
> DoTestFileDisplay(), though.)
> Also, "audioOpenDrive" is more reader-friendly than "audioOpen" +
> volume->GetName().

Sure, they can. I were this way for consistency. Fixed, done.

https://codereview.chromium.org/15984003/diff/2001/chrome/test/data/extension...
File chrome/test/data/extensions/api_test/file_manager_browsertest/test_cases.js
(right):

https://codereview.chromium.org/15984003/diff/2001/chrome/test/data/extension...
chrome/test/data/extensions/api_test/file_manager_browsertest/test_cases.js:327:
function(appId) {
On 2013/05/28 07:59:46, hirono wrote:
> Please use 'inAppId'.

Done.

https://codereview.chromium.org/15984003/diff/2001/chrome/test/data/extension...
chrome/test/data/extensions/api_test/file_manager_browsertest/test_cases.js:345:
chrome.test.succeed);
On 2013/05/28 07:59:46, hirono wrote:
> It seems that chrome.test.succeed just records success log.
> Even if it is not called, the test will succeed.
> We can use chrome.test.callbackPass to ensure the callback function is called.

I think chrome.test.succeed marks the test case as successful. However, the
default response is successful. Moreover, callbackPass implementation calls
chrome.test.succeed when there are no other callbacks pending, on closure end.

I think we should get rid of chrome.test.succeed in the future, since it seem to
be a no-op here. I may be wrong though. I have to double check it.

I've fixed here. Done.

[hashimoto, 2013-05-28 10:28:24]

file_manager_browsertest.cc lgtm

[hirono, 2013-05-28 10:56:35]

On 2013/05/28 10:28:24, hashimoto wrote:
> file_manager_browsertest.cc lgtm

lgtm for js files.
Thank you!

[commit-bot: I haz the power, 2013-05-29 03:21:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mtomasz@chromium.org/15984003/20001

[commit-bot: I haz the power, 2013-05-29 04:39:48]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-05-29 04:43:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mtomasz@chromium.org/15984003/20001

[commit-bot: I haz the power, 2013-05-29 08:56:45]

Change committed as 202829