drive: Make FileManagerBrowserTest more robust

drive: Make FileManagerBrowserTest more robust

Previously, getFileList() was called directly from testFileDisplay().
This was bad, as it's not guaranteed that the file list is already rendered
by the time testFileDisplay() is called, hence the test was flaky.
We should use waitForFileListChange() instead.

Likewise, doKeyboardTest() and doKeyboardTestWithConfirmation() are fixed
in the same way.

BUG=224534
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=192550

[mtomasz, 2013-04-05 07:59:44]

https://codereview.chromium.org/13450004/diff/1/chrome/test/data/extensions/a...
File chrome/test/data/extensions/api_test/file_manager_browsertest/background.js
(left):

https://codereview.chromium.org/13450004/diff/1/chrome/test/data/extensions/a...
chrome/test/data/extensions/api_test/file_manager_browsertest/background.js:117:
function doKeyboardTest(code) {
I think this can also be flaky.

It can pass if copying doesn't work. A change in file list is interpreted as a
successful copy, while it could be just the file list initialized.

Shall we wrap it with waitForFileListChange(0, ...) to wait util the file list
is rendered?

[mtomasz, 2013-04-05 08:10:49]

I'm ok if this is addressed in a patch. Let me know.

https://codereview.chromium.org/13450004/diff/1/chrome/test/data/extensions/a...
File chrome/test/data/extensions/api_test/file_manager_browsertest/background.js
(left):

https://codereview.chromium.org/13450004/diff/1/chrome/test/data/extensions/a...
chrome/test/data/extensions/api_test/file_manager_browsertest/background.js:117:
function doKeyboardTest(code) {
On 2013/04/05 07:59:44, mtomasz wrote:
> I think this can also be flaky.
> 
> It can pass if copying doesn't work. A change in file list is interpreted as a
> successful copy, while it could be just the file list initialized.
> 
> Shall we wrap it with waitForFileListChange(0, ...) to wait util the file list
> is rendered?

Actually it can even pass if the list is not yet initialized!

[mtomasz, 2013-04-05 08:11:15]

On 2013/04/05 08:10:49, mtomasz wrote:
> I'm ok if this is addressed in a patch. Let me know.
> 
>
https://codereview.chromium.org/13450004/diff/1/chrome/test/data/extensions/a...
> File
chrome/test/data/extensions/api_test/file_manager_browsertest/background.js
> (left):
> 
>
https://codereview.chromium.org/13450004/diff/1/chrome/test/data/extensions/a...
>
chrome/test/data/extensions/api_test/file_manager_browsertest/background.js:117:
> function doKeyboardTest(code) {
> On 2013/04/05 07:59:44, mtomasz wrote:
> > I think this can also be flaky.
> > 
> > It can pass if copying doesn't work. A change in file list is interpreted as
a
> > successful copy, while it could be just the file list initialized.
> > 
> > Shall we wrap it with waitForFileListChange(0, ...) to wait util the file
list
> > is rendered?
> 
> Actually it can even pass if the list is not yet initialized!

s/in a patch/in a separate patch/

[satorux1, 2013-04-05 08:15:11]

https://codereview.chromium.org/13450004/diff/1/chrome/test/data/extensions/a...
File chrome/test/data/extensions/api_test/file_manager_browsertest/background.js
(left):

https://codereview.chromium.org/13450004/diff/1/chrome/test/data/extensions/a...
chrome/test/data/extensions/api_test/file_manager_browsertest/background.js:103:
chrome.tabs.executeScript(null, {file: 'fake_keypress.js'}, function(result) {
fixed here too

https://codereview.chromium.org/13450004/diff/1/chrome/test/data/extensions/a...
chrome/test/data/extensions/api_test/file_manager_browsertest/background.js:117:
function doKeyboardTest(code) {
On 2013/04/05 07:59:44, mtomasz wrote:
> I think this can also be flaky.
> 
> It can pass if copying doesn't work. A change in file list is interpreted as a
> successful copy, while it could be just the file list initialized.
> 
> Shall we wrap it with waitForFileListChange(0, ...) to wait util the file list
> is rendered?

Good idea! Done.

[mtomasz, 2013-04-05 08:17:12]

lgtm, thanks!

[commit-bot: I haz the power, 2013-04-05 09:52:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/satorux@chromium.org/13450004/3

[commit-bot: I haz the power, 2013-04-05 13:05:05]

Change committed as 192550