New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 608106 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit 29 days ago
Closed: May 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 608109



Sign in to add a comment

DevTools: [LiveSASS] support watchdog interop

Project Member Reported by lushnikov@chromium.org, Apr 29 2016

Issue description

1. Setup Workspace for a website which uses SASS.
2. Launch WatchDog for the project
3. Open DevTools on the website, enable LiveSASS
4. Try to edit a SASS property in StylesSidebarPane
--
5. SASSSourceMapping listens to SASS updates and propogates changes to  uiSourceCodes 
6. Workspace takes care of changed uiSourceCodes and saves them to the file system.
7. Watchdog notices changes on FileSystem and re-compiles css/sourcemap.
8. Workspace updates CSS file and SourceMap. 

This is bad: the CSS file from the file system might be outdated, given that user kept editing SASS in DevTools after (4) during all this watchdog processing (5-8).

Possible solution here would be postponing either (5) or (8).
In case of postponing (8):
- Workspace should update CSS file from workspace only if LiveSASS is not alive or elements panel lost its focus.
- As the CSS/SourceMap files got updated, LiveSASS should be restarted

In case of postponing (5):
- uiSourceCodes should be updated only on ElementsPanel blur event.

 
Blocking: 608109
Also supported 
Project Member

Comment 3 by bugdroid1@chromium.org, May 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a05638f5d217da949f614a6aaae67a0711b0e151

commit a05638f5d217da949f614a6aaae67a0711b0e151
Author: lushnikov <lushnikov@chromium.org>
Date: Wed May 11 00:14:47 2016

DevTools: [LiveSASS] do not commit updated SASS sources to file system.

With this patch in place, SASS changes will not be saved to file system; instead, a working copy of files will be changed.

User will have to save changed SASS files
manually.

BUG= 608106 
R=pfeldman

Review-Url: https://codereview.chromium.org/1947943004
Cr-Commit-Position: refs/heads/master@{#392781}

[modify] https://crrev.com/a05638f5d217da949f614a6aaae67a0711b0e151/third_party/WebKit/Source/devtools/front_end/bindings/SASSSourceMapping.js
[modify] https://crrev.com/a05638f5d217da949f614a6aaae67a0711b0e151/third_party/WebKit/Source/devtools/front_end/workspace/UISourceCode.js

Project Member

Comment 4 by bugdroid1@chromium.org, May 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee0c104c20b4dd17f1f6ae00b1d09c88236ebda9

commit ee0c104c20b4dd17f1f6ae00b1d09c88236ebda9
Author: lushnikov <lushnikov@chromium.org>
Date: Thu May 12 01:09:52 2016

DevTools: liveSASS should tolerate race between iNotify and BrowserSync

In a specific setup, with BrowserSync, SASS watchdog and
DevTools Workspaces, there's a race between iNotify and BrowserSync.
Both are pushing updated StyleSheet into inspected page.

The problem here is that BrowserSync also uses a cache-busting URL for
the stylesheet, appending "?rel=<timestamp>" to stylesheet URL.

As a result, the following sequence of events takes place:
1. As SCSS file is updated on file system, watchDog picks up changes and
   produces new CSS files
2. BrowserSync streamlines new CSS into inspected page, with a cache-busting
   URL
3. DevTools start loading new SourceMap for the StyleSheet
4. At this moment, DevTools Workspace realizes CSS file changes and updates
   style sheets one more time. The stylesheet with a specific cache-busted URL
   goes away.
5. LiveSASS tries to fetch content for original StyleSheet with cache-busting
   URL (which was already removed) and fails.

Nevertheless, the LiveSASS should not be affected with
this race condition - both BrowserSync and iNotify push
the same content.

This patch stops relying on StyleSheet URL and starts looking for styleSheets
which refer to the sourceMap.

BUG= 608106 
R=dgozman, pfeldman

Review-Url: https://codereview.chromium.org/1965223003
Cr-Commit-Position: refs/heads/master@{#393127}

[add] https://crrev.com/ee0c104c20b4dd17f1f6ae00b1d09c88236ebda9/third_party/WebKit/LayoutTests/inspector/sass/test-mapping-with-cache-busting-url-expected.txt
[add] https://crrev.com/ee0c104c20b4dd17f1f6ae00b1d09c88236ebda9/third_party/WebKit/LayoutTests/inspector/sass/test-mapping-with-cache-busting-url.html
[modify] https://crrev.com/ee0c104c20b4dd17f1f6ae00b1d09c88236ebda9/third_party/WebKit/Source/devtools/front_end/sass/SASSSourceMapFactory.js

Status: Fixed (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3e2695d9380a54e59770b10a35c0de88e0e2a78d

commit 3e2695d9380a54e59770b10a35c0de88e0e2a78d
Author: lushnikov <lushnikov@chromium.org>
Date: Mon Jun 27 23:06:27 2016

DevTools: eliminate race condition in WI.StylesSourceMapping.

If the style file was disposed, it should not do any other scheduled
actions.

BUG= 608106 
R=pfeldman

Review-Url: https://codereview.chromium.org/2016943004
Cr-Commit-Position: refs/heads/master@{#402326}

[modify] https://crrev.com/3e2695d9380a54e59770b10a35c0de88e0e2a78d/third_party/WebKit/Source/devtools/front_end/bindings/StylesSourceMapping.js

Components: Platform>DevTools>Authoring
Components: Platform>DevTools

Sign in to add a comment