Bug 84681 – Web Inspector: native nodes may have snapshot id less than base snapshot max JS object id

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Yury Semikhatsky]

We should bare that in mind when calculating heap snapshot diff.

[Yury Semikhatsky]

Created attachment 138515 [details]
Patch

[Yury Semikhatsky]

(In reply to comment #1)
> Created an attachment (id=138515) [details]
> Patch

The patch restores part of the logic removed in http://trac.webkit.org/changeset/114894 and adds a test for it.

[Yury Semikhatsky]

Committed r115031: <http://trac.webkit.org/changeset/115031>

[Csaba Osztrogonác]

It fails on Qt:

--- /home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/layout-test-results/inspector/profiler/heap-snapshot-comparison-dom-groups-change-expected.txt 
+++ /home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/layout-test-results/inspector/profiler/heap-snapshot-comparison-dom-groups-change-actual.txt 
@@ -1,12 +1,4 @@
 Tests that Comparison view of heap snapshots will contain added nodes even if their ids are less than the maximumm JS object id in the base snapshot.
 
-Profiler was enabled.
-Detailed heap profiles were enabled.
+Heap profiler is disabled
 
-Running: testShowAll
-Delta: +4 -1
-Deleted node id(s): 40
-Added node id(s): 15,25,35,5
-
-Profiler was disabled.
-


Is heap profiler disabled intentionally? Should we skip this test or add a platform specific expected file?

[Yury Semikhatsky]

(In reply to comment #4)
> It fails on Qt:
> Is heap profiler disabled intentionally? Should we skip this test or add a platform specific expected file?

Sorry about that, I should have skipped the test for all non-v8 platforms as JSC doesn't support heap profiling.

[Csaba Osztrogonác]

(In reply to comment #5)
> (In reply to comment #4)
> > It fails on Qt:
> > Is heap profiler disabled intentionally? Should we skip this test or add a platform specific expected file?
> 
> Sorry about that, I should have skipped the test for all non-v8 platforms as JSC doesn't support heap profiling.

Not problem, we found this section in the skipped list, and added this test too - http://trac.webkit.org/changeset/115035

[Yury Semikhatsky]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > It fails on Qt:
> > > Is heap profiler disabled intentionally? Should we skip this test or add a platform specific expected file?
> > 
> > Sorry about that, I should have skipped the test for all non-v8 platforms as JSC doesn't support heap profiling.
> 
> Not problem, we found this section in the skipped list, and added this test too - http://trac.webkit.org/changeset/115035

Thanks! I've just updated Skipped lists on other platforms as well.