Reset child_process_launcher after notifying observers

Reset child_process_launcher after notifying observers

Some observers need to know the ProcessHandle of the process that just
exited.  If they listen for NOTIFICATION_RENDERER_PROCESS_CLOSED, then
they can get a valid handle but not if they register as a
RenderProcessHostObserver because the child_process_launcher is reset
before those observers are notified.

Fix this by resetting the child_process_launcher after all the
notifications have been sent out.

BUG=none

[Chirantan Ekbote, 2014-10-17 18:58:19]

chirantan@chromium.org changed reviewers:
+ creis@chromium.org, sievers@chromium.org

[Chirantan Ekbote, 2014-10-17 18:58:19]

Please take a look.

[no sievers, 2014-10-17 19:08:29]

https://codereview.chromium.org/662103002/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_process_host_impl.cc (left):

https://codereview.chromium.org/662103002/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_process_host_impl.cc:1906:
child_process_launcher_.reset();
This should already provide the old handle because it is queried above.

Of course the handle is not valid anymore, but with your proposed change you are
changing both NOTIFICATION_RENDERER_PROCESS_CLOSED and RenderProcessExited to
NOTIFICATION_RENDERER_PROCESS_ABOUT_TO_CLOSE/RenderProcessAboutToExit which
might have subtle implications.

[Charlie Reis, 2014-10-17 19:16:00]

creis@chromium.org changed reviewers:
+ nasko@chromium.org

[Charlie Reis, 2014-10-17 19:16:00]

Not LGTM.  Nasko intentionally changed this behavior in
https://codereview.chromium.org/613173004.

I'd suggest coordinating with Nasko to figure out the right path forward for
your use case.

[nasko, 2014-10-17 19:53:40]

Can you be more specific with which observers need this? What is broken that
requires this change?

[Chirantan Ekbote, 2014-10-17 20:18:00]

On 2014/10/17 19:53:40, nasko wrote:
> Can you be more specific with which observers need this? What is broken that
> requires this change?

No existing observers need this but a new one that I'm writing uses
ProcessHandles (crrev.com/656953002).  Looking at it again, I really only need
the handle at process creation time.  Everything else that I was using the
handle for can be changed to use the RenderProcessHost id instead.

I'm going to close this CL because I have a workaround but I still think the
current behavior is a little weird.  Maybe the ProcessHandle should also be
removed from RendererClosedDetails and everything that uses it should switch to
something else (like the RPH id).