Patch to follow.
Created attachment 84021 [details] Patch
Comment on attachment 84021 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=84021&action=review > LayoutTests/http/tests/inspector/inspector-test.js:308 > layoutTestController.closeWebInspector(); Please delete watchdog timer here to make sure we don't close inspector twice. > LayoutTests/http/tests/inspector/network/network-size.html:33 > + var resource1 = WebInspector.panels.network.resources[0]; Image order might change. Can you sort the results to avoid inconsistent output? r- for this. > LayoutTests/http/tests/inspector/network/network-timing.html:34 > + InspectorTest.addResult(resource1.url); Results' order should not depend on network activity. > LayoutTests/http/tests/inspector/network/network-timing.html:36 > + InspectorTest.assertGreaterOrEqual(300, resource1.duration * 1000, "Duration of the first resource"); I don't like that we deliberately slow down layout tests. Can we use mocks/something else to avoid this. Inspector tests are already notorious for being quite slow. > LayoutTests/http/tests/inspector/network/network-timing.html:41 > + InspectorTest.assertGreaterOrEqual(100, resource2.latency * 1000, "Latency of the second resource"); I think we should print something in case of success. > LayoutTests/http/tests/inspector/network/resources/resource.php:42 > + var now = new Date(); Just use Date.now(). This way you won't need Number(now) cast below. > LayoutTests/http/tests/inspector/network/resources/resource.php:55 > + echo("function foo() {}"); Could be rewritten as if (!$jscontent) $jscontent = "function foo() {}"; and use same logic for both branches.
Created attachment 84037 [details] Patch
Committed r79849: <http://trac.webkit.org/changeset/79849>
This watchdog is evil, evil, evil, evil! It prevents us from using slow/timeout/etc expectations as we want. You should _never_ need to do your own watchdog timer for layout tests, the layout test harness has a timeout per test that we can modify as needed (for example SLOW modifiers, debug/release tweaks, TIMEOUT expectations). Can you please remove this? Otherwise we'll have to =TEXT a lot of slow-running inspector tests - which is a whole lot of them in debug!
(In reply to comment #5) > This watchdog is evil, evil, evil, evil! It prevents us from using slow/timeout/etc expectations as we want. Ok, ok. We need this for debugging mostly because we are special. Inspector tests are happening in the front-end mostly, and when there is a problem, we end up timed out. In that case, not all the outputs are flushed into the page, so I added a watchdog doing that. I did not think of SLOW. Let me follow up and make sure watchdog is flushing, but not notifying done. As a result we all will be happy.