many inspector tests are flaky. I think the reason for this is a race between two pages. We have inspected page and Inspector's frontend page. When frontend page is loaded it begins to interact with Inspector's backend to gather information about the state of the inspecting page. This process is completely asynchronous. A number of messages will be sent back and forth. At the same time the inspected page tries do its work and send their messages to check the state inspector and trying to do some things with it. However, the state of Inspector's page is not stable until it finishes its initialization process. it would be better to wait until Inspector's frontend page will send all their requests and receive back all the answers. We definitely know how many requests are waiting for answers and need just wait until the number becomes 0 and process test command after that.
Created attachment 82931 [details] [patch] initial version
Comment on attachment 82931 [details] [patch] initial version View in context: https://bugs.webkit.org/attachment.cgi?id=82931&action=review > Source/WebCore/ChangeLog:6 > + You should specify a bug URL after the issue title > Source/WebCore/inspector/CodeGeneratorInspector.pm:629 > + return !!this._waitForResponse; The boolean cast is not necessary, and we normally do not use it (according to one of pfeldman's reviews)
Comment on attachment 82931 [details] [patch] initial version We already have a counter for runAfterPendingDispatches.
Created attachment 83293 [details] [patch] initial version now test harness: 1) doesn't use setTimeout for processing pending test scripts; 2) has no difference between WebKit and Chromium implementations; 3) eval pending test scripts only when all the responses are received.
Comment on attachment 83293 [details] [patch] initial version View in context: https://bugs.webkit.org/attachment.cgi?id=83293&action=review > Source/WebCore/inspector/CodeGeneratorInspector.pm:714 > + this._waitForResponse = 0; this._pendingResponseCount > Source/WebCore/inspector/CodeGeneratorInspector.pm:732 > + --this._waitForResponse; move this line to after callback.apply > Source/WebCore/inspector/CodeGeneratorInspector.pm:815 > + if (this.runAfterPendingDispatches && WebInspector.pendingDispatches === 0 && !this.isWaitingForResponse()) pendingDispatches is of no use now. > Source/WebCore/inspector/front-end/TestController.js:78 > +WebInspector.testController = new WebInspector.TestController(); You should not need it (leave as it was). > Source/WebCore/inspector/front-end/TestController.js:79 > +WebInspector.evaluateForTestInFrontend = WebInspector.testController._evaluateForTestInFrontend.bind(WebInspector.testController); Leave as it was.
Created attachment 83301 [details] [patch] third version comments addressed
Comment on attachment 83301 [details] [patch] third version View in context: https://bugs.webkit.org/attachment.cgi?id=83301&action=review > Source/WebCore/inspector/CodeGeneratorInspector.pm:733 > + --this._pendingResponsesCount; you should move this into dispatch (after the if/else). > Source/WebCore/inspector/CodeGeneratorInspector.pm:815 > + if (this.runAfterPendingDispatches && !this.isWaitingForResponse()) Just move it here. > Source/WebCore/inspector/front-end/TestController.js:77 > + WebInspector.testController.runAfterPendingDispatches(invokeMethod); I'd rather implement runAfterPendingDispatches on the InspectorBackendStub and use it both: from test controller and inspectortest.
Committed r79322 M LayoutTests/http/tests/inspector/inspector-test.js M LayoutTests/ChangeLog M Source/WebKit/chromium/ChangeLog M Source/WebKit/chromium/src/js/DevTools.js M Source/WebCore/ChangeLog M Source/WebCore/inspector/CodeGeneratorInspector.pm M Source/WebCore/inspector/front-end/inspector.js M Source/WebCore/inspector/front-end/TestController.js D Tools/DumpRenderTree/chromium/DRTDevToolsCallArgs.cpp D Tools/DumpRenderTree/chromium/DRTDevToolsCallArgs.h M Tools/DumpRenderTree/chromium/DRTDevToolsClient.cpp M Tools/DumpRenderTree/chromium/DRTDevToolsAgent.h M Tools/DumpRenderTree/chromium/DRTDevToolsClient.h M Tools/DumpRenderTree/chromium/DRTDevToolsAgent.cpp M Tools/DumpRenderTree/DumpRenderTree.gypi M Tools/ChangeLog W: -empty_dir: trunk/Tools/DumpRenderTree/chromium/DRTDevToolsCallArgs.cpp W: -empty_dir: trunk/Tools/DumpRenderTree/chromium/DRTDevToolsCallArgs.h r79322 = 169c0aa5e9b5d1327052690447638f5bcfa812cc (refs/remotes/trunk)
Created attachment 83311 [details] the patch I'm going to land.