Remove 2-stage RenderWidget initialization

content/public/browser/render_process_host.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CONTENT_PUBLIC_BROWSER_RENDER_PROCESS_HOST_H_
 #define CONTENT_PUBLIC_BROWSER_RENDER_PROCESS_HOST_H_
 
 #include <list>
 
 #include "base/basictypes.h"
@@ skipping 96 matching lines @@
 
   // Returns the storage partition associated with this process.
   //
   // TODO(nasko): Remove this function from the public API once
   // URLRequestContextGetter's creation is moved into StoragePartition.
   // http://crbug.com/158595
   virtual StoragePartition* GetStoragePartition() const = 0;
 
   // Try to shut down the associated renderer process without running unload
   // handlers, etc, giving it the specified exit code. If |wait| is true, wait
-  // for the process to be actually terminated before returning.
-  // Returns true if it was able to shut down.
+  // for the process to be actually terminated before returning.  Returns true
+  // if it was able to shut down.  On Windows, this must not be called before
+  // RenderProcessReady was called on a RenderProcessHostObserver, otherwise
+  // RenderProcessExited may never be called.

[ncarter (slow), 2015/09/29 18:28:40]

Is there a DCHECK that catches this case? (If not, this might be another reason
to consider adding an explicit render_process_ready_ bool to
RenderProcessHostImpl).

[piman, 2015/09/29 23:54:27]

There's not, and I considered it, but I did not add one. Calling Shutdown is
still valid even if the channel is not connected, the only thing is that we rely
on the channel error to report RenderProcessExited, and on Windows we only see a
channel error if the channel was connected. But Shutdown will still kill the
process (assuming it was started), and there is no difference if there is no
observer.

Note that I'm not changing behavior here (just documenting existing logic).

   virtual bool Shutdown(int exit_code, bool wait) = 0;
 
   // Try to shut down the associated renderer process as fast as possible.
   // If this renderer has any RenderViews with unload handlers, then this
   // function does nothing.
   // Returns true if it was able to do fast shutdown.
   virtual bool FastShutdownIfPossible() = 0;
 
   // Returns true if fast shutdown was started for the renderer.
   virtual bool FastShutdownStarted() const = 0;
 
   // Returns the process object associated with the child process.  In certain
   // tests or single-process mode, this will actually represent the current
   // process.
   //
   // NOTE: this is not necessarily valid immediately after calling Init, as
   // Init starts the process asynchronously.  It's guaranteed to be valid after
-  // the first IPC arrives.
+  // the first IPC arrives or RenderProcessReady was called on a
+  // RenderProcessHostObserver for this. At that point, IsReady() returns true.
   virtual base::ProcessHandle GetHandle() const = 0;
 
+  // Returns whether the process is ready. The process is ready once it is
+  // launched and the channel is connected. At that point, GetHandle() is valid,

[ncarter (slow), 2015/09/29 18:28:40]

I'm on the fence here, as to whether it's better to keep things simple and say
"Process is launched", or be more explicit, like, "the process launcher reported
a successful launch" or "the process launcher reported back a valid process
handle". With the simple wording it's not clear that the two conditions can
happen in either order. I'll leave it up to you.

[piman, 2015/09/29 23:54:27]

Ok, I clarified a bit in the latest PS.

+  // and deferred messages have been sent.
+  virtual bool IsReady() const = 0;
+
   // Returns the user browser context associated with this renderer process.
   virtual content::BrowserContext* GetBrowserContext() const = 0;
 
   // Returns whether this process is using the same StoragePartition as
   // |partition|.
   virtual bool InSameStoragePartition(StoragePartition* partition) const = 0;
 
   // Returns the unique ID for this child process host. This can be used later
   // in a call to FromID() to get back to this object (this is used to avoid
   // sending non-threadsafe pointers to other threads).
@@ skipping 184 matching lines @@
   static void SetMaxRendererProcessCount(size_t count);
 
   // Returns the current maximum number of renderer process hosts kept by the
   // content module.
   static size_t GetMaxRendererProcessCount();
 };
 
 }  // namespace content.
 
 #endif  // CONTENT_PUBLIC_BROWSER_RENDER_PROCESS_HOST_H_