Improve comments on WebContentsObserver RenderFrame* methods and rename.

content/public/browser/web_contents_observer.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_WEB_CONTENTS_OBSERVER_H_
 #define CONTENT_PUBLIC_BROWSER_WEB_CONTENTS_OBSERVER_H_
 
 #include "base/process/kill.h"
 #include "base/process/process_handle.h"
 #include "content/common/content_export.h"
@@ skipping 30 matching lines @@
 // RenderViewHost the observer cares about.
 //
 // Usually, observers should only care about the current RenderViewHost as
 // returned by GetRenderViewHost().
 //
 // TODO(creis, jochen): Hide the fact that there are several RenderViewHosts
 // from the WebContentsObserver API. http://crbug.com/173325
 class CONTENT_EXPORT WebContentsObserver : public IPC::Listener,
                                            public IPC::Sender {
  public:
-  // Called when a RenderFrameHost associated with this WebContents is created.
+  // Called when a RenderFrame corresponding to the |render_frame_host| is

[Charlie Reis, 2015/01/22 00:17:15]

nit: Called when a RenderFrame for |render_frame_host| is

Also add:
Use |RenderFrameDeleted| to listen for when this RenderFrame goes away.

[nasko, 2015/01/22 00:27:23]

Done.

+  // created in the renderer process.
   virtual void RenderFrameCreated(RenderFrameHost* render_frame_host) {}
 
-  // Called whenever a RenderFrameHost associated with this WebContents is
-  // deleted.
+  // Called when a RenderFrame corresponding to the |render_frame_host| is

[Charlie Reis, 2015/01/22 00:17:16]

Same nit, and mention RenderFrameCreated so that the two methods have references
to each other.  (Seems likely that people will usually want to override both,
not just one.)

[nasko, 2015/01/22 00:27:23]

Done.

+  // deleted or the renderer process in which it runs it has died.
   virtual void RenderFrameDeleted(RenderFrameHost* render_frame_host) {}
 
+  // This method is invoked whenever one of the current frames of a WebContents
+  // swaps its RenderFrameHost with another one; for example because that frame
+  // navigated and the new content is in a different process. The
+  // RenderFrameHost that has been replaced is in |old_host|, which can be
+  // nullptr if the old RenderFrameHost was shut down or a new frame has been
+  // created and no old RenderFrameHost exists.
+  //
+  // This method, in combination with FrameDeleted, is appropriate for

[Charlie Reis, 2015/01/22 00:17:16]

nit: |FrameDeleted|

[nasko, 2015/01/22 00:27:23]

Done.

+  // observers wishing to track the set of active RenderFrameHosts -- i.e.,
+  // those hosts that would be visited by calling WebContents::ForEachFrame.
+  virtual void RenderFrameHostChanged(RenderFrameHost* old_host,
+                                      RenderFrameHost* new_host) {}
+
+  // This method is invoked when a subframe associated with a WebContents is
+  // deleted or the WebContents is destroyed and the top-level frame is deleted.

[Charlie Reis, 2015/01/22 00:17:15]

Also mention |RenderFrameHostChanged|, so that the pair of methods have
references to each other.

[nasko, 2015/01/22 00:27:23]

Done.

+  virtual void FrameDeleted(RenderFrameHost* render_frame_host) {}
+
   // This is called when a RVH is created for a WebContents, but not if it's an
   // interstitial.
   virtual void RenderViewCreated(RenderViewHost* render_view_host) {}
 
   // Called for every RenderFrameHost that's created for an interstitial.
   virtual void RenderFrameForInterstitialPageCreated(
       RenderFrameHost* render_frame_host) {}
 
   // This method is invoked when the RenderView of the current RenderViewHost
   // is ready, e.g. because we recreated it after a crash.
@@ skipping 14 matching lines @@
   // RenderProcessHostObserver::RenderProcessExited().
   virtual void RenderProcessGone(base::TerminationStatus status) {}
 
   // This method is invoked when a WebContents swaps its visible RenderViewHost
   // with another one, possibly changing processes. The RenderViewHost that has
   // been replaced is in |old_host|, which is nullptr if the old RVH was shut
   // down.
   virtual void RenderViewHostChanged(RenderViewHost* old_host,
                                      RenderViewHost* new_host) {}
 
-  // This method is invoked whenever one of the current frames of a WebContents
-  // swaps its RenderFrameHost with another one; for example because that frame
-  // navigated and the new content is in a different process. The
-  // RenderFrameHost that has been replaced is in |old_host|, which can be
-  // nullptr if the old RFH was shut down.
-  //
-  // This method, in combination with RenderFrameDeleted, is appropriate for
-  // observers wishing to track the set of active RenderFrameHosts -- i.e.,
-  // those hosts that would be visited by calling WebContents::ForEachFrame.
-  virtual void RenderFrameHostChanged(RenderFrameHost* old_host,
-                                      RenderFrameHost* new_host) {}
-
   // This method is invoked after the WebContents decides which RenderFrameHost
   // to use for the next browser-initiated navigation, but before the navigation
   // starts.  It is not called for most renderer-initiated navigations, and it
   // does not guarantee that the navigation will commit (e.g., 204s, downloads).
   //
   // DEPRECATED.  This method is difficult to use correctly and should be
   // removed.  TODO(creis): Remove in http://crbug.com/424641.
   virtual void AboutToNavigateRenderFrame(RenderFrameHost* render_frame_host) {}
 
   // This method is invoked after the browser process starts a navigation to a
@@ skipping 106 matching lines @@
   // an action in the observed WebContents, e.g. a link with target=_blank was
   // clicked. The |source_render_frame_host| is the frame in which the action
   // took place.
   virtual void DidOpenRequestedURL(WebContents* new_contents,
                                    RenderFrameHost* source_render_frame_host,
                                    const GURL& url,
                                    const Referrer& referrer,
                                    WindowOpenDisposition disposition,
                                    ui::PageTransition transition) {}
 
-  virtual void FrameDetached(RenderFrameHost* render_frame_host) {}
-
-  // This method is invoked when the renderer has completed its first paint
-  // after a non-empty layout.
+  // This method is invoked when the renderer process has completed its first
+  // 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(RenderViewHost* render_view_host) {}
   virtual void DidStopLoading(RenderViewHost* render_view_host) {}
 
   // 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.
@@ skipping 13 matching lines @@
   // This methods is invoked when the title of the WebContents is set. If the
   // title was explicitly set, |explicit_set| is true, otherwise the title was
   // synthesized and |explicit_set| is false.
   virtual void TitleWasSet(NavigationEntry* entry, bool explicit_set) {}
 
   virtual void AppCacheAccessed(const GURL& manifest_url,
                                 bool blocked_by_policy) {}
 
   // Notification that a plugin has crashed.
   // |plugin_pid| is the process ID identifying the plugin process. Note that
-  // this ID is supplied by the renderer, so should not be trusted. Besides, the
-  // corresponding process has probably died at this point. The ID may even have
-  // been reused by a new process.
+  // this ID is supplied by the renderer process, so should not be trusted.
+  // Besides, the corresponding process has probably died at this point. The ID
+  // may even have been reused by a new process.
   virtual void PluginCrashed(const base::FilePath& plugin_path,
                              base::ProcessId plugin_pid) {}
 
   // Notification that the given plugin has hung or become unhung. This
   // notification is only for Pepper plugins.
   //
   // The plugin_child_id is the unique child process ID from the plugin. Note
-  // that this ID is supplied by the renderer, so should be validated before
-  // it's used for anything in case there's an exploited renderer.
+  // that this ID is supplied by the renderer process, so should be validated
+  // before it's used for anything in case there's an exploited renderer
+  // process.
   virtual void PluginHungStatusChanged(int plugin_child_id,
                                        const base::FilePath& plugin_path,
                                        bool is_hung) {}
 
   // Invoked when WebContents::Clone() was used to clone a WebContents.
   virtual void DidCloneToNewWebContents(WebContents* old_web_contents,
                                         WebContents* new_web_contents) {}
 
   // Invoked when the WebContents is being destroyed. Gives subclasses a chance
   // to cleanup. After the whole loop over all WebContentsObservers has been
   // finished, web_contents() returns nullptr.
   virtual void WebContentsDestroyed() {}
 
   // Called when the user agent override for a WebContents has been changed.
   virtual void UserAgentOverrideSet(const std::string& user_agent) {}
 
-  // Invoked when new FaviconURL candidates are received from the renderer.
+  // Invoked when new FaviconURL candidates are received from the renderer
+  // process.
   virtual void DidUpdateFaviconURL(const std::vector<FaviconURL>& candidates) {}
 
   // Invoked when a pepper plugin creates and shows or destroys a fullscreen
-  // render widget.
+  // RenderWidget.
   virtual void DidShowFullscreenWidget(int routing_id) {}
   virtual void DidDestroyFullscreenWidget(int routing_id) {}
 
-  // Invoked when the renderer has toggled the tab into/out of fullscreen mode.
+  // Invoked when the renderer process has toggled the tab into/out of
+  // fullscreen mode.
   virtual void DidToggleFullscreenModeForTab(bool entered_fullscreen) {}
 
   // Invoked when an interstitial page is attached or detached.
   virtual void DidAttachInterstitialPage() {}
   virtual void DidDetachInterstitialPage() {}
 
   // Invoked before a form repost warning is shown.
   virtual void BeforeFormRepostWarningShow() {}
 
-  // Invoked when the beforeunload handler fires. The time is from the renderer.
+  // Invoked when the beforeunload handler fires. The time is from the renderer
+  // process.
   virtual void BeforeUnloadFired(const base::TimeTicks& proceed_time) {}
 
   // Invoked when a user cancels a before unload dialog.
   virtual void BeforeUnloadDialogCancelled() {}
 
-  // Invoked when an accessibility event is received from the renderer.
+  // Invoked when an accessibility event is received from the renderer process.
   virtual void AccessibilityEventReceived(
       const std::vector<AXEventNotificationDetails>& details) {}
 
   // Invoked when theme color is changed to |theme_color|.
   virtual void DidChangeThemeColor(SkColor theme_color) {}
 
   // Invoked if an IPC message is coming from a specific RenderFrameHost.
   virtual bool OnMessageReceived(const IPC::Message& message,
                                  RenderFrameHost* render_frame_host);
 
@@ skipping 27 matching lines @@
   void ResetWebContents();
 
   WebContentsImpl* web_contents_;
 
   DISALLOW_COPY_AND_ASSIGN(WebContentsObserver);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_BROWSER_WEB_CONTENTS_OBSERVER_H_