<webview>: Fix an issue with destroying an opener that has unattached guests.

<webview>: Fix an issue with destroying an opener that has unattached guests.

This CL also adds a test that exercises the code path and makes
sure destroying an opener that has unattached window doesn't
cause any undesired side effects.

If we destroy an opener that had a pending newwindow, the
newwindow's GC would try to setPermission to deny the
newwindow later. But the opener is gone and its guestInstanceId
isn't valid anymore, this would throw a JavaScript exception b/c
we call webViewInternal.setPermission with undefined instance id:
"Uncaught Error: Invocation of form ..."
The fix is to ignore calling webViewInternal.setPermission in this
case.

Note that the added test doesn't necessarily check for the above
exception because GC can be delayed arbitrarily. The test checks
the regular code path.

BUG=406616
Test=See the test added in newwindow/embedder.js for details.
Open a chrome app window that has a <webview>.
Add a newwindow event listener to the <webview>.
Now trigger a new window from the <webview> so the listener
fires.
In the listener, destroy the <webview>.
Every thing should work fine and should not exhibit any bad
behavior. Without this change it would throw a JavaScript
exception.

Committed: https://crrev.com/40341e4ab145e7e4daa57de8ea1152f6018233b6
Cr-Commit-Position: refs/heads/master@{#294053}

[lazyboy, 2014-09-09 21:37:46]

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

[Fady Samuel, 2014-09-09 21:55:35]

https://chromiumcodereview.appspot.com/558813002/diff/1/chrome/test/data/exte...
File chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js
(right):

https://chromiumcodereview.appspot.com/558813002/diff/1/chrome/test/data/exte...
chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js:347:
window.setTimeout(function() { embedder.test.succeed(); }, 0);
What happens if we try e.window.attach on another webview?

[lazyboy, 2014-09-09 22:52:22]

Comment addressed in patchset #2

https://chromiumcodereview.appspot.com/558813002/diff/1/chrome/test/data/exte...
File chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js
(right):

https://chromiumcodereview.appspot.com/558813002/diff/1/chrome/test/data/exte...
chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js:347:
window.setTimeout(function() { embedder.test.succeed(); }, 0);
On 2014/09/09 21:55:34, Fady Samuel wrote:
> What happens if we try e.window.attach on another webview?

It would throw the same exception, added guards to attach/discard as well.
Added test cases for e.window.attach() and e.window.discard() calls.

[Fady Samuel, 2014-09-09 23:01:34]

lgtm with nits

https://chromiumcodereview.appspot.com/558813002/diff/20001/chrome/test/data/...
File chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js
(right):

https://chromiumcodereview.appspot.com/558813002/diff/20001/chrome/test/data/...
chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js:251:
//window.setTimeout(function() { embedder.test.succeed(); }, 0);
Delete this line?

https://chromiumcodereview.appspot.com/558813002/diff/20001/chrome/test/data/...
chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js:254: //
Trying to discard() will fail silencly.
s/discard/attach
s/silencly/silently.

https://chromiumcodereview.appspot.com/558813002/diff/20001/chrome/test/data/...
chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js:338:
//window.setTimeout(function() { embedder.test.succeed(); }, 0);
Remove this line?

https://chromiumcodereview.appspot.com/558813002/diff/20001/chrome/test/data/...
chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js:341: //
Trying to discard() will fail silencly.
s/silency/silently

[lazyboy, 2014-09-09 23:18:36]

Comments address in patchset #3, #4.

https://chromiumcodereview.appspot.com/558813002/diff/20001/chrome/test/data/...
File chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js
(right):

https://chromiumcodereview.appspot.com/558813002/diff/20001/chrome/test/data/...
chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js:251:
//window.setTimeout(function() { embedder.test.succeed(); }, 0);
On 2014/09/09 23:01:34, Fady Samuel wrote:
> Delete this line?

Done.

https://chromiumcodereview.appspot.com/558813002/diff/20001/chrome/test/data/...
chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js:254: //
Trying to discard() will fail silencly.
On 2014/09/09 23:01:33, Fady Samuel wrote:
> s/discard/attach
> s/silencly/silently.

Done.

https://chromiumcodereview.appspot.com/558813002/diff/20001/chrome/test/data/...
chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js:338:
//window.setTimeout(function() { embedder.test.succeed(); }, 0);
On 2014/09/09 23:01:33, Fady Samuel wrote:
> Remove this line?

Done.

[lazyboy, 2014-09-09 23:37:56]

The CQ bit was checked by lazyboy@chromium.org

[commit-bot: I haz the power, 2014-09-09 23:51:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/lazyboy@chromium.org/558813002/60001

[commit-bot: I haz the power, 2014-09-10 00:38:06]

Committed patchset #4 (id:60001) as 68fc36cb07eaf5ff4536b8ce3040f1c2ba951ebf

[commit-bot: I haz the power, 2014-09-10 03:57:02]

Patchset 4 (id:??) landed as
https://crrev.com/40341e4ab145e7e4daa57de8ea1152f6018233b6
Cr-Commit-Position: refs/heads/master@{#294053}