Summary: | Fix uninitialized public members in NetworkProcess | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tomas Popela <tpopela> | ||||||||||
Component: | WebKit2 | Assignee: | Tomas Popela <tpopela> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | achristensen, ap, buildbot, cdumez, cgarcia, clopez, commit-queue, darin, mcatanzaro, rniwa, ryanhaddad | ||||||||||
Priority: | P2 | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 104114 | ||||||||||||
Attachments: |
|
Description
Tomas Popela
2017-03-14 06:33:50 PDT
Created attachment 304374 [details]
Patch
Comment on attachment 304374 [details] Patch Attachment 304374 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3320362 Number of test failures exceeded the failure limit. Created attachment 304378 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 304374 [details] Patch Attachment 304374 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3320418 Number of test failures exceeded the failure limit. Created attachment 304381 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
LGTM, needs an owner. Comment on attachment 304374 [details] Patch Clearing flags on attachment: 304374 Committed r213915: <http://trac.webkit.org/changeset/213915> All reviewed patches have been landed. Closing bug. (In reply to comment #7) > Comment on attachment 304374 [details] > Patch > > Clearing flags on attachment: 304374 > > Committed r213915: <http://trac.webkit.org/changeset/213915> This change appears to have caused LayoutTests to exit early with timeouts on WK2: https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK2%20%28Tests%29/builds/4362 https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20%28Tests%29/builds/13970 https://build.webkit.org/builders/Apple%20iOS%2010%20Simulator%20Release%20WK2%20%28Tests%29/builds/4575 Reverted r213915 for reason: Caused WK2 LayoutTests to exit early with timeouts. Committed r213927: <http://trac.webkit.org/changeset/213927> Alexey asked in bug #169603#c4: (In reply to comment #4) > Comment on attachment 304377 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=304377&action=review > > > Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.h:52 > > + bool privateBrowsingEnabled { false }; > > Can any of the creation parameters actually be uninitialized? Using default > values seems as much of a problem as using uninitialised one - perhaps even > worse, as UBSan won't catch that. Yes, in fact there already exist NetworkProcessCreationParameters members that are never initialized. Valgrind always complains when starting WebKitGTK+ because we always pass uninitialized memory into the kernel at startup via NetworkProcessCreationParameters. I guess/hope this patch should fix that. See bug #146729#c5. You have a point that it might be useful to force callers to initialize all data members, but if so then it should be enforced at compile time by adding a constructor. It's much easier and safer to placate the static analyzer by ensuring objects cannot be created with uninitialized data members than it is to check for such issues at runtime. But the counterargument is that having a constructor with tons of parameters for a Parameters object might be crazy. Created attachment 304763 [details] Patch Moving patch here from bug 169604, where is was reviwed by Alex Christensen. Comment on attachment 304763 [details] Patch Clearing flags on attachment: 304763 Committed r214101: <http://trac.webkit.org/changeset/214101> All reviewed patches have been landed. Closing bug. |