Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(93)

Issues Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 1196193016: DevTools: [CSS] promisify CSS domain (Closed)

Created:
4 years, 10 months ago by lushnikov
Modified:
4 years, 10 months ago
Reviewers:
alph, pfeldman
CC:
blink-reviews, caseq+blink_chromium.org, blink-reviews-style_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, sergeyv+blink_chromium.org, kozyatinskiy+blink_chromium.org
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

DevTools: [CSS] promisify CSS domain The patch makes all css agent commands to return promise. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197841

Patch Set 1 #

Patch Set 2 : fix compiler #

Patch Set 3 : do not console.error protocol errors #

Patch Set 4 : compile types #

Total comments: 7

Patch Set 5 : catchException type checks #

Total comments: 8

Patch Set 6 : address comments #

Total comments: 6

Patch Set 7 : address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+340 lines, -285 lines) Patch
M LayoutTests/inspector/agents-enable-disable.html View 1 2 3 1 chunk +10 lines, -6 lines 0 comments Download
M LayoutTests/inspector/elements/styles-3/selector-source-data.html View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/inspector/elements/styles-4/styles-new-API.html View 1 2 3 1 chunk +6 lines, -4 lines 0 comments Download
M LayoutTests/inspector/elements/styles-4/styles-source-offsets.html View 1 2 3 1 chunk +5 lines, -3 lines 0 comments Download
M LayoutTests/inspector/elements/styles-4/undo-add-new-rule.html View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/inspector/inspector-backend-commands.html View 1 2 3 1 chunk +21 lines, -17 lines 0 comments Download
M LayoutTests/inspector/inspector-backend-commands-expected.txt View 1 2 1 chunk +9 lines, -10 lines 0 comments Download
M LayoutTests/inspector/profiler/cpu-profiler-stopped-removed-race-expected.txt View 1 2 1 chunk +0 lines, -1 line 0 comments Download
M Source/devtools/front_end/elements/StylesSidebarPane.js View 1 2 3 4 5 6 7 chunks +46 lines, -54 lines 0 comments Download
M Source/devtools/front_end/platform/utilities.js View 1 2 3 4 1 chunk +12 lines, -0 lines 0 comments Download
M Source/devtools/front_end/sdk/CPUProfilerModel.js View 1 2 3 1 chunk +5 lines, -4 lines 0 comments Download
M Source/devtools/front_end/sdk/CSSStyleModel.js View 1 2 3 4 5 6 14 chunks +193 lines, -144 lines 0 comments Download
M Source/devtools/front_end/sdk/InspectorBackend.js View 1 2 3 5 chunks +6 lines, -15 lines 0 comments Download
M Source/devtools/front_end/timeline/TimelineModel.js View 1 2 3 1 chunk +5 lines, -4 lines 0 comments Download
M Source/devtools/scripts/generate_protocol_externs.py View 1 2 3 2 chunks +20 lines, -21 lines 0 comments Download

Depends on Patchset:

Issue 988523003 Patch 380001

Dependent Patchsets:

Issue 1204393002 Patch 1

Messages

