Disable using transferrables for ArrayBuffers in Safari(#9003)

mapbox · Nov 21, 2019 · c9bb0f3 · c9bb0f3
1 parent aa0f7c3
commit c9bb0f3
Show file tree

Hide file tree

Showing 10 changed files with 107 additions and 9 deletions.
diff --git a/build/rollup_plugins.js b/build/rollup_plugins.js
@@ -5,9 +5,10 @@ import resolve from 'rollup-plugin-node-resolve';
 import commonjs from 'rollup-plugin-commonjs';
 import unassert from 'rollup-plugin-unassert';
 import json from 'rollup-plugin-json';
-import { terser } from 'rollup-plugin-terser';
+import {terser} from 'rollup-plugin-terser';
 import minifyStyleSpec from './rollup_plugin_minify_style_spec';
-import { createFilter } from 'rollup-pluginutils';
+import strip from '@rollup/plugin-strip';
+import {createFilter} from 'rollup-pluginutils';
 
 // Common set of plugins/transformations shared across different rollup
 // builds (main mapboxgl bundle, style-spec package, benchmarks bundle)
@@ -16,6 +17,9 @@ export const plugins = (minified, production) => [
     flow(),
     minifyStyleSpec(),
     json(),
+    production ? strip({
+        functions: ['Debug.*']
+    }) : false,
     glsl('./src/shaders/*.glsl', production),
     buble({transforms: {dangerousForOf: true}, objectAssign: "Object.assign"}),
     minified ? terser() : false,

diff --git a/debug/is-safari.html b/debug/is-safari.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>Mapbox GL JS debug page</title>
+    <meta charset='utf-8'>
+    <meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=no">
+    <link rel='stylesheet' href='../dist/mapbox-gl.css' />
+    <style>
+        body { margin: 0; padding: 0; }
+        html, body, #map { height: 100%; }
+        #overlay {position: fixed; top: 10px; left: 10px; background: white }
+    </style>
+</head>
+
+<body>
+<div id='map'></div>
+<div id='overlay'></div>
+
+<script src='../dist/mapbox-gl-dev.js'></script>
+<script src='../debug/access_token_generated.js'></script>
+<script>
+
+var map = window.map = new mapboxgl.Map({
+    container: 'map',
+    zoom: 12.5,
+    center: [-77.01866, 38.888],
+    style: 'mapbox://styles/mapbox/streets-v10',
+    hash: true
+});
+
+map.on('load', function() {
+    const not = mapboxgl.isSafari(window) ? '' : 'NOT';
+    document.getElementById('overlay').innerHTML = 'THIS IS ' + not + ' SAFARI!!!';
+});
+
+</script>
+</body>
+</html>
diff --git a/package.json b/package.json
@@ -43,6 +43,7 @@
     "@mapbox/mapbox-gl-rtl-text": "^0.2.1",
     "@mapbox/mapbox-gl-test-suite": "file:test/integration",
     "@octokit/rest": "^16.30.1",
+    "@rollup/plugin-strip": "^1.3.1",
     "address": "^1.1.2",
     "babel-eslint": "^10.0.1",
     "benchmark": "^2.1.4",

diff --git a/src/data/feature_position_map.js b/src/data/feature_position_map.js
@@ -63,7 +63,9 @@ export default class FeaturePositionMap {
 
         sort(ids, positions, 0, ids.length - 1);
 
-        transferables.push(ids.buffer, positions.buffer);
+        if (transferables) {
+            transferables.push(ids.buffer, positions.buffer);
+        }
 
         return {ids, positions};
     }

diff --git a/src/index.js b/src/index.js
@@ -19,6 +19,8 @@ import Point from '@mapbox/point-geometry';
 import MercatorCoordinate from './geo/mercator_coordinate';
 import {Evented} from './util/evented';
 import config from './util/config';
+import {Debug} from './util/debug';
+import {isSafari} from './util/util';
 import {setRTLTextPlugin, getRTLTextPluginStatus} from './source/rtl_text_plugin';
 import WorkerPool from './util/worker_pool';
 import {clearTileCache} from './util/tile_request_cache';
@@ -130,6 +132,9 @@ const exported = {
     workerUrl: ''
 };
 
+//This gets automatically stripped out in production builds.
+Debug.extend(exported, {isSafari});
+
 /**
  * The version of Mapbox GL JS in use as specified in `package.json`,
  * `CHANGELOG.md`, and the GitHub release.

diff --git a/src/util/actor.js b/src/util/actor.js
@@ -1,6 +1,7 @@
 // @flow
 
-import {bindAll, isWorker} from './util';
+import {bindAll, isWorker, isSafari} from './util';
+import window from './window';
 import {serialize, deserialize} from './web_worker_transfer';
 import ThrottledInvoker from './throttled_invoker';
 
@@ -28,6 +29,7 @@ class Actor {
     taskQueue: Array<number>;
     cancelCallbacks: { number: Cancelable };
     invoker: ThrottledInvoker;
+    globalScope: any;
 
     constructor(target: any, parent: any, mapId: ?number) {
         this.target = target;
@@ -40,6 +42,7 @@ class Actor {
         bindAll(['receive', 'process'], this);
         this.invoker = new ThrottledInvoker(this.process);
         this.target.addEventListener('message', this.receive, false);
+        this.globalScope = isWorker() ? target : window;
     }
 
     /**
@@ -59,7 +62,7 @@ class Actor {
         if (callback) {
             this.callbacks[id] = callback;
         }
-        const buffers: Array<Transferable> = [];
+        const buffers: ?Array<Transferable> = isSafari(this.globalScope) ? undefined : [];
         this.target.postMessage({
             id,
             type,
@@ -158,10 +161,10 @@ class Actor {
             }
         } else {
             let completed = false;
+            const buffers: ?Array<Transferable> = isSafari(this.globalScope) ? undefined : [];
             const done = task.hasCallback ? (err, data) => {
                 completed = true;
                 delete this.cancelCallbacks[id];
-                const buffers: Array<Transferable> = [];
                 this.target.postMessage({
                     id,
                     type: '<response>',

diff --git a/src/util/debug.js b/src/util/debug.js
@@ -0,0 +1,12 @@
+// @flow
+import {extend} from './util';
+
+/**
+ * This is a private namespace for utility functions that will get automatically stripped
+ * out in production builds.
+ */
+export const Debug = {
+    extend(dest: Object, ...sources: Array<?Object>): Object {
+        return extend(dest, ...sources);
+    }
+};
diff --git a/src/util/util.js b/src/util/util.js
@@ -449,6 +449,30 @@ export function parseCacheControl(cacheControl: string): Object {
     return header;
 }
 
+let _isSafari = null;
+
+/**
+ * Returns true when run in WebKit derived browsers.
+ * This is used as a workaround for a memory leak in Safari caused by using Transferable objects to
+ * transfer data between WebWorkers and the main thread.
+ * https://github.com/mapbox/mapbox-gl-js/issues/8771
+ *
+ * This should be removed once the underlying Safari issue is fixed.
+ *
+ * @private
+ * @param scope {WindowOrWorkerGlobalScope} Since this function is used both on the main thread and WebWorker context,
+ *      let the calling scope pass in the global scope object.
+ * @returns {boolean}
+ */
+export function isSafari(scope: any): boolean {
+    if (_isSafari == null) {
+        const userAgent = scope.navigator ? scope.navigator.userAgent : null;
+        _isSafari = !!scope.safari ||
+        !!(userAgent && (/\b(iPad|iPhone|iPod)\b/.test(userAgent) || (!!userAgent.match('Safari') && !userAgent.match('Chrome'))));
+    }
+    return _isSafari;
+}
+
 export function storageAvailable(type: string): boolean {
     try {
         const storage = window[type];

diff --git a/src/util/web_worker_transfer.js b/src/util/web_worker_transfer.js
@@ -119,7 +119,7 @@ function isArrayBuffer(val: any): boolean {
  *
  * @private
  */
-export function serialize(input: mixed, transferables?: Array<Transferable>): Serialized {
+export function serialize(input: mixed, transferables: ?Array<Transferable>): Serialized {
     if (input === null ||
         input === undefined ||
         typeof input === 'boolean' ||

diff --git a/yarn.lock b/yarn.lock
@@ -1137,6 +1137,15 @@
   dependencies:
     "@types/node" "^12.11.1"
 
+"@rollup/plugin-strip@^1.3.1":
+  version "1.3.1"
+  resolved "https://registry.yarnpkg.com/@rollup/plugin-strip/-/plugin-strip-1.3.1.tgz#ef7b01c81c837863aec3885db4ac47a7e87a0ec5"
+  integrity sha512-JzjtQSdRVkrm/17SscKN0iEdnmvsV/9kLfAVx/MOilsdKI8EPY5x3hnLIhi2i/hXjXE4IlJxkv582nGxmt9qUg==
+  dependencies:
+    estree-walker "^0.6.0"
+    magic-string "^0.25.1"
+    rollup-pluginutils "^2.8.1"
+
 "@sinonjs/commons@^1", "@sinonjs/commons@^1.3.0", "@sinonjs/commons@^1.4.0":
   version "1.6.0"
   resolved "https://registry.yarnpkg.com/@sinonjs/commons/-/commons-1.6.0.tgz#ec7670432ae9c8eb710400d112c201a362d83393"
@@ -4053,7 +4062,7 @@ estraverse@^4.0.0, estraverse@^4.1.0, estraverse@^4.1.1, estraverse@^4.2.0:
   resolved "https://registry.yarnpkg.com/estraverse/-/estraverse-4.3.0.tgz#398ad3f3c5a24948be7725e83d11a7de28cdbd1d"
   integrity sha512-39nnKffWz8xN1BU/2c79n9nB9HDzo0niYUqx6xyqUnyoAnQyyWpOTdZEeiCch8BBu515t4wp9ZmgVfVhn9EBpw==
 
-estree-walker@^0.6.1:
+estree-walker@^0.6.0, estree-walker@^0.6.1:
   version "0.6.1"
   resolved "https://registry.yarnpkg.com/estree-walker/-/estree-walker-0.6.1.tgz#53049143f40c6eb918b23671d1fe3219f3a1b362"
   integrity sha512-SqmZANLWS0mnatqbSfRP5g8OXZC12Fgg1IwNtLsyHDzJizORW4khDfjPqJZsemPWBB2uqykUah5YpQ6epsqC/w==
@@ -6232,7 +6241,7 @@ macos-release@^2.2.0:
   resolved "https://registry.yarnpkg.com/macos-release/-/macos-release-2.3.0.tgz#eb1930b036c0800adebccd5f17bc4c12de8bb71f"
   integrity sha512-OHhSbtcviqMPt7yfw5ef5aghS2jzFVKEFyCJndQt2YpSQ9qRVSEv2axSJI1paVThEu+FFGs584h/1YhxjVqajA==
 
-magic-string@^0.25.2, magic-string@^0.25.3:
+magic-string@^0.25.1, magic-string@^0.25.2, magic-string@^0.25.3:
   version "0.25.4"
   resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.25.4.tgz#325b8a0a79fc423db109b77fd5a19183b7ba5143"
   integrity sha512-oycWO9nEVAP2RVPbIoDoA4Y7LFIJ3xRYov93gAyJhZkET1tNuB0u7uWkZS2LpBWTJUWnmau/To8ECWRC+jKNfw==