Remove GetActiveEntry usage from content.

content/browser/web_contents/web_contents_impl.cc

 // 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.
 
 #include "content/browser/web_contents/web_contents_impl.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
@@ skipping 856 matching lines @@
 
   renderer_preferences_.user_agent_override = override;
 
   // Send the new override string to the renderer.
   RenderViewHost* host = GetRenderViewHost();
   if (host)
     host->SyncRendererPrefs();
 
   // 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);
 
   FOR_EACH_OBSERVER(WebContentsObserver, observers_,
                     UserAgentOverrideSet(override));
 }
 
 const std::string& WebContentsImpl::GetUserAgentOverride() const {
   return renderer_preferences_.user_agent_override;
 }
@@ skipping 1017 matching lines @@
   if (!GetURL().is_valid())
     return;
   bool is_main_frame = (url == GetURL());
 
   DownloadManager* dlm =
       BrowserContext::GetDownloadManager(GetBrowserContext());
   if (!dlm)
     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();
   }
   scoped_ptr<DownloadUrlParameters> params(
       DownloadUrlParameters::FromWebContents(this, url));
   params->set_referrer(referrer);
   params->set_post_id(post_id);
   params->set_prefer_cache(true);
   if (post_id >= 0)
     params->set_method("POST");
   params->set_prompt(true);
   dlm->DownloadUrl(params.Pass());
 }
 
 void WebContentsImpl::GenerateMHTML(
     const base::FilePath& file,
     const base::Callback<void(const base::FilePath&, int64)>& callback) {
   MHTMLGenerationManager::GetInstance()->GenerateMHTML(this, file, callback);
 }
 
 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);
 }
 
 const std::string& WebContentsImpl::GetContentsMimeType() const {
   return contents_mime_type_;
 }
 
 bool WebContentsImpl::WillNotifyDisconnection() const {
@@ skipping 64 matching lines @@
       HostZoomMap::GetForBrowserContext(GetBrowserContext()));
   if (!zoom_map)
     return 0;
 
   double zoom_level;
   if (temporary_zoom_settings_) {
     zoom_level = zoom_map->GetTemporaryZoomLevel(
         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();
     zoom_level = zoom_map->GetZoomLevelForHostAndScheme(url.scheme(),
         net::GetHostOrSpecFromURL(url));
   }
   return zoom_level;
 }
 
 int WebContentsImpl::GetZoomPercent(bool* enable_increment,
                                     bool* enable_decrement) const {
   *enable_decrement = *enable_increment = false;
   // Calculate the zoom percent from the factor. Round up to the nearest whole
   // number.
   int percent = static_cast<int>(
       ZoomLevelToZoomFactor(GetZoomLevel()) * 100 + 0.5);
   *enable_decrement = percent > minimum_zoom_percent_;
   *enable_increment = percent < maximum_zoom_percent_;
   return percent;
 }
 
 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;
 
   delegate_->ViewSourceForTab(this, active_entry->GetURL());
 }
 
 void WebContentsImpl::ViewFrameSource(const GURL& url,
                                       const PageState& page_state) {
   if (!delegate_)
     return;
@@ skipping 40 matching lines @@
                                    uint32_t max_image_size,
                                    const ImageDownloadCallback& callback) {
   RenderViewHost* host = GetRenderViewHost();
   int id = StartDownload(
       host, url, is_favicon, preferred_image_size, max_image_size);
   image_download_map_[id] = callback;
   return id;
 }
 
 bool WebContentsImpl::FocusLocationBarByDefault() {
-  NavigationEntry* entry = controller_.GetActiveEntry();
+  NavigationEntry* entry = controller_.GetVisibleEntry();
   if (entry && entry->GetURL() == GURL(kAboutBlankURL))
     return true;
   return delegate_ && delegate_->ShouldFocusLocationBarByDefault(this);
 }
 
 void WebContentsImpl::SetFocusToLocationBar(bool select_all) {
   if (delegate_)
     delegate_->SetFocusToLocationBar(select_all);
 }
 
@@ skipping 735 matching lines @@
   if (static_cast<RenderViewHostImpl*>(render_view_host)->is_swapped_out())
     return;
 
   if (delegate_)
     view_->SetOverscrollControllerEnabled(delegate_->CanOverscrollContent());
 
   NotificationService::current()->Notify(
       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;
 
   // When we're creating views, we're still doing initial setup, so we always
   // use the pending Web UI rather than any possibly existing committed one.
   if (render_manager_.pending_web_ui())
     render_manager_.pending_web_ui()->RenderViewCreated(render_view_host);
 
   if (entry->IsViewSourceMode()) {
     // Put the renderer in view source mode.
@@ skipping 118 matching lines @@
     // For AUTO_SUBFRAME navigations, an event for the main frame is generated
     // that is not recorded in the navigation history. For the purpose of
     // tracking navigation events, we treat this event as a sub frame navigation
     // event.
     bool is_main_frame = did_navigate ? details.is_main_frame : false;
     PageTransition transition_type = params.transition;
     // Whether or not a page transition was triggered by going backward or
     // 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);
     }
     // Notify observers about the commit of the provisional load.
     FOR_EACH_OBSERVER(WebContentsObserver, observers_,
                       DidCommitProvisionalLoadForFrame(params.frame_id,
                       is_main_frame, params.url, transition_type, rvh));
   }
 
@@ skipping 402 matching lines @@
                                           const string16& source_id) {
   if (!delegate_)
     return false;
   return delegate_->AddMessageToConsole(this, level, message, line_no,
                                         source_id);
 }
 
 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);
 }
 
 int WebContentsImpl::CreateSwappedOutRenderView(
     SiteInstance* instance) {
   return render_manager_.CreateRenderView(instance, MSG_ROUTING_NONE, true);
 }
 
 void WebContentsImpl::OnUserGesture() {
   // Notify observers.
@@ skipping 281 matching lines @@
 }
 
 void WebContentsImpl::ClearAllPowerSaveBlockers() {
   for (PowerSaveBlockerMap::iterator i(power_save_blockers_.begin());
        i != power_save_blockers_.end(); ++i)
     STLDeleteValues(&power_save_blockers_[i->first]);
   power_save_blockers_.clear();
 }
 
 }  // namespace content