Expose Chrome version to todos webview (fix #1211) (#1478)

* Expose Chrome version to todos webview (fix #1211)

The TickTick todo service fails to load if the Chrome version number
does not appear in the User-Agent string. However, login to Google Tasks
is prevented by the same.

We adopt the "chromeless" User-Agent logic from the service webview,
which selectively exposes the Chrome version everywhere except the
Google login screen. The common logic was moved into the
userAgent-helpers module.

* Refactor user agent switching

* "Chromeless" user agent switching is extracted into a separate model
* Both the service and the todos webview uses the same model

2 files changed, 88 insertions, 31 deletions

diff --git a/src/models/Service.js b/src/models/Service.js
index b01881beb..0d1dff431 100644
--- a/src/models/Service.js
+++ b/src/models/Service.js
@@ -4,9 +4,9 @@ import { webContents } from '@electron/remote';
 import normalizeUrl from 'normalize-url';
 import path from 'path';
 
-import userAgent from '../helpers/userAgent-helpers';
 import { TODOS_RECIPE_ID, todosStore } from '../features/todos';
 import { isValidExternalURL } from '../helpers/url-helpers';
+import UserAgent from './UserAgent';
 
 const debug = require('debug')('Ferdi:Service');
 
@@ -94,7 +94,7 @@ export default class Service {
 
   @observable lostRecipeReloadAttempt = 0;
 
-  @observable chromelessUserAgent = false;
+  @observable userAgentModel = null;
 
   constructor(data, recipe) {
     if (!data) {
@@ -156,6 +156,8 @@ export default class Service {
       this.isHibernating = true;
     }
 
+    this.userAgentModel = new UserAgent(recipe.overrideUserAgent);
+
     autorun(() => {
       if (!this.isEnabled) {
         this.webview = null;
@@ -234,12 +236,7 @@ export default class Service {
   }
 
   @computed get userAgent() {
-    let ua = userAgent(this.chromelessUserAgent);
-    if (typeof this.recipe.overrideUserAgent === 'function') {
-      ua = this.recipe.overrideUserAgent();
-    }
-
-    return ua;
+    return this.userAgentModel.userAgent;
   }
 
   @computed get partition() {
@@ -250,6 +247,8 @@ export default class Service {
   initializeWebViewEvents({ handleIPCMessage, openWindow, stores }) {
     const webviewWebContents = webContents.fromId(this.webview.getWebContentsId());
 
+    this.userAgentModel.setWebviewReference(this.webview);
+
     // If the recipe has implemented modifyRequestHeaders,
     // Send those headers to ipcMain so that it can be set in session
     if (typeof this.recipe.modifyRequestHeaders === 'function') {
@@ -263,23 +262,6 @@ export default class Service {
       debug(this.name, 'modifyRequestHeaders is not defined in the recipe');
     }
 
-    const handleUserAgent = (url, forwardingHack = false) => {
-      if (url.startsWith('https://accounts.google.com')) {
-        if (!this.chromelessUserAgent) {
-          debug('Setting user agent to chromeless for url', url);
-          this.webview.setUserAgent(userAgent(true));
-          if (forwardingHack) {
-            this.webview.loadURL(url);
-          }
-          this.chromelessUserAgent = true;
-        }
-      } else if (this.chromelessUserAgent) {
-        debug('Setting user agent to contain chrome');
-        this.webview.setUserAgent(this.userAgent);
-        this.chromelessUserAgent = false;
-      }
-    };
-
     this.webview.addEventListener('ipc-message', e => handleIPCMessage({
       serviceId: this.id,
       channel: e.channel,
@@ -306,8 +288,6 @@ export default class Service {
       }
     });
 
-    this.webview.addEventListener('will-navigate', event => handleUserAgent(event.url, true));
-
     this.webview.addEventListener('did-start-loading', (event) => {
       debug('Did start load', this.name, event);
 
@@ -325,10 +305,7 @@ export default class Service {
     };
 
     this.webview.addEventListener('did-frame-finish-load', didLoad.bind(this));
-    this.webview.addEventListener('did-navigate', (event) => {
-      handleUserAgent(event.url);
-      didLoad();
-    });
+    this.webview.addEventListener('did-navigate', didLoad.bind(this));
 
     this.webview.addEventListener('did-fail-load', (event) => {
       debug('Service failed to load', this.name, event);
diff --git a/src/models/UserAgent.js b/src/models/UserAgent.js
new file mode 100644
index 000000000..f51f2e5a6
--- /dev/null
+++ b/src/models/UserAgent.js
@@ -0,0 +1,80 @@
+import {
+  action,
+  computed,
+  observe,
+  observable,
+} from 'mobx';
+
+import defaultUserAgent, { isChromeless } from '../helpers/userAgent-helpers';
+
+const debug = require('debug')('Ferdi:UserAgent');
+
+export default class UserAgent {
+  _willNavigateListener = null;
+
+  _didNavigateListener = null;
+
+  @observable.ref webview = null;
+
+  @observable chromelessUserAgent = false;
+
+  @observable getUserAgent = defaultUserAgent;
+
+  constructor(overrideUserAgent = null) {
+    if (typeof overrideUserAgent === 'function') {
+      this.getUserAgent = overrideUserAgent;
+    }
+
+    observe(this, 'webview', (change) => {
+      const { oldValue, newValue } = change;
+      if (oldValue !== null) {
+        this._removeWebviewEvents(oldValue);
+      }
+      if (newValue !== null) {
+        this._addWebviewEvents(newValue);
+      }
+    });
+  }
+
+  @computed get userAgent() {
+    return this.chromelessUserAgent ? defaultUserAgent(true) : this.getUserAgent();
+  }
+
+  @action setWebviewReference(webview) {
+    this.webview = webview;
+  }
+
+  @action _handleNavigate(url, forwardingHack = false) {
+    if (isChromeless(url)) {
+      if (!this.chromelessUserAgent) {
+        debug('Setting user agent to chromeless for url', url);
+        this.chromelessUserAgent = true;
+        this.webview.userAgent = this.userAgent;
+        if (forwardingHack) {
+          this.webview.loadURL(url);
+        }
+      }
+    } else if (this.chromelessUserAgent) {
+      debug('Setting user agent to contain chrome for url', url);
+      this.chromelessUserAgent = false;
+      this.webview.userAgent = this.userAgent;
+    }
+  }
+
+  _addWebviewEvents(webview) {
+    debug('Adding event handlers');
+
+    this._willNavigateListener = event => this._handleNavigate(event.url, true);
+    webview.addEventListener('will-navigate', this._willNavigateListener);
+
+    this._didNavigateListener = event => this._handleNavigate(event.url);
+    webview.addEventListener('did-navigate', this._didNavigateListener);
+  }
+
+  _removeWebviewEvents(webview) {
+    debug('Removing event handlers');
+
+    webview.removeEventListener('will-navigate', this._willNavigateListener);
+    webview.removeEventListener('did-navigate', this._didNavigateListener);
+  }
+}