Bug 159145

Summary: REGRESSION(r202337) [WebRTC] Crash when loading html5test.com
Product: WebKit Reporter: Carlos Alberto Lopez Perez <clopez>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, adam.bergkvist, bugs-noreply, cgarcia, commit-queue, eric.carlson, youennf
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 143211, 158832    
Attachments:
Description Flags
Proposed patch
none
Updated patch none

Carlos Alberto Lopez Perez
Reported 2016-06-27 06:07:33 PDT
With current trunk (r202484) when I try to load html5test.com with the minibrowser: $ Tools/Scripts/run-minibrowser --gtk http://html5test.com I get a crash. The backtrace: http://sprunge.us/REVR
Attachments
Proposed patch (4.78 KB, patch)
2016-06-28 01:43 PDT, Adam Bergkvist 		no flags
DetailsFormatted DiffDiff
Updated patch (4.78 KB, patch)
2016-06-28 22:29 PDT, Adam Bergkvist 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1 2016-06-27 06:14:04 PDT
I don't see anything specific to GTK+ in the bt. This looks like a crash in WebRTC implementation.
Carlos Alberto Lopez Perez
Comment 2 2016-06-27 07:23:12 PDT
I have identified r202337 <http://trac.webkit.org/r202337> as the one causing this regression. I manually reverted r202337 locally, rebuilt webkit and the crash is gone.
Adam Bergkvist
Comment 3 2016-06-27 22:34:20 PDT
I see the problem. For testing, we create a MockMediaEndpoint (WebRTC backend), but when the mini-browser is run, we have a nullptr MediaEndpoint. The changes introduced in r202337 doesn't handle that properly.
Adam Bergkvist
Comment 4 2016-06-28 01:43:39 PDT
Created attachment 282224 [details] Proposed patch
Alex Christensen
Comment 5 2016-06-28 11:18:36 PDT
Comment on attachment 282224 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=282224&action=review > Source/WebCore/platform/mediastream/MediaEndpoint.cpp:56 > +class NullMediaEndpoint : public MediaEndpoint { I think EmptyMediaEndpoint would be a better name. > Source/WebCore/platform/mediastream/MediaEndpoint.cpp:80 > + return std::unique_ptr<MediaEndpoint>(new NullMediaEndpoint(client)); make_unique. Also, this function is not necessary. Just use std::make_unique instead of this.
Adam Bergkvist
Comment 6 2016-06-28 14:36:53 PDT
(In reply to comment #5) Thanks for reviewing, Alex > Comment on attachment 282224 [details] > Proposed patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=282224&action=review > > > Source/WebCore/platform/mediastream/MediaEndpoint.cpp:56 > > +class NullMediaEndpoint : public MediaEndpoint { > > I think EmptyMediaEndpoint would be a better name. EmptyMediaEndpoint and EmptyRealtimeMediaSource works for me. > > Source/WebCore/platform/mediastream/MediaEndpoint.cpp:80 > > + return std::unique_ptr<MediaEndpoint>(new NullMediaEndpoint(client)); > > make_unique. Also, this function is not necessary. Just use > std::make_unique instead of this. Are you referring to the createMediaEndpoint() function here? The MediaEndpointPeerConnection constructor calls createMediaEndpoint() to create its MediaEndpoint - in this case an "empty" one. When a port has a "real" MediaEndponit implementation it provides its own createMediaEndpoint() that's called instead.
Adam Bergkvist
Comment 7 2016-06-28 22:29:59 PDT
Created attachment 282321 [details] Updated patch
Adam Bergkvist
Comment 8 2016-06-29 08:50:02 PDT
Comment on attachment 282321 [details] Updated patch Thanks Eric and Alex
WebKit Commit Bot
Comment 9 2016-06-29 09:11:06 PDT
Comment on attachment 282321 [details] Updated patch Clearing flags on attachment: 282321 Committed r202624: <http://trac.webkit.org/changeset/202624>
WebKit Commit Bot
Comment 10 2016-06-29 09:11:11 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.