DevTools: [Persistence] validate persistence binding.

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

Committed: https://crrev.com/7b5206fba773fc4f5ce4ba32691fd825a9eed17a
Cr-Commit-Position: refs/heads/master@{#437761}

[lushnikov, 2016-12-01 20:00:02]

please, take a look

[lushnikov, 2016-12-07 01:10:12]

Description was changed from

==========
DevTools: [Persistence] validate binding before creating.

This patch compares contents of the files before establishing the
binding.

This patch will result in requesting contents for every network resource
which is about to be "bound" to the fileSystem. This is additional work
to do, however I didn't notice any slow-down in my tests.

BUG=649837
R=dgozman
==========

to

==========
DevTools: [Persistence] validate persistence binding.

This patch ensures that persistence binding is destroyed if the
contents of network uiSourceCode do not match with contents of filesystem
uiSourceCode.

This is done in the following way:
- if at the moment of binding creation either binding.network or
  binding.filesystem uiSourceCodes have loaded content, then the
  binding will be validated right away.
- otherwise, the binding will be validated as soon as one of the uiSourceCodes
  will load its content.

BUG=649837
R=dgozman
==========

[lushnikov, 2016-12-07 01:11:26]

please, take a look!

[lushnikov, 2016-12-07 01:11:31]

The CQ bit was checked by lushnikov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-07 01:12:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dgozman, 2016-12-07 01:45:42]

lgtm

https://codereview.chromium.org/2542073002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-validate-binding-on-content-loaded-expected.txt
(right):

https://codereview.chromium.org/2542073002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-validate-binding-on-content-loaded-expected.txt:11:
Running: openFileSystemTab
Let's also test that network.requestContent() does the same.

https://codereview.chromium.org/2542073002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js
(right):

