Remove ref dependency from Frame

Resolves #8322

Auditors: @bsclifton @bridiver

Test Plan:
- tests are green

app/browser/reducers/tabsReducer.js

@@ -66,6 +66,15 @@ const tabsReducer = (state, action) => {
     case appConstants.APP_LOAD_URL_IN_ACTIVE_TAB_REQUESTED:
       state = tabs.loadURLInActiveTab(state, action)
       break
+    case appConstants.APP_ON_NAVIGATE_BACK:
+      state = tabs.goBack(state, action)
+      break
+    case appConstants.APP_ON_NAVIGATE_FORWARD:
+      state = tabs.goForward(state, action)
+      break
+    case appConstants.APP_ON_NAVIGATE_INDEX:
+      state = tabs.goToIndex(state, action)
+      break
     case appConstants.APP_FRAME_CHANGED:
       state = tabState.updateFrame(state, action)
       break

app/browser/tabs.js

@@ -617,6 +617,27 @@ const api = {
       api.createTab(state, action)
     }
     return state
+  },
+
+  goBack: (state, action) => {
+    action = makeImmutable(action)
+    const tab = api.getWebContents(action.get('tabId'))
+    tab.goBack()
+    return state
+  },
+
+  goForward: (state, action) => {
+    action = makeImmutable(action)
+    const tab = api.getWebContents(action.get('tabId'))
+    tab.goForward()
+    return state
+  },
+
+  goToIndex: (state, action) => {
+    action = makeImmutable(action)
+    const tab = api.getWebContents(action.get('tabId'))
+    tab.goToIndex(action.get('index'))
+    return state
   }
 }
 

app/renderer/components/navigation/navigator.js

@@ -66,7 +66,7 @@ class Navigator extends ImmutableComponent {
         })
       }
     } else {
-      navAction.call(this.activeFrame)
+      navAction.call(this, this.props.activeTab.get('tabId'))
     }
   }
 
@@ -119,19 +119,21 @@ class Navigator extends ImmutableComponent {
   }
 
   onBack (e) {
-    this.onNav(e, 'canGoBack', 'back', this.activeFrame.goBack)
+    this.onNav(e, 'canGoBack', 'back', appActions.onNavigateBack)
   }
 
   onForward (e) {
-    this.onNav(e, 'canGoForward', 'forward', this.activeFrame.goForward)
+    this.onNav(e, 'canGoForward', 'forward', appActions.onNavigateForward)
   }
 
   onBackLongPress (target) {
-    contextMenus.onBackButtonHistoryMenu(this.activeFrame, this.activeFrame.getHistory(this.props.appState), target)
+    const activeTab = this.props.activeTab
+    contextMenus.onBackButtonHistoryMenu(activeTab.get('tabId'), activeTab.get('partitionNumber'), this.activeFrame.getHistory(this.props.appState), target)
   }
 
   onForwardLongPress (target) {
-    contextMenus.onForwardButtonHistoryMenu(this.activeFrame, this.activeFrame.getHistory(this.props.appState), target)
+    const activeTab = this.props.activeTab
+    contextMenus.onForwardButtonHistoryMenu(activeTab.get('tabId'), activeTab.get('partitionNumber'), this.activeFrame.getHistory(this.props.appState), target)
   }
 
   onDragOver (e) {

docs/appActions.md

@@ -955,6 +955,38 @@ Notifies the app that a drop operation occurred
 
 
 
+### onNavigateBack(tabId) 
+
+Go back in a history for a given tab
+
+**Parameters**
+
+**tabId**: `number`, Tab id used for an action
+
+
+
+### onNavigateForward(tabId) 
+
+Go forward in a history for a given tab
+
+**Parameters**
+
+**tabId**: `number`, Tab id used for an action
+
+
+
+### onNavigateIndex(tabId, index) 
+
+Go to specific item in a history for a given tab
+
+**Parameters**
+
+**tabId**: `number`, Tab id used for an action
+
+**index**: `number`, Index in the history
+
+
+
 
 * * *
 

js/actions/appActions.js

@@ -1198,6 +1198,41 @@ const appActions = {
       actionType: appConstants.APP_DRAGGED_OVER,
       draggedOverData
     })
+  },
+
+  /**
+   * Go back in a history for a given tab
+   * @param {number} tabId - Tab id used for an action
+   */
+  onNavigateBack: function (tabId) {
+    AppDispatcher.dispatch({
+      actionType: appConstants.APP_ON_NAVIGATE_BACK,
+      tabId
+    })
+  },
+
+  /**
+   * Go forward in a history for a given tab
+   * @param {number} tabId - Tab id used for an action
+   */
+  onNavigateForward: function (tabId) {
+    AppDispatcher.dispatch({
+      actionType: appConstants.APP_ON_NAVIGATE_FORWARD,
+      tabId
+    })
+  },
+
+  /**
+   * Go to specific item in a history for a given tab
+   * @param {number} tabId - Tab id used for an action
+   * @param {number} index - Index in the history
+   */
+  onNavigateIndex: function (tabId, index) {
+    AppDispatcher.dispatch({
+      actionType: appConstants.APP_ON_NAVIGATE_INDEX,
+      tabId,
+      index
+    })
   }
 }
 

js/components/frame.js

