DevTools: Bring Sass support from behind the experiment

DevTools: Bring Sass support from behind the experiment

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

[apavlov, 2013-04-18 15:29:59]

Remove the "Support for Sass" experiment flag.

[pfeldman, 2013-04-18 16:14:15]

lgtm

Could you confirm that it does not result in additional cost to the users that
are not using sass prior to landing?

[apavlov, 2013-04-18 16:55:47]

On 2013/04/18 16:14:15, pfeldman wrote:
> lgtm
> 
> Could you confirm that it does not result in additional cost to the users that
> are not using sass prior to landing?

SASSSourceMapping will request and split all external stylesheets contents by
"\r?\n" and then match the last line against the sourceMapURL regex. This is
suboptimal (remnants from the previous @media solution) - we can just scan
everything starting from contents.lastIndexOf("\n") + 1. Other than that, no
performance impact.

[pfeldman, 2013-04-18 16:58:31]

Requesting stylesheet sounds like a serious performance impact. Would we not do
that otherwise? I'd much rather get the sourceMap info as a part of the CSS
header information as we do for JS source maps. Please don't land this until
this issue is fixed.

[apavlov, 2013-04-18 17:44:00]

On 2013/04/18 16:58:31, pfeldman wrote:
> Requesting stylesheet sounds like a serious performance impact. Would we not
do
> that otherwise? I'd much rather get the sourceMap info as a part of the CSS
> header information as we do for JS source maps. Please don't land this until
> this issue is fixed.

Yes, this sounds like a good solution. Will add the sourceMapURL to the
stylesheet header.

[apavlov, 2013-05-28 14:55:23]

Pavel, I guess this patch needs your approval from the scratch?

[vsevik, 2013-05-28 14:58:24]

lgtm

[commit-bot: I haz the power, 2013-05-28 14:58:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/apavlov@chromium.org/13845021/7001

[pfeldman, 2013-05-28 15:26:33]

Lets rename "SASS" to CSS source maps in the UI.

[commit-bot: I haz the power, 2013-05-28 17:35:34]

Retried try job too often on win_layout_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...

[commit-bot: I haz the power, 2013-05-28 19:21:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/apavlov@chromium.org/13845021/7001

[commit-bot: I haz the power, 2013-05-28 21:42:05]

Retried try job too often on win_layout_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...

[commit-bot: I haz the power, 2013-05-29 06:06:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/apavlov@chromium.org/13845021/7001

[commit-bot: I haz the power, 2013-05-29 09:11:39]

Retried try job too often on win_layout_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...

[vsevik, 2013-05-29 10:03:11]

not lgtm, we should improve auto reload support first and get rid of the word
"sass" in the frontend.

[apavlov, 2013-07-08 10:04:18]

On 2013/05/29 10:03:11, vsevik wrote:
> not lgtm, we should improve auto reload support first and get rid of the word
> "sass" in the frontend.

Hopefully, this patch is clear to land.

[vsevik, 2013-07-12 13:07:08]

https://codereview.chromium.org/13845021/diff/33001/Source/devtools/front_end...
File Source/devtools/front_end/SASSSourceMapping.js (right):

https://codereview.chromium.org/13845021/diff/33001/Source/devtools/front_end...
Source/devtools/front_end/SASSSourceMapping.js:47: this._mappingsEnabled =
WebInspector.settings.cssSourceMapsEnabled.get();
not needed

https://codereview.chromium.org/13845021/diff/33001/Source/devtools/front_end...
File Source/devtools/front_end/SettingsScreen.js (right):

https://codereview.chromium.org/13845021/diff/33001/Source/devtools/front_end...
Source/devtools/front_end/SettingsScreen.js:354:
p.appendChild(this._createCheckboxSetting(WebInspector.UIString("Auto-reload
generated CSS"), WebInspector.settings.cssReloadEnabled));
Gray out when css source maps are disabled.

[vsevik, 2013-07-12 13:07:34]

lgtm

[apavlov, 2013-07-12 14:11:12]

On 2013/07/12 13:07:08, vsevik wrote:
>
https://codereview.chromium.org/13845021/diff/33001/Source/devtools/front_end...
> File Source/devtools/front_end/SASSSourceMapping.js (right):
> 
>
https://codereview.chromium.org/13845021/diff/33001/Source/devtools/front_end...
> Source/devtools/front_end/SASSSourceMapping.js:47: this._mappingsEnabled =
> WebInspector.settings.cssSourceMapsEnabled.get();
> not needed

Done.

>
https://codereview.chromium.org/13845021/diff/33001/Source/devtools/front_end...
> File Source/devtools/front_end/SettingsScreen.js (right):
> 
>
https://codereview.chromium.org/13845021/diff/33001/Source/devtools/front_end...
> Source/devtools/front_end/SettingsScreen.js:354:
> p.appendChild(this._createCheckboxSetting(WebInspector.UIString("Auto-reload
> generated CSS"), WebInspector.settings.cssReloadEnabled));
> Gray out when css source maps are disabled.

Done.

[commit-bot: I haz the power, 2013-07-12 14:11:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/apavlov@chromium.org/13845021/44001

[commit-bot: I haz the power, 2013-07-12 15:31:46]

Change committed as 154123