Bug 138976 – Web Inspector: timeline probe records have inaccurate per-probe hit counts

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Brian Burg]

Given a timeline record for breakpointActionProbe, we need to be able to find the relevant probe sample so that the user can click on a timeline row and jump to the specific probe sample associated with that timeline row.

The current implementation uses a per-JSContext m_hitCount variable, which is similar to the global sampleId but not guaranteed to be the same. These should be combined into the same thing and renamed in the protocol.

This design might be revisited once the probe UI becomes replay-aware, but it's fine for now.

[Radar WebKit Bug Importer]

<rdar://problem/19061273>

[Brian Burg]

    I have a patch forthcoming to fix the backend parts.

    madonk will write a test to be written to cover sampleId behavior. In particular, the samples obtained from the timeline record should match what is seen in the probe manager.

[Brian Burg]

Created attachment 242094 [details]
Patch

[Katie Madonna]

Created attachment 242428 [details]
Proposed Patch

[Brian Burg]

Created attachment 242436 [details]
Proposed Fix

[Build Bot]

Comment on attachment 242436 [details]
Proposed Fix

Attachment 242436 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5347395973939200

New failing tests:
transitions/default-timing-function.html

[Build Bot]

Created attachment 242452 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Brian Burg]

Created attachment 242498 [details]
Proposed Fix (fixed ChangeLog)

[Joseph Pecoraro]

Comment on attachment 242498 [details]
Proposed Fix (fixed ChangeLog)

View in context: https://bugs.webkit.org/attachment.cgi?id=242498&action=review

r=me

> LayoutTests/inspector-protocol/debugger/didSampleProbe-multiple-probes.html:42
> +                return samples.every(function(sample, idx) { return sample.sampleId = idx + 1; });

I think the every predicate callback should be:

    return sample.sampleId === idx + 1;

Otherwise it will always be returning true!

[Brian Burg]

(In reply to comment #9)
> Comment on attachment 242498 [details]
> Proposed Fix (fixed ChangeLog)
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=242498&action=review
> 
> r=me
> 
> > LayoutTests/inspector-protocol/debugger/didSampleProbe-multiple-probes.html:42
> > +                return samples.every(function(sample, idx) { return sample.sampleId = idx + 1; });
> 
> I think the every predicate callback should be:
> 
>     return sample.sampleId === idx + 1;
> 
> Otherwise it will always be returning true!

Doh, will fix before landing.

[Brian Burg]

Committed r176817: <http://trac.webkit.org/changeset/176817>

[Joseph Pecoraro]

This test fails when run multiple times, the sample ids seem to increase (5-8) instead of (1-4).

shell> run-webkit-tests inspector-protocol/debugger/didSampleProbe-multiple-probes.html --repeat-each 2 -v --no-retry -g --additional-env-var="JSC_slowPathAllocsBetweenGCs=1"