Bug 180877 - Web Inspector: CRASH at WebCore::InspectorCanvas::resetRecordingData + 416
Summary: Web Inspector: CRASH at WebCore::InspectorCanvas::resetRecordingData + 416
Status: RESOLVED MOVED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-15 11:59 PST by BJ Burg
Modified: 2017-12-15 22:24 PST (History)
6 users (show)

See Also:


Attachments
Patch (4.51 KB, patch)
2017-12-15 12:07 PST, Devin Rousso
bburg: review+
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews116 for mac-elcapitan (3.21 MB, application/zip)
2017-12-15 16:54 PST, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2017-12-15 11:59:21 PST
CRASHING TEST: webgl/1.0.2/conformance/attribs/gl-disabled-vertex-attrib.html

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x000000017bfbfe50 WebCore::InspectorCanvas::resetRecordingData() + 416
1   com.apple.WebCore             	0x000000017bfbfb13 WebCore::InspectorCanvas::~InspectorCanvas() + 19
2   com.apple.WebCore             	0x000000017bff3ab8 WebCore::InspectorCanvasAgent::clearCanvasData() + 120
3   com.apple.WebCore             	0x000000017bfcfd39 WebCore::InspectorInstrumentation::didCommitLoadImpl(WebCore::InstrumentingAgents&, WebCore::Frame&, WebCore::DocumentLoader*) + 217
4   com.apple.WebCore             	0x000000017c05b849 WebCore::FrameLoader::dispatchDidCommitLoad(std::optional<WebCore::HasInsecureContent>) + 137
5   com.apple.WebCore             	0x000000017c05b461 WebCore::FrameLoader::receivedFirstData() + 33
6   com.apple.WebCore             	0x000000017c03fdeb WebCore::DocumentLoader::commitData(char const*, unsigned long) + 1627
7   com.apple.WebCore             	0x000000017c03f173 WebCore::DocumentLoader::finishedLoading() + 531
...

Devin thinks we need to just not reset recording data in ~InspectorCanvas, as there is no reason to do that. I am skeptical that InspectorCanvas is not being destructed underneath ~CanvasRenderingContext, as that was the intent of the design.
Comment 1 BJ Burg 2017-12-15 11:59:51 PST
<rdar://problem/36071940>
Comment 2 Devin Rousso 2017-12-15 12:07:14 PST
Created attachment 329503 [details]
Patch
Comment 3 Joseph Pecoraro 2017-12-15 15:57:04 PST
Comment on attachment 329503 [details]
Patch

This looks good to me. The original patch however is triggering ASSERTions on the Debug bots, so I think it is about to be rolled out. In which case this can be re-included in that when relanding.
Comment 4 BJ Burg 2017-12-15 16:49:03 PST
Comment on attachment 329503 [details]
Patch

r=me
Comment 5 EWS Watchlist 2017-12-15 16:53:59 PST
Comment on attachment 329503 [details]
Patch

Attachment 329503 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5678445

New failing tests:
webgl/1.0.2/conformance/context/context-release-upon-reload.html
Comment 6 EWS Watchlist 2017-12-15 16:54:01 PST
Created attachment 329543 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 7 Devin Rousso 2017-12-15 22:24:03 PST
I'm going to merge this change with <https://webkit.org/b/180770> since it has been reopened.