Ignore:
Timestamp:
Oct 3, 2019, 8:46:30 AM (6 years ago)
Author:
[email protected]
Message:

Web Inspector: tests under LayoutTests/inspector/debugger are flaky
https://bugs.webkit.org/show_bug.cgi?id=137131
<rdar://problem/18461335>

Patch by Yury Semikhatsky <[email protected]> on 2019-10-03
Reviewed by Devin Rousso.

Source/JavaScriptCore:

Changed breakpoint resolution logic to make it consistent across platforms and
better handle the case when there are several DebuggerPausePositions at the same
offset (but with different types).

  • bytecode/CodeBlock.cpp:

(JSC::CodeBlock::hasOpDebugForLineAndColumn):

  • bytecode/CodeBlock.h:
  • debugger/Breakpoint.h: Removed Breakpoint::unspecifiedColumn, Optional<unsigned>

is used instead where needed. It allows to avoid code that relies on (int)UINT_MAX => -1
conversion.

  • debugger/Debugger.cpp:

(JSC::Debugger::resolveBreakpoint): clarified that in the map columns are 0-based.

  • debugger/DebuggerParseData.cpp:

(JSC::DebuggerPausePositions::breakpointLocationForLineColumn): replaced custom
binary search with std::lower_bound. If there are several pause positions at the
same offset they will be sorted by the type and the algorithm is guaranteed to see
leftmost one first.

(JSC::DebuggerPausePositions::sort): use type as secondary ordering component.

  • debugger/DebuggerParseData.h: Rearranged type constants so that Enter < Pause < Leave

this change along with sorting by type should guarantee that in case of several pause
positions at the same line Enter goes before Pause before Leave and the breakpoint
resolution will yield result similar to that when each pause locations has different
position.

  • inspector/protocol/Debugger.json: clarified that positions are 0-based.
  • parser/ParserTokens.h:

(JSC::JSTextPosition::column const): added helper method for computing column.

Source/WebCore:

Fix debugger tests on GTK. All tests that pause on breakpoint didn't work because
in layout tests InspectorFrontendClientLocal was using Timer to dispatch commands
sent from the local front-end page to the inspected one. When paused inside a script
triggered by the front-end nested timer event would be scheduled but never fired
because in glib implementation of RunLoop::TimerBase uses event source which doesn't
allow recursion (g_source_set_can_recurse is not called on the source), so dispatching
Debugger.resume command didn't work when paused inside another inspector command (e.g.
eval). RunLoop itself uses event source which does allow recursion. So instead of using
a timer for asynchronous command dispatching with delay=0 we now schedule a task in
RunLoop's queue.

  • inspector/InspectorFrontendClientLocal.cpp:

(WebCore::InspectorBackendDispatchTask::dispatch):
(WebCore::InspectorBackendDispatchTask::reset):
(WebCore::InspectorBackendDispatchTask::InspectorBackendDispatchTask):
(WebCore::InspectorBackendDispatchTask::scheduleOneShot): ensures that there is one inspector
dispatch task in the queue.
(WebCore::InspectorBackendDispatchTask::dispatchOneMessage): this is mostly the same behavior
as was with timerFired, we should be able to dispatch all accumulated messages from the queue
in one batch but for now I'd like to keep it one per iteration.

LayoutTests:

Enable inspector/debugger tests on GTK.

  • inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt: Rebaselined the test

after changes in the breakpoint resolution code. Now the output on GTK is the same as on Mac.

  • platform/gtk/TestExpectations:
File:
1 edited

Legend:

Unmodified
Added
Removed
Note: See TracChangeset for help on using the changeset viewer.