Bug 173386 – [iOS] Generate a simulated crash when the WebThread starts in MobileSafari

• Component: Platform
• Status: RESOLVED WONTFIX

[David Kilzer (:ddkilzer)]

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(-)

[David Kilzer (:ddkilzer)]

Created attachment 312923 [details]
Patch

[David Kilzer (:ddkilzer)]

<rdar://problem/32776426>

[Andy Estes]

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 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)]

Created attachment 312950 [details]
Patch v2

[Build Bot]

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)]

Created attachment 312966 [details]
Patch v3

[Build Bot]

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)]

Created attachment 312981 [details]
Patch v4

[Build Bot]

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)]

Created attachment 312983 [details]
Patch v5

[David Kilzer (:ddkilzer)]

(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]

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 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)]

Committed r218339: <http://trac.webkit.org/changeset/218339>

[David Kilzer (:ddkilzer)]

(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)]

Reopening to attach new patch.

[David Kilzer (:ddkilzer)]

Created attachment 312994 [details]
Build fix v1

[David Kilzer (:ddkilzer)]

Created attachment 312997 [details]
Build fix v2

[David Kilzer (:ddkilzer)]

Committed r218347: <http://trac.webkit.org/changeset/218347>

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)]

Okay, reverted this change in:

Committed r218370: <http://trac.webkit.org/changeset/218370>

Moving to RESOLVED/WONTFIX.