[DevTools] Fixed breakpoints in hotreloaded scripts (issue 2384953002 by kozyatinskiy@chromium.org)

0 views
Skip to first unread message

kozyat...@chromium.org

unread,
Oct 1, 2016, 1:11:43 AM10/1/16
to lush...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
Reviewers: lushnikov
CL: https://codereview.chromium.org/2384953002/

Message:
Andrey, please take a look! If it is fine, I'll add a test on Monday.

Description:
[DevTools] Fixed breakpoints in hotreloaded scripts

We need to use last arrived script for ResourceScriptFile.
Otherwise _isDiverged check will always return true in case of:
eval("source1 //# sourceURL=foo.js")
eval("source2 //# sourceURL=foo.js")

BUG=617450,623150
R=lush...@chromium.org

Affected files (+2, -2 lines):
M third_party/WebKit/Source/devtools/front_end/bindings/ResourceScriptMapping.js


Index: third_party/WebKit/Source/devtools/front_end/bindings/ResourceScriptMapping.js
diff --git a/third_party/WebKit/Source/devtools/front_end/bindings/ResourceScriptMapping.js b/third_party/WebKit/Source/devtools/front_end/bindings/ResourceScriptMapping.js
index 04f8b130c49ce00587b952663f6ee129858e562a..a8caac26e89246b4dd31f7e2bf8dd479195003b1 100644
--- a/third_party/WebKit/Source/devtools/front_end/bindings/ResourceScriptMapping.js
+++ b/third_party/WebKit/Source/devtools/front_end/bindings/ResourceScriptMapping.js
@@ -91,7 +91,7 @@ WebInspector.ResourceScriptMapping.prototype = {
{
var scripts = this._scriptsForUISourceCode(uiSourceCode);
console.assert(scripts.length);
- var script = scripts[0];
+ var script = scripts[scripts.length - 1];
if (script.isInlineScriptWithSourceURL())
return this._debuggerModel.createRawLocation(script, lineNumber + script.lineOffset, lineNumber ? columnNumber : columnNumber + script.columnOffset);
return this._debuggerModel.createRawLocation(script, lineNumber, columnNumber);
@@ -281,7 +281,7 @@ WebInspector.ResourceScriptFile = function(resourceScriptMapping, uiSourceCode,
this._uiSourceCode = uiSourceCode;

if (this._uiSourceCode.contentType().isScript())
- this._script = scripts[0];
+ this._script = scripts[scripts.length - 1];

this._uiSourceCode.addEventListener(WebInspector.UISourceCode.Events.WorkingCopyChanged, this._workingCopyChanged, this);
this._uiSourceCode.addEventListener(WebInspector.UISourceCode.Events.WorkingCopyCommitted, this._workingCopyCommitted, this);


dgo...@chromium.org

unread,
Oct 2, 2016, 11:02:05 PM10/2/16
to kozyat...@chromium.org, lush...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
On 2016/10/01 05:11:42, kozyatinskiy wrote:
> Andrey, please take a look! If it is fine, I'll add a test on Monday.

We definitely need a test :-)

https://codereview.chromium.org/2384953002/

kozyat...@chromium.org

unread,
Oct 3, 2016, 3:01:52 PM10/3/16
to lush...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
test added, please take a look!

https://codereview.chromium.org/2384953002/

lush...@chromium.org

unread,
Oct 3, 2016, 4:07:54 PM10/3/16
to kozyat...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
lgtm (don't forget to merge!)


https://codereview.chromium.org/2384953002/diff/50001/third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html
File
third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html
(right):

https://codereview.chromium.org/2384953002/diff/50001/third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html#newcode32
third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html:32:
InspectorTest.completeDebuggerTest();
return;

https://codereview.chromium.org/2384953002/diff/50001/third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html#newcode39
third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html:39:
scriptFile.addEventListener(WebInspector.ResourceScriptFile.Events.DidDivergeFromVM,
dumpDivergeFromVM, this);
scriptFile is a new one; this won't fire

https://codereview.chromium.org/2384953002/

kozyat...@chromium.org

unread,
Oct 3, 2016, 4:45:31 PM10/3/16
to lush...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
thanks! all done.



https://codereview.chromium.org/2384953002/diff/50001/third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html
File
third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html
(right):

https://codereview.chromium.org/2384953002/diff/50001/third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html#newcode32
third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html:32:
InspectorTest.completeDebuggerTest();
On 2016/10/03 20:07:53, lushnikov wrote:
> return;

Done.


https://codereview.chromium.org/2384953002/diff/50001/third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html#newcode39
third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html:39:
scriptFile.addEventListener(WebInspector.ResourceScriptFile.Events.DidDivergeFromVM,
dumpDivergeFromVM, this);
On 2016/10/03 20:07:53, lushnikov wrote:
> scriptFile is a new one; this won't fire

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 3, 2016, 4:46:24 PM10/3/16
to kozyat...@chromium.org, lush...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 3, 2016, 6:01:15 PM10/3/16
to kozyat...@chromium.org, lush...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
Try jobs failed on following builders:
linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/152976)

https://codereview.chromium.org/2384953002/

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 3, 2016, 7:02:34 PM10/3/16
to kozyat...@chromium.org, lush...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 3, 2016, 8:23:21 PM10/3/16
to kozyat...@chromium.org, lush...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
Committed patchset #3 (id:70001)

https://codereview.chromium.org/2384953002/

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 3, 2016, 8:26:49 PM10/3/16
to kozyat...@chromium.org, lush...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
Patchset 3 (id:??) landed as
https://crrev.com/c0d25adc44ebd36c2dc2565942a1884479c40004
Cr-Commit-Position: refs/heads/master@{#422622}

https://codereview.chromium.org/2384953002/
Reply all
Reply to author
Forward
0 new messages