Change NavigationEntry to use virtual URL in error pages for blocked navigations

chrome/browser/extensions/navigation_observer.cc

Index: chrome/browser/extensions/navigation_observer.cc
diff --git a/chrome/browser/extensions/navigation_observer.cc b/chrome/browser/extensions/navigation_observer.cc
index 730ddcdbeee52c7758411e9d5c0acd5131a8254b..948d8bcb03a1b649a850579254e807fce0cdcf05 100644
--- a/chrome/browser/extensions/navigation_observer.cc
+++ b/chrome/browser/extensions/navigation_observer.cc
@@ -69,7 +69,14 @@ void NavigationObserver::PromptToEnableExtensionIfNecessary(
   if (!nav_entry)
     return;
 
-  const GURL& url = nav_entry->GetURL();
+  // With a disabled extension, the navigation is blocked, which results in
+  // a NavigationEntry with "about:blank" URL. In this case, the virtual
+  // URL is the real URL the user navigated to, so if it is non-empty, use
+  // it instead.
+  const GURL& url = nav_entry->GetVirtualURL().is_empty()

[Charlie Reis, 2017/05/24 21:50:52]

This condition makes me nervous.  If GetURL is an extension URL and
GetVirtualURL is not, then we're potentially breaking the code below.  I know
most component extensions won't be disabled, but I'm not 100% sure that no
extension that can be disabled has a non-extension virtual URL.

Maybe we should only use the virtual URL (instead of GetURL) if the virtual URL
is an extension and GetURL() is about:blank, to limit to this case?

And/or your idea of checking if nav_entry has an error in GetPageType?

[nasko, 2017/05/24 22:16:41]

Fixed by combining all 3 checks.

+                        ? nav_entry->GetURL()
+                        : nav_entry->GetVirtualURL();
+
   // NOTE: We only consider chrome-extension:// urls, and deliberately don't
   // consider hosted app urls. This is because it's really annoying to visit the
   // site associated with a hosted app (like calendar.google.com or