DevTools: Only provide static script content to UISourceCode

DevTools: Only provide static script content to UISourceCode

This fixes live edited scripts losing their original content.

BUG=610061
Review-Url: https://codereview.chromium.org/2695123004
Cr-Commit-Position: refs/heads/master@{#454150}
Committed: https://chromium.googlesource.com/chromium/src/+/303b2827e8aac3bc43fc93aec9c79d6cd1834806

[einbinder, 2017-02-14 23:44:04]

einbinder@chromium.org changed reviewers:
+ pfeldman@chromium.org

[einbinder, 2017-02-14 23:44:05]

ptal.

Still too much?

[pfeldman, 2017-02-15 22:33:15]

https://codereview.chromium.org/2695123004/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/bindings/ContentProviderBasedProject.js
(right):

https://codereview.chromium.org/2695123004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/ContentProviderBasedProject.js:66:
requestOriginalFileContent(uiSourceCode) {
Still too much

[einbinder, 2017-02-15 23:26:20]

On 2017/02/15 at 22:33:15, pfeldman wrote:
>
https://codereview.chromium.org/2695123004/diff/20001/third_party/WebKit/Sour...
> File
third_party/WebKit/Source/devtools/front_end/bindings/ContentProviderBasedProject.js
(right):
> 
>
https://codereview.chromium.org/2695123004/diff/20001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/devtools/front_end/bindings/ContentProviderBasedProject.js:66:
requestOriginalFileContent(uiSourceCode) {
> Still too much

Now there is less

[einbinder, 2017-02-15 23:32:55]

Description was changed from

==========
DevTools: Add requestOriginalFileContent to Projects

This fixes live edited scripts losing their original content.

BUG=610061
==========

to

==========
DevTools: Only provide static script content to UISourceCode

This fixes live edited scripts losing their original content.

BUG=610061
==========

[pfeldman, 2017-02-15 23:33:38]

https://codereview.chromium.org/2695123004/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/inspector/uisourcecode-revisions.html
(right):

https://codereview.chromium.org/2695123004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/uisourcecode-revisions.html:17:
project.requestOriginalFileContent = function(uri)
Still some extra stuff

https://codereview.chromium.org/2695123004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/Script.js (right):

https://codereview.chromium.org/2695123004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/Script.js:191:
callback(this._source);
Can we just say this._originalSource = this._source; before this line?

https://codereview.chromium.org/2695123004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/Script.js:199: if
(!this._originalContentProvider) {
then you would say

var lazyContent = this._originalSource ? Promise.resolve(this._originalSource) :
this.requestContent();

and that would be it.

https://codereview.chromium.org/2695123004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/Script.js:210:
requestOriginalContent() {
And you would not need this.

[einbinder, 2017-02-16 00:12:36]

https://codereview.chromium.org/2695123004/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/inspector/uisourcecode-revisions.html
(right):

https://codereview.chromium.org/2695123004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/uisourcecode-revisions.html:17:
project.requestOriginalFileContent = function(uri)
On 2017/02/15 at 23:33:38, pfeldman wrote:
> Still some extra stuff

You were too fast.

https://codereview.chromium.org/2695123004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/Script.js (right):

https://codereview.chromium.org/2695123004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/Script.js:191:
callback(this._source);
On 2017/02/15 at 23:33:38, pfeldman wrote:
> Can we just say this._originalSource = this._source; before this line?

That will cache the content on first request, not first change.

https://codereview.chromium.org/2695123004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/Script.js:210:
requestOriginalContent() {
On 2017/02/15 at 23:33:38, pfeldman wrote:
> And you would not need this.

This can be private

[pfeldman, 2017-02-23 01:11:13]

ping

[einbinder, 2017-02-27 21:21:44]

https://codereview.chromium.org/2695123004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/Script.js (right):

https://codereview.chromium.org/2695123004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/Script.js:199: if
(!this._originalContentProvider) {
On 2017/02/15 at 23:33:38, pfeldman wrote:
> then you would say
> 
> var lazyContent = this._originalSource ? Promise.resolve(this._originalSource)
: this.requestContent();
> 
> and that would be it.

StaticContent provider takes a function():Promise<string>, not a
Promise<string>. I can't bind the requestContent, because the content may have
changed between when the static content provider was created and the
requestContent is fired. So that's why the _requestOriginalContent method
exists.

[pfeldman, 2017-02-27 22:56:09]

https://codereview.chromium.org/2695123004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/sdk/Script.js (right):

https://codereview.chromium.org/2695123004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sdk/Script.js:201: if
(!this._originalContentProvider) {
return this.requestContent().then(() => this._originalSource);

[einbinder, 2017-02-28 00:51:23]

https://codereview.chromium.org/2695123004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/sdk/Script.js (right):

https://codereview.chromium.org/2695123004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sdk/Script.js:201: if
(!this._originalContentProvider) {
On 2017/02/27 at 22:56:09, pfeldman wrote:
> return this.requestContent().then(() => this._originalSource);

() => this.requestContent().then(() => this._originalSource);

[pfeldman, 2017-03-01 23:21:04]

lgtm

[einbinder, 2017-03-01 23:53:22]

The CQ bit was checked by einbinder@chromium.org

[commit-bot: I haz the power, 2017-03-01 23:54:01]

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, 2017-03-02 02:06:29]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1488412402062570,
"parent_rev": "4a49718819f42a744f99682a9571006b99258f8a", "commit_rev":
"303b2827e8aac3bc43fc93aec9c79d6cd1834806"}

[commit-bot: I haz the power, 2017-03-02 02:07:21]

Description was changed from

==========
DevTools: Only provide static script content to UISourceCode

This fixes live edited scripts losing their original content.

BUG=610061
==========

to

==========
DevTools: Only provide static script content to UISourceCode

This fixes live edited scripts losing their original content.

BUG=610061

Review-Url: https://codereview.chromium.org/2695123004
Cr-Commit-Position: refs/heads/master@{#454150}
Committed:
https://chromium.googlesource.com/chromium/src/+/303b2827e8aac3bc43fc93aec9c7...
==========

[commit-bot: I haz the power, 2017-03-02 02:07:22]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/303b2827e8aac3bc43fc93aec9c7...