Skip to content

Commit

Permalink
fix review's issues
Browse files Browse the repository at this point in the history
  • Loading branch information
LavrovArtem committed Aug 31, 2017
1 parent 87b5895 commit d60e74c
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 25 deletions.
32 changes: 15 additions & 17 deletions src/client/sandbox/xhr.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export default class XhrSandbox extends SandboxBase {
nativeMethods.xhrRemoveEventListener.call(this, 'readystatechange', syncCookieWithClient);
};

const xhrConstrWrapper = function () {
const xmlHttpRequestWrapper = function () {
const xhr = new nativeMethods.XMLHttpRequest();

nativeMethods.xhrAddEventListener.call(xhr, 'readystatechange', emitXhrCompletedEvent);
Expand All @@ -90,13 +90,13 @@ export default class XhrSandbox extends SandboxBase {
return xhr;
};

window.XMLHttpRequest = xhrConstrWrapper;
xhrConstrWrapper.prototype = xmlHttpRequestProto;
xhrConstrWrapper.toString = () => xhrConstructorString;
xmlHttpRequestProto.constructor = xhrConstrWrapper;
window.XMLHttpRequest = xmlHttpRequestWrapper;
xmlHttpRequestWrapper.prototype = xmlHttpRequestProto;
xmlHttpRequestWrapper.toString = () => xhrConstructorString;
xmlHttpRequestProto.constructor = xmlHttpRequestWrapper;

const defineConstructorConst = (name, value) =>
nativeMethods.objectDefineProperty.call(window.Object, xhrConstrWrapper, name, { value, enumerable: true });
nativeMethods.objectDefineProperty.call(window.Object, xmlHttpRequestWrapper, name, { value, enumerable: true });

defineConstructorConst('UNSENT', 0);
defineConstructorConst('OPENED', 1);
Expand Down Expand Up @@ -129,28 +129,26 @@ export default class XhrSandbox extends SandboxBase {
};

xmlHttpRequestProto.send = function () {
const xhr = this;

xhrSandbox.emit(xhrSandbox.BEFORE_XHR_SEND_EVENT, { xhr });
xhrSandbox.emit(xhrSandbox.BEFORE_XHR_SEND_EVENT, { xhr: this });

// NOTE: Add the XHR request mark, so that a proxy can recognize a request as a XHR request. As all
// requests are passed to the proxy, we need to perform Same Origin Policy compliance checks on the
// server side. So, we pass the CORS support flag to inform the proxy that it can analyze the
// Access-Control_Allow_Origin flag and skip "preflight" requests.
nativeMethods.xhrSetRequestHeader.call(xhr, XHR_HEADERS.requestMarker, 'true');
nativeMethods.xhrSetRequestHeader.call(xhr, XHR_HEADERS.origin, getOriginHeader());
nativeMethods.xhrSetRequestHeader.call(this, XHR_HEADERS.requestMarker, 'true');
nativeMethods.xhrSetRequestHeader.call(this, XHR_HEADERS.origin, getOriginHeader());

if (xhrSandbox.corsSupported)
nativeMethods.xhrSetRequestHeader.call(xhr, XHR_HEADERS.corsSupported, 'true');
nativeMethods.xhrSetRequestHeader.call(this, XHR_HEADERS.corsSupported, 'true');

if (xhr.withCredentials)
nativeMethods.xhrSetRequestHeader.call(xhr, XHR_HEADERS.withCredentials, 'true');
if (this.withCredentials)
nativeMethods.xhrSetRequestHeader.call(this, XHR_HEADERS.withCredentials, 'true');

nativeMethods.xhrSend.apply(xhr, arguments);
nativeMethods.xhrSend.apply(this, arguments);

// NOTE: For xhr with the sync mode
emitXhrCompletedEvent.call(xhr);
syncCookieWithClient.call(xhr);
emitXhrCompletedEvent.call(this);
syncCookieWithClient.call(this);
};

xmlHttpRequestProto.getResponseHeader = function (name) {
Expand Down
17 changes: 9 additions & 8 deletions test/client/fixtures/sandbox/xhr-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ test('constructor', function () {
module('regression');

asyncTest('unexpected text modifying during typing text in the search input on the http://www.google.co.uk (B238528)', function () {
var timeout = 100;
var timeoutId = 100;
var syncActionExecuted = false;
var xhr = new XMLHttpRequest();

Expand All @@ -101,7 +101,7 @@ asyncTest('unexpected text modifying during typing text in the search input on t
};

xhr.onreadystatechange = ready;
xhr.open('GET', '/xhr-test/' + timeout);
xhr.open('GET', '/xhr-test/' + timeoutId);
xhr.send(null);

syncActionExecuted = true;
Expand Down Expand Up @@ -219,11 +219,11 @@ asyncTest('authorization headers by client should be processed (GH-1016)', funct
xhr.send();
});

asyncTest('our internal the "readystatechange" handler must be first (GH-1283)', function () {
var xhr = new XMLHttpRequest();
var timeout = null;
var testDone = function (eventObj) {
clearTimeout(timeout);
asyncTest('"XHR_COMPLETED_EVENT" for completed requests should be raised before calling outer handlers (GH-1283)', function () {
var xhr = new XMLHttpRequest();
var timeoutId = null;
var testDone = function (eventObj) {
clearTimeout(timeoutId);
ok(!!eventObj);
start();
};
Expand All @@ -235,8 +235,9 @@ asyncTest('our internal the "readystatechange" handler must be first (GH-1283)',

xhr.addEventListener('readystatechange', readyStateChangeHandler, true);
xhr.onreadystatechange = readyStateChangeHandler;

xhrSandbox.on(xhrSandbox.XHR_COMPLETED_EVENT, testDone);
timeout = setTimeout(testDone, 2000);
timeoutId = setTimeout(testDone, 2000);

xhr.open('GET', '/xhr-test/', true);
xhr.send();
Expand Down

0 comments on commit d60e74c

Please sign in to comment.