Enabling two tests that were disabled in app_shell_browsertests.

Enabling two tests that were disabled in app_shell_browsertests.

We can enable those tests now that crbug.com/419020 is fixed.

BUG=352293
Committed: https://crrev.com/9b10e4b077ee7106ab57d4728c5199a03638701a
Cr-Commit-Position: refs/heads/master@{#298062}

Committed: https://crrev.com/3bc410dd3d82e0024f69a08adf9cbdd532cfd583
Cr-Commit-Position: refs/heads/master@{#299177}

[lfg, 2014-10-03 16:37:51]

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

[lfg, 2014-10-03 16:37:51]

PTAL

[Fady Samuel, 2014-10-03 16:50:23]

lgtm

[lfg, 2014-10-03 18:13:03]

The CQ bit was checked by lfg@chromium.org

[commit-bot: I haz the power, 2014-10-03 18:14:25]

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

[commit-bot: I haz the power, 2014-10-03 18:21:30]

Committed patchset #1 (id:1) as 7f4ec96677558a4fdd79882504cbe510bc1cc4b5

[commit-bot: I haz the power, 2014-10-03 18:22:07]

Patchset 1 (id:??) landed as
https://crrev.com/9b10e4b077ee7106ab57d4728c5199a03638701a
Cr-Commit-Position: refs/heads/master@{#298062}

[lfg, 2014-10-03 20:15:56]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/630453002/ by lfg@chromium.org.

The reason for reverting is: The RenderProcessHost may not be destroyed because
the message to destroy it may be queued in the message loop after the test pass
message.
.

[lfg, 2014-10-03 20:47:16]

Fixed the flaky test -- PTAL.

[Fady Samuel, 2014-10-03 20:51:36]

https://codereview.chromium.org/623193002/diff/20001/extensions/browser/guest...
File extensions/browser/guest_view/web_view/web_view_apitest.cc (right):

https://codereview.chromium.org/623193002/diff/20001/extensions/browser/guest...
extensions/browser/guest_view/web_view/web_view_apitest.cc:411:
GetGuestViewManager()->WaitForGuestDeleted();
Can we move this to the bottom of RunTest?

[lfg, 2014-10-03 20:55:01]

On 2014/10/03 20:51:36, Fady Samuel wrote:
>
https://codereview.chromium.org/623193002/diff/20001/extensions/browser/guest...
> File extensions/browser/guest_view/web_view/web_view_apitest.cc (right):
> 
>
https://codereview.chromium.org/623193002/diff/20001/extensions/browser/guest...
> extensions/browser/guest_view/web_view/web_view_apitest.cc:411:
> GetGuestViewManager()->WaitForGuestDeleted();
> Can we move this to the bottom of RunTest?

We could, but it's very test-dependant. There are tests that creates multiple
guestviews, and for those cases it would be a wrong assertion, since it only
verifies that a single guestview was destroyed.

I think if we want to move if to the end of RunTest we need to make it more
robust -- i.e. observe every guest view created and make sure that every one
gets destroyed.

I'll try to get that working, but on a new CL, does that SGTY?

[Fady Samuel, 2014-10-03 20:59:28]

https://codereview.chromium.org/623193002/diff/20001/extensions/browser/guest...
File extensions/browser/guest_view/web_view/web_view_apitest.cc (right):

https://codereview.chromium.org/623193002/diff/20001/extensions/browser/guest...
extensions/browser/guest_view/web_view/web_view_apitest.cc:411:
GetGuestViewManager()->WaitForGuestDeleted();
On 2014/10/03 20:51:36, Fady Samuel wrote:
> Can we move this to the bottom of RunTest?

LGTM. I like the idea of observing that every guest created gets destroyed in a
future CL. That seems like a great extra way to test for leaks on all tests.

[lfg, 2014-10-10 20:26:05]

The CQ bit was checked by lfg@chromium.org

[commit-bot: I haz the power, 2014-10-10 20:27:16]

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

[commit-bot: I haz the power, 2014-10-10 20:35:31]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-10 20:36:15]

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