Summary: | [WebRTC] LibWebRTCEndpoint should not own objects that should be destroyed on the main thread | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||||||||
Component: | WebCore Misc. | Assignee: | youenn fablet <youennf> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | achristensen, commit-queue | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
youenn fablet
2017-02-03 14:56:15 PST
Created attachment 300585 [details]
Patch
Created attachment 300763 [details]
Patch
Attachment 300763 [details] did not pass style-queue:
ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp:93: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp:119: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp:134: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.h:48: local_streams is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.h:49: remote_streams is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.h:51: local_description is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.h:52: remote_description is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.h:55: signaling_state is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.h:56: ice_connection_state is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/testing/MockLibWebRTCPeerConnection.h:57: ice_gathering_state is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Total errors found: 10 in 19 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 300763 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300763&action=review What problem did you see that motivated this change? > LayoutTests/ChangeLog:15 > +2017-02-03 Youenn Fablet <youenn@apple.com> two ChangeLog entries. > What problem did you see that motivated this change? I haven't noticed any crash yet. But libwebrtc network process may well keep the last reference to LibWebRTCMediaEndpoint, thus deleting objects that are supposed to be deleted in the main thread. > > > LayoutTests/ChangeLog:15 > > +2017-02-03 Youenn Fablet <youenn@apple.com> > > two ChangeLog entries. OK, I'll fix it. Comment on attachment 300763 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300763&action=review > Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp:93 > + explicit MockLibWebRTCPeerConnectionReleasedInNetworkThreadWhileCreatingOffer(webrtc::PeerConnectionObserver& observer) : MockLibWebRTCPeerConnection(observer) { } We should fix the style checker if we really want to allow one initializer on the same line. > Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp:142 > +void MockLibWebRTCPeerConnectionReleasedInNetworkThreadWhileSettingDescription::SetLocalDescription(webrtc::SetSessionDescriptionObserver* observer, webrtc::SessionDescriptionInterface*) This shouldn't have its own line. Created attachment 300830 [details]
Patch for landing
Comment on attachment 300830 [details] Patch for landing Rejecting attachment 300830 [details] from commit-queue. Number of test failures exceeded the failure limit. Full output: http://webkit-queues.webkit.org/results/3021081 Created attachment 300835 [details]
Archive of layout-test-results from webkit-cq-01 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: webkit-cq-01 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 300830 [details] Patch for landing Rejecting attachment 300830 [details] from commit-queue. Number of test failures exceeded the failure limit. Full output: http://webkit-queues.webkit.org/results/3021306 Created attachment 300844 [details]
Archive of layout-test-results from webkit-cq-03 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: webkit-cq-03 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 300845 [details]
Patch for landing
Comment on attachment 300845 [details] Patch for landing Clearing flags on attachment: 300845 Committed r211837: <http://trac.webkit.org/changeset/211837> All reviewed patches have been landed. Closing bug. |