Gross
Created attachment 333874 [details] Patch
This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)
Created attachment 333928 [details] Patch
Comment on attachment 333928 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333928&action=review r=me, great cleanup! Thanks! > Source/JavaScriptCore/inspector/InspectorProtocolTypes.h:36 > +template<typename> struct BindingTraits; I didn't know you could do that.
Created attachment 333941 [details] Patch
Created attachment 333957 [details] Patch
Committed r228546: <https://trac.webkit.org/changeset/228546>
<rdar://problem/37591967>
This change seems to have broken inspector/heap/getPreview.html, in WebKitGTK+ and Mac as well. Might be related to the change in signature of InspectorHeapAgent::getPreview(), but I am not sure. Please check it? Below is the failure diff. Seems that the string variable in the test is undefined. --- /home/slave/webkitgtk/gtk-linux-64-release-tests/build/layout-test-results/inspector/heap/getPreview-expected.txt +++ /home/slave/webkitgtk/gtk-linux-64-release-tests/build/layout-test-results/inspector/heap/getPreview-actual.txt @@ -9,7 +9,7 @@ -- Running test case: GetPreviewForString PASS: Should not have an error creating a snapshot. PASS: Should not have an error getting preview. -STRING: This is the test string. +STRING: undefined -- Running test case: GetPreviewForFunction PASS: Should not have an error creating a snapshot.
It looks like it has actually cause the same test to crash with an assertion failure on Debug macOS testers as well. sderr: LEAK: 6 CachedResource LEAK: 25 WebCoreNode Assertion failed: (initialized()), function operator*, file /Volumes/Data/slave/highsierra-debug/build/WebKitBuild/Debug/usr/local/include/wtf/Optional.h, line 595. LEAK: 2 WebPageProxy https://build.webkit.org/results/Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/r228557%20(2088)/results.html https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/builds/2088
Reverted r228546 for reason: This caused a consistent crash on all macOS WK2 platforms. Committed r228563: <https://trac.webkit.org/changeset/228563>
(In reply to Claudio Saavedra from comment #9) > This change seems to have broken inspector/heap/getPreview.html, in > WebKitGTK+ and Mac as well. Might be related to the change in signature of > InspectorHeapAgent::getPreview(), but I am not sure. Please check it? Below > is the failure diff. Seems that the string variable in the test is undefined. > > > --- > /home/slave/webkitgtk/gtk-linux-64-release-tests/build/layout-test-results/ > inspector/heap/getPreview-expected.txt > +++ > /home/slave/webkitgtk/gtk-linux-64-release-tests/build/layout-test-results/ > inspector/heap/getPreview-actual.txt > @@ -9,7 +9,7 @@ > -- Running test case: GetPreviewForString > PASS: Should not have an error creating a snapshot. > PASS: Should not have an error getting preview. > -STRING: This is the test string. > +STRING: undefined > > -- Running test case: GetPreviewForFunction > PASS: Should not have an error creating a snapshot. The code used to do *result = ..., but now it should not use operator*. It should just assign to result. So the crash is using operator* on an empty std::optional. (I'm not sure what the semantics of this, or whether it's allowed. Should just not use operator* here.) As for the other ServiceWorker crashes, I have no idea what's going on and it looks like unrelated code.
Committed r228600: <https://trac.webkit.org/changeset/228600>
Re-landed with the offending "*" character removed.
Thanks, test passing again.