Total messages: 24 (8 generated)
lushnikov
ptal
4 years, 10 months ago (2015-06-24 13:51:17 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1196193016/40001
4 years, 10 months ago (2015-06-24 13:52:20 UTC) #4
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 10 months ago (2015-06-24 15:23:48 UTC) #6
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1196193016/60001
4 years, 10 months ago (2015-06-24 16:04:34 UTC) #8
pfeldman
https://codereview.chromium.org/1196193016/diff/60001/Source/devtools/front_end/sdk/CSSStyleModel.js File Source/devtools/front_end/sdk/CSSStyleModel.js (right): https://codereview.chromium.org/1196193016/diff/60001/Source/devtools/front_end/sdk/CSSStyleModel.js#newcode48 Source/devtools/front_end/sdk/CSSStyleModel.js:48: this._agent.enable().spread(this._wasEnabled.bind(this)); Why spread? https://codereview.chromium.org/1196193016/diff/60001/Source/devtools/front_end/sdk/CSSStyleModel.js#newcode102 Source/devtools/front_end/sdk/CSSStyleModel.js:102: .catchException([]) 1. It is ...
4 years, 10 months ago (2015-06-24 16:38:12 UTC) #9
lushnikov
https://codereview.chromium.org/1196193016/diff/60001/Source/devtools/front_end/sdk/CSSStyleModel.js File Source/devtools/front_end/sdk/CSSStyleModel.js (right): https://codereview.chromium.org/1196193016/diff/60001/Source/devtools/front_end/sdk/CSSStyleModel.js#newcode48 Source/devtools/front_end/sdk/CSSStyleModel.js:48: this._agent.enable().spread(this._wasEnabled.bind(this)); Oops. Accidental. https://codereview.chromium.org/1196193016/diff/60001/Source/devtools/front_end/sdk/CSSStyleModel.js#newcode102 Source/devtools/front_end/sdk/CSSStyleModel.js:102: .catchException([]) On 2015/06/24 16:38:12, ...
4 years, 10 months ago (2015-06-24 16:45:25 UTC) #10
pfeldman
> It gets reported in the devtools console. Where? There is no catch after userCallback.
4 years, 10 months ago (2015-06-24 16:47:36 UTC) #11
pfeldman
i.e. throttler still dies, you are not fixing anything. Lets have a test for what ...
4 years, 10 months ago (2015-06-24 16:47:57 UTC) #12
alph
https://codereview.chromium.org/1196193016/diff/60001/Source/devtools/front_end/timeline/TimelineModel.js File Source/devtools/front_end/timeline/TimelineModel.js (right): https://codereview.chromium.org/1196193016/diff/60001/Source/devtools/front_end/timeline/TimelineModel.js#newcode656 Source/devtools/front_end/timeline/TimelineModel.js:656: return target.profilerAgent().stop(extractProfile).then(this._addCpuProfile.bind(this, target.id())); what's that? why stop gets a ...
4 years, 10 months ago (2015-06-24 16:54:18 UTC) #14
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: win_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/68148)
4 years, 10 months ago (2015-06-24 19:37:51 UTC) #16
pfeldman
https://codereview.chromium.org/1196193016/diff/80001/Source/devtools/front_end/sdk/CSSStyleModel.js File Source/devtools/front_end/sdk/CSSStyleModel.js (right): https://codereview.chromium.org/1196193016/diff/80001/Source/devtools/front_end/sdk/CSSStyleModel.js#newcode48 Source/devtools/front_end/sdk/CSSStyleModel.js:48: this._agent.enable().spread(this._wasEnabled.bind(this)); ? https://codereview.chromium.org/1196193016/diff/80001/Source/devtools/front_end/sdk/CSSStyleModel.js#newcode197 Source/devtools/front_end/sdk/CSSStyleModel.js:197: * @return {!Array.<?>} Please don't ...
4 years, 10 months ago (2015-06-25 10:00:52 UTC) #17
lushnikov
ptal https://codereview.chromium.org/1196193016/diff/80001/Source/devtools/front_end/sdk/CSSStyleModel.js File Source/devtools/front_end/sdk/CSSStyleModel.js (right): https://codereview.chromium.org/1196193016/diff/80001/Source/devtools/front_end/sdk/CSSStyleModel.js#newcode48 Source/devtools/front_end/sdk/CSSStyleModel.js:48: this._agent.enable().spread(this._wasEnabled.bind(this)); On 2015/06/25 10:00:52, pfeldman wrote: > ? ...
4 years, 10 months ago (2015-06-25 15:20:53 UTC) #18
pfeldman
lgtm https://codereview.chromium.org/1196193016/diff/100001/Source/devtools/front_end/elements/StylesSidebarPane.js File Source/devtools/front_end/elements/StylesSidebarPane.js (right): https://codereview.chromium.org/1196193016/diff/100001/Source/devtools/front_end/elements/StylesSidebarPane.js#newcode372 Source/devtools/front_end/elements/StylesSidebarPane.js:372: _getMatchedStylesForNode: function(node) _matchedStylesForNode https://codereview.chromium.org/1196193016/diff/100001/Source/devtools/front_end/sdk/CSSStyleModel.js File Source/devtools/front_end/sdk/CSSStyleModel.js (right): https://codereview.chromium.org/1196193016/diff/100001/Source/devtools/front_end/sdk/CSSStyleModel.js#newcode244 ...
4 years, 10 months ago (2015-06-25 16:16:11 UTC) #19
lushnikov
https://codereview.chromium.org/1196193016/diff/100001/Source/devtools/front_end/elements/StylesSidebarPane.js File Source/devtools/front_end/elements/StylesSidebarPane.js (right): https://codereview.chromium.org/1196193016/diff/100001/Source/devtools/front_end/elements/StylesSidebarPane.js#newcode372 Source/devtools/front_end/elements/StylesSidebarPane.js:372: _getMatchedStylesForNode: function(node) On 2015/06/25 16:16:11, pfeldman wrote: > _matchedStylesForNode ...
4 years, 10 months ago (2015-06-25 16:40:30 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1196193016/120001
4 years, 10 months ago (2015-06-25 16:41:10 UTC) #23
commit-bot: I haz the power
4 years, 10 months ago (2015-06-25 18:14:26 UTC) #24
Message was sent while issue was closed. 
Committed patchset #7 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197841

Powered by Google App Engine
This is Rietveld 408576698