Implement layout tests for JS debugger in Web Inspector.
Created attachment 59258 [details] Patch
Comment on attachment 59258 [details] Patch LayoutTests/http/tests/inspector/inspector-test.js:35 + if (window.layoutTestController) { You don't need this change, right? Same here. We should rely on DRT. Is there a reason we can't trust it? LayoutTests/http/tests/inspector/inspector-test.js:107 + var toolbar = document.getElementById("toolbar"); What is wrong with assigning to the current panel? (that is what IC is doing by means of WebInspector.show*Panel calls. WebCore/platform/mac/EventLoopMac.mm:35 + NSTimeInterval interval = [[NSDate date] timeIntervalSinceReferenceDate]; I am hesitant getting into this code / reviewing it in connection with inspector tests. Why did you change this? Does it work on all other platforms? (You are only changing the mac's platform). What non-inspector flows will it affect? r- until this is cleared properly.
(In reply to comment #2) > (From update of attachment 59258 [details]) > LayoutTests/http/tests/inspector/inspector-test.js:35 > + if (window.layoutTestController) { > You don't need this change, right? Same here. We should rely on DRT. Is there a reason we can't trust it? > This is needed to ensure that debugger is resumed after test is completed. > LayoutTests/http/tests/inspector/inspector-test.js:107 > + var toolbar = document.getElementById("toolbar"); > What is wrong with assigning to the current panel? (that is what IC is doing by means of WebInspector.show*Panel calls. > Will change this. > WebCore/platform/mac/EventLoopMac.mm:35 > + NSTimeInterval interval = [[NSDate date] timeIntervalSinceReferenceDate]; > I am hesitant getting into this code / reviewing it in connection with inspector tests. Why did you change this? Does it work on all other platforms? (You are only changing the mac's platform). What non-inspector flows will it affect? r- until this is cleared properly. As I wrote in ChangeLog there may be no events to be processed in the EventLoop if script execution is paused in DRT. If there were no timeout the test would hang since ScriptDebugServer would never leave the event loop and wouldn't have a chance to check if the execution was resumed.
Created attachment 59372 [details] Patch
> WebCore/platform/mac/EventLoopMac.mm:35 > + NSTimeInterval interval = [[NSDate date] timeIntervalSinceReferenceDate]; > I am hesitant getting into this code / reviewing it in connection with inspector tests. Why did you change this? Does it work on all other platforms? (You are only changing the mac's platform). What non-inspector flows will it affect? r- until this is cleared properly. Would be nice if the author of the code look at this change. Tim, could you review this stuff?
Comment on attachment 59372 [details] Patch WebCore/platform/mac/EventLoopMac.mm:36 + interval += 0.05; From the IRC: Timothy is suggesting longer timeout (1s). Other than that, he is fine.
> From the IRC: Timothy is suggesting longer timeout (1s). Other than that, he is fine. ... with the change :)
(In reply to comment #6) > (From update of attachment 59372 [details]) > WebCore/platform/mac/EventLoopMac.mm:36 > + interval += 0.05; > From the IRC: Timothy is suggesting longer timeout (1s). Other than that, he is fine. 1s timeout would slow down each pause/continue pair for 1s. So I'd rather continue with a shorter timeout.
Comment on attachment 59372 [details] Patch Clearing flags on attachment: 59372 Committed r61749: <http://trac.webkit.org/changeset/61749>
All reviewed patches have been landed. Closing bug.
Appears to cause Qt failure: http://build.webkit.org/results/Qt%20Linux%20Release/r61749%20(13882)/inspector/debugger-pause-on-debugger-statement-pretty-diff.html
http://trac.webkit.org/changeset/61749 might have broken Qt Linux Release