[webkit-reviews] review denied: [Bug 67647] Web Inspector: do not re-create RawSourceCode when toggling pretty-print mode. : [Attachment 106423] Patch

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has denied Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 67647: Web Inspector: do not re-create RawSourceCode when toggling
pretty-print mode.
https://bugs.webkit.org/show_bug.cgi?id=67647

Attachment 106423: Patch
https://bugs.webkit.org/attachment.cgi?id=106423&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=106423&action=review


> LayoutTests/inspector/debugger/raw-source-code.html:9
> +    var sourceMapping = {

Can this test be split into a few smaller ones?

> LayoutTests/inspector/debugger/raw-source-code.html:83
> +	   var timerId = originalSetTimeout(invokeCallback, timeout)

It should be originalSetTimeout.call(window,...)

> LayoutTests/inspector/debugger/raw-source-code.html:93
> +	   for (var i = 0; i < 3; ++i) {

Why do you do it exactly thrice? Should it loop until timers is empty? If you
assume that firing timers may modify "timers" map it should be cloned before
the iteration.

> LayoutTests/inspector/debugger/raw-source-code.html:105
> +	       var script = createScript("foo.js", [0, 0, 20, 80], true,
"<script source>");

Here and below script content and start/end line/column should be in sync,
otherwise the test uses invalid scripts. r- for this.