Fix for a memory corruption.

Fix for a memory corruption.

As identified by http://crbug.com/90867, we sometimes leaked render view hosts by overwriting them in the swapped out map of the render view host manager.
 
This should not happen (two different hosts with the same instance id) and will eventually be fixed, but in the mean time this CL recovers from that problem and prevent the leak, and also the memory corruptions that were caused by it.

The memory corruption were caused by the fact that the leaked host would not be told when their delegate_ would die and might try to call them post-mortem.

BUG=90867
TEST=RenderViewHostManagerTest.LeakingRenderViewHosts

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98249

[MAD, 2011-08-24 20:32:50]

How about this?

The fails without the fix and succeeds with it, of course... :-)

Thanks!

BYE
MAD

[Charlie Reis, 2011-08-24 21:18:52]

LGTM

http://codereview.chromium.org/7725005/diff/1/content/browser/renderer_host/r...
File content/browser/renderer_host/render_view_host_manager_browsertest.cc
(right):

http://codereview.chromium.org/7725005/diff/1/content/browser/renderer_host/r...
content/browser/renderer_host/render_view_host_manager_browsertest.cc:332:
render_view_host());
Maybe add an EXPECT_EQ(3, rvh_observers.GetNumObservers()) here?

http://codereview.chromium.org/7725005/diff/1/content/browser/tab_contents/re...
File content/browser/tab_contents/render_view_host_manager.cc (right):

http://codereview.chromium.org/7725005/diff/1/content/browser/tab_contents/re...
content/browser/tab_contents/render_view_host_manager.cc:641: // TODO(creis):
Clean this up.
Ugh, I'll try to fix the duplicated code in my next CL as well.  Can you add a
reminder to this TODO to also see SwapInRenderViewHost?

http://codereview.chromium.org/7725005/diff/1/content/browser/tab_contents/re...
content/browser/tab_contents/render_view_host_manager.cc:647:
iter->second->Shutdown();
Please add just a little more comment here, like "Delete the previous RVH before
we overwrite it."

[MAD, 2011-08-24 21:31:07]

Cool... Thanks!

Done!

BYE
MAD

http://codereview.chromium.org/7725005/diff/1/content/browser/renderer_host/r...
File content/browser/renderer_host/render_view_host_manager_browsertest.cc
(right):

http://codereview.chromium.org/7725005/diff/1/content/browser/renderer_host/r...
content/browser/renderer_host/render_view_host_manager_browsertest.cc:332:
render_view_host());
On 2011/08/24 21:18:52, creis wrote:
> Maybe add an EXPECT_EQ(3, rvh_observers.GetNumObservers()) here?

Done.

http://codereview.chromium.org/7725005/diff/1/content/browser/tab_contents/re...
File content/browser/tab_contents/render_view_host_manager.cc (right):

http://codereview.chromium.org/7725005/diff/1/content/browser/tab_contents/re...
content/browser/tab_contents/render_view_host_manager.cc:641: // TODO(creis):
Clean this up.
On 2011/08/24 21:18:52, creis wrote:
> Ugh, I'll try to fix the duplicated code in my next CL as well.  Can you add a
> reminder to this TODO to also see SwapInRenderViewHost?

Added a note, thought you already had one here:

void RenderViewHostManager::SwapInRenderViewHost(RenderViewHost* rvh) {
  // TODO(creis): Abstract out the common code between this and CommitPending.


:-)

http://codereview.chromium.org/7725005/diff/1/content/browser/tab_contents/re...
content/browser/tab_contents/render_view_host_manager.cc:647:
iter->second->Shutdown();
On 2011/08/24 21:18:52, creis wrote:
> Please add just a little more comment here, like "Delete the previous RVH
before
> we overwrite it."

Done.

[commit-bot: I haz the power, 2011-08-25 14:11:53]

Presubmit check for 7725005-8001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for:
content/browser/renderer_host/render_view_host_manager_browsertest.cc,content/browser/tab_contents/render_view_host_manager.cc

Presubmit checks took 4.6s to calculate.

[commit-bot: I haz the power, 2011-08-25 14:16:42]

Presubmit check for 7725005-8001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for:
content/browser/renderer_host/render_view_host_manager_browsertest.cc,content/browser/tab_contents/render_view_host_manager.cc

Presubmit checks took 3.8s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[MAD, 2011-08-25 14:19:55]

I need an owner's LGTM... Anyone? Anyone?... :-)

Thanks!

BYE
MAD

[Avi (use Gerrit), 2011-08-25 14:35:20]

Owner LGTM.

[commit-bot: I haz the power, 2011-08-25 16:57:36]

Change committed as 98249