Rename WebContentsObserver::NavigateToPendingEntry to DidStartNavigationToPendingEntry.

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 fbc5206f1f90af2ef4534942ae83bfe8d38a3774..7204f1d6476bc5bd87e855e7e64d682ccd8d6533 100644
--- a/content/public/browser/web_contents_observer.h
+++ b/content/public/browser/web_contents_observer.h
@@ -80,7 +80,7 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener,
       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) {}