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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 297923002: DevTools: Decouple CSS model from UI entities (Closed)

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

Description

DevTools: Decouple CSS model from UI entities CSSPresentationModel has been introduced in order to - manage source mappings, - store live locations, - perform the rawLocationToUILocation conversions. R=pfeldman, vsevik Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179012

Patch Set 1 : Correct patch #

Patch Set 2 : Remove asserts, clear model on main frame navigation #

Patch Set 3 : Rebased patch #

Patch Set 4 : Rebase atop new workspace bindings #

Total comments: 8

Patch Set 5 : Address comments #

Total comments: 2

Patch Set 6 : Merge CSSStyleSheetMapping into CSSWorkspaceBinding #

Patch Set 7 : Remove unused argument in test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+373 lines, -304 lines) Patch
M LayoutTests/http/tests/inspector/elements/styles/selector-line-deprecated.html View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M LayoutTests/http/tests/inspector/elements/styles/update-locations-on-filesystem-scss-load.html View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/inspector/stylesheet-source-mapping.html View 1 2 3 4 5 6 2 chunks +3 lines, -3 lines 0 comments Download
M Source/devtools/devtools.gypi View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M Source/devtools/front_end/inspector.html View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M Source/devtools/front_end/main/Main.js View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M Source/devtools/front_end/sdk/CSSStyleModel.js View 1 2 3 4 5 6 8 chunks +22 lines, -217 lines 0 comments Download
D Source/devtools/front_end/sdk/CSSStyleSheetMapping.js View 1 2 3 4 1 chunk +0 lines, -68 lines 0 comments Download
M Source/devtools/front_end/sdk/CSSWorkspaceBinding.js View 1 2 3 4 5 2 chunks +339 lines, -3 lines 0 comments Download
M Source/devtools/front_end/sdk/SASSSourceMapping.js View 1 2 3 4 4 chunks +4 lines, -4 lines 0 comments Download
M Source/devtools/front_end/sdk/StylesSourceMapping.js View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M Source/devtools/scripts/frontend_modules.json View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 12 (0 generated)
apavlov
6 years, 7 months ago (2014-05-23 10:45:49 UTC) #1
apavlov
PTAL, the patch has been rebased
6 years, 5 months ago (2014-07-14 15:23:35 UTC) #2
apavlov
Ping? This is blocking my further code motion (along with the DebuggerPresentationModel extraction).
6 years, 5 months ago (2014-07-16 09:47:07 UTC) #3
apavlov
PTAL at the rebased patch
6 years, 5 months ago (2014-07-22 13:46:27 UTC) #4
vsevik
https://codereview.chromium.org/297923002/diff/100007/Source/devtools/front_end/sdk/CSSStyleModel.js File Source/devtools/front_end/sdk/CSSStyleModel.js (left): https://codereview.chromium.org/297923002/diff/100007/Source/devtools/front_end/sdk/CSSStyleModel.js#oldcode1524 Source/devtools/front_end/sdk/CSSStyleModel.js:1524: if (!this.header() || typeof this.lineNumberInSource() === "undefined") Shouldn't it ...
6 years, 5 months ago (2014-07-22 14:59:01 UTC) #5
apavlov
https://codereview.chromium.org/297923002/diff/100007/Source/devtools/front_end/sdk/CSSStyleModel.js File Source/devtools/front_end/sdk/CSSStyleModel.js (left): https://codereview.chromium.org/297923002/diff/100007/Source/devtools/front_end/sdk/CSSStyleModel.js#oldcode1524 Source/devtools/front_end/sdk/CSSStyleModel.js:1524: if (!this.header() || typeof this.lineNumberInSource() === "undefined") On 2014/07/22 ...
6 years, 5 months ago (2014-07-22 16:19:53 UTC) #6
vsevik
https://chromiumcodereview.appspot.com/297923002/diff/140001/LayoutTests/http/tests/inspector/stylesheet-source-mapping.html File LayoutTests/http/tests/inspector/stylesheet-source-mapping.html (right): https://chromiumcodereview.appspot.com/297923002/diff/140001/LayoutTests/http/tests/inspector/stylesheet-source-mapping.html#newcode15 LayoutTests/http/tests/inspector/stylesheet-source-mapping.html:15: var cssModel = new WebInspector.CSSStyleModel(target, InspectorTest.testWorkspace); workspace is not ...
6 years, 5 months ago (2014-07-22 16:37:27 UTC) #7
vsevik
https://chromiumcodereview.appspot.com/297923002/diff/140001/Source/devtools/front_end/sdk/CSSWorkspaceBinding.js File Source/devtools/front_end/sdk/CSSWorkspaceBinding.js (right): https://chromiumcodereview.appspot.com/297923002/diff/140001/Source/devtools/front_end/sdk/CSSWorkspaceBinding.js#newcode390 Source/devtools/front_end/sdk/CSSWorkspaceBinding.js:390: WebInspector.CSSStyleSheetMapping = function(cssModel, workspace, networkWorkspaceBinding) This is essentially the ...
6 years, 5 months ago (2014-07-22 16:37:59 UTC) #8
vsevik
lgtm
6 years, 4 months ago (2014-07-28 08:13:29 UTC) #9
apavlov
The CQ bit was checked by apavlov@chromium.org
6 years, 4 months ago (2014-07-28 08:13:45 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/apavlov@chromium.org/297923002/180001
6 years, 4 months ago (2014-07-28 08:14:35 UTC) #11
commit-bot: I haz the power
6 years, 4 months ago (2014-07-28 09:15:39 UTC) #12
Message was sent while issue was closed. 
Change committed as 179012

Powered by Google App Engine
This is Rietveld 408576698