Get rid of content::DidEndMainMessageLoop since it was declared in content but defined in chrome,...

Get rid of content::DidEndMainMessageLoop since it was declared in content but defined in chrome, and called by chrome.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=101924

[willchan no longer on Chromium, 2011-09-19 21:52:49]

I see.

I'm going to redo my CL not to use DidEndMainMessageLoop() and rather create a
new function in browser_process_impl.cc. My change is cross platform anyway, so
there's no reason for me to muck with all these platform specific files.

http://codereview.chromium.org/7948006/diff/1/chrome/browser/chrome_browser_m...
File chrome/browser/chrome_browser_main_mac.mm (right):

http://codereview.chromium.org/7948006/diff/1/chrome/browser/chrome_browser_m...
chrome/browser/chrome_browser_main_mac.mm:131: void
ChromeBrowserMainPartsMac::DidEndMainMessageLoop() {
I think we should just move this into PostMainMessageLoopRun(). Then we don't
need chrome_browser_main.cc to know about this function.

http://codereview.chromium.org/7948006/diff/1/content/browser/browser_main.cc
File content/browser/browser_main.cc (right):

http://codereview.chromium.org/7948006/diff/1/content/browser/browser_main.cc...
content/browser/browser_main.cc:170: #if defined(OS_WIN)
Looks to me like this should be in BrowserMainPartsWin. Same with the
OleInitialize() call below.

[willchan no longer on Chromium, 2011-09-19 22:08:29]

Oops, foirgot to send LGTM

On Mon, Sep 19, 2011 at 2:52 PM,  <willchan@chromium.org> wrote:
> I see.
>
> I'm going to redo my CL not to use DidEndMainMessageLoop() and rather create
> a
> new function in browser_process_impl.cc. My change is cross platform anyway,
> so
> there's no reason for me to muck with all these platform specific files.
>
>
>
http://codereview.chromium.org/7948006/diff/1/chrome/browser/chrome_browser_m...
> File chrome/browser/chrome_browser_main_mac.mm (right):
>
>
http://codereview.chromium.org/7948006/diff/1/chrome/browser/chrome_browser_m...
> chrome/browser/chrome_browser_main_mac.mm:131: void
> ChromeBrowserMainPartsMac::DidEndMainMessageLoop() {
> I think we should just move this into PostMainMessageLoopRun(). Then we
> don't need chrome_browser_main.cc to know about this function.
>
> http://codereview.chromium.org/7948006/diff/1/content/browser/browser_main.cc
> File content/browser/browser_main.cc (right):
>
>
http://codereview.chromium.org/7948006/diff/1/content/browser/browser_main.cc...
> content/browser/browser_main.cc:170: #if defined(OS_WIN)
> Looks to me like this should be in BrowserMainPartsWin. Same with the
> OleInitialize() call below.
>
> http://codereview.chromium.org/7948006/
>

[jam, 2011-09-19 22:13:31]

http://codereview.chromium.org/7948006/diff/1/chrome/browser/chrome_browser_m...
File chrome/browser/chrome_browser_main_mac.mm (right):

http://codereview.chromium.org/7948006/diff/1/chrome/browser/chrome_browser_m...
chrome/browser/chrome_browser_main_mac.mm:131: void
ChromeBrowserMainPartsMac::DidEndMainMessageLoop() {
On 2011/09/19 21:52:49, willchan wrote:
> I think we should just move this into PostMainMessageLoopRun(). Then we don't
> need chrome_browser_main.cc to know about this function.

That's what I had tried last week, but that didn't work since that function
needs the message loop to be running and some other objects that are destructed
by the time PostMainMessageLoopRun is run.

http://codereview.chromium.org/7948006/diff/1/content/browser/browser_main.cc
File content/browser/browser_main.cc (right):

http://codereview.chromium.org/7948006/diff/1/content/browser/browser_main.cc...
content/browser/browser_main.cc:170: #if defined(OS_WIN)
On 2011/09/19 21:52:49, willchan wrote:
> Looks to me like this should be in BrowserMainPartsWin. Same with the
> OleInitialize() call below.

BrowserMainPartsWin is in chrome, but we want this function for all embedders of
content, hence why it's here

[willchan no longer on Chromium, 2011-09-19 22:25:42]

http://codereview.chromium.org/7948006/diff/1/chrome/browser/chrome_browser_m...
File chrome/browser/chrome_browser_main_mac.mm (right):

http://codereview.chromium.org/7948006/diff/1/chrome/browser/chrome_browser_m...
chrome/browser/chrome_browser_main_mac.mm:131: void
ChromeBrowserMainPartsMac::DidEndMainMessageLoop() {
On 2011/09/19 22:13:31, John Abd-El-Malek wrote:
> On 2011/09/19 21:52:49, willchan wrote:
> > I think we should just move this into PostMainMessageLoopRun(). Then we
don't
> > need chrome_browser_main.cc to know about this function.
> 
> That's what I had tried last week, but that didn't work since that function
> needs the message loop to be running and some other objects that are
destructed
> by the time PostMainMessageLoopRun is run.

Sounds like shutdown ordering is busted here. I still don't think this should
exist and we should create a better pattern with more defined shutdown ordering,
but there's no need to block this changelist on it.

http://codereview.chromium.org/7948006/diff/1/content/browser/browser_main.cc
File content/browser/browser_main.cc (right):

http://codereview.chromium.org/7948006/diff/1/content/browser/browser_main.cc...
content/browser/browser_main.cc:170: #if defined(OS_WIN)
On 2011/09/19 22:13:31, John Abd-El-Malek wrote:
> On 2011/09/19 21:52:49, willchan wrote:
> > Looks to me like this should be in BrowserMainPartsWin. Same with the
> > OleInitialize() call below.
> 
> BrowserMainPartsWin is in chrome, but we want this function for all embedders
of
> content, hence why it's here

Make sense. I suspect we should probably be created the content equivalents of
BrowserMainParts{Win|Mac|etc} eventually.

[jam, 2011-09-19 22:40:48]

http://codereview.chromium.org/7948006/diff/1/content/browser/browser_main.cc
File content/browser/browser_main.cc (right):

http://codereview.chromium.org/7948006/diff/1/content/browser/browser_main.cc...
content/browser/browser_main.cc:170: #if defined(OS_WIN)
On 2011/09/19 22:25:43, willchan wrote:
> On 2011/09/19 22:13:31, John Abd-El-Malek wrote:
> > On 2011/09/19 21:52:49, willchan wrote:
> > > Looks to me like this should be in BrowserMainPartsWin. Same with the
> > > OleInitialize() call below.
> > 
> > BrowserMainPartsWin is in chrome, but we want this function for all
embedders
> of
> > content, hence why it's here
> 
> Make sense. I suspect we should probably be created the content equivalents of
> BrowserMainParts{Win|Mac|etc} eventually.

I avoided that because Chrome also has its own platform versions, which derive
from a common chrome class. so if content also had 3 versions, we'd have to make
the base chrome one derive from three different classes depending on the
platform, which seemed a little hard on the eyes