Assert that new renderer processes aren't created during shutdown.

Assert that new renderer processes aren't created during shutdown.

Puts CHECKs in RenderProcessHost's constructor to make sure the main MessageLoop hasn't stopped running yet.

BUG=91398
TEST=none

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

[willchan no longer on Chromium, 2011-10-09 18:23:29]

jam: This is part of my ongoing shutdown crusade to catch these renderer IPCs at
shutdown. I think I'd actually like this CHECK in there long-term to prevent
regressions.

michaeln: I'm hoping this will catch the causes of the OnChannelOpened() and
other renderer IPCs on shutdown.

[jam, 2011-10-10 18:26:30]

I think it's simpler to check BrowserShutdown (which is in chrome) from Chrome's
ContentBrowserClient::BrowserRenderProcessHostCreated

[willchan no longer on Chromium, 2011-10-10 20:36:41]

On 2011/10/10 18:26:30, John Abd-El-Malek wrote:
> I think it's simpler to check BrowserShutdown (which is in chrome) from
Chrome's
> ContentBrowserClient::BrowserRenderProcessHostCreated

We talked about this offline and there was some discussion about whether this
should be within content/ or within chrome/. I'm fine doing it in chrome/, but I
think content/ makes more sense.

[jam, 2011-10-10 20:43:58]

On 2011/10/10 20:36:41, willchan wrote:
> On 2011/10/10 18:26:30, John Abd-El-Malek wrote:
> > I think it's simpler to check BrowserShutdown (which is in chrome) from
> Chrome's
> > ContentBrowserClient::BrowserRenderProcessHostCreated
> 
> We talked about this offline and there was some discussion about whether this
> should be within content/ or within chrome/. I'm fine doing it in chrome/, but
I
> think content/ makes more sense.

content is fine if it'll be permanent. in that case, we should probably keep the
state that shutdown has started somewhere else? i'm a little nervous of other
classes starting to do this. if it's temporary, then this is fine.

[willchan no longer on Chromium, 2011-10-10 21:19:31]

On Mon, Oct 10, 2011 at 1:43 PM, <jam@chromium.org> wrote:

> On 2011/10/10 20:36:41, willchan wrote:
>
>> On 2011/10/10 18:26:30, John Abd-El-Malek wrote:
>> > I think it's simpler to check BrowserShutdown (which is in chrome) from
>> Chrome's
>> > ContentBrowserClient::**BrowserRenderProcessHostCreate**d
>>
>
>  We talked about this offline and there was some discussion about whether
>> this
>> should be within content/ or within chrome/. I'm fine doing it in chrome/,
>> but
>>
> I
>
>> think content/ makes more sense.
>>
>
> content is fine if it'll be permanent. in that case, we should probably
> keep the
> state that shutdown has started somewhere else? i'm a little nervous of
> other
> classes starting to do this. if it's temporary, then this is fine.
>

What do you think about adding a HasMainMessageLoopExited() function to
browser_main.h? We could set a static boolean in browser_main.cc for this.


>
>
http://codereview.chromium.**org/8210004/<http://codereview.chromium.org/8210...
>

[jam, 2011-10-10 21:26:47]

On 2011/10/10 21:19:31, willchan wrote:
> On Mon, Oct 10, 2011 at 1:43 PM, <mailto:jam@chromium.org> wrote:
> 
> > On 2011/10/10 20:36:41, willchan wrote:
> >
> >> On 2011/10/10 18:26:30, John Abd-El-Malek wrote:
> >> > I think it's simpler to check BrowserShutdown (which is in chrome) from
> >> Chrome's
> >> > ContentBrowserClient::**BrowserRenderProcessHostCreate**d
> >>
> >
> >  We talked about this offline and there was some discussion about whether
> >> this
> >> should be within content/ or within chrome/. I'm fine doing it in chrome/,
> >> but
> >>
> > I
> >
> >> think content/ makes more sense.
> >>
> >
> > content is fine if it'll be permanent. in that case, we should probably
> > keep the
> > state that shutdown has started somewhere else? i'm a little nervous of
> > other
> > classes starting to do this. if it's temporary, then this is fine.
> >
> 
> What do you think about adding a HasMainMessageLoopExited() function to
> browser_main.h? We could set a static boolean in browser_main.cc for this.
> 
> 
> >
> >
>
http://codereview.chromium.**org/8210004/%3Chttp://codereview.chromium.org/82...>
> >

sgtm

[willchan no longer on Chromium, 2011-10-10 22:17:37]

Updated

On 2011/10/10 21:26:47, John Abd-El-Malek wrote:
> On 2011/10/10 21:19:31, willchan wrote:
> > On Mon, Oct 10, 2011 at 1:43 PM, <mailto:jam@chromium.org> wrote:
> > 
> > > On 2011/10/10 20:36:41, willchan wrote:
> > >
> > >> On 2011/10/10 18:26:30, John Abd-El-Malek wrote:
> > >> > I think it's simpler to check BrowserShutdown (which is in chrome) from
> > >> Chrome's
> > >> > ContentBrowserClient::**BrowserRenderProcessHostCreate**d
> > >>
> > >
> > >  We talked about this offline and there was some discussion about whether
> > >> this
> > >> should be within content/ or within chrome/. I'm fine doing it in
chrome/,
> > >> but
> > >>
> > > I
> > >
> > >> think content/ makes more sense.
> > >>
> > >
> > > content is fine if it'll be permanent. in that case, we should probably
> > > keep the
> > > state that shutdown has started somewhere else? i'm a little nervous of
> > > other
> > > classes starting to do this. if it's temporary, then this is fine.
> > >
> > 
> > What do you think about adding a HasMainMessageLoopExited() function to
> > browser_main.h? We could set a static boolean in browser_main.cc for this.
> > 
> > 
> > >
> > >
> >
>
http://codereview.chromium.**org/8210004/%253Chttp://codereview.chromium.org/...>
> > >
> 
> sgtm

[jam, 2011-10-10 22:25:44]

lgtm

[commit-bot: I haz the power, 2011-10-10 23:37:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/willchan@chromium.org/8210004/6002

[commit-bot: I haz the power, 2011-10-11 00:36:53]

Try job failure for 8210004-6002 (retry) on win_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2011-10-11 03:15:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/willchan@chromium.org/8210004/6002

[commit-bot: I haz the power, 2011-10-11 03:16:59]

Try job failure for 8210004-6002 on mac_rel for step "update".
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2011-10-11 04:38:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/willchan@chromium.org/8210004/6002

[commit-bot: I haz the power, 2011-10-11 04:39:44]

Try job failure for 8210004-6002 on win_rel for step "update".
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

Step "update" is always a major failure.
Look at the try server FAQ for more details.