Browser Tab cleanup work (chrome part 3)

This is just a refactor PR, nothing new was added.

+ cleaned up the way tab ids were managed:
Previously, tab Ids were either firefox actors or chrome tab ids. This
created an issue when it came to persisting an ID in the url query.
Previously, we were keeping just the child/tab of the firefox actor, but
now that it can be either firefox or chrome, the ID was easier. So, to
make this work, the tab ID is now either the chrome tab id or the
firefox child or tab part of the actor.

You might be asking, "why not use the entire actor?" The reason is that
the tab actor includes a connection id which is incremented everytime
the debugger is refreshed, which is exactly what we're trying to avoid.
What about if the user changes the tab order in firefox and then
refreshes the debugger? This will probably cause the debugger to debug
a different tab. We should check though...

Reasons:

+ debugTab seems like something you would want in your redux history
+ it's nice for the one action to know to call the firefox or chrome
client debugTab functions
+ it's nice for it to first invoke the select tab action.

public/js/actions/tabs.js

@@ -5,6 +5,8 @@
 "use strict";
 
 const constants = require("../constants");
+const { debugFirefoxTab } = require("../clients/firefox");
+const { debugChromeTab } = require("../clients/chrome");
 
 function newTabs(tabs) {
   return {
@@ -13,24 +15,27 @@ function newTabs(tabs) {
   };
 }
 
-function selectTab({ tabActor }) {
-  // set selected tab in the URL hash
-  let childId;
-  if (tabActor.includes("child")) {
-    childId = tabActor.match(/child\d+/)[0];
-  } else {
-    childId = tabActor.match(/tab\d+/)[0];
-  }
-
-  window.location.hash = `tab=${childId}`;
+function selectTab({ id }) {
+  window.location.hash = `tab=${id}`;
 
   return {
     type: constants.SELECT_TAB,
-    tabActor: tabActor,
+    id: id,
+  };
+}
+
+function debugTab(tab, actions) {
+  return ({ getState }) => {
+    const isFirefox = tab.browser == "firefox";
+    actions.selectTab({ id: tab.id });
+
+    const _debugTab = isFirefox ? debugFirefoxTab : debugChromeTab;
+    return _debugTab(tab.tab, actions);
   };
 }
 
 module.exports = {
   newTabs,
-  selectTab
+  selectTab,
+  debugTab
 };

public/js/clients/chrome.js

@@ -28,7 +28,8 @@ function presentTabs(tabs) {
         title: tab.title,
         url: tab.url,
         id: tab.id,
-        chrome: tab
+        tab,
+        browser: "chrome"
       };
     });
 }
@@ -60,7 +61,7 @@ function onConnection(connection) {
   runtimeAgent.run();
 }
 
