Summary: | REGRESSION(r202337) [WebRTC] Crash when loading html5test.com | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Alberto Lopez Perez <clopez> | ||||||
Component: | WebKitGTK | Assignee: | 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
Carlos Alberto Lopez Perez
2016-06-27 06:07:33 PDT
I don't see anything specific to GTK+ in the bt. This looks like a crash in WebRTC implementation. 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. 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. Created attachment 282224 [details]
Proposed patch
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. (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. Created attachment 282321 [details]
Updated patch
Comment on attachment 282321 [details]
Updated patch
Thanks Eric and Alex
Comment on attachment 282321 [details] Updated patch Clearing flags on attachment: 282321 Committed r202624: <http://trac.webkit.org/changeset/202624> All reviewed patches have been landed. Closing bug. |