Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(387)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 438333003: DevTools: use setImmediate instead of setTimeout for dispatching messages on devtools frontend in d… (Closed)

Created:
6 years, 4 months ago by vsevik
Modified:
6 years, 2 months ago
Reviewers:
dgozman, pfeldman, yurys
CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, malch+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, sergeyv+blink_chromium.org, aandrey+blink_chromium.org
Project:
blink
Visibility:
Public.

Description

DevTools: use setImmediate instead of setTimeout for dispatching messages on devtools frontend in debug mode. R=pfeldman, dgozman Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179924

Patch Set 1 #

Total comments: 2

Patch Set 2 : Promises based impl #

Patch Set 3 : #

Patch Set 4 : #

Patch Set 5 : Fixed tests #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -13 lines) Patch
M Source/devtools/front_end/common/utilities.js View 1 2 3 4 1 chunk +16 lines, -0 lines 2 comments Download
M Source/devtools/front_end/externs.js View 1 chunk +6 lines, -0 lines 0 comments Download
M Source/devtools/front_end/host/InspectorFrontendHost.js View 3 chunks +3 lines, -13 lines 0 comments Download

Messages

Total messages: 20 (0 generated)
dgozman
lgtm https://codereview.chromium.org/438333003/diff/1/Source/devtools/front_end/sources/SourcesPanel.js File Source/devtools/front_end/sources/SourcesPanel.js (right): https://codereview.chromium.org/438333003/diff/1/Source/devtools/front_end/sources/SourcesPanel.js#newcode1 Source/devtools/front_end/sources/SourcesPanel.js:1: nit: remove this accidental empty line.
6 years, 4 months ago (2014-08-07 16:55:30 UTC) #1
yurys
https://codereview.chromium.org/438333003/diff/1/Source/devtools/front_end/common/utilities.js File Source/devtools/front_end/common/utilities.js (right): https://codereview.chromium.org/438333003/diff/1/Source/devtools/front_end/common/utilities.js#newcode1812 Source/devtools/front_end/common/utilities.js:1812: Object.observe(dummy, dummyChanged); Wouldn't it be more straightforward with Promise ...
6 years, 4 months ago (2014-08-07 17:02:13 UTC) #2
vsevik
On 2014/08/07 17:02:13, yurys wrote: > https://codereview.chromium.org/438333003/diff/1/Source/devtools/front_end/common/utilities.js > File Source/devtools/front_end/common/utilities.js (right): > > https://codereview.chromium.org/438333003/diff/1/Source/devtools/front_end/common/utilities.js#newcode1812 > ...
6 years, 4 months ago (2014-08-07 17:04:12 UTC) #3
vsevik
FYI: http://jsperf.com/setimmediate-test/16
6 years, 4 months ago (2014-08-08 09:07:13 UTC) #4
vsevik
The CQ bit was checked by vsevik@chromium.org
6 years, 4 months ago (2014-08-08 09:16:29 UTC) #5
vsevik
The CQ bit was unchecked by vsevik@chromium.org
6 years, 4 months ago (2014-08-08 09:16:52 UTC) #6
vsevik
The CQ bit was checked by vsevik@chromium.org
6 years, 4 months ago (2014-08-08 09:17:38 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vsevik@chromium.org/438333003/40001
6 years, 4 months ago (2014-08-08 09:18:25 UTC) #8
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_blink_dbg on tryserver.blink ...
6 years, 4 months ago (2014-08-08 12:29:54 UTC) #9
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 4 months ago (2014-08-08 12:31:34 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/19008)
6 years, 4 months ago (2014-08-08 12:31:35 UTC) #11
vsevik
The CQ bit was checked by vsevik@chromium.org
6 years, 4 months ago (2014-08-08 13:35:48 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vsevik@chromium.org/438333003/60001
6 years, 4 months ago (2014-08-08 13:36:10 UTC) #13
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_blink_dbg on tryserver.blink ...
6 years, 4 months ago (2014-08-08 17:03:56 UTC) #14
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 4 months ago (2014-08-08 17:44:25 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/19060)
6 years, 4 months ago (2014-08-08 17:44:27 UTC) #16
vsevik
The CQ bit was checked by vsevik@chromium.org
6 years, 4 months ago (2014-08-11 07:46:30 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vsevik@chromium.org/438333003/80001
6 years, 4 months ago (2014-08-11 07:47:32 UTC) #18
commit-bot: I haz the power
Change committed as 179924
6 years, 4 months ago (2014-08-11 08:45:06 UTC) #19
pfeldman
6 years, 2 months ago (2014-09-26 09:42:20 UTC) #20
Message was sent while issue was closed. 
https://codereview.chromium.org/438333003/diff/80001/Source/devtools/front_en...
File Source/devtools/front_end/common/utilities.js (right):

https://codereview.chromium.org/438333003/diff/80001/Source/devtools/front_en...
Source/devtools/front_end/common/utilities.js:1753: self.setImmediate =
(function() {
@vsevik, @dgozman: do you want to count the number of style violations in this
function declaration? I can see about a dozen.

https://codereview.chromium.org/438333003/diff/80001/Source/devtools/front_en...
Source/devtools/front_end/common/utilities.js:1762: new
Promise(function(resolve,reject){ resolve(null);}).then(run);
self.setImmediate = function(callback)
{
    Promise.resolve().then(callback).catch(function(e) { console.error(e); });
}

Powered by Google App Engine
This is Rietveld 408576698