DevTools: Add support for //# sourceURL (sourceMappingURL) comments and deprecate //@ ones

DevTools: Add support for //# sourceURL (sourceMappingURL) comments and deprecate //@ ones

Added support for a new type of comments and a console warning when the older one is used.

R=yurys@chromium.org

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

[vsevik, 2013-05-29 12:56:18]

Please review.

[yurys, 2013-05-29 13:34:12]

https://codereview.chromium.org/15832007/diff/1/LayoutTests/inspector/console...
File LayoutTests/inspector/console/console-trace-in-eval.html (right):

https://codereview.chromium.org/15832007/diff/1/LayoutTests/inspector/console...
LayoutTests/inspector/console/console-trace-in-eval.html:47: with //#
sourceURL=url it will dump a stack trace that will have
Can we continue to test the old functionality as well until it is removed?

[apavlov, 2013-05-29 13:58:04]

https://codereview.chromium.org/15832007/diff/1/LayoutTests/http/tests/inspec...
File LayoutTests/http/tests/inspector/debugger-test.js (right):

https://codereview.chromium.org/15832007/diff/1/LayoutTests/http/tests/inspec...
LayoutTests/http/tests/inspector/debugger-test.js:288: var hasSourceURL =
!!source.match(/\/\/#\ssourceURL=\s*(\S*?)\s*$/m) ||
!!source.match(/\/\/@\ssourceURL=\s*(\S*?)\s*$/m);
You could use RegExp.prototype.test here (and avoid the !! thing)

https://codereview.chromium.org/15832007/diff/1/Source/core/inspector/Inspect...
File Source/core/inspector/InspectorDebuggerAgent.cpp (right):

https://codereview.chromium.org/15832007/diff/1/Source/core/inspector/Inspect...
Source/core/inspector/InspectorDebuggerAgent.cpp:642:
addConsoleMessage(NetworkMessageSource, WarningMessageLevel, "\"//@
sourceMapURL=\" source mapping url declaration is deprecated, \"//#
sourceMapURL=\" declaration should be used instead.", script.url);
url -> URL ?

https://codereview.chromium.org/15832007/diff/1/Source/core/inspector/Inspect...
Source/core/inspector/InspectorDebuggerAgent.cpp:642:
addConsoleMessage(NetworkMessageSource, WarningMessageLevel, "\"//@
sourceMapURL=\" source mapping url declaration is deprecated, \"//#
sourceMapURL=\" declaration should be used instead.", script.url);
Perhaps, "...is deprecated and will be unsupported soon" ?

https://codereview.chromium.org/15832007/diff/1/Source/core/inspector/Inspect...
Source/core/inspector/InspectorDebuggerAgent.cpp:668:
addConsoleMessage(NetworkMessageSource, WarningMessageLevel, "\"//@ sourceURL=\"
source url declaration is deprecated, \"//# sourceURL=\" declaration should be
used instead.", script.url);
Ditto for the message contents.

https://codereview.chromium.org/15832007/diff/1/Source/core/inspector/Inspect...
File Source/core/inspector/InspectorPageAgent.cpp (right):

https://codereview.chromium.org/15832007/diff/1/Source/core/inspector/Inspect...
Source/core/inspector/InspectorPageAgent.cpp:928:
m_page->console()->addMessage(NetworkMessageSource, WarningMessageLevel,
"Resource is served with deprecated header X-SourceMap, SourceMap should be used
instead.", url, 0);
Perhaps a note about phasing out the deprecated header could be useful here,
too?

https://codereview.chromium.org/15832007/diff/1/Source/core/inspector/Inspect...
File Source/core/inspector/InspectorStyleSheet.cpp (right):

https://codereview.chromium.org/15832007/diff/1/Source/core/inspector/Inspect...
Source/core/inspector/InspectorStyleSheet.cpp:1493:
m_pageAgent->page()->console()->addMessage(NetworkMessageSource,
WarningMessageLevel, "\"/*@ sourceURL=\" source url declaration is deprecated,
\"/*# sourceURL=\" declaration should be used instead.", finalURL(), 0);
We need a kind of utility for this :(

https://codereview.chromium.org/15832007/diff/1/Source/devtools/front_end/Con...
File Source/devtools/front_end/ConsoleMessage.js (right):

https://codereview.chromium.org/15832007/diff/1/Source/devtools/front_end/Con...
Source/devtools/front_end/ConsoleMessage.js:152: var isExternal =
!WebInspector.resourceForURL(this.url) &&
!WebInspector.workspace.uiSourceCodeForURL(this.url);
This seems unrelated?

[vsevik, 2013-05-29 14:00:05]

PTAL

On 2013/05/29 13:34:12, Yury Semikhatsky wrote:
>
https://codereview.chromium.org/15832007/diff/1/LayoutTests/inspector/console...
> File LayoutTests/inspector/console/console-trace-in-eval.html (right):
> 
>
https://codereview.chromium.org/15832007/diff/1/LayoutTests/inspector/console...
> LayoutTests/inspector/console/console-trace-in-eval.html:47: with //#
> sourceURL=url it will dump a stack trace that will have
> Can we continue to test the old functionality as well until it is removed?

I have added two (one for stylesheet and one for script) simple tests for
deprecated type of sourceURL comment back.
Doing this for sourceMappingURL would require too much copy paste, so I am not
testing it (this should be safe assuming the same findMagicComment method is
used for both comments).

[yurys, 2013-05-29 14:08:06]

lgtm

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

On 2013/05/29 13:58:04, apavlov wrote:
>
https://codereview.chromium.org/15832007/diff/1/LayoutTests/http/tests/inspec...
> File LayoutTests/http/tests/inspector/debugger-test.js (right):
> 
>
https://codereview.chromium.org/15832007/diff/1/LayoutTests/http/tests/inspec...
> LayoutTests/http/tests/inspector/debugger-test.js:288: var hasSourceURL =
> !!source.match(/\/\/#\ssourceURL=\s*(\S*?)\s*$/m) ||
> !!source.match(/\/\/@\ssourceURL=\s*(\S*?)\s*$/m);
> You could use RegExp.prototype.test here (and avoid the !! thing)
I'd like to keep this change as simple as possible, so I would avoid changing
something that is already there.

> 
>
https://codereview.chromium.org/15832007/diff/1/Source/core/inspector/Inspect...
> File Source/core/inspector/InspectorDebuggerAgent.cpp (right):
> 
>
https://codereview.chromium.org/15832007/diff/1/Source/core/inspector/Inspect...
> Source/core/inspector/InspectorDebuggerAgent.cpp:642:
> addConsoleMessage(NetworkMessageSource, WarningMessageLevel, "\"//@
> sourceMapURL=\" source mapping url declaration is deprecated, \"//#
> sourceMapURL=\" declaration should be used instead.", script.url);
> url -> URL ?
Done.

 
>
https://codereview.chromium.org/15832007/diff/1/Source/core/inspector/Inspect...
> Source/core/inspector/InspectorDebuggerAgent.cpp:642:
> addConsoleMessage(NetworkMessageSource, WarningMessageLevel, "\"//@
> sourceMapURL=\" source mapping url declaration is deprecated, \"//#
> sourceMapURL=\" declaration should be used instead.", script.url);
> Perhaps, "...is deprecated and will be unsupported soon" ?
I believe deprecated makes it clear enough.

>
https://codereview.chromium.org/15832007/diff/1/Source/devtools/front_end/Con...
> File Source/devtools/front_end/ConsoleMessage.js (right):
> 
>
https://codereview.chromium.org/15832007/diff/1/Source/devtools/front_end/Con...
> Source/devtools/front_end/ConsoleMessage.js:152: var isExternal =
> !WebInspector.resourceForURL(this.url) &&
> !WebInspector.workspace.uiSourceCodeForURL(this.url);
> This seems unrelated?
This is needed to linkify source map url.

[commit-bot: I haz the power, 2013-05-29 14:29:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vsevik@chromium.org/15832007/4003

[commit-bot: I haz the power, 2013-05-29 15:19:28]

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

[vsevik, 2013-06-04 15:40:40]

Committed patchset #4 manually as r151755 (presubmit successful).