ChangeLog

2019-09-24  Yury Semikhatsky  <yurys@chromium.org>

        Web Inspector: tests under LayoutTests/inspector/debugger are flaky

        https://bugs.webkit.org/show_bug.cgi?id=137131

        <rdar://problem/18461335>

        Reviewed by NOBODY (OOPS!).

        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.

2019-09-26  Yusuke Suzuki  <ysuzuki@apple.com>

        [JSC] DFG recursive-tail-call optimization should not emit jump to call-frame with varargs

Source/WebCore/ChangeLog

2019-09-20  Yury Semikhatsky  <yurys@chromium.org>

        Web Inspector: tests under LayoutTests/inspector/debugger are flaky

        https://bugs.webkit.org/show_bug.cgi?id=137131

        <rdar://problem/18461335>

        Reviewed by NOBODY (OOPS!).

        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.

2019-09-27  Zalan Bujtas  <zalan@apple.com>

        [LFC][IFC] Replace Line::Run's logicalRect/textContext with Display::Run

Source/JavaScriptCore/bytecode/CodeBlock.cpp

@@void CodeBlock::expressionRangeForBytecodeOffset(unsigned bytecodeOffset, int& d

    line += ownerExecutable()->firstLine();

}

1869 bool CodeBlock::hasOpDebugForLineAndColumn(unsigned line, unsigned column)

1869bool CodeBlock::hasOpDebugForLineAndColumn(unsigned line, Optional<unsigned> column)

{

    const InstructionStream& instructionStream = instructions();

    for (const auto& it : instructionStream) {

@@bool CodeBlock::hasOpDebugForLineAndColumn(unsigned line, unsigned column)

            unsigned opDebugLine;

            unsigned opDebugColumn;

            expressionRangeForBytecodeOffset(it.offset(), unused, unused, unused, opDebugLine, opDebugColumn);

1878 if (line == opDebugLine && (~~column == Breakpoint::unspecifiedC~~olumn || column == opDebugColumn))

1878 if (line == opDebugLine && (!column || column == opDebugColumn))

                return true;

        }

    }

Source/JavaScriptCore/bytecode/CodeBlock.h

@@public:

    unsigned frameRegisterCount();

    int stackPointerOffset();

790 bool hasOpDebugForLineAndColumn(unsigned line, unsigned column);

790 bool hasOpDebugForLineAndColumn(unsigned line, Optional<unsigned> column);

    bool hasDebuggerRequests() const { return m_debuggerRequests; }

    void* debuggerRequestsAddress() { return &m_debuggerRequests; }

Source/JavaScriptCore/debugger/Breakpoint.h

