Bug 220809 - REGRESSION (r270874): Some React Native apps are reported broken on iOS
Summary: REGRESSION (r270874): Some React Native apps are reported broken on iOS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Major
Assignee: Alexey Shvayka
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-01-21 10:29 PST by Alexey Shvayka
Modified: 2021-01-25 18:13 PST (History)
9 users (show)

See Also:

Attachments
Patch (18.90 KB, patch)
2021-01-21 10:33 PST, Alexey Shvayka 		no flags Details | Formatted Diff | Diff
Patch (18.94 KB, patch)
2021-01-21 11:10 PST, Alexey Shvayka 		no flags Details | Formatted Diff | Diff
Patch for landing (14.37 KB, patch)
2021-01-25 17:27 PST, Alexey Shvayka 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Shvayka 2021-01-21 10:29:52 PST 
REGRESSION (r270874): Some React Native apps are reported broken on iOS
Comment 1 Alexey Shvayka 2021-01-21 10:33:05 PST 
Created attachment 418061 [details]
Patch
Comment 2 Alexey Shvayka 2021-01-21 10:37:45 PST 
Comment on attachment 418061 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=418061&action=review

> Source/JavaScriptCore/runtime/JSObject.cpp:2001
> +    bool reportAsEnumerable = !(slot.attributes() & PropertyAttribute::DontEnum) || structure(vm)->typeInfo().getOwnPropertySlotIsWrongAboutDontEnum();

The flag should probably be queried from slotBase() rather than |this|.
Comment 3 Alexey Shvayka 2021-01-21 11:10:50 PST 
Created attachment 418064 [details]
Patch

Query GetOwnPropertySlotIsWrongAboutDontEnum from the slotBase().
Comment 4 Radar WebKit Bug Importer 2021-01-21 16:51:17 PST 
<rdar://problem/73475533>
Comment 5 Saam Barati 2021-01-22 11:05:22 PST 
I'm going to test this soon and can review too
Comment 6 Saam Barati 2021-01-25 16:09:36 PST 
Comment on attachment 418064 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=418064&action=review

Confirmed this fixes the React native crashes we were seeing

> Source/JavaScriptCore/runtime/JSTypeInfo.h:62
> +static constexpr unsigned GetOwnPropertySlotIsWrongAboutDontEnum = 1 << 20;

nit, I think I'd call this:
GetOwnPropertySlotMayBeWrongAboutDontEnum

since we don't know definitively, but the runtime must be conservative.

> Source/WebCore/bridge/runtime_array.h:38
> +    static constexpr unsigned StructureFlags = Base::StructureFlags | OverridesGetOwnPropertySlot | OverridesGetOwnPropertyNames | InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | GetOwnPropertySlotIsWrongAboutDontEnum;

is it worth reverting these back to how they used to be and provide this flag? Or should we make them truthful? I don't think we have evidence that this part of the patch broke stuff
Comment 7 Alexey Shvayka 2021-01-25 17:27:53 PST 
Created attachment 418359 [details]
Patch for landing
Comment 8 EWS 2021-01-25 18:13:16 PST 
Committed r271873: <https://trac.webkit.org/changeset/271873>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 418359 [details].