-function debugTab(tab) {
+function debugChromeTab(tab) {
   function toTitleCase() {
     return this.substring(0, 1).toUpperCase() + this.substring(1);
   }
@@ -84,10 +85,8 @@ function debugTab(tab) {
   );
 }
 
-window.debugTab = debugTab;
-
 module.exports = {
   chromeTabs,
-  debugTab,
+  debugChromeTab,
   getAgent
 };

public/js/clients/chrome/api.js

@@ -1,6 +1,5 @@
 /* eslint-disable */
-
-var WebInspector = {};
+var WebInspector = {}, window = window || {};
 
 /*
  * Copyright (C) 2008 Apple Inc. All Rights Reserved.

public/js/clients/chrome/docs.md

@@ -24,7 +24,7 @@ InspectorBackend.registerCommand("Debugger.getBacktrace", [], ["callFrames", "as
 #### Steps to upgrade `inspectorBackend.js`:
 
 + add line `/* eslint-disable */`
-+ add line `var WebInspector = {};`
++ add line `var WebInspector = {}, window = window || {};`
 + copy front_end\/common\/Object.js
 + copy front_end\/sdk\/InspectorBackend.js
 + copy front_end\/main\/Connections.js

public/js/clients/firefox.js

@@ -22,7 +22,8 @@ function presentTabs(tabs) {
       title: tab.title,
       url: tab.url,
       id: tab.actor,
-      firefox: tab
+      tab,
+      browser: "firefox"
     };
   });
 }
@@ -55,10 +56,9 @@ function connectThread(tab, onNavigate) {
   });
 }
 
-function debugTab(tab, actions) {
+function debugFirefoxTab(tab, actions) {
   return Task.spawn(function* () {
     yield connectThread(tab);
-    actions.selectTab({ tabActor: tab.actor });
 
     const target = yield getTabTarget(tab);
     target.on("will-navigate", actions.willNavigate);
@@ -79,6 +79,6 @@ function debugTab(tab, actions) {
 module.exports = {
   connectClient,
   connectThread,
-  getThreadClient,
-  debugTab
+  debugFirefoxTab,
+  getThreadClient
 };

public/js/components/Tabs.js

@@ -5,15 +5,18 @@ const { connect } = require("react-redux");
 const actions = require("../actions");
 const { bindActionCreators } = require("redux");
 const { getTabs } = require("../selectors");
-const { debugTab } = require("../clients/firefox");
 
 require("./Tabs.css");
 const dom = React.DOM;
 
+function getTabsByBrowser(tabs, browser) {
+  return tabs.valueSeq()
+             .filter(tab => tab.get("browser") == browser);
+}
+
 function renderTab(tab, boundActions) {
   function onSelect(selectedTab) {
-    selectedTab = selectedTab.get("firefox") || selectedTab.get("chrome");
-    debugTab(selectedTab, boundActions);
+    boundActions.debugTab(tab.toJS(), boundActions);
   }
 
   return dom.li(
@@ -39,8 +42,8 @@ function renderTabs(tabTitle, tabs, boundActions) {
 }
 
 function Tabs({ tabs, actions: boundActions }) {
-  const firefoxTabs = tabs.valueSeq().filter(tab => tab.get("firefox"));
-  const chromeTabs = tabs.valueSeq().filter(tab => tab.get("chrome"));
+  const firefoxTabs = getTabsByBrowser(tabs, "firefox");
+  const chromeTabs = getTabsByBrowser(tabs, "chrome");
 
   return dom.div({ className: "tabs" },
     renderTabs("Firefox Tabs", firefoxTabs, boundActions),

public/js/main.js

@@ -21,7 +21,7 @@ if (isEnabled("development")) {
 
 const configureStore = require("./create-store");
 const reducers = require("./reducers");
-const { connectClient, getThreadClient, debugTab } = require("./clients/firefox");
+const { connectClient, getThreadClient } = require("./clients/firefox");
 const { chromeTabs } = require("./clients/chrome");
 const TabList = React.createFactory(require("./components/TabList"));
 
@@ -43,9 +43,8 @@ connectClient(tabs => {
   // if there's a pre-selected tab, connect to it and load the sources.
   // otherwise, just show the toolbox.
   if (hasSelectedTab()) {
-    const selectedTab = getSelectedTab(store.getState().tabs.get("tabs"));
-    const tab = selectedTab.get("firefox") || selectedTab.get("chrome");
-    debugTab(tab, actions).then(renderToolbox);
+    const tab = getTabFromUri(store.getState());
+    actions.debugTab(tab.toJS(), actions).then(renderToolbox);
   } else {
     renderToolbox();
   }
@@ -74,9 +73,10 @@ function hasSelectedTab() {
  * tab id is always 1.
  *
  */
-function getSelectedTab(tabs) {
-  const childId = window.location.hash.split("=")[1];
-  return tabs.find(tab => tab.get("id").includes(childId));
+function getTabFromUri(state) {
+  const tabs = getTabs(state);
+  const id = window.location.hash.split("=")[1];
+  return tabs.get(id);
 }
 
 setTimeout(function() {

public/js/reducers/tabs.js

@@ -21,14 +21,30 @@ function update(state = initialState, action) {
 
       return state.mergeIn(
         ["tabs"],
-        Immutable.Map(tabs.map(tab => [tab.id, Immutable.Map(tab)]))
+        Immutable.Map(tabs.map(tab => {
+          tab.id = getTabId(tab);
+          return [tab.id, Immutable.Map(tab)];
+        }))
       );
     case constants.SELECT_TAB:
-      const tab = state.getIn(["tabs", action.tabActor]);
+      const tab = state.getIn(["tabs", action.id]);
       return state.setIn(["selectedTab"], tab);
   }
 
   return state;
 }
 
+function getTabId(tab) {
+  let id = tab.id;
+  const isFirefox = tab.browser == "firefox";
+
+  // NOTE: we're getting the last part of the actor because
+  // we want to ignore the connection id
+  if (isFirefox) {
+    id = tab.id.split(".").pop();
+  }
+
+  return id;
+}
+
 module.exports = update;