RESOLVED WONTFIX 173386
[iOS] Generate a simulated crash when the WebThread starts in MobileSafari
https://bugs.webkit.org/show_bug.cgi?id=173386
Summary [iOS] Generate a simulated crash when the WebThread starts in MobileSafari
David Kilzer (:ddkilzer)
Reported 2017-06-14 15:02:09 PDT
Need the bug URL (OOPS!). <rdar://problem/32776426> Reviewed by NOBODY (OOPS!). * platform/ios/wak/WebCoreThread.mm: (WebThreadEnable): Add release assert for MobileSafari. --- 2 files changed, 13 insertions(+), 1 deletion(-)
Attachments
Patch (1.64 KB, patch)
2017-06-14 15:02 PDT, David Kilzer (:ddkilzer)
no flags
Patch v2 (13.46 KB, patch)
2017-06-14 20:54 PDT, David Kilzer (:ddkilzer)
no flags
Patch v3 (13.47 KB, patch)
2017-06-15 02:41 PDT, David Kilzer (:ddkilzer)
no flags
Patch v4 (14.68 KB, patch)
2017-06-15 08:11 PDT, David Kilzer (:ddkilzer)
no flags
Patch v5 (14.68 KB, patch)
2017-06-15 08:16 PDT, David Kilzer (:ddkilzer)
aestes: review+
Build fix v1 (4.52 KB, patch)
2017-06-15 11:57 PDT, David Kilzer (:ddkilzer)
no flags
Build fix v2 (4.42 KB, patch)
2017-06-15 12:27 PDT, David Kilzer (:ddkilzer)
no flags
David Kilzer (:ddkilzer)
Comment 1 2017-06-14 15:02:10 PDT
David Kilzer (:ddkilzer)
Comment 2 2017-06-14 15:06:53 PDT
Andy Estes
Comment 3 2017-06-14 16:22:41 PDT
Actually I’m not sure about this. In general, we intentionally support WKWebView and legacy WebKit in the same app. I’m not sure why MobileSafari should be an exception to this rule.
David Kilzer (:ddkilzer)
Comment 4 2017-06-14 19:46:16 PDT
Comment on attachment 312923 [details] Patch After talking to Andy, we're going to switch to use a simulated crash since there might be a mix of expected and unexpected use cases, and we don't want to make MobileSafari unusable until we understand (all) the scenarios where the WebThread becomes active.
David Kilzer (:ddkilzer)
Comment 5 2017-06-14 20:54:50 PDT
Created attachment 312950 [details] Patch v2
Build Bot
Comment 6 2017-06-14 20:56:05 PDT
Attachment 312950 [details] did not pass style-queue: ERROR: Source/WebCore/platform/spi/ios/CrashReporterSupportSPI.h:36: The parameter name "pid" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
David Kilzer (:ddkilzer)
Comment 7 2017-06-15 02:41:10 PDT
Created attachment 312966 [details] Patch v3
Build Bot
Comment 8 2017-06-15 02:42:56 PDT
Attachment 312966 [details] did not pass style-queue: ERROR: Source/WebCore/platform/spi/ios/CrashReporterSupportSPI.h:36: The parameter name "pid" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
David Kilzer (:ddkilzer)
Comment 9 2017-06-15 08:11:46 PDT
Created attachment 312981 [details] Patch v4
Build Bot
Comment 10 2017-06-15 08:14:20 PDT
Attachment 312981 [details] did not pass style-queue: ERROR: Source/WebCore/platform/spi/ios/CrashReporterSupportSPI.h:37: The parameter name "pid" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
David Kilzer (:ddkilzer)
Comment 11 2017-06-15 08:16:34 PDT
Created attachment 312983 [details] Patch v5
David Kilzer (:ddkilzer)
Comment 12 2017-06-15 08:17:36 PDT
(In reply to David Kilzer (:ddkilzer) from comment #11) > Created attachment 312983 [details] > Patch v5 Last one! Minor header adjustment after fixing build issues in v4.
Build Bot
Comment 13 2017-06-15 08:19:07 PDT
Attachment 312983 [details] did not pass style-queue: ERROR: Source/WebCore/platform/spi/ios/CrashReporterSupportSPI.h:38: The parameter name "pid" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Andy Estes
Comment 14 2017-06-15 09:58:35 PDT
Comment on attachment 312983 [details] Patch v5 View in context: https://bugs.webkit.org/attachment.cgi?id=312983&action=review > Source/WebCore/WebCore.xcodeproj/project.pbxproj:21096 > + 44CA12971EF23A6500E4B3AD /* CrashReporterSupportSPI.h */, > CE1252421A16C01A00864480 /* CoreUISPI.h */, Sorting problem.
David Kilzer (:ddkilzer)
Comment 15 2017-06-15 10:05:21 PDT
David Kilzer (:ddkilzer)
Comment 16 2017-06-15 11:22:57 PDT
(In reply to David Kilzer (:ddkilzer) from comment #15) > Committed r218339: <http://trac.webkit.org/changeset/218339> This broke internal iOS Simulator builds. Working on a fix now.
David Kilzer (:ddkilzer)
Comment 17 2017-06-15 11:57:28 PDT
Reopening to attach new patch.
David Kilzer (:ddkilzer)
Comment 18 2017-06-15 11:57:28 PDT
Created attachment 312994 [details] Build fix v1
David Kilzer (:ddkilzer)
Comment 19 2017-06-15 12:27:13 PDT
Created attachment 312997 [details] Build fix v2
David Kilzer (:ddkilzer)
Comment 20 2017-06-15 12:30:18 PDT
mitz
Comment 21 2017-06-15 13:16:32 PDT
Comment on attachment 312983 [details] Patch v5 View in context: https://bugs.webkit.org/attachment.cgi?id=312983&action=review > Source/WebCore/platform/ios/wak/WebCoreThread.mm:1007 > + SimulateCrash(getpid(), kExceptionCode, @"MobileSafari should never run a WebThread"); This is a strange assertion for a framework to make about one of its clients, and it would have made more sense (and less of a layering violation) to let clients register for a callback when the Web thread is created so that they can enforce whatever policy they might have. It is also strange for WebKit to assert this about MobileSafari, which is a rather complex app that uses many frameworks in the system which it might not have direct control over, but this is not a good place to get into such details of how Apple develops Safari and iOS.
David Kilzer (:ddkilzer)
Comment 22 2017-06-15 17:46:07 PDT
Okay, reverted this change in: Committed r218370: <http://trac.webkit.org/changeset/218370> Moving to RESOLVED/WONTFIX.
Note You need to log in before you can comment on or make changes to this bug.