Remove WebContentsObserver::DidCommitNavigation

content/public/browser/web_contents_observer.h

Index: content/public/browser/web_contents_observer.h
diff --git a/content/public/browser/web_contents_observer.h b/content/public/browser/web_contents_observer.h
index 6a2b1acdd7976fa13a5855cae2faa574cabb1530..aa8f07343ff901c4885d35218b95ee8c6ffe5186 100644
--- a/content/public/browser/web_contents_observer.h
+++ b/content/public/browser/web_contents_observer.h
@@ -122,8 +122,8 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener,
   // Called when a navigation started in the WebContents. |navigation_handle|
   // is unique to a specific navigation. The same |navigation_handle| will be
   // provided on subsequent calls to
-  // DidRedirect/Commit/FinishNavigation/ReadyToCommitNavigation related to
-  // this navigation.
+  // DidRedirect/FinishNavigation/ReadyToCommitNavigation related to this
+  // navigation.
   //
   // Note that this is fired by navigations in any frame of the WebContents,
   // not just the main frame.
@@ -141,22 +141,64 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener,
   virtual void DidRedirectNavigation(NavigationHandle* navigation_handle) {}
 
   // PlzNavigate
-  // Called when the navigation is ready to be committed in a renderer. This is
-  // the first point in time where a RenderFrameHost is associated with the
-  // navigation. Observers that want to initialize any renderer side
-  // structures/state before the RenderFrame is navigated, should use this
-  // method as opposed to DidCommitNavigation, which is after the fact.
+  // Called when the navigation is ready to be committed in a renderer. Most
+  // observers should use DidFinishNavigation instead, which happens right
+  // after the navigation commits. This method is for observers that want to
+  // initialize renderer-side state just before the RenderFrame commits the
+  // navigation.
+  //
+  // This is the first point in time where a RenderFrameHost is associated with
+  // the navigation.
   virtual void ReadyToCommitNavigation(NavigationHandle* navigation_handle) {}
 
-  // Called when a navigation was committed.
-  virtual void DidCommitNavigation(NavigationHandle* navigation_handle) {}
-
-  // Called when a navigation stopped in the WebContents. This happens when a
-  // navigation is either aborted, replaced by a new one, or the document load
-  // finishes. Note that |navigation_handle| will be destroyed at the end of
-  // this call, so do not keep a reference to it afterward.
+  // Called when a navigation finished in the WebContents. This happens when a
+  // navigation is committed, aborted or replaced by a new one. Note that

[Charlie Reis, 2015/09/21 16:40:42]

Please mention NavigationHandle's HasCommitted and IsErrorPage methods as a way
to tell the difference between these cases, since almost all users will need to
check them.

[clamy, 2015/09/22 00:38:05]

Done.

+  // |navigation_handle| will be destroyed at the end of this call, so do not
+  // keep a reference to it afterward.
+  // If this is called because the navigation committed, then the document load

[Charlie Reis, 2015/09/21 16:40:42]

nit: Add an empty comment line above this paragraph, similar to line 149.

[clamy, 2015/09/22 00:38:05]

Done.

+  // will still be ongoing in the RenderFrameHost returned by
+  // |navigation_handle|. Use the document loads events such as DidStopLoading
+  // and related methods to listen for continued events from this
+  // RenderFrameHost.
   virtual void DidFinishNavigation(NavigationHandle* navigation_handle) {}
 
+  // Document load events ------------------------------------------------------
+
+  // These two methods correspond to the points in time when the spinner of the
+  // tab starts and stops spinning.
+  virtual void DidStartLoading() {}
+  virtual void DidStopLoading() {}
+
+  // This method is invoked once the window.document object of the main frame
+  // was created.
+  virtual void DocumentAvailableInMainFrame() {}
+
+  // This method is invoked once the onload handler of the main frame has
+  // completed.
+  virtual void DocumentOnLoadCompletedInMainFrame() {}
+
+  // This method is invoked when the document in the given frame finished
+  // loading. At this point, scripts marked as defer were executed, and
+  // content scripts marked "document_end" get injected into the frame.
+  virtual void DocumentLoadedInFrame(RenderFrameHost* render_frame_host) {}
+
+  // This method is invoked when the navigation is done, i.e. the spinner of
+  // the tab will stop spinning, and the onload event was dispatched.
+  //
+  // If the WebContents is displaying replacement content, e.g. network error
+  // pages, DidFinishLoad is invoked for frames that were not sending
+  // navigational events before. It is safe to ignore these events.
+  virtual void DidFinishLoad(RenderFrameHost* render_frame_host,
+                             const GURL& validated_url) {}
+
+  // This method is like DidFinishLoad, but when the load failed or was
+  // cancelled, e.g. window.stop() is invoked.
+  virtual void DidFailLoad(RenderFrameHost* render_frame_host,
+                           const GURL& validated_url,
+                           int error_code,
+                           const base::string16& error_description,
+                           bool was_ignored_by_handler) {}
+
   // ---------------------------------------------------------------------------
 
   // This method is invoked after the WebContents decides which RenderFrameHost
@@ -233,36 +275,6 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener,
       SecurityStyle security_style,
       const SecurityStyleExplanations& security_style_explanations) {}
 
-  // This method is invoked once the window.document object of the main frame
-  // was created.
-  virtual void DocumentAvailableInMainFrame() {}
-
-  // This method is invoked once the onload handler of the main frame has
-  // completed.
-  virtual void DocumentOnLoadCompletedInMainFrame() {}
-
-  // This method is invoked when the document in the given frame finished
-  // loading. At this point, scripts marked as defer were executed, and
-  // content scripts marked "document_end" get injected into the frame.
-  virtual void DocumentLoadedInFrame(RenderFrameHost* render_frame_host) {}
-
-  // This method is invoked when the navigation is done, i.e. the spinner of
-  // the tab will stop spinning, and the onload event was dispatched.
-  //
-  // If the WebContents is displaying replacement content, e.g. network error
-  // pages, DidFinishLoad is invoked for frames that were not sending
-  // navigational events before. It is safe to ignore these events.
-  virtual void DidFinishLoad(RenderFrameHost* render_frame_host,
-                             const GURL& validated_url) {}
-
-  // This method is like DidFinishLoad, but when the load failed or was
-  // cancelled, e.g. window.stop() is invoked.
-  virtual void DidFailLoad(RenderFrameHost* render_frame_host,
-                           const GURL& validated_url,
-                           int error_code,
-                           const base::string16& error_description,
-                           bool was_ignored_by_handler) {}
-
   // This method is invoked when content was loaded from an in-memory cache.
   virtual void DidLoadResourceFromMemoryCache(
       const LoadFromMemoryCacheDetails& details) {}
@@ -300,11 +312,6 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener,
   // paint after a non-empty layout.
   virtual void DidFirstVisuallyNonEmptyPaint() {}
 
-  // These two methods correspond to the points in time when the spinner of the
-  // tab starts and stops spinning.
-  virtual void DidStartLoading() {}
-  virtual void DidStopLoading() {}
-
   // When WebContents::Stop() is called, the WebContents stops loading and then
   // invokes this method. If there are ongoing navigations, their respective
   // failure methods will also be invoked.