Make GTK and Aura parts orthogonal to OS parts

content/browser/browser_main.cc

Index: content/browser/browser_main.cc
diff --git a/content/browser/browser_main.cc b/content/browser/browser_main.cc
index 5bccf00027bca08ea502993b69ec06709f2e11e8..46d47ece6835a7014a85aeef6dcf920fcc29db51 100644
--- a/content/browser/browser_main.cc
+++ b/content/browser/browser_main.cc
@@ -167,7 +167,103 @@ static void SetUpGLibLogHandler() {
 
 namespace content {
 
-BrowserMainParts::BrowserMainParts(const MainFunctionParams& parameters)
+// BrowserMainLoop

[jam, 2011/10/24 19:36:00]

nit: this should go into its own file, and also have a header

[stevenjb, 2011/10/25 02:51:04]

Done.

+
+// This class contains different "stages" to be executed in |BrowserMain()|,

[jam, 2011/10/24 19:36:00]

nit: these comments are really meant for embedders in order to figure out how to
hook startup, so it seems that putting them in the implementation file will be
hard to find.

[stevenjb, 2011/10/25 02:51:04]

Done.

+// Each stage is represented by a single BrowserMainLoop method
+// (e.g., "EarlyInitialization()"), which does the following:
+//  - calls a method (e.g., "PreEarlyInitialization()") for each member of
+//    |parts_|. Parts will imlement platform or tookit specific code for that
+//    stage.
+//  - calls various methods for things common to all platforms (for that stage).
+//  - calls a method (e.g., "PostEarlyInitialization()") for platform-specific
+//    code to be called after the common code.
+//
+// Stages:
+//  - EarlyInitialization: things which should be done as soon as possible on
+//    program start (such as setting up signal handlers) and things to be done
+//    at some generic time before the start of the main message loop.
+//  - MainMessageLoopStart: things beginning with the start of the main message
+//    loop and ending with initialization of the main thread; platform-specific
+//    things which should be done immediately before the start of the main
+//    message loop should go in |PreMainMessageLoopStart()|.
+//  - RunMainMessageLoopParts:  things to be done before and after invoking the
+//    main message loop run method (e.g. MessageLoopForUI::current()->Run()).
+//
+// How to add stuff (to existing parts):
+//  - Figure out when your new code should be executed. What must happen
+//    before/after your code is executed? Are there performance reasons for
+//    running your code at a particular time? Document these things!
+//  - Split out any platform-specific bits. Please avoid #ifdefs it at all
+//    possible. You have two choices for platform-specific code: (1) Execute it
+//    from one of the platform-specific |Pre/Post...()| methods; do this if the
+//    code is unique to a platform type. Or (2) execute it from one of the
+//    "parts" (e.g., |EarlyInitialization()|) and provide platform-specific
+//    implementations of your code (in a virtual method); do this if you need to
+//    provide different implementations across most/all platforms.
+//  - Unless your new code is just one or two lines, put it into a separate
+//    method with a well-defined purpose. (Likewise, if you're adding to an
+//    existing chunk which makes it longer than one or two lines, please move
+//    the code out into a separate method.)
+
+class BrowserMainLoop {
+ public:
+  explicit BrowserMainLoop(const MainFunctionParams& parameters);
+  virtual ~BrowserMainLoop();
+  void Init();
+
+  // Parts to be called by |BrowserMain()|.
+  void EarlyInitialization();
+  void InitializeToolkit();
+  void MainMessageLoopStart();
+  void RunMainMessageLoopParts();
+  void MainMessageLoopRun();
+
+  int result_code() const { return result_code_; }
+
+ protected:
+  // Run main message loop. Invokes MessageLoopForUI::current()->Run unless
+  // overridden.
+
+  // Accessors for data members (below) ----------------------------------------
+  const MainFunctionParams& parameters() const {
+    return parameters_;
+  }
+  const CommandLine& parsed_command_line() const {
+    return parsed_command_line_;
+  }
+  MessageLoop& main_message_loop() const {
+    return *main_message_loop_;
+  }
+
+ private:
+  void InitializeMainThread();
+
+  // Members initialized on construction ---------------------------------------
+
+  const MainFunctionParams& parameters_;
+  const CommandLine& parsed_command_line_;
+  int result_code_;
+
+  // Vector of BrowserMainParts set by CreateBrowserMainParts ------------------
+  // The BrowserParts fucntions for each part are called in the order added.
+  // They are released (destroyed) in the reverse order.
+  typedef ContentBrowserClient::BrowserMainPartsList PartsList;
+  PartsList parts_;
+
+  // Members initialized in |MainMessageLoopStart()| ---------------------------
+  scoped_ptr<MessageLoop> main_message_loop_;
+  scoped_ptr<base::SystemMonitor> system_monitor_;
+  scoped_ptr<HighResolutionTimerManager> hi_res_timer_manager_;
+  scoped_ptr<net::NetworkChangeNotifier> network_change_notifier_;
+  scoped_ptr<BrowserThread> main_thread_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserMainLoop);
+};
+
+// BrowserMainLoop construction / destruction ==================================
+
+BrowserMainLoop::BrowserMainLoop(const MainFunctionParams& parameters)
     : parameters_(parameters),
       parsed_command_line_(parameters.command_line_),
       result_code_(content::RESULT_CODE_NORMAL_EXIT) {
@@ -176,14 +272,33 @@ BrowserMainParts::BrowserMainParts(const MainFunctionParams& parameters)
 #endif
 }
 
-BrowserMainParts::~BrowserMainParts() {
+BrowserMainLoop::~BrowserMainLoop() {
+  // Destroy added parts in reverse order.
+  for (PartsList::reverse_iterator riter = parts_.rbegin();
+       riter != parts_.rend(); ++riter) {
+    delete *riter;
+  }
+  parts_.clear();
+
 #if defined(OS_WIN)
   OleUninitialize();
 #endif
 }
 
-void BrowserMainParts::EarlyInitialization() {
-  PreEarlyInitialization();
+void BrowserMainLoop::Init() {
+  content::GetContentClient()->browser()->CreateBrowserMainParts(
+      parameters_, parts_);
+}
+
+// BrowserMainLoop stages =====================================================
+
+void BrowserMainLoop::EarlyInitialization() {
+  for (PartsList::iterator iter = parts_.begin();
+       iter != parts_.end(); ++iter) {
+    (*iter)->PreEarlyInitialization();
+  }
+
+  // Start watching for jank during shutdown. It gets disarmed when
 
 #if defined(OS_WIN)
   net::EnsureWinsockInit();
@@ -192,15 +307,17 @@ void BrowserMainParts::EarlyInitialization() {
   // Use NSS for SSL by default.
   // The default client socket factory uses NSS for SSL by default on
   // Windows and Mac.
+  bool init_nspr = false;
 #if defined(OS_WIN) || defined(OS_MACOSX)
   if (parsed_command_line().HasSwitch(switches::kUseSystemSSL)) {
     net::ClientSocketFactory::UseSystemSSL();
   } else {
+    init_nspr = true;
+  }
 #elif defined(USE_NSS)
-  if (true) {
-#else
-  if (false) {
+  init_nspr = true;
 #endif
+  if (init_nspr) {
     // We want to be sure to init NSPR on the main thread.
     crypto::EnsureNSPRInit();
   }
@@ -225,11 +342,17 @@ void BrowserMainParts::EarlyInitialization() {
   if (parsed_command_line().HasSwitch(switches::kEnableTcpFastOpen))
     net::set_tcp_fastopen_enabled(true);
 
-  PostEarlyInitialization();
+  for (PartsList::iterator iter = parts_.begin();

[jam, 2011/10/24 19:36:00]

convention is usually to not use an iterator for a vector..

[stevenjb, 2011/10/25 02:51:04]

Interesting, I did not know that. That reduces flexibility and debug validity
checks, although it is easier to read. I didn't see that anywhere in our style
guide? Anyway, it's easy enough to change.

+       iter != parts_.end(); ++iter) {
+    (*iter)->PostEarlyInitialization();
+  }
 }
 
-void BrowserMainParts::MainMessageLoopStart() {
-  PreMainMessageLoopStart();
+void BrowserMainLoop::MainMessageLoopStart() {
+  for (PartsList::iterator iter = parts_.begin();
+       iter != parts_.end(); ++iter) {
+    (*iter)->PreMainMessageLoopStart();
+  }
 
 #if defined(OS_WIN)
   // If we're running tests (ui_task is non-null), then the ResourceBundle
@@ -249,27 +372,50 @@ void BrowserMainParts::MainMessageLoopStart() {
 
   network_change_notifier_.reset(net::NetworkChangeNotifier::Create());
 
-  PostMainMessageLoopStart();
+  for (PartsList::iterator iter = parts_.begin();
+       iter != parts_.end(); ++iter) {
+    (*iter)->PostMainMessageLoopStart();
+  }
 }
 
 static bool g_exited_main_message_loop = false;
 
-void BrowserMainParts::RunMainMessageLoopParts() {
-  PreMainMessageLoopRun();
+void BrowserMainLoop::RunMainMessageLoopParts() {
+  for (PartsList::iterator iter = parts_.begin();
+       iter != parts_.end(); ++iter) {
+    int result = (*iter)->PreMainMessageLoopRun();
+    if (result != content::RESULT_CODE_NORMAL_EXIT)
+      result_code_ = result;
+  }
 
   TRACE_EVENT_BEGIN_ETW("BrowserMain:MESSAGE_LOOP", 0, "");
   // If the UI thread blocks, the whole UI is unresponsive.
   // Do not allow disk IO from the UI thread.
   base::ThreadRestrictions::SetIOAllowed(false);
-  MainMessageLoopRun();
+
+  // Iterate through each of the parts. If any of them ran the main
+  // message loop then they should return |true|. Otherwise
+  // BrowserMainLoop::MainMessageLoopRun loop will be run.
+  bool ran_main_loop = false;
+  for (PartsList::iterator iter = parts_.begin();
+       iter != parts_.end(); ++iter) {
+    if ((*iter)->MainMessageLoopRun())
+      ran_main_loop = true;
+  }
+  if (!ran_main_loop)
+    MainMessageLoopRun();
+
   TRACE_EVENT_END_ETW("BrowserMain:MESSAGE_LOOP", 0, "");
 
   g_exited_main_message_loop = true;
 
-  PostMainMessageLoopRun();
+  for (PartsList::iterator iter = parts_.begin();
+       iter != parts_.end(); ++iter) {
+    (*iter)->PostMainMessageLoopRun();
+  }
 }
 
-void BrowserMainParts::InitializeMainThread() {
+void BrowserMainLoop::InitializeMainThread() {
   const char* kThreadName = "CrBrowserMain";
   base::PlatformThread::SetName(kThreadName);
   main_message_loop().set_thread_name(kThreadName);
@@ -283,11 +429,12 @@ void BrowserMainParts::InitializeMainThread() {
                                        MessageLoop::current()));
 }
 
-void BrowserMainParts::InitializeToolkit() {
+void BrowserMainLoop::InitializeToolkit() {
   // TODO(evan): this function is rather subtle, due to the variety
   // of intersecting ifdefs we have.  To keep it easy to follow, there
   // are no #else branches on any #ifs.
-
+  // TODO(stevenjb): Move platform specific code into platform specific Parts
+  // (Need to add InitializeToolkit stage to BrowserParts).
 #if defined(OS_LINUX)
   // We want to call g_thread_init(), but in some codepaths (tests) it
   // is possible it has already been called.  In older versions of
@@ -330,25 +477,13 @@ void BrowserMainParts::InitializeToolkit() {
     LOG_GETLASTERROR(FATAL);
 #endif
 
-  ToolkitInitialized();
-}
-
-void BrowserMainParts::PreEarlyInitialization() {
-}
-
-void BrowserMainParts::PostEarlyInitialization() {
-}
-
-void BrowserMainParts::PreMainMessageLoopStart() {
-}
-
-void BrowserMainParts::PostMainMessageLoopStart() {
-}
-
-void BrowserMainParts::PreMainMessageLoopRun() {
+  for (PartsList::iterator iter = parts_.begin();
+       iter != parts_.end(); ++iter) {
+    (*iter)->ToolkitInitialized();
+  }
 }
 
-void BrowserMainParts::MainMessageLoopRun() {
+void BrowserMainLoop::MainMessageLoopRun() {
   if (parameters().ui_task)
     MessageLoopForUI::current()->PostTask(FROM_HERE, parameters().ui_task);
 
@@ -359,12 +494,6 @@ void BrowserMainParts::MainMessageLoopRun() {
 #endif
 }
 
-void BrowserMainParts::PostMainMessageLoopRun() {
-}
-
-void BrowserMainParts::ToolkitInitialized() {
-}
-
 bool ExitedMainMessageLoop() {
   return g_exited_main_message_loop;
 }
@@ -377,18 +506,17 @@ int BrowserMain(const MainFunctionParams& parameters) {
 
   NotificationServiceImpl main_notification_service;
 
-  scoped_ptr<content::BrowserMainParts> parts(
-      content::GetContentClient()->browser()->CreateBrowserMainParts(
-          parameters));
-  if (!parts.get())
-    parts.reset(new content::BrowserMainParts(parameters));
+  scoped_ptr<content::BrowserMainLoop> main_loop(
+      new content::BrowserMainLoop(parameters));
 
-  parts->EarlyInitialization();
+  main_loop->Init();
+
+  main_loop->EarlyInitialization();
 
   // Must happen before we try to use a message loop or display any UI.
-  parts->InitializeToolkit();
+  main_loop->InitializeToolkit();
 
-  parts->MainMessageLoopStart();
+  main_loop->MainMessageLoopStart();
 
   // WARNING: If we get a WM_ENDSESSION, objects created on the stack here
   // are NOT deleted. If you need something to run during WM_ENDSESSION add it
@@ -434,8 +562,9 @@ int BrowserMain(const MainFunctionParams& parameters) {
 
   base::StatisticsRecorder statistics;
 
-  parts->RunMainMessageLoopParts();
+  main_loop->RunMainMessageLoopParts();
 
   TRACE_EVENT_END_ETW("BrowserMain", 0, 0);
-  return parts->result_code();
+
+  return main_loop->result_code();
 }