extensions: tabs: set tab id to -1 for app/devtools windows

chrome/browser/extensions/extension_tab_util.cc

Index: chrome/browser/extensions/extension_tab_util.cc
diff --git a/chrome/browser/extensions/extension_tab_util.cc b/chrome/browser/extensions/extension_tab_util.cc
index 5d4b89b667faac9facefd526cd0de2a974db0499..9dd5564d40eb7532a0e129b0e3b26330d509ec42 100644
--- a/chrome/browser/extensions/extension_tab_util.cc
+++ b/chrome/browser/extensions/extension_tab_util.cc
@@ -350,10 +350,13 @@ base::ListValue* ExtensionTabUtil::CreateTabList(
   base::ListValue* tab_list = new base::ListValue();
   TabStripModel* tab_strip = browser->tab_strip_model();
   for (int i = 0; i < tab_strip->count(); ++i) {
-    tab_list->Append(CreateTabValue(tab_strip->GetWebContentsAt(i),
-                                    tab_strip,
-                                    i,
-                                    extension));
+    base::DictionaryValue* value =
+        CreateTabValue(tab_strip->GetWebContentsAt(i), tab_strip, i, extension);
+    // Remove tab id for devtools as we don't want them to be
+    // manipulated through the chrome.tabs API.
+    if (browser->is_devtools())

[dgozman, 2015/07/14 15:16:16]

What about TabUpdatedEvent or TabsGetCurrent? Perhaps, we should filter out id
(or even a whole tab entry?) from inside CreateTabValue?

[not at google - send to devlin, 2015/07/14 17:15:13]

CreateTabValue looks right to me.

[not at google - send to devlin, 2015/07/14 17:40:00]

Actually, GetTabID looks even right-er to me. More subtle and wide-reaching, but
skimming over the call sites, it looks safe. Perhaps double-check?

It already returns -1 if there is no tab; you could additionally special-case
devtools to return -1, then not set tab ID here if it's == -1.

[not at google - send to devlin, 2015/07/14 17:41:09]

More comments:

In fact I wonder if this would break backwards compatibility at all. We must
already be setting -1 for the tab ID in some cases already, and not setting it
if it == -1 would break that. In which case, I'd set it to -1 for devtools.

+      value->Remove(keys::kIdKey, nullptr);
+    tab_list->Append(value);
   }
 
   return tab_list;