Move chromeos specific code to ChromeBrowserMainPartsChromeos

Move chromeos specific code to ChromeBrowserMainPartsChromeos

Change-Id: I77a05a62fa652a1f79a536761c037cffc2d318bb

BUG=chromium-os:103821
TEST=Make sure all chromeos tests and autotests run

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

[stevenjb, 2011-11-23 22:43:47]

Taking advantage of the ChromeBrowserMain re-factoring to migrate most of the
chromeos specific startup/shutdown code into
ChromeBrowserMainExtraPartsChromeos.

[satorux1, 2011-11-23 23:51:53]

LGTM. Nice refactoring. Fingers crossed.

http://codereview.chromium.org/8670001/diff/6004/chrome/browser/chrome_browse...
File chrome/browser/chrome_browser_main.cc (left):

http://codereview.chromium.org/8670001/diff/6004/chrome/browser/chrome_browse...
chrome/browser/chrome_browser_main.cc:149: #include
"chrome/browser/ui/views/browser_dialogs.h"
Great to see them gone!

http://codereview.chromium.org/8670001/diff/6004/chrome/browser/chrome_browse...
File chrome/browser/chrome_browser_main.h (right):

http://codereview.chromium.org/8670001/diff/6004/chrome/browser/chrome_browse...
chrome/browser/chrome_browser_main.h:69: virtual void PostBrowserStart();
BTW, will these stages be executed in this order? The execution order is unclear
from the function names alone, so it'd be nice to document the execution order.

The same can be said to the stages in content::BrowserMainParts().

http://codereview.chromium.org/8670001/diff/6004/chrome/browser/chromeos/chro...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

http://codereview.chromium.org/8670001/diff/6004/chrome/browser/chromeos/chro...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:330: void
ChromeBrowserMainPartsChromeos::PostProfileInitialized() {
This looks like an useful hook. Maybe we can move the nasty stuff from
browser_init.cc to here? Of course, you don't need to do this in this patch,
though.

    // This observer is a singleton. It is never deleted but the pointer is kept
                                                  
    // in a static so that it isn't reported as a leak.                         
                                                  
    static chromeos::LowBatteryObserver* low_battery_observer =
        new chromeos::LowBatteryObserver(profile);
    chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->AddObserver(
	low_battery_observer);

    static chromeos::NetworkMessageObserver* network_message_observer =
        new chromeos::NetworkMessageObserver(profile);
    chromeos::CrosLibrary::Get()->GetNetworkLibrary()
        ->AddNetworkManagerObserver(network_message_observer);
    chromeos::CrosLibrary::Get()->GetNetworkLibrary()
        ->AddCellularDataPlanObserver(network_message_observer);
    chromeos::CrosLibrary::Get()->GetNetworkLibrary()
        ->AddUserActionObserver(network_message_observer);

    static chromeos::SmsObserver* sms_observer =
        new chromeos::SmsObserver(profile);
    chromeos::CrosLibrary::Get()->GetNetworkLibrary()
        ->AddNetworkManagerObserver(sms_observer);

    profile->SetupChromeOSEnterpriseExtensionObserver();

[stevenjb, 2011-11-24 01:35:39]

http://codereview.chromium.org/8670001/diff/6004/chrome/browser/chrome_browse...
File chrome/browser/chrome_browser_main.h (right):

http://codereview.chromium.org/8670001/diff/6004/chrome/browser/chrome_browse...
chrome/browser/chrome_browser_main.h:69: virtual void PostBrowserStart();
On 2011/11/23 23:51:53, satorux1 wrote:
> BTW, will these stages be executed in this order? The execution order is
unclear
> from the function names alone, so it'd be nice to document the execution
order.
> 
> The same can be said to the stages in content::BrowserMainParts().

Yes. I will add comments to clarify this.

http://codereview.chromium.org/8670001/diff/6004/chrome/browser/chromeos/chro...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

http://codereview.chromium.org/8670001/diff/6004/chrome/browser/chromeos/chro...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:330: void
ChromeBrowserMainPartsChromeos::PostProfileInitialized() {
On 2011/11/23 23:51:53, satorux1 wrote:
> This looks like an useful hook. Maybe we can move the nasty stuff from
> browser_init.cc to here? Of course, you don't need to do this in this patch,
> though.
> 
>     // This observer is a singleton. It is never deleted but the pointer is
kept
>                                                   
>     // in a static so that it isn't reported as a leak.                       
 
>                                                   
>     static chromeos::LowBatteryObserver* low_battery_observer =
>         new chromeos::LowBatteryObserver(profile);
>     chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->AddObserver(
> 	low_battery_observer);
> 
>     static chromeos::NetworkMessageObserver* network_message_observer =
>         new chromeos::NetworkMessageObserver(profile);
>     chromeos::CrosLibrary::Get()->GetNetworkLibrary()
>         ->AddNetworkManagerObserver(network_message_observer);
>     chromeos::CrosLibrary::Get()->GetNetworkLibrary()
>         ->AddCellularDataPlanObserver(network_message_observer);
>     chromeos::CrosLibrary::Get()->GetNetworkLibrary()
>         ->AddUserActionObserver(network_message_observer);
> 
>     static chromeos::SmsObserver* sms_observer =
>         new chromeos::SmsObserver(profile);
>     chromeos::CrosLibrary::Get()->GetNetworkLibrary()
>         ->AddNetworkManagerObserver(sms_observer);
> 
>     profile->SetupChromeOSEnterpriseExtensionObserver();

Definitely sounds like a good idea. I will make a note to myself. Should
probably be able to be called from PostBrowserStart().

[DaveMoore, 2011-11-30 23:02:22]

lgtm

[commit-bot: I haz the power, 2011-12-01 00:59:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/8670001/11001

[commit-bot: I haz the power, 2011-12-01 02:46:23]

Can't apply patch for file chrome/browser/chrome_browser_main.cc.
While running patch -p1 --forward --force;
patching file chrome/browser/chrome_browser_main.cc
Hunk #2 FAILED at 127.
Hunk #3 succeeded at 446 (offset -1 lines).
Hunk #7 FAILED at 1444.
Hunk #8 succeeded at 1503 (offset -4 lines).
Hunk #9 succeeded at 1549 (offset -4 lines).
Hunk #10 succeeded at 1703 (offset -8 lines).
Hunk #11 succeeded at 1727 (offset -3 lines).
Hunk #12 succeeded at 1744 (offset -3 lines).
Hunk #13 succeeded at 1822 (offset -3 lines).
Hunk #14 succeeded at 1850 (offset -3 lines).
Hunk #15 succeeded at 1907 (offset -3 lines).
Hunk #16 succeeded at 1921 (offset -3 lines).
2 out of 16 hunks FAILED -- saving rejects to file
chrome/browser/chrome_browser_main.cc.rej

[commit-bot: I haz the power, 2011-12-01 03:07:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/8670001/12012

[commit-bot: I haz the power, 2011-12-01 04:38:29]

Change committed as 112405