https://codereview.chromium.org/2542073002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js:49:
function onBindingValidated(isValid) {
@param

https://codereview.chromium.org/2542073002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js:68: *
@param {!Persistence.PersistenceBinding} binding
@return promise.....

https://codereview.chromium.org/2542073002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js:73:
function onContentsLoaded() {
@return boolean

https://codereview.chromium.org/2542073002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js:83: var
fileSystemContent = binding.fileSystem.workingCopy().replace(/\s*$/, '');
.trimRight()

https://codereview.chromium.org/2542073002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js:122:
return;
Drop this return, and the if.

https://codereview.chromium.org/2542073002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js:382:
Persistence.Persistence._prevalidatePromise =
Symbol('Persistence.prevalidatePromise');
Let's have private fields instead.

[commit-bot: I haz the power, 2016-12-07 04:11:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-07 04:11:41]

Dry run: This issue passed the CQ dry run.

[lushnikov, 2016-12-08 05:21:43]

Description was changed from

==========
DevTools: [Persistence] validate persistence binding.

This patch ensures that persistence binding is destroyed if the
contents of network uiSourceCode do not match with contents of filesystem
uiSourceCode.

This is done in the following way:
- if at the moment of binding creation either binding.network or
  binding.filesystem uiSourceCodes have loaded content, then the
  binding will be validated right away.
- otherwise, the binding will be validated as soon as one of the uiSourceCodes
  will load its content.

BUG=649837
R=dgozman
==========

to

==========
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 bindings of source map sources.
These could be slow to fetch, and they don't break us in any way. Instead,
we request contents only of those 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
==========

[lushnikov, 2016-12-08 05:22:36]

this is a total rewrite according to our discussion. Please, take a look!

[dgozman, 2016-12-08 22:38:53]

lgtm

https://codereview.chromium.org/2542073002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/main/Main.js (right):

https://codereview.chromium.org/2542073002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/main/Main.js:107:
Runtime.experiments.register('persistenceValidation', 'Validate persistence
bindings');
You should manage to keep this list sorted by both strings at the same time.

https://codereview.chromium.org/2542073002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js
(right):

https://codereview.chromium.org/2542073002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js:104: if
(!binding.network[Persistence.Persistence._binding] ||
!binding.fileSystem[Persistence.Persistence._binding])
if (binding.network[Persistence.Persistence._binding] !== binding || )
  return;
console.assert(they are equal)

[lushnikov, 2016-12-08 23:02:24]

thanks!

https://codereview.chromium.org/2542073002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js
(right):

https://codereview.chromium.org/2542073002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js:104: if
(!binding.network[Persistence.Persistence._binding] ||
!binding.fileSystem[Persistence.Persistence._binding])
On 2016/12/08 22:38:53, dgozman wrote:
> if (binding.network[Persistence.Persistence._binding] !== binding || )
>   return;
> console.assert(they are equal)

Done.

[lushnikov, 2016-12-08 23:02:28]

The CQ bit was checked by lushnikov@chromium.org

[lushnikov, 2016-12-08 23:02:28]

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/2542073002/#ps160001
(title: "address comments")

[commit-bot: I haz the power, 2016-12-08 23:03:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-08 23:14:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-08 23:14:13]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[lushnikov, 2016-12-08 23:38:08]

The CQ bit was checked by lushnikov@chromium.org

[commit-bot: I haz the power, 2016-12-08 23:38:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-08 23:44:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-08 23:44:56]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[lushnikov, 2016-12-09 00:23:46]

The CQ bit was checked by lushnikov@chromium.org

[commit-bot: I haz the power, 2016-12-09 00:24:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lushnikov, 2016-12-09 00:34:26]

Description was changed from

==========
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 bindings of source map sources.
These could be slow to fetch, and they don't break us in any way. Instead,
we request contents only of those 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
==========

to

==========
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
==========

[lushnikov, 2016-12-09 00:35:31]

The CQ bit was checked by lushnikov@chromium.org to run a CQ dry run

[lushnikov, 2016-12-09 00:35:56]

The CQ bit was unchecked by lushnikov@chromium.org

[lushnikov, 2016-12-09 00:35:56]

The CQ bit was checked by lushnikov@chromium.org

[lushnikov, 2016-12-09 00:35:57]

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/2542073002/#ps180001
(title: "skip binding validation in case of non-text type of filesystem")

[commit-bot: I haz the power, 2016-12-09 00:36:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-09 02:28:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 02:28:46]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[lushnikov, 2016-12-09 02:45:30]

The CQ bit was checked by lushnikov@chromium.org

[commit-bot: I haz the power, 2016-12-09 02:45:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-09 04:04:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 04:04:38]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[lushnikov, 2016-12-09 05:14:22]

The CQ bit was checked by lushnikov@chromium.org

[commit-bot: I haz the power, 2016-12-09 05:14:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-09 06:26:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 06:26:28]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[lushnikov, 2016-12-10 02:18:42]

The CQ bit was checked by lushnikov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-10 02:19:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-10 03:58:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-10 03:58:24]

Dry run: This issue passed the CQ dry run.

[lushnikov, 2016-12-10 06:34:17]

The CQ bit was checked by lushnikov@chromium.org

[lushnikov, 2016-12-10 06:34:17]

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/2542073002/#ps200001
(title: "rebaseline tests")

[commit-bot: I haz the power, 2016-12-10 06:34:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-10 06:39:09]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1481351657568200,
"parent_rev": "5e99fe11b254e6d37c2112b9d9274ec3d28ea3e6", "commit_rev":
"af5d2ffaa662f8b68532a09509f0ab6cac8f6653"}

[commit-bot: I haz the power, 2016-12-10 06:40:50]

Description was changed from

==========
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
==========

to

==========
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
==========

[commit-bot: I haz the power, 2016-12-10 06:40:51]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-12-12 15:06:42]

Description was changed from

==========
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
==========

to

==========
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

Committed: https://crrev.com/7b5206fba773fc4f5ce4ba32691fd825a9eed17a
Cr-Commit-Position: refs/heads/master@{#437761}
==========

[commit-bot: I haz the power, 2016-12-12 15:06:43]

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