Skip to content

Commit

Permalink
Switch back to using host url instead of a blob-url to load rtl-text-…
Browse files Browse the repository at this point in the history
…plugin on the worker (mapbox#9122)
  • Loading branch information
Arindam Bose authored and mike-unearth committed Mar 18, 2020
1 parent 2b38770 commit 42fee37
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 30 deletions.
30 changes: 8 additions & 22 deletions src/source/rtl_text_plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import {Event, Evented} from '../util/evented';
import {getArrayBuffer} from '../util/ajax';
import browser from '../util/browser';
import window from '../util/window';
import assert from 'assert';
import {isWorker} from '../util/util';

Expand All @@ -17,8 +16,7 @@ const status = {

export type PluginState = {
pluginStatus: $Values<typeof status>;
pluginURL: ?string,
pluginBlobURL: ?string
pluginURL: ?string
};

type ErrorCallback = (error: ?Error) => void;
Expand All @@ -28,7 +26,6 @@ let _completionCallback = null;
//Variables defining the current state of the plugin
let pluginStatus = status.unavailable;
let pluginURL = null;
let pluginBlobURL = null;

export const triggerPluginCompletionEvent = function(error: ?Error) {
if (_completionCallback) {
Expand All @@ -37,7 +34,7 @@ export const triggerPluginCompletionEvent = function(error: ?Error) {
};

function sendPluginStateToWorker() {
evented.fire(new Event('pluginStateChange', {pluginStatus, pluginURL, pluginBlobURL}));
evented.fire(new Event('pluginStateChange', {pluginStatus, pluginURL}));
}

export const evented = new Evented();
Expand All @@ -48,7 +45,7 @@ export const getRTLTextPluginStatus = function () {

export const registerForPluginStateChange = function(callback: PluginStateSyncCallback) {
// Do an initial sync of the state
callback({pluginStatus, pluginURL, pluginBlobURL});
callback({pluginStatus, pluginURL});
// Listen for all future state changes
evented.on('pluginStateChange', callback);
return callback;
Expand All @@ -57,10 +54,6 @@ export const registerForPluginStateChange = function(callback: PluginStateSyncCa
export const clearRTLTextPlugin = function() {
pluginStatus = status.unavailable;
pluginURL = null;
if (pluginBlobURL) {
window.URL.revokeObjectURL(pluginBlobURL);
}
pluginBlobURL = null;
};

export const setRTLTextPlugin = function(url: string, callback: ?ErrorCallback, deferred: boolean = false) {
Expand All @@ -85,12 +78,10 @@ export const downloadRTLTextPlugin = function() {
pluginStatus = status.loading;
sendPluginStateToWorker();
if (pluginURL) {
getArrayBuffer({url: pluginURL}, (error, data) => {
getArrayBuffer({url: pluginURL}, (error) => {
if (error) {
triggerPluginCompletionEvent(error);
} else {
const rtlBlob = new window.Blob([data], {type: 'application/javascript'});
pluginBlobURL = window.URL.createObjectURL(rtlBlob);
pluginStatus = status.loaded;
sendPluginStateToWorker();
}
Expand All @@ -106,7 +97,7 @@ export const plugin: {
isLoading: () => boolean,
setState: (state: PluginState) => void,
isParsed: () => boolean,
getURLs: () => { blob: ?string, host: ?string }
getPluginURL: () => ?string
} = {
applyArabicShaping: null,
processBidirectionalText: null,
Expand All @@ -123,7 +114,6 @@ export const plugin: {

pluginStatus = state.pluginStatus;
pluginURL = state.pluginURL;
pluginBlobURL = state.pluginBlobURL;
},
isParsed(): boolean {
assert(isWorker(), 'rtl-text-plugin is only parsed on the worker-threads');
Expand All @@ -132,13 +122,9 @@ export const plugin: {
plugin.processBidirectionalText != null &&
plugin.processStyledBidirectionalText != null;
},
getURLs(): { blob: ?string, host: ?string } {
assert(isWorker(), 'rtl-text-plugin urls can only be queried from the worker threads');

return {
blob: pluginBlobURL,
host: pluginURL,
};
getPluginURL(): ?string {
assert(isWorker(), 'rtl-text-plugin url can only be queried from the worker threads');
return pluginURL;
}
};

Expand Down
8 changes: 4 additions & 4 deletions src/source/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,15 @@ export default class Worker {
syncRTLPluginState(map: string, state: PluginState, callback: Callback<boolean>) {
try {
globalRTLTextPlugin.setState(state);
const {blob, host} = globalRTLTextPlugin.getURLs();
const pluginURL = globalRTLTextPlugin.getPluginURL();
if (
globalRTLTextPlugin.isLoaded() &&
!globalRTLTextPlugin.isParsed() &&
blob != null && host != null // Not possible when `isLoaded` is true, but keeps flow happy
pluginURL != null // Not possible when `isLoaded` is true, but keeps flow happy
) {
this.self.importScripts(blob);
this.self.importScripts(pluginURL);
const complete = globalRTLTextPlugin.isParsed();
const error = complete ? undefined : new Error(`RTL Text Plugin failed to import scripts from ${host}`);
const error = complete ? undefined : new Error(`RTL Text Plugin failed to import scripts from ${pluginURL}`);
callback(error, complete);
}
} catch (e) {
Expand Down
3 changes: 1 addition & 2 deletions src/style/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,7 @@ class Style extends Evented {
this._rtlTextPluginCallback = Style.registerForPluginStateChange((event) => {
const state = {
pluginStatus: event.pluginStatus,
pluginURL: event.pluginURL,
pluginBlobURL: event.pluginBlobURL
pluginURL: event.pluginURL
};
self.dispatcher.broadcast('syncRTLPluginState', state, (err, results) => {
triggerPluginCompletionEvent(err);
Expand Down
3 changes: 1 addition & 2 deletions test/unit/style/style.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ test('Style', (t) => {
setRTLTextPlugin("/plugin.js",);
t.ok(style.dispatcher.broadcast.calledWith('syncRTLPluginState', {
pluginStatus: 'deferred',
pluginURL: "/plugin.js",
pluginBlobURL: null
pluginURL: "/plugin.js"
}));
window.clearFakeWorkerPresence();
t.end();
Expand Down

0 comments on commit 42fee37

Please sign in to comment.