Remove GetActiveEntry usage from content.

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index cac53853a7d6fb2abb9a5c78395cd596b253da5b..293ce684403985eabe7c3d8c9f47f6e8f75653c8 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -874,7 +874,7 @@ void WebContentsImpl::SetUserAgentOverride(const std::string& override) {
 
   // Reload the page if a load is currently in progress to avoid having
   // different parts of the page loaded using different user agents.
-  NavigationEntry* entry = controller_.GetActiveEntry();
+  NavigationEntry* entry = controller_.GetVisibleEntry();
   if (is_loading_ && entry != NULL && entry->GetIsOverridingUserAgent())
     controller_.ReloadIgnoringCache(true);
 
@@ -1912,7 +1912,7 @@ void WebContentsImpl::SaveFrame(const GURL& url,
     return;
   int64 post_id = -1;
   if (is_main_frame) {
-    const NavigationEntry* entry = controller_.GetActiveEntry();
+    const NavigationEntry* entry = controller_.GetVisibleEntry();

[Charlie Reis, 2013/08/16 22:10:14]

I have similar concerns here as with the SavePackage stuff.  Doesn't that refer
to the last committed page?

[nasko, 2013/09/12 17:56:31]

Done.

     if (entry)
       post_id = entry->GetPostID();
   }
@@ -1935,7 +1935,7 @@ void WebContentsImpl::GenerateMHTML(
 
 bool WebContentsImpl::IsActiveEntry(int32 page_id) {

[Charlie Reis, 2013/08/16 22:10:14]

We should at least have a TODO to change this to IsVisibleEntry.

[nasko, 2013/09/12 17:56:31]

Done.

   NavigationEntryImpl* active_entry =
-      NavigationEntryImpl::FromNavigationEntry(controller_.GetActiveEntry());
+      NavigationEntryImpl::FromNavigationEntry(controller_.GetVisibleEntry());
   return (active_entry != NULL &&
           active_entry->site_instance() == GetSiteInstance() &&
           active_entry->GetPageID() == page_id);
@@ -2020,7 +2020,7 @@ double WebContentsImpl::GetZoomLevel() const {
         GetRenderProcessHost()->GetID(), GetRenderViewHost()->GetRoutingID());
   } else {
     GURL url;
-    NavigationEntry* active_entry = GetController().GetActiveEntry();
+    NavigationEntry* active_entry = GetController().GetVisibleEntry();

[Charlie Reis, 2013/08/16 22:10:14]

nit: visible_entry

Actually, this is broken.  It's using the zoom level from the current process,
which is the last committed entry.

[nasko, 2013/09/12 17:56:31]

Done.

     // Since zoom map is updated using rewritten URL, use rewritten URL
     // to get the zoom level.
     url = active_entry ? active_entry->GetURL() : GURL::EmptyGURL();
@@ -2046,7 +2046,7 @@ void WebContentsImpl::ViewSource() {
   if (!delegate_)
     return;
 
-  NavigationEntry* active_entry = GetController().GetActiveEntry();
+  NavigationEntry* active_entry = GetController().GetVisibleEntry();

[Charlie Reis, 2013/08/16 22:10:14]

nit: visible_entry

Is this correct?  I'm seeing a view source of the last committed page even when
there's a pending entry, so I'm not sure how this works.

[nasko, 2013/09/12 17:56:31]

The URL passed in isn't used. Eventually down the road, we end up calling
GetLastCommittedURL, so that's why it works. I've made those two match.

   if (!active_entry)
     return;
 
@@ -2107,7 +2107,7 @@ int WebContentsImpl::DownloadImage(const GURL& url,
 }
 
 bool WebContentsImpl::FocusLocationBarByDefault() {
-  NavigationEntry* entry = controller_.GetActiveEntry();
+  NavigationEntry* entry = controller_.GetVisibleEntry();
   if (entry && entry->GetURL() == GURL(kAboutBlankURL))
     return true;
   return delegate_ && delegate_->ShouldFocusLocationBarByDefault(this);
@@ -2863,7 +2863,7 @@ void WebContentsImpl::RenderViewCreated(RenderViewHost* render_view_host) {
       NOTIFICATION_WEB_CONTENTS_RENDER_VIEW_HOST_CREATED,
       Source<WebContents>(this),
       Details<RenderViewHost>(render_view_host));
-  NavigationEntry* entry = controller_.GetActiveEntry();
+  NavigationEntry* entry = controller_.GetVisibleEntry();

[Charlie Reis, 2013/08/16 22:10:14]

Is this buggy?  Depends on whether render_view_host always matches entry, since
we're using entry below to decide what to do with render_view_host.

[nasko, 2013/09/12 17:56:31]

I'm not sure I follow your comment, but the code has changed and the entry is
only used to judge view-source mode. We can't be in view-source for pending
entries, hence I'm changing this to be LastCommitted.

   if (!entry)
     return;
 
@@ -3002,7 +3002,7 @@ void WebContentsImpl::DidNavigate(
     // forward in the history is only stored in the navigation controller's
     // entry list.
     if (did_navigate &&
-        (controller_.GetActiveEntry()->GetTransitionType() &
+        (controller_.GetVisibleEntry()->GetTransitionType() &

[Charlie Reis, 2013/08/16 22:10:14]

Can this ever be different from GetLastCommittedEntry, given that we just
committed?  (Maybe for a transient entry?)

[nasko, 2013/09/12 17:56:31]

I think you are right and it can't be different if we just committed.
Furthermore, I don't see how we can have a transient entry at the time of
commit. In case of SSL warnings, it is displayed before we even complete the
connection and send the request, so to even receive a response (aka commit), we
need to have agreed on the interstitial page. In the case of safe browsing, we
need to have loaded the content and check it, which is also post-commit.

             PAGE_TRANSITION_FORWARD_BACK)) {
       transition_type = PageTransitionFromInt(
           params.transition | PAGE_TRANSITION_FORWARD_BACK);
@@ -3425,8 +3425,8 @@ bool WebContentsImpl::AddMessageToConsole(int32 level,
 WebPreferences WebContentsImpl::GetWebkitPrefs() {
   // We want to base the page config off of the real URL, rather than the
   // display URL.
-  GURL url = controller_.GetActiveEntry()
-      ? controller_.GetActiveEntry()->GetURL() : GURL::EmptyGURL();
+  GURL url = controller_.GetVisibleEntry()

[Charlie Reis, 2013/08/16 22:10:14]

This looks buggy, since GetVisibleEntry does not match GetRenderViewHost.

[nasko, 2013/09/12 17:56:31]

Done.

+      ? controller_.GetVisibleEntry()->GetURL() : GURL::EmptyGURL();
   return GetWebkitPrefs(GetRenderViewHost(), url);
 }