Make RenderMessageCompletionCallback delete unsent Message

Make RenderMessageCompletionCallback delete unsent Message

Prior to this CL, if SendReplyAndDeleteThis is never called, the IPC::Message
gets leaked.

BUG=370588
R=jam@chromium.org

Committed: https://crrev.com/215d44f2ff830604efa30652f8940502283f7ec4
Cr-Commit-Position: refs/heads/master@{#308486}

[jam, 2014-12-03 00:01:26]

if this condition happens, isn't the renderer hung? that seems much worse than a
leak.

[dmichael (off chromium), 2014-12-03 15:59:42]

I agree in general. There are more details in the bug. In this case, I *think*
it's happening if the channel closes before
PluginServiceImpl::FinishOpenChannelToPlugin is called. I.e.:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...
happens before:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/pl...

So the renderer isn't getting hung; it probably shut down or crashed. We could
alternately add a "DeleteWithoutSending" method to make it clearer what's
happening.

[jam, 2014-12-05 01:20:21]

On 2014/12/03 15:59:42, dmichael wrote:
> I agree in general. There are more details in the bug. In this case, I *think*
> it's happening if the channel closes before
> PluginServiceImpl::FinishOpenChannelToPlugin is called. I.e.:
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...
> happens before:
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/pl...
> 
> So the renderer isn't getting hung; it probably shut down or crashed. We could
> alternately add a "DeleteWithoutSending" method to make it clearer what's
> happening.

to be safe, in the destructor if the message is still there, can you call
set_reply_error() on it and then Send it? Then if for some reason if whoever is
using this class forgets to call it, the renderer won't hang

[dmichael (off chromium), 2014-12-09 18:41:01]

Comment addressed.

[jam, 2014-12-15 19:15:06]

lgtm, sorry for delay just saw the update

https://codereview.chromium.org/766173003/diff/20001/content/browser/renderer...
File content/browser/renderer_host/render_message_filter.cc (right):

https://codereview.chromium.org/766173003/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_message_filter.cc:152:
scoped_ptr<IPC::Message> reply_msg_;
nit: no need to use scoped_ptr then?

[dmichael (off chromium), 2014-12-15 21:44:52]

The CQ bit was checked by dmichael@chromium.org

[commit-bot: I haz the power, 2014-12-15 21:46:21]

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

[commit-bot: I haz the power, 2014-12-15 23:15:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-15 23:15:40]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dmichael (off chromium), 2014-12-15 23:46:18]

The CQ bit was checked by dmichael@chromium.org

[commit-bot: I haz the power, 2014-12-15 23:48:08]

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

[commit-bot: I haz the power, 2014-12-16 01:01:28]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2014-12-16 01:02:08]

Patchset 5 (id:??) landed as
https://crrev.com/215d44f2ff830604efa30652f8940502283f7ec4
Cr-Commit-Position: refs/heads/master@{#308486}