Fix uses of NavigationEntries in RenderViewContextMenu.

chrome/browser/renderer_context_menu/render_view_context_menu.cc

 // Copyright 2014 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 "chrome/browser/renderer_context_menu/render_view_context_menu.h"
 
 #include <algorithm>
 #include <set>
 #include <utility>
 
@@ skipping 1128 matching lines @@
     case IDC_CONTENT_CONTEXT_VIEWFRAMESOURCE:
       return source_web_contents_->GetController().CanViewSource();
 
     case IDC_CONTENT_CONTEXT_INSPECTELEMENT:
     case IDC_CONTENT_CONTEXT_INSPECTBACKGROUNDPAGE:
     case IDC_CONTENT_CONTEXT_RELOAD_PACKAGED_APP:
     case IDC_CONTENT_CONTEXT_RESTART_PACKAGED_APP:
       return IsDevCommandEnabled(id);
 
     case IDC_CONTENT_CONTEXT_VIEWPAGEINFO:
-      if (source_web_contents_->GetController().GetActiveEntry() == NULL)
+      if (source_web_contents_->GetController().GetVisibleEntry() == NULL)

[Charlie Reis, 2014/05/09 22:48:50]

This needed to match the VIEWPAGEINFO case below, so that we don't show "View
Page Info" when there's no visible entry (e.g., during a spoof attempt).

         return false;
       // Disabled if no browser is associated (e.g. desktop notifications).
       if (chrome::FindBrowserWithWebContents(source_web_contents_) == NULL)
         return false;
       return true;
 
     case IDC_CONTENT_CONTEXT_TRANSLATE: {
       TranslateTabHelper* translate_tab_helper =
           TranslateTabHelper::FromWebContents(source_web_contents_);
       if (!translate_tab_helper)
@@ skipping 115 matching lines @@
       if (core_delegate &&
           !core_delegate->CanSaveContents(source_web_contents_))
         return false;
 
       PrefService* local_state = g_browser_process->local_state();
       DCHECK(local_state);
       // Test if file-selection dialogs are forbidden by policy.
       if (!local_state->GetBoolean(prefs::kAllowFileSelectionDialogs))
         return false;
 
-      // Instead of using GetURL here, we use url() (which is the "real" url of

[Charlie Reis, 2014/05/09 22:48:50]

John: It looks like you changed this behavior in http://crrev.com/115766 but
didn't update the comment.  I'm not sure if it has been buggy since that change,
but the code matches the comment now.

[jam, 2014/05/09 23:06:54]

I just looked at that cl, GetURL() there returned url_, which is what url() used
to do before. so i think that old comment was wrong?

[Charlie Reis, 2014/05/09 23:34:43]

Huh.  That confused me as well, but now I think "GetURL()" in the old comment
referred to TabContents::GetURL(), which returned the virtual URL.

-      // the page) from the NavigationEntry because its reflects their origin
-      // rather than the display one (returned by GetURL) which may be
-      // different (like having "view-source:" on the front).
-      // TODO(nasko): Audit all GetActiveEntry calls in this file.

[Charlie Reis, 2014/05/09 22:48:50]

Nasko: I think this takes care of the rest of them.

-      NavigationEntry* active_entry =
-          source_web_contents_->GetController().GetActiveEntry();
-      return content::IsSavableURL(
-          (active_entry) ? active_entry->GetURL() : GURL());
+      // We save the last committed entry (which the user is looking at), as

[Charlie Reis, 2014/05/09 22:48:50]

I think it makes sense to save the page you're viewing rather than the one
that's pending, since you got to it by right clicking.

[nasko, 2014/05/09 22:56:08]

Agreed.

+      // opposed to any pending URL that hasn't committed yet.
+      NavigationEntry* entry =
+          source_web_contents_->GetController().GetLastCommittedEntry();
+      return content::IsSavableURL(entry ? entry->GetURL() : GURL());
     }
 
     case IDC_CONTENT_CONTEXT_RELOADFRAME:
       return params_.frame_url.is_valid();
 
     case IDC_CONTENT_CONTEXT_UNDO:
       return !!(params_.edit_flags & WebContextMenuData::CanUndo);
 
     case IDC_CONTENT_CONTEXT_REDO:
       return !!(params_.edit_flags & WebContextMenuData::CanRedo);
@@ skipping 374 matching lines @@
       DCHECK(platform_app);
       DCHECK(platform_app->is_platform_app());
 
       extensions::devtools_util::InspectBackgroundPage(platform_app, profile_);
       break;
     }
 
     case IDC_CONTENT_CONTEXT_VIEWPAGEINFO: {
       NavigationController* controller = &source_web_contents_->GetController();
       // Important to use GetVisibleEntry to match what's showing in the
-      // omnibox.
+      // omnibox.  This may return null.
       NavigationEntry* nav_entry = controller->GetVisibleEntry();
+      if (!nav_entry)
+        return;
       Browser* browser =
           chrome::FindBrowserWithWebContents(source_web_contents_);
       chrome::ShowWebsiteSettings(browser, source_web_contents_,
                                   nav_entry->GetURL(), nav_entry->GetSSL());
       break;
     }
 
     case IDC_CONTENT_CONTEXT_TRANSLATE: {
       // A translation might have been triggered by the time the menu got
       // selected, do nothing in that case.
@@ skipping 267 matching lines @@
   source_web_contents_->GetRenderViewHost()->
       ExecuteMediaPlayerActionAtLocation(location, action);
 }
 
 void RenderViewContextMenu::PluginActionAt(
     const gfx::Point& location,
     const WebPluginAction& action) {
   source_web_contents_->GetRenderViewHost()->
       ExecutePluginActionAtLocation(location, action);
 }