When a pending contents is created but deleted before it is shown, remove it from the list of pendi…

When a pending contents is created but deleted before it is shown, remove it from the list of pending contents

This happens during running layout tests when all newly opened windows are
closed, however, an IPC message to show one of the new windows is already in
flight

The other direction, when the opener is deleted, is already covered by existing
code.

BUG=180969
R=creis@chromium.org
TEST=content_unittests:WebContentsImplTest.PendingContents

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

[Charlie Reis, 2013-03-07 18:20:00]

Nice catch.  Can you file a bug for this?  It'll help with tracking it.

https://codereview.chromium.org/12602003/diff/1/content/browser/web_contents/...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/12602003/diff/1/content/browser/web_contents/...
content/browser/web_contents/web_contents_impl.cc:1240: NOTREACHED();
Why is this here?  It's failing in all the tests, because we usually get here
when a non-pending contents is deleted.

https://codereview.chromium.org/12602003/diff/1/content/browser/web_contents/...
File content/browser/web_contents/web_contents_impl_unittest.cc (right):

https://codereview.chromium.org/12602003/diff/1/content/browser/web_contents/...
content/browser/web_contents/web_contents_impl_unittest.cc:2041: // creash.
nit: crash

[jochen (gone - plz use gerrit), 2013-03-07 19:45:10]

https://codereview.chromium.org/12602003/diff/1/content/browser/web_contents/...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/12602003/diff/1/content/browser/web_contents/...
content/browser/web_contents/web_contents_impl.cc:1240: NOTREACHED();
On 2013/03/07 18:20:00, creis wrote:
> Why is this here?  It's failing in all the tests, because we usually get here
> when a non-pending contents is deleted.

Ah, because I forgot to add a return in the if() block above.

Since we only listen for the destruction of a pending web contents and the
opener, we shouldn't get here for other cases.

https://codereview.chromium.org/12602003/diff/1/content/browser/web_contents/...
File content/browser/web_contents/web_contents_impl_unittest.cc (right):

https://codereview.chromium.org/12602003/diff/1/content/browser/web_contents/...
content/browser/web_contents/web_contents_impl_unittest.cc:2041: // creash.
On 2013/03/07 18:20:00, creis wrote:
> nit: crash

Done.

[Charlie Reis, 2013-03-07 22:01:02]

This is still failing the DisownOpener test, which suggests it's possible to
arrive at WebContentsDestroyed after the WebContents has set its  opener_ to
null.

https://codereview.chromium.org/12602003/diff/8001/content/browser/web_conten...
File content/browser/web_contents/web_contents_impl_unittest.cc (right):

https://codereview.chromium.org/12602003/diff/8001/content/browser/web_conten...
content/browser/web_contents/web_contents_impl_unittest.cc:2040: // Test that a
if a pending contents is deleted before it is shown, we don't
typo: that a if a

[jochen (gone - plz use gerrit), 2013-03-07 22:06:32]

On 2013/03/07 22:01:02, creis wrote:
> This is still failing the DisownOpener test, which suggests it's possible to
> arrive at WebContentsDestroyed after the WebContents has set its  opener_ to
> null.

Indeed. Fixed that place.

> 
>
https://codereview.chromium.org/12602003/diff/8001/content/browser/web_conten...
> File content/browser/web_contents/web_contents_impl_unittest.cc (right):
> 
>
https://codereview.chromium.org/12602003/diff/8001/content/browser/web_conten...
> content/browser/web_contents/web_contents_impl_unittest.cc:2040: // Test that
a
> if a pending contents is deleted before it is shown, we don't
> typo: that a if a

done

[Charlie Reis, 2013-03-07 22:08:46]

Thanks, LGTM.

[commit-bot: I haz the power, 2013-03-07 22:36:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jochen@chromium.org/12602003/14001

[commit-bot: I haz the power, 2013-03-08 04:59:53]

Change committed as 186893