Move extensions code from WebContents::GetURL to GetLastCommittedURL.

chrome/browser/extensions/extension_tab_util.h

Index: chrome/browser/extensions/extension_tab_util.h
diff --git a/chrome/browser/extensions/extension_tab_util.h b/chrome/browser/extensions/extension_tab_util.h
index fe47b014c11a4ce3153396aaf483e1ce962c0f0b..ba27c83bbc6a796f091aae14d87675a2c10f0f4f 100644
--- a/chrome/browser/extensions/extension_tab_util.h
+++ b/chrome/browser/extensions/extension_tab_util.h
@@ -84,6 +84,8 @@ class ExtensionTabUtil {
       int window_id,
       std::string* error_message);
 
+  enum UrlType { LastCommitted, Visible };

[nasko, 2016/10/20 00:48:42]

This along with the methods taking it can potentially move into the anonymous
namespace in the .cc file. I did hit some linker errors when I tried that, but
didn't dig further.

[Devlin, 2016/10/20 17:27:24]

I'm fine with it here (and think it should be; see below).  But we should use an
enum class, and unfortunately chrome requires SCREAMING_STYLE enums (even though
it hurts my ears).

+
   // Creates a Tab object (see chrome/common/extensions/api/tabs.json) with
   // information about the state of a browser tab.  Depending on the
   // permissions of the extension, the object may or may not include sensitive
@@ -91,24 +93,48 @@ class ExtensionTabUtil {
   static std::unique_ptr<api::tabs::Tab> CreateTabObject(
       content::WebContents* web_contents,
       const Extension* extension) {
-    return CreateTabObject(web_contents, nullptr, -1, extension);
+    return CreateTabObject(web_contents, nullptr, -1, extension,
+                           UrlType::LastCommitted);
   }
   static std::unique_ptr<api::tabs::Tab> CreateTabObject(
       content::WebContents* web_contents,
       TabStripModel* tab_strip,
       int tab_index,
-      const Extension* extension);
+      const Extension* extension) {
+    return CreateTabObject(web_contents, nullptr, -1, extension,
+                           UrlType::LastCommitted);
+  }
+  static std::unique_ptr<api::tabs::Tab> CreateTabObjectVisibleUrl(

[Devlin, 2016/10/20 17:27:24]

In the tabs API, we'll also scrub the tab object of private data (like urls and
favicons) if the extension doesn't have the "tabs" permission. As such, I think
in this context, "VisibleUrl" is a little ambiguous (visible vs last committed,
or visible meaning present?).  Overall, I'd probably just recommend using the
overload that takes the UrlType in the places we want to specify using the
visible url, rather than adding this separate method.

+      content::WebContents* web_contents,
+      const Extension* extension) {
+    return CreateTabObject(web_contents, nullptr, -1, extension,
+                           UrlType::Visible);
+  }
+  static std::unique_ptr<api::tabs::Tab> CreateTabObject(
+      content::WebContents* web_contents,
+      TabStripModel* tab_strip,
+      int tab_index,
+      const Extension* extension,
+      UrlType url_type);
 
   // Creates a Tab object but performs no extension permissions checks; the
   // returned object will contain privacy-sensitive data.
   static std::unique_ptr<api::tabs::Tab> CreateTabObject(
       content::WebContents* web_contents) {
-    return CreateTabObject(web_contents, nullptr, -1);
+    return CreateTabObject(web_contents, nullptr, -1, UrlType::LastCommitted);
+  }
+  static std::unique_ptr<api::tabs::Tab> CreateTabObject(
+      content::WebContents* web_contents,
+      TabStripModel* tab_strip,
+      int tab_index) {
+    return CreateTabObject(web_contents, tab_strip, tab_index,
+                           UrlType::LastCommitted);
   }
   static std::unique_ptr<api::tabs::Tab> CreateTabObject(
       content::WebContents* web_contents,
       TabStripModel* tab_strip,
-      int tab_index);
+      int tab_index,
+      UrlType url_type);
 
   // Creates a tab MutedInfo object (see chrome/common/extensions/api/tabs.json)
   // with information about the mute state of a browser tab.