Run the DOM Storage layout tests in the UI test framework since the test shel...

Run the DOM Storage layout tests in the UI test framework since the test shell doesn't hit very much of the new code.  I still plan to write unit tests and we're still goign to enable the layout tests in test shell, but this is still a good way for us to test the full stack ASAP.

Workers has used this technique for a while now.  This CL factors out the layout test running code from the workers ui test and moves arounds directories a bit so not everything is in the "workers" directory.

Doing this via a v8 extension is the right way to go long term, but I ran into a couple snags, so I think this is a good first step.

TEST=none
BUG=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=22466

[jorlow, 2009-07-31 18:35:00]

I think this is ready for review.

Darin, you'll notice the lack of v8 extension stuff.  I'm not abandoning that
work, but I am going to put off doing it "the right way" for now since there are
higher priorities and it's taking more time than I expected.

http://codereview.chromium.org/159720/diff/1/4
File chrome/test/data/layout_tests/layout_test_controller.html (right):

http://codereview.chromium.org/159720/diff/1/4#newcode21
Line 21: window.layoutTestController = layoutTestController;
Hmm...is this a no-op?

[jianli, 2009-07-31 19:01:26]

http://codereview.chromium.org/159720/diff/1/4
File chrome/test/data/layout_tests/layout_test_controller.html (right):

http://codereview.chromium.org/159720/diff/1/4#newcode22
Line 22: window.addEventListener('load', layoutTestController.OnLoadEvent,
false);
What do we want to accomplish with this?

http://codereview.chromium.org/159720/diff/1/5
File chrome/test/ui/ui_layout_test.cc (right):

http://codereview.chromium.org/159720/diff/1/5#newcode53
Line 53: printf("Retrying %d ms to delete temp layout test directory.\n",
retry_time);
Over 80-char.