@@struct Breakpoint : public DoublyLinkedListNode<Breakpoint> {

    unsigned hitCount { 0 };

    bool resolved { false };

60 static constexpr unsigned unspecifiedColumn = UINT_MAX;

61

private:

    Breakpoint* m_prev;

    Breakpoint* m_next;

Source/JavaScriptCore/debugger/Debugger.cpp

@@void Debugger::toggleBreakpoint(CodeBlock* codeBlock, Breakpoint& breakpoint, Br

    if (breakpoint.sourceID != sourceID)

        return;

    unsigned line = breakpoint.line;

    unsigned column = breakpoint.column;

    unsigned startLine = executable->firstLine();

    unsigned startColumn = executable->startColumn();

    unsigned endLine = executable->lastLine();

@@void Debugger::toggleBreakpoint(CodeBlock* codeBlock, Breakpoint& breakpoint, Br

    // Inspector breakpoint line and column values are zero-based but the executable

    // and CodeBlock line and column values are one-based.

281 line += 1;

282 column = column ? column + 1 : Breakpoint::unspecifiedColumn;

    unsigned line = breakpoint.line + 1;

    Optional<unsigned> column;

    if (breakpoint.column)

        column = breakpoint.column + 1;

    if (line < startLine || line > endLine)

        return;

286 if (column ~~!= Breakpoint::unspecifiedColumn~~) {

285 if (column) {

        if (line == startLine && column < startColumn)

            return;

        if (line == endLine && column > endColumn)

@@void Debugger::resolveBreakpoint(Breakpoint& breakpoint, SourceProvider* sourceP

    // FIXME: <https://webkit.org/b/162771> Web Inspector: Adopt TextPosition in Inspector to avoid oneBasedInt/zeroBasedInt ambiguity

    // Inspector breakpoint line and column values are zero-based but the executable

    // and CodeBlock line and column values are one-based.

    unsigned line = breakpoint.line + 1;

    unsigned column = breakpoint.column ? breakpoint.column : Breakpoint::unspecifiedColumn;

    // and CodeBlock line values are one-based while column is zero-based.

    int line = breakpoint.line + 1;

    int column = breakpoint.column;

365364

366365 // Account for a <script>'s start position on the first line only.

    unsigned providerStartLine = sourceProvider->startPosition().m_line.oneBasedInt(); // One based to match the already adjusted line.

    unsigned providerStartColumn = sourceProvider->startPosition().m_column.zeroBasedInt(); // Zero based so column zero is zero.

    if (line == providerStartLine && column != Breakpoint::unspecifiedColumn) {

    int providerStartLine = sourceProvider->startPosition().m_line.oneBasedInt(); // One based to match the already adjusted line.

    int providerStartColumn = sourceProvider->startPosition().m_column.zeroBasedInt(); // Zero based so column zero is zero.

    if (line == providerStartLine && breakpoint.column) {

        ASSERT(providerStartColumn <= column);

        if (providerStartColumn)

            column -= providerStartColumn;

    }

    DebuggerParseData& parseData = debuggerParseData(breakpoint.sourceID, sourceProvider);

376 Optional<JSTextPosition> resolvedPosition = parseData.pausePositions.breakpointLocationForLineColumn(~~(int)~~line, ~~(int)~~column);

375 Optional<JSTextPosition> resolvedPosition = parseData.pausePositions.breakpointLocationForLineColumn(line, column);

    if (!resolvedPosition)

        return;

380 u~~nsigned~~ resolvedLine = resolvedPosition->line;

381 u~~nsigned~~ resolvedColumn = resolvedPosition->~~offset - resolvedPosition->lineStartOffset + 1~~;

379 int resolvedLine = resolvedPosition->line;

380 int resolvedColumn = resolvedPosition->column();

382381

383382 // Re-account for a <script>'s start position on the first line only.

384 if (resolvedLine == providerStartLine && ~~column != B~~reakpoint~~::unspecifiedC~~olumn) {

383 if (resolvedLine == providerStartLine && breakpoint.column) {

        if (providerStartColumn)

            resolvedColumn += providerStartColumn;

    }

    breakpoint.line = resolvedLine - 1;

390 breakpoint.column = resolvedColumn ~~- 1~~;

389 breakpoint.column = resolvedColumn;

    breakpoint.resolved = true;

}

Source/JavaScriptCore/debugger/DebuggerParseData.cpp

@@namespace JSC {

Optional<JSTextPosition> DebuggerPausePositions::breakpointLocationForLineColumn(int line, int column)

{

    unsigned start = 0;

    unsigned end = m_positions.size();

    while (start != end) {

        unsigned middle = start + ((end - start) / 2);

        DebuggerPausePosition& pausePosition = m_positions[middle];

        int pauseLine = pausePosition.position.line;

        int pauseColumn = pausePosition.position.offset - pausePosition.position.lineStartOffset;

        if (line < pauseLine) {

            end = middle;

            continue;

        }

        if (line > pauseLine) {

            start = middle + 1;

            continue;

        }

        if (column == pauseColumn) {

            // Found an exact position match. Roll forward if this was a function Entry.

            // We are guarenteed to have a Leave for an Entry so we don't need to bounds check.

            while (true) {

                if (pausePosition.type != DebuggerPausePositionType::Enter)

                    return Optional<JSTextPosition>(pausePosition.position);

                pausePosition = m_positions[middle++];

            }

        }

    DebuggerPausePosition position = { DebuggerPausePositionType::Invalid, JSTextPosition(line, column, 0) };

    auto it = std::lower_bound(m_positions.begin(), m_positions.end(), position, [] (const DebuggerPausePosition& a, const DebuggerPausePosition& b) {

        if (a.position.line == b.position.line)

            return a.position.column() < b.position.column();

        return a.position.line < b.position.line;

    });

    if (it == m_positions.end())

        return WTF::nullopt;

6244

        if (column < pauseColumn)

            end = middle;

        else

            start = middle + 1;

    if (line == it->position.line && column == it->position.column()) {

        // Found an exact position match. Roll forward if this was a function Entry.

        // We are guaranteed to have a Leave for an Entry so we don't need to bounds check.

        while (it->type == DebuggerPausePositionType::Enter)

            ++it;

        return it->position;

6751 }

6852

    // Past the end, no possible pause locations.

    if (start >= m_positions.size())

        return WTF::nullopt;

    // If the next location is a function Entry we will need to decide if we should go into

    // the function or go past the function. We decide to go into the function if the

    // input is on the same line as the function entry. For example:

@@Optional<JSTextPosition> DebuggerPausePositions::breakpointLocationForLineColumn

    // If the input was line 3, go into the function to pause on line 4.

    // Valid pause location. Use it.

89 ~~DebuggerPausePosition~~& firstSlidePosition = ~~m_positions[start]~~;

69 auto& firstSlidePosition = *it;

    if (firstSlidePosition.type != DebuggerPausePositionType::Enter)

        return Optional<JSTextPosition>(firstSlidePosition.position);

@@Optional<JSTextPosition> DebuggerPausePositions::breakpointLocationForLineColumn

    // If entryStackSize is > 0 we are skipping functions.

    bool shouldEnterFunction = firstSlidePosition.position.line == line;

    int entryStackSize = shouldEnterFunction ? 0 : 1;

97 for (unsigned i = start + 1; i < m_positions.size(); ++i) {

98 DebuggerPausePosition& slidePosition = m_positions[i];

    ++it;

    for (; it != m_positions.end(); ++it) {

        auto& slidePosition = *it;

        ASSERT(entryStackSize >= 0);

        // Already skipping functions.

@@Optional<JSTextPosition> DebuggerPausePositions::breakpointLocationForLineColumn

void DebuggerPausePositions::sort()

{

    std::sort(m_positions.begin(), m_positions.end(), [] (const DebuggerPausePosition& a, const DebuggerPausePosition& b) {

108 if (a.position.offset == b.position.offset)

109 return a.type < b.type;

        return a.position.offset < b.position.offset;

    });

}

Source/JavaScriptCore/debugger/DebuggerParseData.h

@@namespace JSC {

class SourceProvider;

class VM;

36 enum class DebuggerPausePositionType { Enter, Leave, Pause };

// The order of the constants here matters as it is used for

// sorting pause positions that have same offset.

enum class DebuggerPausePositionType { Invalid, Enter, Pause, Leave };

3739struct DebuggerPausePosition {

38 DebuggerPausePositionType type;

40 DebuggerPausePositionType type { DebuggerPausePositionType::Invalid };

    JSTextPosition position;

};

Source/JavaScriptCore/inspector/protocol/Debugger.json

            "description": "Location in the source code.",

            "properties": [

                { "name": "scriptId", "$ref": "ScriptId", "description": "Script identifier as reported in the <code>Debugger.scriptParsed</code>." },

31 { "name": "lineNumber", "type": "integer", "description": "Line number in the script." },

32 { "name": "columnNumber", "type": "integer", "optional": true, "description": "Column number in the script." }

31 { "name": "lineNumber", "type": "integer", "description": "Line number in the script (0-based)." },

32 { "name": "columnNumber", "type": "integer", "optional": true, "description": "Column number in the script (0-based)." }

Source/JavaScriptCore/parser/ParserTokens.h

@@struct JSTextPosition {

        return !(*this == other);

    }

220 int column() const { return offset - lineStartOffset; }

221

    int line { 0 };

    int offset { 0 };

    int lineStartOffset { 0 };

Source/WebCore/inspector/InspectorFrontendClientLocal.cpp

@@public:

        ASSERT_ARG(message, !message.isEmpty());

        m_messages.append(message);

84 if (!m_timer.isActive())

85 m_timer.startOneShot(0_s);

84 scheduleOneShot();

    }

    void reset()

    {

        m_messages.clear();

91 m_timer.stop();

        m_inspectedPageController = nullptr;

    }

95 void timerFired()

private:

    InspectorBackendDispatchTask(InspectorController* inspectedPageController)

        : m_inspectedPageController(inspectedPageController)

    {

        ASSERT_ARG(inspectedPageController, inspectedPageController);

    }

    void scheduleOneShot()

96101 {

97 ASSERT(m_inspectedPageController);

        if (m_hasScheduledTask)

            return;

        m_hasScheduledTask = true;

        // The frontend can be closed and destroy the owning frontend client before or in the

        // process of dispatching the task, so keep a protector reference here.

        RunLoop::current().dispatch([this, protectedThis = makeRef(*this)] {

            m_hasScheduledTask = false;

            dispatchOneMessage();

        });

    }

98113

        // Dispatching a message can possibly close the frontend and destroy

        // the owning frontend client, so keep a protector reference here.

        Ref<InspectorBackendDispatchTask> protectedThis(*this);

    void dispatchOneMessage()

    {

        // Owning frontend client may have been destroyed after the task was scheduled.

        if (!m_inspectedPageController) {

            ASSERT(m_messages.isEmpty());

            return;

        }

        if (!m_messages.isEmpty())

            m_inspectedPageController->dispatchMessageFromFrontend(m_messages.takeFirst());

        if (!m_messages.isEmpty() && m_inspectedPageController)

            m_timer.startOneShot(0_s);

    }

private:

    InspectorBackendDispatchTask(InspectorController* inspectedPageController)

        : m_inspectedPageController(inspectedPageController)

        , m_timer(*this, &InspectorBackendDispatchTask::timerFired)

    {

        ASSERT_ARG(inspectedPageController, inspectedPageController);

126 scheduleOneShot();

    }

    InspectorController* m_inspectedPageController { nullptr };

119 Timer m_timer;

120130 Deque<String> m_messages;

131 bool m_hasScheduledTask { false };

};

String InspectorFrontendClientLocal::Settings::getProperty(const String&)

LayoutTests/ChangeLog

2019-09-20  Yury Semikhatsky  <yurys@chromium.org>

        Web Inspector: tests under LayoutTests/inspector/debugger are flaky

        https://bugs.webkit.org/show_bug.cgi?id=137131

        <rdar://problem/18461335>

        Reviewed by NOBODY (OOPS!).

        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:

2019-09-27  John Wilander  <wilander@apple.com>

        Storage Access API: document.hasStorageAccess() should return false by default

LayoutTests/inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt

@@PAUSES AT: 89:7

     92        x

INSERTING AT: 89:8

2169 PAUSES AT: 94:0

2169PAUSES AT: 98:0

  

  x => x;

  () => 1;

@@PAUSES AT: 94:0

  (x) => {

      x

  };

2178 => 94 |() => {

2178 94 () => {

      var x;

  };

  

 =>  98    |var fObj = {

      f1: function() {

          var x;

      },

INSERTING AT: 90:0

PAUSES AT: 90:0

@@PAUSES AT: 93:0

      var x;

  };

INSERTING AT: 94:0

PAUSES AT: 94:0

     91    (x) => {

     92        x

     93    };

-=>  94    |() => {

     95        var x;

     96    };

     97

INSERTING AT: 94:1

PAUSES AT: 95:4

     91    (x) => {

@@PAUSES AT: 96:0

  var fObj = {

      f1: function() {

INSERTING AT: 96:1

PAUSES AT: 98:0

     93    };

     94    () => {

     95        var x;

 ->  96    }#;

     97    

 =>  98    |var fObj = {

     99        f1: function() {

    100            var x;

    101        },

INSERTING AT: 99:0

PAUSES AT: 100:8

     96    };

LayoutTests/platform/gtk/TestExpectations

@@webkit.org/b/116957 media/track/track-automatic-subtitles.html [ Timeout ]

webkit.org/b/120682 inspector/page/archive.html [ Timeout ]

2323webkit.org/b/147518 inspector/debugger/nested-inspectors.html [ Timeout ]

23232324webkit.org/b/122571 http/tests/inspector/dom/cross-domain-inspected-node-access.html [ Timeout Pass ]

webkit.org/b/122571 inspector/debugger/setBreakpoint.html [ Timeout Pass ]

webkit.org/b/122571 inspector/debugger/call-frame-function-name.html [ Timeout Pass ]

webkit.org/b/122571 inspector/debugger/setBreakpoint-options-exception.html [ Timeout Pass ]

webkit.org/b/122571 inspector/debugger/setPauseOnExceptions-all.html [ Timeout Pass ]

webkit.org/b/122571 inspector/debugger/setPauseOnExceptions-none.html [ Timeout Pass ]

webkit.org/b/122571 inspector/debugger/call-frame-this-host.html [ Timeout Pass Crash ]

webkit.org/b/122571 inspector/debugger/setBreakpoint-autoContinue.html [ Timeout Pass ]

webkit.org/b/122571 inspector/debugger/setPauseOnExceptions-uncaught.html [ Timeout Pass ]

webkit.org/b/122571 inspector/debugger/setBreakpoint-actions.html [ Failure Timeout Pass ]

webkit.org/b/122571 inspector/debugger/call-frame-this-nonstrict.html [ Timeout Pass ]

webkit.org/b/122571 inspector/debugger/setBreakpoint-column.html [ Timeout Pass ]

webkit.org/b/122571 inspector/debugger/setBreakpoint-condition.html [ Timeout Pass ]

webkit.org/b/122571 inspector/debugger/removeBreakpoint.html [ Timeout Pass ]

webkit.org/b/122571 inspector/debugger/call-frame-this-strict.html [ Timeout Pass ]

webkit.org/b/122571 inspector/dom/request-child-nodes-depth.html [ Timeout Pass ]

webkit.org/b/122571 inspector/dom/focus.html [ Failure Timeout Pass ]

webkit.org/b/122571 inspector/runtime/getProperties.html [ Timeout Pass Crash ]

@@webkit.org/b/136674 media/track/track-cue-rendering-with-padding.html [ Timeout

webkit.org/b/137698 media/video-controls-drag.html [ Timeout ]

2382 webkit.org/b/137131 inspector/debugger [ Skip ]

webkit.org/b/147518 inspector/dom [ Skip ]

webkit.org/b/147518 inspector/model [ Skip ]

webkit.org/b/147518 inspector/timeline [ Skip ]