DevTools: merge UISourceCode's parentPath, name, originURL and uri.

DevTools: merge UISourceCode's parentPath, name, originURL and uri.

BUG=569457
Committed: https://crrev.com/008594f9e765e0a860e8b7123f51148e306e8486
Cr-Commit-Position: refs/heads/master@{#365639}

[dgozman, 2015-12-15 22:22:46]

dgozman@chromium.org changed reviewers:
+ dgozman@chromium.org

[dgozman, 2015-12-15 22:22:48]

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger/resource-script-mapping-expected.txt
(right):

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/sources/debugger/resource-script-mapping-expected.txt:6:
UISourceCode: debugger:///VM1 foo.js
Don't we have a convenience decorator for VMXX?

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/bindings/DefaultScriptMapping.js
(right):

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/DefaultScriptMapping.js:91:
name = "debugger:///VM" + script.scriptId + (name ? "(" + name + ")" : "");
" (" looks better

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/bindings/NetworkMapping.js
(right):

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/NetworkMapping.js:172:
_urlForPath: function(fileSystemPath, filePath)
Does anybody use it?

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js
(right):

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:155: *
@param {!WebInspector.Project} project
indentation off

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:168: if
(path.length === 1 && path[0] === "file://")
What is this for? Needs a comment.

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:176: var
isFromNetwork = project.type() !== WebInspector.projectTypes.FileSystem;
I'd rather call some NetworkProject method here.

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:188:
folderNode = new WebInspector.NavigatorFolderTreeNode(this, null, folderPath,
type, folderPath, name);
3rd and 5th parameters are always equal. Remove one?

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:188:
folderNode = new WebInspector.NavigatorFolderTreeNode(this, null, folderPath,
type, folderPath, name);
Why passing null as project?

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/workspace/FileSystemMapping.js
(right):

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/workspace/FileSystemMapping.js:61:
var entry = new WebInspector.FileSystemMapping.Entry(fileSystemPath,
savedEntry.urlPrefix, savedEntry.pathPrefix, true);
Why change this?

[pfeldman, 2015-12-16 01:34:28]

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger/resource-script-mapping-expected.txt
(right):

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/sources/debugger/resource-script-mapping-expected.txt:6:
UISourceCode: debugger:///VM1 foo.js
On 2015/12/15 22:22:47, dgozman wrote:
> Don't we have a convenience decorator for VMXX?

It is not installed everywhere.

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/bindings/DefaultScriptMapping.js
(right):

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/DefaultScriptMapping.js:91:
name = "debugger:///VM" + script.scriptId + (name ? "(" + name + ")" : "");
On 2015/12/15 22:22:47, dgozman wrote:
> " (" looks better

Needed to revert it to " " since I want to preserve the pseudo extension.

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/bindings/NetworkMapping.js
(right):

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/NetworkMapping.js:172:
_urlForPath: function(fileSystemPath, filePath)
On 2015/12/15 22:22:47, dgozman wrote:
> Does anybody use it?

Yep, line 80. And some tests.

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js
(right):

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:155: *
@param {!WebInspector.Project} project
On 2015/12/15 22:22:47, dgozman wrote:
> indentation off

Done.

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:168: if
(path.length === 1 && path[0] === "file://")
On 2015/12/15 22:22:47, dgozman wrote:
> What is this for? Needs a comment.

Done.

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:176: var
isFromNetwork = project.type() !== WebInspector.projectTypes.FileSystem;
On 2015/12/15 22:22:47, dgozman wrote:
> I'd rather call some NetworkProject method here.

Done.

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:188:
folderNode = new WebInspector.NavigatorFolderTreeNode(this, null, folderPath,
type, folderPath, name);
On 2015/12/15 22:22:47, dgozman wrote:
> Why passing null as project?

To preserve the behavior of context menu...

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:188:
folderNode = new WebInspector.NavigatorFolderTreeNode(this, null, folderPath,
type, folderPath, name);
On 2015/12/15 22:22:47, dgozman wrote:
> 3rd and 5th parameters are always equal. Remove one?

They are no longer equal.

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/workspace/FileSystemMapping.js
(right):

https://codereview.chromium.org/1523193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/workspace/FileSystemMapping.js:61:
var entry = new WebInspector.FileSystemMapping.Entry(fileSystemPath,
savedEntry.urlPrefix, savedEntry.pathPrefix, true);
On 2015/12/15 22:22:47, dgozman wrote:
> Why change this?

Done. (there were more changes)

[pfeldman, 2015-12-16 01:35:16]

Description was changed from

==========
DevTools: [wip] path/parentPath/url
==========

to

==========
DevTools: merge UISourceCode's parentPath, name, originURL and uri.

BUG=569457
==========

[dgozman, 2015-12-16 07:58:30]

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/file-system-mapping-overrides-expected.txt
(right):

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/file-system-mapping-overrides-expected.txt:5:
http://localhost:1234/right_urlfile.txt
Looks suspicious.

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger/file-system-project-mapping.html
(right):

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/sources/debugger/file-system-project-mapping.html:60:
networkUISourceCode =
InspectorTest.testWorkspace.uiSourceCode(WebInspector.NetworkProject.projectId(target),
"http://localhost/html/foo.js");
...projectId(target, false)

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/bindings/DefaultScriptMapping.js
(right):

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/DefaultScriptMapping.js:90:
var name = splitURL[splitURL.length - 1];
name -> url?

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/DefaultScriptMapping.js:91:
name = "debugger:///VM" + script.scriptId + (name ? " " + name: "");
Why 3 slashes? I think there should be just 2 of them.

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/bindings/FileSystemWorkspaceBinding.js
(right):

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/FileSystemWorkspaceBinding.js:78:
var baseURL = /** @type
{!WebInspector.FileSystemWorkspaceBinding.FileSystem}*/(uiSourceCode.project())._fileSystemBaseURL;
assert project type?

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/bindings/NetworkMapping.js
(right):

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/NetworkMapping.js:127:
return project ? project.uiSourceCode("file://" + file.fileSystemPath + "/" +
file.filePath) : null;
Can we incapsulate this logic in FSWB static method?

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js
(right):

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:179: var
name = path[path.length - 1];
peekLast

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:332:
project.excludeFolder("file:///" + path + "/");
This looks bad. Can we just pass |path| and left FSWB handle file:/// part?

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:1016:
this._treeElement.tooltip = this._uiSourceCode.originURL();
(from source map) is now gone. Restore?

[pfeldman, 2015-12-16 18:02:18]

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/file-system-mapping-overrides-expected.txt
(right):

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/file-system-mapping-overrides-expected.txt:5:
http://localhost:1234/right_urlfile.txt
On 2015/12/16 07:58:30, dgozman wrote:
> Looks suspicious.

Done.

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger/file-system-project-mapping.html
(right):

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/sources/debugger/file-system-project-mapping.html:60:
networkUISourceCode =
InspectorTest.testWorkspace.uiSourceCode(WebInspector.NetworkProject.projectId(target),
"http://localhost/html/foo.js");
On 2015/12/16 07:58:30, dgozman wrote:
> ...projectId(target, false)

Done.

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/bindings/DefaultScriptMapping.js
(right):

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/DefaultScriptMapping.js:90:
var name = splitURL[splitURL.length - 1];
On 2015/12/16 07:58:30, dgozman wrote:
> name -> url?

Done.

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/bindings/FileSystemWorkspaceBinding.js
(right):

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/FileSystemWorkspaceBinding.js:78:
var baseURL = /** @type
{!WebInspector.FileSystemWorkspaceBinding.FileSystem}*/(uiSourceCode.project())._fileSystemBaseURL;
It crashes otherwise.

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/bindings/NetworkMapping.js
(right):

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/NetworkMapping.js:127:
return project ? project.uiSourceCode("file://" + file.fileSystemPath + "/" +
file.filePath) : null;
It is currently a mess that requires separate brushing up. Moved it into mapping
for now.

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js
(right):

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:179: var
name = path[path.length - 1];
On 2015/12/16 07:58:30, dgozman wrote:
> peekLast

Acknowledged.

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:332:
project.excludeFolder("file:///" + path + "/");
On 2015/12/16 07:58:30, dgozman wrote:
> This looks bad. Can we just pass |path| and left FSWB handle file:/// part?

There are no "path"s in the Workspace / UISourceCode API anymore.

https://codereview.chromium.org/1523193002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/NavigatorView.js:1016:
this._treeElement.tooltip = this._uiSourceCode.originURL();
On 2015/12/16 07:58:30, dgozman wrote:
> (from source map) is now gone. Restore?

Okay

[dgozman, 2015-12-16 18:51:08]

lgtm

[pfeldman, 2015-12-16 19:54:43]

The CQ bit was checked by pfeldman@chromium.org

[pfeldman, 2015-12-16 19:54:43]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/1523193002/#ps120001
(title: "rebaselined")

[commit-bot: I haz the power, 2015-12-16 19:55:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1523193002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1523193002/120001

[commit-bot: I haz the power, 2015-12-16 22:26:51]

Description was changed from

==========
DevTools: merge UISourceCode's parentPath, name, originURL and uri.

BUG=569457
==========

to

==========
DevTools: merge UISourceCode's parentPath, name, originURL and uri.

BUG=569457
==========

[commit-bot: I haz the power, 2015-12-16 22:26:52]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-12-16 22:27:50]

Description was changed from

==========
DevTools: merge UISourceCode's parentPath, name, originURL and uri.

BUG=569457
==========

to

==========
DevTools: merge UISourceCode's parentPath, name, originURL and uri.

BUG=569457

Committed: https://crrev.com/008594f9e765e0a860e8b7123f51148e306e8486
Cr-Commit-Position: refs/heads/master@{#365639}
==========

[commit-bot: I haz the power, 2015-12-16 22:27:51]

Patchset 7 (id:??) landed as
https://crrev.com/008594f9e765e0a860e8b7123f51148e306e8486
Cr-Commit-Position: refs/heads/master@{#365639}