Browser Plugin: Navigating to the same URL after crash should load a new guest

Browser Plugin: Navigating to the same URL after crash should load a new guest

This patch contains a few other small changes in behavior:

1. Removing the src attribute after navigation in <webview> will immediately restore it.
2. browserPlugin.terminate() will not issue an IPC as long as the BrowserPlugin is in a crashed state.
3. WebViewTest.Shim was misbehaving because it had lingering <webview>s in the DOM all in the same partition and so terminating one webview terminated them all, and caused multiple exit listeners to fire. This was fixed.
4. Fixed a potential infinite loop that could happen if the src attribute is modified while in a crashed state.

BUG=175206
Test=WebViewTest.Shim: webViewAssignSrcAfterCrash, webViewRemoveSrcAttribute

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

[lazyboy, 2013-02-11 18:23:18]

One question about the test.

https://codereview.chromium.org/12224094/diff/1/chrome/test/data/extensions/p...
File chrome/test/data/extensions/platform_apps/web_view/shim/main.js (right):

https://codereview.chromium.org/12224094/diff/1/chrome/test/data/extensions/p...
chrome/test/data/extensions/platform_apps/web_view/shim/main.js:84:
webview.parentNode.removeChild(webview);
Instead of doing this on every test can we set different partition attributes on
each webview? Will that work?

https://codereview.chromium.org/12224094/diff/1/content/renderer/browser_plug...
File content/renderer/browser_plugin/browser_plugin_bindings.cc (right):

https://codereview.chromium.org/12224094/diff/1/content/renderer/browser_plug...
content/renderer/browser_plugin/browser_plugin_bindings.cc:714: if
(new_value.empty()) {
Just noting: We used to have this check (setting empty src) on shim before?

[Fady Samuel, 2013-02-12 20:44:40]

PTAL

https://codereview.chromium.org/12224094/diff/1/chrome/test/data/extensions/p...
File chrome/test/data/extensions/platform_apps/web_view/shim/main.js (right):

https://codereview.chromium.org/12224094/diff/1/chrome/test/data/extensions/p...
chrome/test/data/extensions/platform_apps/web_view/shim/main.js:84:
webview.parentNode.removeChild(webview);
On 2013/02/11 18:23:18, lazyboy wrote:
> Instead of doing this on every test can we set different partition attributes
on
> each webview? Will that work?

It works! It uncovered a subtle infinite navigation loop on guest crashes that I
fixed in web_view.js.

The issue was that we would continue bouncing between the webview's mutation
observer and the browser plugin's mutation observer, setting the src attribute.

The fix is to only mutate an attribute in <webview> if BrowserPlugin's attribute
differs from <webview>'s.

https://codereview.chromium.org/12224094/diff/1/content/renderer/browser_plug...
File content/renderer/browser_plugin/browser_plugin_bindings.cc (right):

https://codereview.chromium.org/12224094/diff/1/content/renderer/browser_plug...
content/renderer/browser_plugin/browser_plugin_bindings.cc:714: if
(new_value.empty()) {
On 2013/02/11 18:23:18, lazyboy wrote:
> Just noting: We used to have this check (setting empty src) on shim before?

I don't recall. Right now 'src' is treated like any other attribute in the shim.

[lazyboy, 2013-02-12 21:50:37]

lgtm

https://chromiumcodereview.appspot.com/12224094/diff/7/chrome/renderer/resour...
File chrome/renderer/resources/extensions/web_view.js (right):

https://chromiumcodereview.appspot.com/12224094/diff/7/chrome/renderer/resour...
chrome/renderer/resources/extensions/web_view.js:161: if (newValue != oldValue)
Needs {} around if block.
Can you also add a note saying why this is needed.

[commit-bot: I haz the power, 2013-02-13 15:59:34]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Fady Samuel, 2013-02-13 16:03:42]

+sadrul for committer rubberstamp.

[sadrul, 2013-02-13 16:13:15]

LGTM

[commit-bot: I haz the power, 2013-02-13 16:13:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fsamuel@chromium.org/12224094/11001

[commit-bot: I haz the power, 2013-02-13 19:10:59]

Change committed as 182278