Rename WebContentsObserver::NavigateToPendingEntry to DidStartNavigationToPendingEntry.

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 62 matching lines @@
   // was shut down.
   virtual void RenderViewHostChanged(RenderViewHost* old_host,
                                      RenderViewHost* new_host) {}
 
   // This method is invoked after the WebContents decided which RenderViewHost
   // to use for the next navigation, but before the navigation starts.
   virtual void AboutToNavigateRenderView(
       RenderViewHost* render_view_host) {}
 
   // This method is invoked right after the navigation was initiated.
-  virtual void NavigateToPendingEntry(
+  virtual void DidCreatePendingEntry(

[Charlie Reis, 2013/11/22 22:09:25]

Sorry to possibly create more work for you, but I want to be sure we end up with
a good name.  As I think about it, there's a lot of things to consider here.

1) Looking at the call sites, it would be nice keep something about "navigation"
in the name, since it's not always obvious from context (e.g.,
FavIconTabHelper::DidCreatePendingEntry).  DidStartNavigation is one option, but
that's admittedly confusing next to DidStartProvisionalLoadForFrame.  Maybe
DidCreatePendingNavigationEntry, as you suggested.

2) Not every navigation goes through this path, and I'm not sure whether that
matters to overriders.  For example, most renderer-initiated navigations won't,
unless they punt to the browser via OpenURL.  Also, some will get here twice
(e.g., cross-process client-redirect).

3) I bet overriders don't realize that subframe navigations will soon pass
through here.  That's more likely to be a problem than (2), but we can ignore it
until we add the cross-process subframe logic.

4) DidCreatePendingEntry accurately conveys that it's for navigations that have
pending entries (which isn't all navigations, and may include subframes). 
However, the event is not fired immediately after creating the pending entry,
but rather after we've told the renderer to go there (possibly confusing).  I
suppose it's still part of the same task, though I wish the name conveyed that
we'd already kicked off the navigation.

5) There could be some confusion around the fact that we've started creating
pending entries for renderer-initiated navigations (in DidStartProvisionalLoad),
but we're not calling this in those cases.  Someone might look at this method
and wonder why we aren't calling it in that case.  That's actually a reasonable
concern, and it applies to the "DidCreatePendingNavigationEntry" name as well.

Other names that don't seem to fit those guidelines: DidStartPendingNavigation,
DidCreatePendingEntryForNavigation, ...

I wish we could just say DidNavigateToPendingEntry.  That's probably the most
accurate, though the "DidNavigate" part of it sounds a lot like
WebContents::DidNavigate, which means commit.  (Should we rename DidNavigate to
something like DidCommit when it moves to Navigator?)

Do you see any better suggestions?  I'm not sure which one to recommend so far.

       const GURL& url,
       NavigationController::ReloadType reload_type) {}
 
   // |render_view_host| is the RenderViewHost for which the provisional load is
   // happening. |frame_id| is a positive, non-zero integer identifying the
   // navigating frame in the given |render_view_host|. |parent_frame_id| is the
   // frame identifier of the frame containing the navigating frame, or -1 if the
   // frame is not contained in another frame.
   //
   // Since the URL validation will strip error URLs, or srcdoc URLs, the boolean
@@ skipping 244 matching lines @@
   void WebContentsImplDestroyed();
 
   WebContentsImpl* web_contents_;
 
   DISALLOW_COPY_AND_ASSIGN(WebContentsObserver);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_BROWSER_WEB_CONTENTS_OBSERVER_H_