Fix uses of NavigationEntries in RenderViewContextMenu.

chrome/browser/renderer_context_menu/render_view_context_menu.cc

Index: chrome/browser/renderer_context_menu/render_view_context_menu.cc
diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu.cc b/chrome/browser/renderer_context_menu/render_view_context_menu.cc
index 406a8f0000a1102c53f61516316a31f926e5d8ea..40deb6bec96c59d5e4288a91322084e5b13eebc9 100644
--- a/chrome/browser/renderer_context_menu/render_view_context_menu.cc
+++ b/chrome/browser/renderer_context_menu/render_view_context_menu.cc
@@ -1146,7 +1146,7 @@ bool RenderViewContextMenu::IsCommandIdEnabled(int id) const {
       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)
@@ -1282,15 +1282,11 @@ bool RenderViewContextMenu::IsCommandIdEnabled(int id) const {
       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:
@@ -1685,8 +1681,10 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) {
     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_,