Skip to content

GitLab

T
tangled
Commit 7b5206fb authored by lushnikov's avatar lushnikov Committed by Commit bot
Browse files
Options

DevTools: [Persistence] validate persistence binding. 

This patch ensures that persistence binding is not established if working copy
of network UISourceCode does not match with the working copy of filesystem
UISourceCode.

This validation is done proactively: whenever automapping reports a binding,
we fetch contents of both network and filesystem UISourceCodes and compare them.

For this to work fast, we do *not* validate the following types of bindings:
- bindings of source map sources. These could be slow to fetch, and they don't break us in any
  way.
- bindings of binary files (e.g. images). These are never going to be edited, and
  thus can't deal any harm.

To sum up, we request contents only of those text resources which were already
succesfully loaded by the website itself, which means they are of manageble size.
However, to be on the safe side, this change is guarded by on-by-default experiment.

BUG=649837
R=dgozman

Review-Url: https://codereview.chromium.org/2542073002
Cr-Commit-Position: refs/heads/master@{#437761}
parent 31d36e81
Hide whitespace changes
Inline Side-by-side
Showing with 98 additions and 128 deletions
third_party/WebKit/LayoutTests/http/tests/inspector/file-system-project-mapping-expected.txt
View file @ 7b5206fb
Tests file system project mappings.
Running: testAutomaticMapping
Adding file system.
Adding network resource.
UISourceCode uri to url mappings:
file:///var/www/html/foo.js ->
file:///var/www/bar.js ->
Adding mapping between network and file system resources.
Emulate reloading inspector.
UISourceCode uri to url mappings:
file:///var/www/html/foo.js -> http://127.0.0.1:8000/inspector/resources/html/foo.js
file:///var/www/bar.js -> http://127.0.0.1:8000/inspector/resources/bar.js
Removing mapping between network and file system resources.
Emulate reloading inspector.
UISourceCode uri to url mappings:
file:///var/www/html/foo.js ->
file:///var/www/bar.js ->
Running: testProjectBasedMapping
Adding file system.
UISourceCode uri to url mappings:
......
third_party/WebKit/LayoutTests/http/tests/inspector/file-system-project-mapping.html
View file @ 7b5206fb
......@@ -2,6 +2,7 @@
<head>
<script src="inspector-test.js"></script>
<script src="debugger-test.js"></script>
<script src="persistence/persistence-test.js"></script>
<script src="isolated-filesystem-test.js"></script>
<script>
function addScript(url)
......@@ -14,7 +15,6 @@ function addScript(url)
function test()
{
var target = InspectorTest.mainTarget;
var persistence = Persistence.persistence;
var fileSystemProjectId = Persistence.FileSystemWorkspaceBinding.projectId("file:///var/www");
function dumpFileSystemUISourceCodesMappings()
......@@ -22,78 +22,19 @@ function test()
var uiSourceCodes = Workspace.workspace.project(fileSystemProjectId).uiSourceCodes();
InspectorTest.addResult("UISourceCode uri to url mappings:");
for (var uiSourceCode of uiSourceCodes) {
var binding = persistence.binding(uiSourceCode);
var binding = Persistence.persistence.binding(uiSourceCode);
var url = binding ? binding.network.url() : "";
InspectorTest.addResult(" " + uiSourceCode.url() + " -> " + url);
}
}
InspectorTest.runTestSuite([
function testAutomaticMapping(next)
{
InspectorTest.addResult("Adding file system.");
var fs = new InspectorTest.TestFileSystem("file:///var/www");
fs.root.mkdir("html").addFile("foo.js", "<foo content>");
fs.root.addFile("bar.js", "<bar content>");
fs.reportCreated(fileSystemCreated1);
var networkUISourceCode;
function fileSystemCreated1()
{
InspectorTest.addResult("Adding network resource.");
InspectorTest.waitForUISourceCode(scriptsAdded, "resources/bar.js");
InspectorTest.evaluateInPage("addScript('resources/html/foo.js')");
InspectorTest.evaluateInPage("addScript('resources/bar.js')");
}
function scriptsAdded()
{
dumpFileSystemUISourceCodesMappings();
var uiSourceCode = Workspace.workspace.uiSourceCode(fileSystemProjectId, "file:///var/www/html/foo.js");
networkUISourceCode = Workspace.workspace.uiSourceCode(Bindings.NetworkProject.projectId(target, InspectorTest.resourceTreeModel.mainFrame, false), "http://127.0.0.1:8000/inspector/resources/html/foo.js");
InspectorTest.addResult("Adding mapping between network and file system resources.");
var fileSystemPath = Persistence.FileSystemWorkspaceBinding.fileSystemPath(uiSourceCode.project().id());
Workspace.fileSystemMapping.addMappingForResource(networkUISourceCode.url(), fileSystemPath, uiSourceCode.url());
var setting = JSON.stringify(Workspace.fileSystemMapping._fileSystemMappingSetting.get());
InspectorTest.addResult("Emulate reloading inspector.");
fs.reportRemoved();
Workspace.fileSystemMapping._fileSystemMappingSetting.set(JSON.parse(setting));
Workspace.fileSystemMapping._loadFromSettings();
fs.reportCreated(fileSystemCreated2);
}
function fileSystemCreated2()
{
dumpFileSystemUISourceCodesMappings();
InspectorTest.addResult("Removing mapping between network and file system resources.");
var uiSourceCode = Workspace.workspace.uiSourceCode(fileSystemProjectId, "file:///var/www/html/foo.js");
Workspace.fileSystemMapping.removeMappingForURL(uiSourceCode.url());
InspectorTest.addResult("Emulate reloading inspector.");
fs.reportRemoved();
fs.reportCreated(fileSystemCreated3);
}
function fileSystemCreated3()
{
dumpFileSystemUISourceCodesMappings();
Workspace.fileSystemMapping.removeMappingForURL(networkUISourceCode.url());
fs.reportRemoved();
next();
}
},
function testProjectBasedMapping(next)
{
InspectorTest.addResult("Adding file system.");
var fs = new InspectorTest.TestFileSystem("file:///var/www");
fs.root.mkdir("html").addFile("foo.js", "<foo content>");
fs.root.mkdir("html2").addFile("bar.js", "<bar content>");
fs.root.mkdir("html").addFile("foo.js", "var foo = 1;");
fs.root.mkdir("html2").addFile("bar.js", "var bar = 2;");
fs.root.addFile(".devtools", JSON.stringify({ mappings: [ { folder: "/html/", url: "http://127.0.0.1:8000/inspector/resources/html/" }, { folder: "/html2/", url: "http://127.0.0.1:8000/inspector/resources/html2/" } ]}));
fs.reportCreated(fileSystemCreated);
......@@ -101,10 +42,13 @@ function test()
{
InspectorTest.evaluateInPage("addScript('resources/html/foo.js')");
InspectorTest.evaluateInPage("addScript('resources/html2/bar.js')");
InspectorTest.waitForUISourceCode(scriptsAdded, "resources/html2/bar.js");
Promise.all([
InspectorTest.waitForBinding("foo.js"),
InspectorTest.waitForBinding("bar.js")
]).then(onBindings);
}
function scriptsAdded()
function onBindings()
{
dumpFileSystemUISourceCodesMappings();
fs.reportRemoved();
......
third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-do-not-bind-dirty-sourcecode-expected.txt
View file @ 7b5206fb
......@@ -4,7 +4,9 @@ Verify that dirty uiSourceCodes are not bound.
Running: waitForUISourceCodes
Running: addFileSystemMapping
Running: dumpConsoleMessages
foo.js can not be persisted to file system due to unsaved changes.
Failed to create binding: {
network: http://127.0.0.1:8000/inspector/persistence/resources/foo.js
fileSystem: file:///var/www/inspector/persistence/resources/foo.js
exactMatch: false
}
third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-do-not-bind-dirty-sourcecode.html
View file @ 7b5206fb
......@@ -26,14 +26,14 @@ function test()
function addFileSystemMapping(next)
{
InspectorTest.addSniffer(Persistence.Persistence.prototype, '_prevalidationFailedForTest', onPrevalidationFailed);
Workspace.fileSystemMapping.addFileMapping(fs.fileSystemPath, "http://127.0.0.1:8000", "/");
next();
},
function dumpConsoleMessages(next)
{
InspectorTest.dumpConsoleMessages();
next();
function onPrevalidationFailed(binding)
{
InspectorTest.addResult("Failed to create binding: " + binding);
next();
}
},
]);
};
......
third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-merge-editor-tabs-expected.txt
View file @ 7b5206fb
Verify that tabs get merged and split when binding is added and removed.
Verify that tabs get merged when binding is added and removed.
Running: addFileSystem
......@@ -13,7 +13,7 @@ Running: openFileSystemTab
SourceFrame: file:///var/www/inspector/persistence/resources/foo.js
selection: {"startLine":0,"startColumn":0,"endLine":0,"endColumn":0}
firstVisibleLine: 0
isDirty: true
isDirty: false
Opened tabs:
file:///var/www/inspector/persistence/resources/foo.js
http://127.0.0.1:8000/inspector/persistence/resources/foo.js
......@@ -24,7 +24,7 @@ Opened tabs:
SourceFrame: file:///var/www/inspector/persistence/resources/foo.js
selection: {"startLine":2,"startColumn":0,"endLine":2,"endColumn":5}
firstVisibleLine: 2
isDirty: true
isDirty: false
Running: removeFileMapping
Opened tabs:
......@@ -32,5 +32,5 @@ Opened tabs:
SourceFrame: file:///var/www/inspector/persistence/resources/foo.js
selection: {"startLine":2,"startColumn":0,"endLine":2,"endColumn":5}
firstVisibleLine: 2
isDirty: true
isDirty: false
third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-merge-editor-tabs.html
View file @ 7b5206fb
......@@ -38,15 +38,9 @@ function test()
function openFileSystemTab(next)
{
InspectorTest.waitForUISourceCode("foo.js", Workspace.projectTypes.FileSystem)
.then(onFileSystemSourceCode)
.then(code => InspectorTest.showUISourceCodePromise(code))
.then(onFileSystemTab);
function onFileSystemSourceCode(code)
{
code.setWorkingCopy("\n\nwindow.foo = ()=>'foo2';");
return InspectorTest.showUISourceCodePromise(code);
}
function onFileSystemTab(sourceFrame)
{
fileSystemSourceFrame = sourceFrame;
......@@ -108,6 +102,6 @@ function test()
</script>
</head>
<body onload="runTest()">
<p>Verify that tabs get merged and split when binding is added and removed.</p>
<p>Verify that tabs get merged when binding is added and removed.</p>
</body>
</html>
third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-sync-content-nodejs-expected.txt
View file @ 7b5206fb
......@@ -40,7 +40,6 @@ Network:
(function (exports, require, module, __filename, __dirname) {
var express = require("express");
network();
workingCopy1();
//TODO
});
......@@ -49,7 +48,6 @@ FileSystem:
#!/usr/bin/env node
var express = require("express");
network();
workingCopy1();
//TODO
......@@ -76,7 +74,6 @@ Network:
(function (exports, require, module, __filename, __dirname) {
var express = require("express");
filesystem();
workingCopy2();
//TODO
});
......@@ -85,7 +82,6 @@ FileSystem:
#!/usr/bin/env node
var express = require("express");
filesystem();
workingCopy2();
//TODO
......
third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-sync-content-nodejs.html
View file @ 7b5206fb
......@@ -52,9 +52,9 @@ function test()
var testSuite = [
function addNetworkUISourceCodeRevision(next)
{
nodeContent = nodeContent.replace("//TODO", "network();\n//TODO");
var newContent = nodeContent.replace("//TODO", "network();\n//TODO");
InspectorTest.addSniffer(Persistence.Persistence.prototype, "_contentSyncedForTest", onSynced);
binding.network.addRevision(nodeContent);
binding.network.addRevision(newContent);
function onSynced()
{
......@@ -65,9 +65,9 @@ function test()
function setNetworkUISourceCodeWorkingCopy(next)
{
nodeContent = nodeContent.replace("//TODO", "workingCopy1();\n//TODO");
var newContent = nodeContent.replace("//TODO", "workingCopy1();\n//TODO");
InspectorTest.addSniffer(Persistence.Persistence.prototype, "_contentSyncedForTest", onSynced);
binding.network.setWorkingCopy(nodeContent);
binding.network.setWorkingCopy(newContent);
function onSynced()
{
......@@ -78,9 +78,9 @@ function test()
function changeFileSystemFile(next)
{
fsContent = fsContent.replace("//TODO", "filesystem();\n//TODO");
var newContent = fsContent.replace("//TODO", "filesystem();\n//TODO");
InspectorTest.addSniffer(Persistence.Persistence.prototype, "_contentSyncedForTest", onSynced);
fsEntry.setContent(fsContent);
fsEntry.setContent(newContent);
function onSynced()
{
......@@ -91,9 +91,9 @@ function test()
function setFileSystemUISourceCodeWorkingCopy(next)
{
nodeContent = fsContent.replace("//TODO", "workingCopy2();\n//TODO");
var newContent = fsContent.replace("//TODO", "workingCopy2();\n//TODO");
InspectorTest.addSniffer(Persistence.Persistence.prototype, "_contentSyncedForTest", onSynced);
binding.fileSystem.setWorkingCopy(nodeContent);
binding.fileSystem.setWorkingCopy(newContent);
function onSynced()
{
......
third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-test.js
View file @ 7b5206fb
......@@ -3,6 +3,8 @@ var initialize_PersistenceTest = function() {
InspectorTest.preloadModule("persistence");
InspectorTest.preloadModule("sources");
Runtime.experiments.enableForTest("persistenceValidation");
Persistence.PersistenceBinding.prototype.toString = function()
{
var lines = [
......
third_party/WebKit/LayoutTests/inspector/sources/debugger/file-system-project-live-edit.html
View file @ 7b5206fb
<html>
<head>
<script src="../../../http/tests/inspector/inspector-test.js"></script>
<script src="../../../http/tests/inspector/persistence/persistence-test.js"></script>
<script src="../../../http/tests/inspector/debugger-test.js"></script>
<script src="../../../http/tests/inspector/isolated-filesystem-test.js"></script>
<script src="../../../http/tests/inspector/live-edit-test.js"></script>
......@@ -26,7 +27,8 @@ function test()
function fileSystemCreated()
{
InspectorTest.evaluateInPage("addScript()", didAddScript);
InspectorTest.evaluateInPage("addScript()");
InspectorTest.waitForBinding("edit-me.js").then(didAddScript);
}
function didAddScript()
......
third_party/WebKit/Source/devtools/front_end/main/Main.js
View file @ 7b5206fb
......@@ -104,6 +104,7 @@ Main.Main = class {
Runtime.experiments.register('liveSASS', 'Live SASS');
Runtime.experiments.register('nodeDebugging', 'Node debugging', true);
Runtime.experiments.register('persistence2', 'Persistence 2.0');
Runtime.experiments.register('persistenceValidation', 'Validate persistence bindings');
Runtime.experiments.register('privateScriptInspection', 'Private script inspection');
Runtime.experiments.register('requestBlocking', 'Request blocking', true);
Runtime.experiments.register('timelineShowAllEvents', 'Show all events on Timeline', true);
......@@ -126,7 +127,7 @@ Main.Main = class {
Runtime.experiments.enableForTest('accessibilityInspection');
}
Runtime.experiments.setDefaultExperiments(['timelineRecordingPerspectives']);
Runtime.experiments.setDefaultExperiments(['timelineRecordingPerspectives', 'persistenceValidation']);
}
/**
......
third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js
View file @ 7b5206fb
......@@ -21,25 +21,62 @@ Persistence.Persistence = class extends Common.Object {
var linkDecorator = new Persistence.PersistenceUtils.LinkDecorator(this);
Components.Linkifier.setLinkDecorator(linkDecorator);
this._mapping =
new Persistence.Automapping(workspace, this._onBindingCreated.bind(this), this._onBindingRemoved.bind(this));
new Persistence.Automapping(workspace, this._validateBinding.bind(this), this._onBindingRemoved.bind(this));
} else {
this._mapping = new Persistence.DefaultMapping(
workspace, fileSystemMapping, this._onBindingCreated.bind(this), this._onBindingRemoved.bind(this));
workspace, fileSystemMapping, this._validateBinding.bind(this), this._onBindingRemoved.bind(this));
}
}
/**
* @param {!Persistence.PersistenceBinding} binding
*/
_onBindingCreated(binding) {
if (binding.network.isDirty()) {
Common.console.log(
Common.UIString('%s can not be persisted to file system due to unsaved changes.', binding.network.name()));
_validateBinding(binding) {
if (!Runtime.experiments.isEnabled('persistenceValidation') || binding.network.contentType().isFromSourceMap() ||
!binding.fileSystem.contentType().isTextType()) {
this._establishBinding(binding);
return;
}
if (binding.fileSystem.isDirty())
binding.network.setWorkingCopy(binding.fileSystem.workingCopy());
Promise.all([binding.network.requestContent(), binding.fileSystem.requestContent()]).then(onContents.bind(this));
/**
* @this {Persistence.Persistence}
*/
function onContents() {
if (binding._removed)
return;
var fileSystemContent = binding.fileSystem.workingCopy();
var networkContent = binding.network.workingCopy();
var target = Bindings.NetworkProject.targetForUISourceCode(binding.network);
var isValid = false;
if (target.isNodeJS()) {
var rewrappedNetworkContent = Persistence.Persistence._rewrapNodeJSContent(
binding, binding.fileSystem, fileSystemContent, networkContent);
isValid = (fileSystemContent === rewrappedNetworkContent);
} else {
// Trim trailing whitespaces because V8 adds trailing newline.
isValid = (fileSystemContent.trimRight() === networkContent.trimRight());
}
if (isValid)
this._establishBinding(binding);
else
this._prevalidationFailedForTest(binding);
}
}
/**
* @param {!Persistence.PersistenceBinding} binding
*/
_prevalidationFailedForTest(binding) {
}
/**
* @param {!Persistence.PersistenceBinding} binding
*/
_establishBinding(binding) {
binding.network[Persistence.Persistence._binding] = binding;
binding.fileSystem[Persistence.Persistence._binding] = binding;
......@@ -64,6 +101,13 @@ Persistence.Persistence = class extends Common.Object {
* @param {!Persistence.PersistenceBinding} binding
*/
_onBindingRemoved(binding) {
binding._removed = true;
if (binding.network[Persistence.Persistence._binding] !== binding)
return;
console.assert(
binding.network[Persistence.Persistence._binding] === binding.fileSystem[Persistence.Persistence._binding],
'ERROR: inconsistent binding for networkURL ' + binding.network.url());
binding.network[Persistence.Persistence._binding] = null;
binding.fileSystem[Persistence.Persistence._binding] = null;
......@@ -95,7 +139,8 @@ Persistence.Persistence = class extends Common.Object {
if (target.isNodeJS()) {
var newContent = uiSourceCode.workingCopy();
other.requestContent().then(() => {
var nodeJSContent = this._rewrapNodeJSContent(binding, other, other.workingCopy(), newContent);
var nodeJSContent =
Persistence.Persistence._rewrapNodeJSContent(binding, other, other.workingCopy(), newContent);
setWorkingCopy.call(this, () => nodeJSContent);
});
return;
......@@ -128,7 +173,7 @@ Persistence.Persistence = class extends Common.Object {
var target = Bindings.NetworkProject.targetForUISourceCode(binding.network);
if (target.isNodeJS()) {
other.requestContent().then(currentContent => {
var nodeJSContent = this._rewrapNodeJSContent(binding, other, currentContent, newContent);
var nodeJSContent = Persistence.Persistence._rewrapNodeJSContent(binding, other, currentContent, newContent);
setContent.call(this, nodeJSContent);
});
return;
......@@ -154,7 +199,7 @@ Persistence.Persistence = class extends Common.Object {
* @param {string} newContent
* @return {string}
*/
_rewrapNodeJSContent(binding, uiSourceCode, currentContent, newContent) {
static _rewrapNodeJSContent(binding, uiSourceCode, currentContent, newContent) {
if (uiSourceCode === binding.fileSystem) {
if (newContent.startsWith(Persistence.Persistence._NodePrefix) &&
newContent.endsWith(Persistence.Persistence._NodeSuffix)) {
......@@ -290,6 +335,7 @@ Persistence.PersistenceBinding = class {
this.network = network;
this.fileSystem = fileSystem;
this.exactMatch = exactMatch;
this._removed = false;
}
};
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment