DevTools: Get rid of synchronous XHRs in the frontend code

DevTools: Get rid of synchronous XHRs in the frontend code

According to https://xhr.spec.whatwg.org/, sync XHRs on the main thread
have detrimental effects on the end user's experience and shall be removed.

R=vsevik

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185043

[apavlov, 2014-11-07 14:11:09]

apavlov@chromium.org changed reviewers:
+ pfeldman@chromium.org

[apavlov, 2014-11-07 14:11:16]

PTAL

[pfeldman, 2014-11-07 14:51:18]

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
File Source/devtools/front_end/Runtime.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
Source/devtools/front_end/Runtime.js:246: Runtime.startWorker =
function(moduleName)
Lets extract WorkerRuntime.

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
Source/devtools/front_end/Runtime.js:750:
.then(loadStylesheetsPromise.bind(this))
make a prototype method? _loadStyleSheets (similarly to loadScripts.

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
File Source/devtools/front_end/profiler/HeapSnapshotProxy.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
Source/devtools/front_end/profiler/HeapSnapshotProxy.js:207:
this._pendingMessages.push(message);
You should chain it to the _pendingWorkerPromise instead:

this._pendingWorkerPromise().then(postMessage.bind(this));
function postMessage()
{
    this._worker.postMessage(message);
}

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
File Source/devtools/front_end/sdk/CSSParser.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
Source/devtools/front_end/sdk/CSSParser.js:22: _init: function(worker)
Here and in other _inits, please annotate worker.

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
Source/devtools/front_end/sdk/CSSParser.js:48: parse: function(text, callback)
Since this is async, you should promisify it as well and get rid of this._text

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
File Source/devtools/front_end/sources/FilteredItemSelectionDialog.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
Source/devtools/front_end/sources/FilteredItemSelectionDialog.js:534:
Runtime.startWorker("script_formatter_worker").then(this._init.bind(this,
uiSourceCode));
nit: this._uiSourceCode = uiSourceCode for simplicity?

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
File Source/devtools/front_end/sources/ScriptFormatter.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
Source/devtools/front_end/sources/ScriptFormatter.js:145: return
Runtime.startWorker("script_formatter_worker").then(started.bind(this));
You want to store this resulting promise and return it to subsequent calls.

[apavlov, 2014-11-10 07:44:02]

Patchset #2 (id:20001) has been deleted

[apavlov, 2014-11-10 07:44:12]

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
File Source/devtools/front_end/Runtime.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
Source/devtools/front_end/Runtime.js:246: Runtime.startWorker =
function(moduleName)
On 2014/11/07 14:51:17, pfeldman wrote:
> Lets extract WorkerRuntime.

Done.

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
Source/devtools/front_end/Runtime.js:750:
.then(loadStylesheetsPromise.bind(this))
On 2014/11/07 14:51:17, pfeldman wrote:
> make a prototype method? _loadStyleSheets (similarly to loadScripts.

Done.

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
File Source/devtools/front_end/profiler/HeapSnapshotProxy.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
Source/devtools/front_end/profiler/HeapSnapshotProxy.js:207:
this._pendingMessages.push(message);
On 2014/11/07 14:51:17, pfeldman wrote:
> You should chain it to the _pendingWorkerPromise instead:
> 
> this._pendingWorkerPromise().then(postMessage.bind(this));
> function postMessage()
> {
>     this._worker.postMessage(message);
> }

Done.

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
File Source/devtools/front_end/sdk/CSSParser.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
Source/devtools/front_end/sdk/CSSParser.js:22: _init: function(worker)
On 2014/11/07 14:51:17, pfeldman wrote:
> Here and in other _inits, please annotate worker.

Done.

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
Source/devtools/front_end/sdk/CSSParser.js:48: parse: function(text, callback)
On 2014/11/07 14:51:17, pfeldman wrote:
> Since this is async, you should promisify it as well and get rid of this._text

Done.

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
File Source/devtools/front_end/sources/FilteredItemSelectionDialog.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
Source/devtools/front_end/sources/FilteredItemSelectionDialog.js:534:
Runtime.startWorker("script_formatter_worker").then(this._init.bind(this,
uiSourceCode));
On 2014/11/07 14:51:17, pfeldman wrote:
> nit: this._uiSourceCode = uiSourceCode for simplicity?

Done.

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
File Source/devtools/front_end/sources/ScriptFormatter.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/1/Source/devtools/front...
Source/devtools/front_end/sources/ScriptFormatter.js:145: return
Runtime.startWorker("script_formatter_worker").then(started.bind(this));
On 2014/11/07 14:51:17, pfeldman wrote:
> You want to store this resulting promise and return it to subsequent calls.

Done.

[pfeldman, 2014-11-10 10:23:26]

https://chromiumcodereview.appspot.com/685203003/diff/40001/Source/devtools/f...
File Source/devtools/front_end/bindings/TempFile.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/40001/Source/devtools/f...
Source/devtools/front_end/bindings/TempFile.js:390: callback();
Now new WebInspector.TempStorageCleaner().ensureStorageCleared(callback) will
immediately issue callback.

https://chromiumcodereview.appspot.com/685203003/diff/40001/Source/devtools/f...
File Source/devtools/front_end/sdk/CSSParser.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/40001/Source/devtools/f...
Source/devtools/front_end/sdk/CSSParser.js:57: return
this._innerParseContentPromise(styleSheetHeader.requestContentPromise());
return styleSheetHeader.requestContent().then(this._innerParse.bind(this));

https://chromiumcodereview.appspot.com/685203003/diff/40001/Source/devtools/f...
Source/devtools/front_end/sdk/CSSParser.js:67: return
this._innerParseContentPromise(Promise.resolve(text));
return this._innerParse(text);

https://chromiumcodereview.appspot.com/685203003/diff/40001/Source/devtools/f...
Source/devtools/front_end/sdk/CSSParser.js:106: this._unlock();
return new Promise(promiseBody);

function promiseBody(fullfill, reject)
{
    this._fullfill = fullfill;
    this._pendingWorkerPromise.then(postMessage.bind(this));
    function postMessage()
    {
        this._worker.postMessage();
    }
}

https://chromiumcodereview.appspot.com/685203003/diff/40001/Source/devtools/f...
Source/devtools/front_end/sdk/CSSParser.js:119: this._fulfillCallback = fulfill;
I am lost here.

https://chromiumcodereview.appspot.com/685203003/diff/40001/Source/devtools/f...
File Source/devtools/front_end/sdk/CSSStyleModel.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/40001/Source/devtools/f...
Source/devtools/front_end/sdk/CSSStyleModel.js:1609:
this.requestContent(callback.bind(null, fulfill));
this.requestContent(fullfill) ?

https://chromiumcodereview.appspot.com/685203003/diff/40001/Source/devtools/f...
File Source/devtools/front_end/sources/FilteredItemSelectionDialog.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/40001/Source/devtools/f...
Source/devtools/front_end/sources/FilteredItemSelectionDialog.js:635: if
(this._outlineWorker) {
What if dispose comes first?

https://chromiumcodereview.appspot.com/685203003/diff/40001/Source/devtools/f...
Source/devtools/front_end/sources/FilteredItemSelectionDialog.js:635: if
(this._outlineWorker) {
We should make an abstract worker class that handles it.

https://chromiumcodereview.appspot.com/685203003/diff/40001/Source/devtools/f...
File Source/devtools/front_end/sources/ScriptFormatter.js (right):

https://chromiumcodereview.appspot.com/685203003/diff/40001/Source/devtools/f...
Source/devtools/front_end/sources/ScriptFormatter.js:133:
this._cachedWorker.postMessage(message);
It would have deferred postMessage on its own.

[pfeldman, 2014-11-10 14:52:30]

lgtm.

[apavlov, 2014-11-10 15:28:05]

The CQ bit was checked by apavlov@chromium.org

[commit-bot: I haz the power, 2014-11-10 15:28:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/685203003/80001

[apavlov, 2014-11-10 15:40:46]

The CQ bit was unchecked by apavlov@chromium.org

[apavlov, 2014-11-10 16:00:10]

The CQ bit was checked by apavlov@chromium.org

[commit-bot: I haz the power, 2014-11-10 16:00:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/685203003/100001

[commit-bot: I haz the power, 2014-11-10 17:04:57]

Committed patchset #5 (id:100001) as 185043