Port aria_util_test.js to run as a WebUI test.

Port aria_util_test.js to run as a WebUI test.

This takes one existing ChromeVox unit test and ports it to
run as a WebUI test in Chrome. It's a WebUI test because it
needs a full DOM to run in, even though it's otherwise most
similar to a unit test.

Only one change to the WebUI test system was needed - to
allow paths relative to the source root for library files for a
test. The other changes improve the error messages when
building, and add filenames to stack traces.

BUG=371692
NOTRY=true

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

[dmazzoni, 2014-05-22 06:42:05]

Ready for an initial look.

[Peter Lundblad, 2014-05-22 15:24:25]

lgtm

Very cool to see the first test cross the border!

A bunch of suggestions below, some of which can wait to a follow up
cl if you prefer.

As an improvement, I think we could just remove copying the test file
in gypv8sh.py and pull it from the source directory as well.  I don't see any
reason for it to be copied, but maybe I am missing something.  Anyhow, a
suggestion
for later.

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
File chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js
(right):

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:15: /**
nit: indentation.

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:19:
browsePreload: 'chrome://settings-frame',
Looks like a temporary hack to me.  Maybe make a base class for all our
similar tests where we put this so we can clean it up easily
when we fix the framework?

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:19:
browsePreload: 'chrome://settings-frame',
There seems to be a DUMMY_URL constant for this purpose.  Doesn't that work
here?

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:21: /**
nit: Indent.

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:30:
loadDoc: function(commentEncodedHtml) {
Looks like a candidate to moved into a base class.

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:45:
'../host/interface/abstract_earcons.js']
Given our dependency situation, I am wondering if we should make some
effeort to automate this.  It works here, but it could be cumbersome to
list all dependencies for other tests.
We could for example make the C++ code pick up goog.requires
automatically.  I can take a peak at that if you like.

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:55: */});
Neat trick!  Hadn't seen it before;)

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:56:
assertEquals(
I think assertThat(..., eqJSON) is a nice and compact way of checking that
two objects are equal.  Should we make that the idiomatic way of doing this
for both arrays and objects?

https://codereview.chromium.org/292313004/diff/40001/tools/gypv8sh.py
File tools/gypv8sh.py (right):

https://codereview.chromium.org/292313004/diff/40001/tools/gypv8sh.py#newcode49
tools/gypv8sh.py:49: print open(cxxoutfile).read()
Awesome!

[dmazzoni, 2014-05-22 16:27:32]

+phajdan.jr for chrome/test/base
+sky for chrome/renderer

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
File chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js
(right):

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:15: /**
On 2014/05/22 15:24:25, Peter Lundblad wrote:
> nit: indentation.

Done.

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:19:
browsePreload: 'chrome://settings-frame',
On 2014/05/22 15:24:25, Peter Lundblad wrote:
> There seems to be a DUMMY_URL constant for this purpose.  Doesn't that work
> here?

Yes! That works, thanks.

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:19:
browsePreload: 'chrome://settings-frame',
On 2014/05/22 15:24:25, Peter Lundblad wrote:
> Looks like a temporary hack to me.  Maybe make a base class for all our
> similar tests where we put this so we can clean it up easily
> when we fix the framework?

Moved to a base class!

I don't think there's an easy path for replacing the framework, we have to go
all-in with WebUI or write our own. Too much of the WebUI system really does
depend on WebUI.

Luckily I think this should work.

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:21: /**
On 2014/05/22 15:24:25, Peter Lundblad wrote:
> nit: Indent.

Done.

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:30:
loadDoc: function(commentEncodedHtml) {
On 2014/05/22 15:24:25, Peter Lundblad wrote:
> Looks like a candidate to moved into a base class.

Done.

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:45:
'../host/interface/abstract_earcons.js']
On 2014/05/22 15:24:25, Peter Lundblad wrote:
> Given our dependency situation, I am wondering if we should make some
> effeort to automate this.  It works here, but it could be cumbersome to
> list all dependencies for other tests.
> We could for example make the C++ code pick up goog.requires
> automatically.  I can take a peak at that if you like.

That would be cool if you'd like to look into that!

https://codereview.chromium.org/292313004/diff/40001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/aria_util_test.js:56:
assertEquals(
On 2014/05/22 15:24:25, Peter Lundblad wrote:
> I think assertThat(..., eqJSON) is a nice and compact way of checking that
> two objects are equal.  Should we make that the idiomatic way of doing this
> for both arrays and objects?

Agreed, that works great, thanks.

[sky, 2014-05-22 19:27:50]

LGTM

[Paweł Hajdan Jr., 2014-05-23 10:50:10]

LGTM with a nit

https://codereview.chromium.org/292313004/diff/80001/chrome/test/base/web_ui_...
File chrome/test/base/web_ui_browsertest.h (right):

https://codereview.chromium.org/292313004/diff/80001/chrome/test/base/web_ui_...
chrome/test/base/web_ui_browsertest.h:143: // User added libraries.
nit: Member variables need to go after _all_ the methods.

[dmazzoni, 2014-05-23 15:38:40]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2014-05-23 15:39:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/292313004/120001

[commit-bot: I haz the power, 2014-05-23 21:35:11]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-24 01:11:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-24 01:11:23]

Try jobs failed on following builders:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[dmazzoni, 2014-05-28 21:26:35]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2014-05-28 21:27:28]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/292313004/140001

[commit-bot: I haz the power, 2014-05-29 01:19:43]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-05-29 03:10:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-29 03:10:37]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[Peter Lundblad, 2014-05-29 04:13:46]

The CQ bit was checked by plundblad@chromium.org

[dmazzoni, 2014-05-29 04:14:29]

The CQ bit was unchecked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2014-05-29 04:14:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/292313004/140001

[dmazzoni, 2014-05-29 04:14:43]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2014-05-29 04:16:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/292313004/140001

[commit-bot: I haz the power, 2014-05-29 20:31:47]

Change committed as 273577