http://codereview.chromium.org/159720/diff/1/5#newcode58
Line 58: const std::wstring& test_case) {
Indentation.

http://codereview.chromium.org/159720/diff/1/5#newcode78
Line 78: // Note that we have to use our own copy of WebKit layout tests because
out.
out. => our

http://codereview.chromium.org/159720/diff/1/6
File chrome/test/ui/ui_layout_test.h (right):

http://codereview.chromium.org/159720/diff/1/6#newcode16
Line 16: void RunTest(const std::wstring& test_dir, const std::wstring&
test_case);
I do not think this is part of the infrastructure to support running layout
tests. Perhaps better to keep it in workers.

[jorlow, 2009-07-31 19:09:05]

Changes made

http://codereview.chromium.org/159720/diff/1/4
File chrome/test/data/layout_tests/layout_test_controller.html (right):

http://codereview.chromium.org/159720/diff/1/4#newcode22
Line 22: window.addEventListener('load', layoutTestController.OnLoadEvent,
false);
On 2009/07/31 19:01:26, jianli wrote:
> What do we want to accomplish with this?

Some tests do not call notifyDone at the end.  This code changes it so that, if
waitUntilDone is not called, then when the page finishes loadeding we assume
it's done.

http://codereview.chromium.org/159720/diff/1/5
File chrome/test/ui/ui_layout_test.cc (right):

http://codereview.chromium.org/159720/diff/1/5#newcode53
Line 53: printf("Retrying %d ms to delete temp layout test directory.\n",
retry_time);
On 2009/07/31 19:01:26, jianli wrote:
> Over 80-char.

Done.

http://codereview.chromium.org/159720/diff/1/5#newcode58
Line 58: const std::wstring& test_case) {
On 2009/07/31 19:01:26, jianli wrote:
> Indentation.

Done.

http://codereview.chromium.org/159720/diff/1/5#newcode78
Line 78: // Note that we have to use our own copy of WebKit layout tests because
out.
On 2009/07/31 19:01:26, jianli wrote:
> out. => our

Done.

http://codereview.chromium.org/159720/diff/1/6
File chrome/test/ui/ui_layout_test.h (right):

http://codereview.chromium.org/159720/diff/1/6#newcode16
Line 16: void RunTest(const std::wstring& test_dir, const std::wstring&
test_case);
On 2009/07/31 19:01:26, jianli wrote:
> I do not think this is part of the infrastructure to support running layout
> tests. Perhaps better to keep it in workers.

I agree it's not technically part of the layout test stuff, but it's layout test
like (if you squint hard enough).

In addition, I think I'm going to be doing some tests like this in the near
future, so I figured it was best to factor out...but I'm happy to move it back
since it technically isn't layout test related.

[jam, 2009-07-31 19:42:53]

lgtm

http://codereview.chromium.org/159720/diff/1007/1013
File chrome/browser/in_process_webkit/dom_storage_uitest.cc (right):

http://codereview.chromium.org/159720/diff/1007/1013#newcode5
Line 5: #include "chrome/common/chrome_switches.h"
nit: no need to do an 'svn mv' on this file, since diffing it with the old
worker_uitest.cc file doesn't help much and makes it a little hard to follow.

http://codereview.chromium.org/159720/diff/1007/1015
File chrome/chrome.gyp (right):

http://codereview.chromium.org/159720/diff/1007/1015#newcode3471
Line 3471: 'test/ui/ui_layout_test.h',
nit: these lists are sorted alphabetically so the cc goes first

[jianli, 2009-07-31 20:01:04]

LGTM.

On Fri, Jul 31, 2009 at 12:09 PM, <jorlow@chromium.org> wrote:

> Changes made
>
>
> http://codereview.chromium.org/159720/diff/1/4
> File chrome/test/data/layout_tests/layout_test_controller.html (right):
>
> http://codereview.chromium.org/159720/diff/1/4#newcode22
> Line 22: window.addEventListener('load',
> layoutTestController.OnLoadEvent, false);
> On 2009/07/31 19:01:26, jianli wrote:
>
>> What do we want to accomplish with this?
>>
>
> Some tests do not call notifyDone at the end.  This code changes it so
> that, if waitUntilDone is not called, then when the page finishes
> loadeding we assume it's done.


Could you please add comment for this?

>
>
> http://codereview.chromium.org/159720/diff/1/5
> File chrome/test/ui/ui_layout_test.cc (right):
>
> http://codereview.chromium.org/159720/diff/1/5#newcode53
> Line 53: printf("Retrying %d ms to delete temp layout test
> directory.\n", retry_time);
> On 2009/07/31 19:01:26, jianli wrote:
>
>> Over 80-char.
>>
>
> Done.
>
> http://codereview.chromium.org/159720/diff/1/5#newcode58
> Line 58: const std::wstring& test_case) {
> On 2009/07/31 19:01:26, jianli wrote:
>
>> Indentation.
>>
>
> Done.
>
> http://codereview.chromium.org/159720/diff/1/5#newcode78
> Line 78: // Note that we have to use our own copy of WebKit layout tests
> because out.
> On 2009/07/31 19:01:26, jianli wrote:
>
>> out. => our
>>
>
> Done.
>
> http://codereview.chromium.org/159720/diff/1/6
> File chrome/test/ui/ui_layout_test.h (right):
>
> http://codereview.chromium.org/159720/diff/1/6#newcode16
> Line 16: void RunTest(const std::wstring& test_dir, const std::wstring&
> test_case);
> On 2009/07/31 19:01:26, jianli wrote:
>
>> I do not think this is part of the infrastructure to support running
>>
> layout
>
>> tests. Perhaps better to keep it in workers.
>>
>
> I agree it's not technically part of the layout test stuff, but it's
> layout test like (if you squint hard enough).
>
> In addition, I think I'm going to be doing some tests like this in the
> near future, so I figured it was best to factor out...but I'm happy to
> move it back since it technically isn't layout test related.
>
> I think it might be better to put it back. If we need it for more
scenarios, we can consider to move it as part of basic UI test.

>
> http://codereview.chromium.org/159720
>

[jorlow, 2009-07-31 20:26:03]

Uploaded new version.  Unfortunately, it seems some of the tests started failing
once I synced my client...will have to investigate.  :-(

http://codereview.chromium.org/159720/diff/1007/1013
File chrome/browser/in_process_webkit/dom_storage_uitest.cc (right):

http://codereview.chromium.org/159720/diff/1007/1013#newcode5
Line 5: #include "chrome/common/chrome_switches.h"
On 2009/07/31 19:42:53, John Abd-El-Malek wrote:
> nit: no need to do an 'svn mv' on this file, since diffing it with the old
> worker_uitest.cc file doesn't help much and makes it a little hard to follow.

Done.

http://codereview.chromium.org/159720/diff/1007/1015
File chrome/chrome.gyp (right):

http://codereview.chromium.org/159720/diff/1007/1015#newcode3471
Line 3471: 'test/ui/ui_layout_test.h',
On 2009/07/31 19:42:53, John Abd-El-Malek wrote:
> nit: these lists are sorted alphabetically so the cc goes first

Done.