Adding app_view tests to app_shell_browsertests.

Adding app_view tests to app_shell_browsertests.

This CL adds the following tests to app_shell_browsertests:
AppViewTest.TestAppViewGoodDataShouldSucceed
AppViewTest.TestAppViewRefusedDataShouldFail
AppViewTest.TestAppViewWithUndefinedDataShouldSucceed

BUG=352293
Committed: https://crrev.com/45e87e9d03f09ffdcbd0d2477ba703380c324ed8
Cr-Commit-Position: refs/heads/master@{#302198}

[lfg, 2014-10-24 17:50:01]

lfg@chromium.org changed reviewers:
+ lazyboy@chromium.org

[lfg, 2014-10-24 17:50:01]

PTAL.

[lazyboy, 2014-10-24 20:13:25]

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/browser/gu...
File extensions/browser/guest_view/app_view/app_view_apitest.cc (right):

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/browser/gu...
extensions/browser/guest_view/app_view/app_view_apitest.cc:48: const Extension*
extension = extension_system_->LoadApp(test_data_dir);
nit: return extension_system_->LoadApp ..

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/browser/gu...
extensions/browser/guest_view/app_view/app_view_apitest.cc:95: // Tests that
<appview> correctly processes parameters passed on connect.
Add a bit more test specific comment here, i.e. pointing out that we expect
failure on ...

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/test/data/...
File extensions/test/data/app_view/apitest/main.js (right):

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/test/data/...
extensions/test/data/app_view/apitest/main.js:60: appview.connect(appToEmbed, {
'foo': 'bleep' }, function(success) {
Here and below: no space after { or before }

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/test/data/...
extensions/test/data/app_view/apitest/main.js:93: // Step 1: Attempt to connect
to a non-existant app.
s/non-existant/non-existent
Also add note that "abc123" is the non-existent app.

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/test/data/...
extensions/test/data/app_view/apitest/main.js:94: LOG('attempting to connect to
non-existant app.');
nit: s/attempting/Attempting

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/test/data/...
extensions/test/data/app_view/apitest/main.js:112: LOG('CONNECTED.');
nit: s/CONNECTED/Connected, SHOUT only when things are wrong =P

[lfg, 2014-10-24 20:24:30]

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/browser/gu...
File extensions/browser/guest_view/app_view/app_view_apitest.cc (right):

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/browser/gu...
extensions/browser/guest_view/app_view/app_view_apitest.cc:48: const Extension*
extension = extension_system_->LoadApp(test_data_dir);
On 2014/10/24 20:13:24, lazyboy wrote:
> nit: return extension_system_->LoadApp ..

Done.

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/browser/gu...
extensions/browser/guest_view/app_view/app_view_apitest.cc:95: // Tests that
<appview> correctly processes parameters passed on connect.
On 2014/10/24 20:13:24, lazyboy wrote:
> Add a bit more test specific comment here, i.e. pointing out that we expect
> failure on ...

Done.

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/test/data/...
File extensions/test/data/app_view/apitest/main.js (right):

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/test/data/...
extensions/test/data/app_view/apitest/main.js:60: appview.connect(appToEmbed, {
'foo': 'bleep' }, function(success) {
On 2014/10/24 20:13:25, lazyboy wrote:
> Here and below: no space after { or before }

Done.

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/test/data/...
extensions/test/data/app_view/apitest/main.js:93: // Step 1: Attempt to connect
to a non-existant app.
On 2014/10/24 20:13:25, lazyboy wrote:
> s/non-existant/non-existent
> Also add note that "abc123" is the non-existent app.

Done.

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/test/data/...
extensions/test/data/app_view/apitest/main.js:94: LOG('attempting to connect to
non-existant app.');
On 2014/10/24 20:13:25, lazyboy wrote:
> nit: s/attempting/Attempting

Done.

https://chromiumcodereview.appspot.com/643703007/diff/1/extensions/test/data/...
extensions/test/data/app_view/apitest/main.js:112: LOG('CONNECTED.');
On 2014/10/24 20:13:25, lazyboy wrote:
> nit: s/CONNECTED/Connected, SHOUT only when things are wrong =P

Done.

[lazyboy, 2014-10-24 20:33:09]

Thanks, LGTM.

[lfg, 2014-10-24 20:46:14]

lfg@chromium.org changed reviewers:
+ yoz@chromium.org

[lfg, 2014-10-24 20:46:15]

yoz@chromium.org: Please review changes in 

extensions/*

[Yoyo Zhou, 2014-10-29 20:37:59]

LGTM

https://chromiumcodereview.appspot.com/643703007/diff/20001/extensions/browse...
File extensions/browser/guest_view/app_view/app_view_apitest.cc (right):

https://chromiumcodereview.appspot.com/643703007/diff/20001/extensions/browse...
extensions/browser/guest_view/app_view/app_view_apitest.cc:28:
extensions::GuestViewManager::set_factory_for_testing(&factory_);
nit: don't need extensions::

https://chromiumcodereview.appspot.com/643703007/diff/20001/extensions/browse...
extensions/browser/guest_view/app_view/app_view_apitest.cc:75: LOG(ERROR) <<
"Unable to start test.";
Does this actually make the test fail? It might be better to write

ASSERT_TRUE(content::ExecuteScript...) << "Unable to start test.";

[lfg, 2014-10-30 23:38:16]

https://codereview.chromium.org/643703007/diff/20001/extensions/browser/guest...
File extensions/browser/guest_view/app_view/app_view_apitest.cc (right):

https://codereview.chromium.org/643703007/diff/20001/extensions/browser/guest...
extensions/browser/guest_view/app_view/app_view_apitest.cc:28:
extensions::GuestViewManager::set_factory_for_testing(&factory_);
On 2014/10/29 20:37:58, Yoyo Zhou wrote:
> nit: don't need extensions::

Done.

https://codereview.chromium.org/643703007/diff/20001/extensions/browser/guest...
extensions/browser/guest_view/app_view/app_view_apitest.cc:75: LOG(ERROR) <<
"Unable to start test.";
On 2014/10/29 20:37:58, Yoyo Zhou wrote:
> Does this actually make the test fail? It might be better to write
> 
> ASSERT_TRUE(content::ExecuteScript...) << "Unable to start test.";

Done. web_view_apitest had the same issue, so I fixed it as well.

[lfg, 2014-10-30 23:53:46]

The CQ bit was checked by lfg@chromium.org

[commit-bot: I haz the power, 2014-10-30 23:54:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/643703007/40001

[commit-bot: I haz the power, 2014-10-31 00:38:11]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-31 00:39:10]

Patchset 3 (id:??) landed as
https://crrev.com/45e87e9d03f09ffdcbd0d2477ba703380c324ed8
Cr-Commit-Position: refs/heads/master@{#302198}