@@ -629,10 +629,10 @@ class Frame extends ImmutableComponent {
           method = () => this.webview.stop()
           break
         case messages.GO_BACK:
-          method = () => this.webview.goBack()
+          method = () => appActions.onNavigateBack(this.props.tabId)
           break
         case messages.GO_FORWARD:
-          method = () => this.webview.goForward()
+          method = () => appActions.onNavigateForward(this.props.tabId)
           break
         case messages.RELOAD:
           method = () => {
@@ -728,7 +728,7 @@ class Frame extends ImmutableComponent {
         } else if (isTargetAboutUrl(e.validatedURL)) {
           // open a new tab for other about urls
           // and send this tab back to wherever it came from
-          this.goBack()
+          appActions.tabNavigateBack(this.props.tabId)
           appActions.createTabRequested({
             url: e.validatedURL,
             active: true
@@ -904,10 +904,6 @@ class Frame extends ImmutableComponent {
     this.webview.addEventListener('mousewheel', this.onMouseWheel.bind(this))
   }
 
-  goBack () {
-    this.webview.goBack()
-  }
-
   getHistoryEntry (sites, index) {
     const url = this.webview.getURLAtIndex(index)
     const title = this.webview.getTitleAtIndex(index)
@@ -951,14 +947,6 @@ class Frame extends ImmutableComponent {
     return history
   }
 
-  goToIndex (index) {
-    this.webview.goToIndex(index)
-  }
-
-  goForward () {
-    this.webview.goForward()
-  }
-
   get origin () {
     return siteUtil.getOrigin(this.props.location)
   }

js/components/main.js

@@ -763,7 +763,7 @@ class Main extends ImmutableComponent {
         <Navigator
           appState={this.props.appState}
           windowState={this.props.windowState}
-          frames={this.frames}
+          frames={this.frames} // TODO remove
           activeTab={activeTab}
           shouldAllowWindowDrag={shouldAllowWindowDrag}
           customTitlebar={customTitlebar}
@@ -941,7 +941,7 @@ class Main extends ImmutableComponent {
           {
             sortedFrames.map((frame) =>
               <Frame
-                ref={(node) => { this.frames[frame.get('key')] = node }}
+                ref={(node) => { this.frames[frame.get('key')] = node }} // TODO remove
                 urlBarFocused={activeFrame && activeFrame.getIn(['navbar', 'urlbar', 'focused'])}
                 tabIndex={frameStateUtil.getFrameIndex(this.props.windowState, frame.get('key'))}
                 prefOpenInForeground={getSetting(settings.SWITCH_TO_NEW_TABS)}

js/constants/appConstants.js

@@ -115,7 +115,10 @@ const appConstants = {
   APP_DRAG_STARTED: _,
   APP_DRAG_STOPPED: _,
   APP_DATA_DROPPED: _,
-  APP_DRAGGED_OVER: _
+  APP_DRAGGED_OVER: _,
+  APP_ON_NAVIGATE_BACK: _,
+  APP_ON_NAVIGATE_FORWARD: _,
+  APP_ON_NAVIGATE_INDEX: _
 }
 
 module.exports = mapValuesByKeys(appConstants)

js/contextMenus.js

@@ -1489,11 +1489,11 @@ function onMoreBookmarksMenu (activeFrame, allBookmarkItems, overflowItems, e) {
   }))
 }
 
-function onBackButtonHistoryMenu (activeFrame, history, target) {
+function onBackButtonHistoryMenu (tabId, partitionNumber, history, target) {
   const rect = target.parentNode.getBoundingClientRect()
   const menuTemplate = []
 
-  if (activeFrame && history && history.entries.length > 0) {
+  if (tabId > -1 && history && history.entries.length > 0) {
     const stopIndex = Math.max(((history.currentIndex - config.navigationBar.maxHistorySites) - 1), -1)
     for (let index = (history.currentIndex - 1); index > stopIndex; index--) {
       const url = history.entries[index].url
@@ -1505,11 +1505,11 @@ function onBackButtonHistoryMenu (activeFrame, history, target) {
           if (eventUtil.isForSecondaryAction(e)) {
             appActions.createTabRequested({
               url,
-              partitionNumber: activeFrame.props.partitionNumber,
+              partitionNumber: partitionNumber,
               active: !!e.shiftKey
             })
           } else {
-            activeFrame.goToIndex(index)
+            appActions.onNavigateIndex(tabId, index)
           }
         }
       })
@@ -1536,11 +1536,11 @@ function onBackButtonHistoryMenu (activeFrame, history, target) {
   }))
 }
 
-function onForwardButtonHistoryMenu (activeFrame, history, target) {
+function onForwardButtonHistoryMenu (tabId, partitionNumber, history, target) {
   const rect = target.parentNode.getBoundingClientRect()
   const menuTemplate = []
 
-  if (activeFrame && history && history.entries.length > 0) {
+  if (tabId > -1 && history && history.entries.length > 0) {
     const stopIndex = Math.min(((history.currentIndex + config.navigationBar.maxHistorySites) + 1), history.entries.length)
     for (let index = (history.currentIndex + 1); index < stopIndex; index++) {
       const url = history.entries[index].url
@@ -1552,11 +1552,11 @@ function onForwardButtonHistoryMenu (activeFrame, history, target) {
           if (eventUtil.isForSecondaryAction(e)) {
             appActions.createTabRequested({
               url,
-              partitionNumber: activeFrame.props.partitionNumber,
+              partitionNumber: partitionNumber,
               active: !!e.shiftKey
             })
           } else {
-            activeFrame.goToIndex(index)
+            appActions.onNavigateIndex(tabId, index)
           }
         }
       })