Fix/service webview unmounting (#1328)

* detach service when underlying webview unmounts

* disable no-param-reassign eslint rule

7 files changed, 180 insertions, 122 deletions

diff --git a/.eslintrc b/.eslintrc
index e15148e96..743946d35 100644
--- a/.eslintrc
+++ b/.eslintrc
@@ -2,6 +2,7 @@
   "parser": "babel-eslint",
   "extends": "eslint-config-airbnb",
   "rules": {
+    "no-param-reassign": 0,
     "import/extensions": 0,
     "import/no-extraneous-dependencies": 0,
     "import/no-unresolved": [2, {
diff --git a/src/actions/service.js b/src/actions/service.js
index 5d483b12a..ceaabc31e 100644
--- a/src/actions/service.js
+++ b/src/actions/service.js
@@ -1,4 +1,5 @@
 import PropTypes from 'prop-types';
+import ServiceModel from '../models/Service';
 
 export default {
   setActive: {
@@ -36,6 +37,9 @@ export default {
     serviceId: PropTypes.string.isRequired,
     webview: PropTypes.object.isRequired,
   },
+  detachService: {
+    service: PropTypes.instanceOf(ServiceModel).isRequired,
+  },
   focusService: {
     serviceId: PropTypes.string.isRequired,
   },
diff --git a/src/components/services/content/ServiceView.js b/src/components/services/content/ServiceView.js
new file mode 100644
index 000000000..5afc54f9d
--- /dev/null
+++ b/src/components/services/content/ServiceView.js
@@ -0,0 +1,136 @@
+import React, { Component, Fragment } from 'react';
+import PropTypes from 'prop-types';
+import { autorun } from 'mobx';
+import { observer } from 'mobx-react';
+import classnames from 'classnames';
+
+import ServiceModel from '../../../models/Service';
+import StatusBarTargetUrl from '../../ui/StatusBarTargetUrl';
+import WebviewLoader from '../../ui/WebviewLoader';
+import WebviewCrashHandler from './WebviewCrashHandler';
+import WebviewErrorHandler from './ErrorHandlers/WebviewErrorHandler';
+import ServiceDisabled from './ServiceDisabled';
+import ServiceWebview from './ServiceWebview';
+
+export default @observer class ServiceView extends Component {
+  static propTypes = {
+    service: PropTypes.instanceOf(ServiceModel).isRequired,
+    setWebviewReference: PropTypes.func.isRequired,
+    detachService: PropTypes.func.isRequired,
+    reload: PropTypes.func.isRequired,
+    edit: PropTypes.func.isRequired,
+    enable: PropTypes.func.isRequired,
+    isActive: PropTypes.bool,
+  };
+
+  static defaultProps = {
+    isActive: false,
+  };
+
+  state = {
+    forceRepaint: false,
+    targetUrl: '',
+    statusBarVisible: false,
+  };
+
+  autorunDisposer = null;
+
+  componentDidMount() {
+    this.autorunDisposer = autorun(() => {
+      if (this.props.service.isActive) {
+        this.setState({ forceRepaint: true });
+        setTimeout(() => {
+          this.setState({ forceRepaint: false });
+        }, 100);
+      }
+    });
+  }
+
+  componentWillUnmount() {
+    this.autorunDisposer();
+  }
+
+  updateTargetUrl = (event) => {
+    let visible = true;
+    if (event.url === '' || event.url === '#') {
+      visible = false;
+    }
+    this.setState({
+      targetUrl: event.url,
+      statusBarVisible: visible,
+    });
+  };
+
+  render() {
+    const {
+      detachService,
+      service,
+      setWebviewReference,
+      reload,
+      edit,
+      enable,
+    } = this.props;
+
+    const webviewClasses = classnames({
+      services__webview: true,
+      'services__webview-wrapper': true,
+      'is-active': service.isActive,
+      'services__webview--force-repaint': this.state.forceRepaint,
+    });
+
+    let statusBar = null;
+    if (this.state.statusBarVisible) {
+      statusBar = (
+        <StatusBarTargetUrl text={this.state.targetUrl} />
+      );
+    }
+
+    return (
+      <div className={webviewClasses}>
+        {service.isActive && service.isEnabled && (
+          <Fragment>
+            {service.hasCrashed && (
+              <WebviewCrashHandler
+                name={service.recipe.name}
+                webview={service.webview}
+                reload={reload}
+              />
+            )}
+            {service.isEnabled && service.isLoading && service.isFirstLoad && (
+              <WebviewLoader
+                loaded={false}
+                name={service.name}
+              />
+            )}
+            {service.isError && (
+              <WebviewErrorHandler
+                name={service.recipe.name}
+                errorMessage={service.errorMessage}
+                reload={reload}
+                edit={edit}
+              />
+            )}
+          </Fragment>
+        )}
+        {!service.isEnabled ? (
+          <Fragment>
+            {service.isActive && (
+              <ServiceDisabled
+                name={service.recipe.name}
+                webview={service.webview}
+                enable={enable}
+              />
+            )}
+          </Fragment>
+        ) : (
+          <ServiceWebview
+            service={service}
+            setWebviewReference={setWebviewReference}
+            detachService={detachService}
+          />
+        )}
+        {statusBar}
+      </div>
+    );
+  }
+}
diff --git a/src/components/services/content/ServiceWebview.js b/src/components/services/content/ServiceWebview.js
index bb577e4cc..7252c695f 100644
--- a/src/components/services/content/ServiceWebview.js
+++ b/src/components/services/content/ServiceWebview.js
@@ -1,145 +1,50 @@
-import React, { Component, Fragment } from 'react';
+import React, { Component } from 'react';
 import PropTypes from 'prop-types';
-import { autorun } from 'mobx';
 import { observer } from 'mobx-react';
-import Webview from 'react-electron-web-view';
-import classnames from 'classnames';
+import ElectronWebView from 'react-electron-web-view';
 
 import ServiceModel from '../../../models/Service';
-import StatusBarTargetUrl from '../../ui/StatusBarTargetUrl';
-import WebviewLoader from '../../ui/WebviewLoader';
-import WebviewCrashHandler from './WebviewCrashHandler';
-import WebviewErrorHandler from './ErrorHandlers/WebviewErrorHandler';
-import ServiceDisabled from './ServiceDisabled';
 
-export default @observer class ServiceWebview extends Component {
+@observer
+class ServiceWebview extends Component {
   static propTypes = {
     service: PropTypes.instanceOf(ServiceModel).isRequired,
     setWebviewReference: PropTypes.func.isRequired,
-    reload: PropTypes.func.isRequired,
-    edit: PropTypes.func.isRequired,
-    enable: PropTypes.func.isRequired,
-    isActive: PropTypes.bool,
+    detachService: PropTypes.func.isRequired,
   };
 
-  static defaultProps = {
-    isActive: false,
-  };
-
-  state = {
-    forceRepaint: false,
-    targetUrl: '',
-    statusBarVisible: false,
-  };
-
-  autorunDisposer = null;
-
   webview = null;
 
-  componentDidMount() {
-    this.autorunDisposer = autorun(() => {
-      if (this.props.service.isActive) {
-        this.setState({ forceRepaint: true });
-        setTimeout(() => {
-          this.setState({ forceRepaint: false });
-        }, 100);
-      }
-    });
-  }
-
   componentWillUnmount() {
-    this.autorunDisposer();
-  }
-
-  updateTargetUrl = (event) => {
-    let visible = true;
-    if (event.url === '' || event.url === '#') {
-      visible = false;
-    }
-    this.setState({
-      targetUrl: event.url,
-      statusBarVisible: visible,
-    });
+    const { service, detachService } = this.props;
+    detachService({ service });
   }
 
   render() {
     const {
       service,
       setWebviewReference,
-      reload,
-      edit,
-      enable,
     } = this.props;
 
-    const webviewClasses = classnames({
-      services__webview: true,
-      'services__webview-wrapper': true,
-      'is-active': service.isActive,
-      'services__webview--force-repaint': this.state.forceRepaint,
-    });
-
-    let statusBar = null;
-    if (this.state.statusBarVisible) {
-      statusBar = (
-        <StatusBarTargetUrl text={this.state.targetUrl} />
-      );
-    }
-
     return (
-      <div className={webviewClasses}>
-        {service.isActive && service.isEnabled && (
-          <Fragment>
-            {service.hasCrashed && (
-              <WebviewCrashHandler
-                name={service.recipe.name}
-                webview={service.webview}
-                reload={reload}
-              />
-            )}
-            {service.isEnabled && service.isLoading && service.isFirstLoad && (
-              <WebviewLoader
-                loaded={false}
-                name={service.name}
-              />
-            )}
-            {service.isError && (
-              <WebviewErrorHandler
-                name={service.recipe.name}
-                errorMessage={service.errorMessage}
-                reload={reload}
-                edit={edit}
-              />
-            )}
-          </Fragment>
-        )}
-        {!service.isEnabled ? (
-          <Fragment>
-            {service.isActive && (
-              <ServiceDisabled
-                name={service.recipe.name}
-                webview={service.webview}
-                enable={enable}
-              />
-            )}
-          </Fragment>
-        ) : (
-          <Webview
-            ref={(element) => { this.webview = element; }}
-            autosize
-            src={service.url}
-            preload="./webview/recipe.js"
-            partition={`persist:service-${service.id}`}
-            onDidAttach={() => setWebviewReference({
-              serviceId: service.id,
-              webview: this.webview.view,
-            })}
-            onUpdateTargetUrl={this.updateTargetUrl}
-            useragent={service.userAgent}
-            allowpopups
-          />
-        )}
-        {statusBar}
-      </div>
+      <ElectronWebView
+        ref={(webview) => { this.webview = webview; }}
+        autosize
+        src={service.url}
+        preload="./webview/recipe.js"
+        partition={`persist:service-${service.id}`}
+        onDidAttach={() => {
+          setWebviewReference({
+            serviceId: service.id,
+            webview: this.webview.view,
+          });
+        }}
+        onUpdateTargetUrl={this.updateTargetUrl}
+        useragent={service.userAgent}
+        allowpopups
+      />
     );
   }
 }
+
+export default ServiceWebview;
diff --git a/src/components/services/content/Services.js b/src/components/services/content/Services.js
index 54f16ba12..8f8c38a11 100644
--- a/src/components/services/content/Services.js
+++ b/src/components/services/content/Services.js
@@ -4,7 +4,7 @@ import { observer, PropTypes as MobxPropTypes } from 'mobx-react';
 import { Link } from 'react-router';
 import { defineMessages, intlShape } from 'react-intl';
 
-import Webview from './ServiceWebview';
+import ServiceView from './ServiceView';
 import Appear from '../../ui/effects/Appear';
 
 const messages = defineMessages({
@@ -22,6 +22,7 @@ export default @observer class Services extends Component {
   static propTypes = {
     services: MobxPropTypes.arrayOrObservableArray,
     setWebviewReference: PropTypes.func.isRequired,
+    detachService: PropTypes.func.isRequired,
     handleIPCMessage: PropTypes.func.isRequired,
     openWindow: PropTypes.func.isRequired,
     reload: PropTypes.func.isRequired,
@@ -42,6 +43,7 @@ export default @observer class Services extends Component {
       services,
       handleIPCMessage,
       setWebviewReference,
+      detachService,
       openWindow,
       reload,
       openSettings,
@@ -71,11 +73,12 @@ export default @observer class Services extends Component {
           </Appear>
         )}
         {services.map(service => (
-          <Webview
+          <ServiceView
             key={service.id}
             service={service}
             handleIPCMessage={handleIPCMessage}
             setWebviewReference={setWebviewReference}
+            detachService={detachService}
             openWindow={openWindow}
             reload={() => reload({ serviceId: service.id })}
             edit={() => openSettings({ path: `services/edit/${service.id}` })}
diff --git a/src/containers/layout/AppLayoutContainer.js b/src/containers/layout/AppLayoutContainer.js
index 749912c59..5a05ce431 100644
--- a/src/containers/layout/AppLayoutContainer.js
+++ b/src/containers/layout/AppLayoutContainer.js
@@ -42,6 +42,7 @@ export default @inject('stores', 'actions') @observer class AppLayoutContainer e
       setActive,
       handleIPCMessage,
       setWebviewReference,
+      detachService,
       openWindow,
       reorder,
       reload,
@@ -105,6 +106,7 @@ export default @inject('stores', 'actions') @observer class AppLayoutContainer e
         services={services.allDisplayedUnordered}
         handleIPCMessage={handleIPCMessage}
         setWebviewReference={setWebviewReference}
+        detachService={detachService}
         openWindow={openWindow}
         reload={reload}
         openSettings={openSettings}
@@ -160,6 +162,7 @@ AppLayoutContainer.wrappedComponent.propTypes = {
       toggleAudio: PropTypes.func.isRequired,
       handleIPCMessage: PropTypes.func.isRequired,
       setWebviewReference: PropTypes.func.isRequired,
+      detachService: PropTypes.func.isRequired,
       openWindow: PropTypes.func.isRequired,
       reloadUpdatedServices: PropTypes.func.isRequired,
       updateService: PropTypes.func.isRequired,
diff --git a/src/stores/ServicesStore.js b/src/stores/ServicesStore.js
index c63bef196..69e616f0c 100644
--- a/src/stores/ServicesStore.js
+++ b/src/stores/ServicesStore.js
@@ -44,6 +44,7 @@ export default class ServicesStore extends Store {
     this.actions.service.deleteService.listen(this._deleteService.bind(this));
     this.actions.service.clearCache.listen(this._clearCache.bind(this));
     this.actions.service.setWebviewReference.listen(this._setWebviewReference.bind(this));
+    this.actions.service.detachService.listen(this._detachService.bind(this));
     this.actions.service.focusService.listen(this._focusService.bind(this));
     this.actions.service.focusActiveService.listen(this._focusActiveService.bind(this));
     this.actions.service.toggleService.listen(this._toggleService.bind(this));
@@ -341,6 +342,11 @@ export default class ServicesStore extends Store {
     service.isAttached = true;
   }
 
+  @action _detachService({ service }) {
+    service.webview = null;
+    service.isAttached = false;
+  }
+
   @action _focusService({ serviceId }) {
     const service = this.one(serviceId);