Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perform XHR in the main context #7818

Merged
merged 1 commit into from
Jan 30, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/style/style.js
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@ import GlyphManager from '../render/glyph_manager';
import Light from './light';
import LineAtlas from '../render/line_atlas';
import { pick, clone, extend, deepEqual, filterObject, mapObject } from '../util/util';
import { getJSON, getReferrer, ResourceType } from '../util/ajax';
import { getJSON, getReferrer, makeRequest, ResourceType } from '../util/ajax';
import { isMapboxURL, normalizeStyleURL } from '../util/mapbox';
import browser from '../util/browser';
import Dispatcher from '../util/dispatcher';
@@ -51,6 +51,7 @@ import type {Callback} from '../types/callback';
import type EvaluationParameters from './evaluation_parameters';
import type {Placement} from '../symbol/placement';
import type {Cancelable} from '../types/cancelable';
import type {RequestParameters, ResponseCallback} from '../util/ajax';
import type {GeoJSON} from '@mapbox/geojson-types';
import type {
LayerSpecification,
@@ -1213,6 +1214,10 @@ class Style extends Evented {
getGlyphs(mapId: string, params: {stacks: {[string]: Array<number>}}, callback: Callback<{[string]: {[number]: ?StyleGlyph}}>) {
this.glyphManager.getGlyphs(params.stacks, callback);
}

getResource(mapId: string, params: RequestParameters, callback: ResponseCallback<any>): Cancelable {
return makeRequest(params, callback);
}
}

Style.getSourceType = getSourceType;
25 changes: 22 additions & 3 deletions src/util/actor.js
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@ import { bindAll } from './util';
import { serialize, deserialize } from './web_worker_transfer';

import type {Transferable} from '../types/transferable';
import type {Cancelable} from '../types/cancelable';

/**
* An implementation of the [Actor design pattern](http://en.wikipedia.org/wiki/Actor_model)
@@ -42,7 +43,7 @@ class Actor {
* @param targetMapId A particular mapId to which to send this message.
* @private
*/
send(type: string, data: mixed, callback: ?Function, targetMapId: ?string) {
send(type: string, data: mixed, callback: ?Function, targetMapId: ?string): ?Cancelable {
const id = callback ? `${this.mapId}:${this.callbackID++}` : null;
if (callback) this.callbacks[id] = callback;
const buffers: Array<Transferable> = [];
@@ -53,6 +54,16 @@ class Actor {
id: String(id),
data: serialize(data, buffers)
}, buffers);
if (callback) {
return {
cancel: () => this.target.postMessage({
targetMapId,
sourceMapId: this.mapId,
type: '<cancel>',
id: String(id)
})
};
}
}

receive(message: Object) {
@@ -64,6 +75,7 @@ class Actor {
return;

const done = (err, data) => {
delete this.callbacks[id];
const buffers: Array<Transferable> = [];
this.target.postMessage({
sourceMapId: this.mapId,
@@ -74,7 +86,7 @@ class Actor {
}, buffers);
};

if (data.type === '<response>') {
if (data.type === '<response>' || data.type === '<cancel>') {
callback = this.callbacks[data.id];
delete this.callbacks[data.id];
if (callback && data.error) {
@@ -84,7 +96,14 @@ class Actor {
}
} else if (typeof data.id !== 'undefined' && this.parent[data.type]) {
// data.type == 'loadTile', 'removeTile', etc.
this.parent[data.type](data.sourceMapId, deserialize(data.data), done);
// Add a placeholder so that we can discover when the done callback was called already.
this.callbacks[data.id] = null;
const cancelable = this.parent[data.type](data.sourceMapId, deserialize(data.data), done);
if (cancelable && this.callbacks[data.id] === null) {
// Only add the cancelable callback if the done callback wasn't already called.
// Otherwise we will never be able to delete it.
this.callbacks[data.id] = cancelable;
}
} else if (typeof data.id !== 'undefined' && this.parent.getWorkerSource) {
// data.type == sourcetype.method
const keys = data.type.split('.');
26 changes: 22 additions & 4 deletions src/util/ajax.js
Original file line number Diff line number Diff line change
@@ -71,14 +71,17 @@ class AJAXError extends Error {
}
}

function isWorker() {
return typeof WorkerGlobalScope !== 'undefined' && typeof self !== 'undefined' &&
self instanceof WorkerGlobalScope;
}

// Ensure that we're sending the correct referrer from blob URL worker bundles.
// For files loaded from the local file system, `location.origin` will be set
// to the string(!) "null" (Firefox), or "file://" (Chrome, Safari, Edge, IE),
// and we will set an empty referrer. Otherwise, we're using the document's URL.
/* global self, WorkerGlobalScope */
export const getReferrer = typeof WorkerGlobalScope !== 'undefined' &&
typeof self !== 'undefined' &&
self instanceof WorkerGlobalScope ?
export const getReferrer = isWorker() ?
() => self.worker && self.worker.referrer :
() => {
const origin = window.location.origin;
@@ -158,7 +161,22 @@ function makeXMLHttpRequest(requestParameters: RequestParameters, callback: Resp
return { cancel: () => xhr.abort() };
}

const makeRequest = window.fetch && window.Request && window.AbortController ? makeFetchRequest : makeXMLHttpRequest;
export const makeRequest = function(requestParameters: RequestParameters, callback: ResponseCallback<any>): Cancelable {
// We're trying to use the Fetch API if possible. However, in some situations we can't use it:
// - IE11 doesn't support it at all. In this case, we dispatch the request to the main thread so
// that we can get an accruate referrer header.
// - Requests for resources with the file:// URI scheme don't work with the Fetch API either. In
// this case we unconditionally use XHR on the current thread since referrers don't matter.
if (!/^file:/.test(requestParameters.url)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't work with relative URLs

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm this, you have to specify the absolute path with the file protocol before passing it in, else it thinks it is an http request and throws an error about the file protocol like before.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't support relative URLs intentionally — see the discussion at #3636 and linked tickets. That said, #8612 should now correctly classify these URLs as file URLs.

if (window.fetch && window.Request && window.AbortController) {
return makeFetchRequest(requestParameters, callback);
}
if (isWorker() && self.worker && self.worker.actor) {
return self.worker.actor.send('getResource', requestParameters, callback);
}
}
return makeXMLHttpRequest(requestParameters, callback);
};

export const getJSON = function(requestParameters: RequestParameters, callback: ResponseCallback<Object>): Cancelable {
return makeRequest(extend(requestParameters, { type: 'json' }), callback);
2 changes: 2 additions & 0 deletions test/ajax_stubs.js
Original file line number Diff line number Diff line change
@@ -67,6 +67,8 @@ export const getArrayBuffer = function({ url }, callback) {
});
};

export const makeRequest = getArrayBuffer;

export const postData = function({ url, body }, callback) {
return request.post(url, body, (error, response, body) => {
if (!error && response.statusCode >= 200 && response.statusCode < 300) {