Bug 155089 - [JSC] StringObject.{put, defineOwnProperty} should realize indexed properties
Summary: [JSC] StringObject.{put, defineOwnProperty} should realize indexed properties
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-06 08:20 PST by Yusuke Suzuki
Modified: 2016-03-07 10:21 PST (History)
9 users (show)

See Also:


Attachments
Patch (10.68 KB, patch)
2016-03-06 08:24 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2016-03-06 08:20:32 PST
[JSC] StringObject.{put, defineOwnProperty} should realize indexed properties
Comment 1 Yusuke Suzuki 2016-03-06 08:24:47 PST
Created attachment 273136 [details]
Patch
Comment 2 Yusuke Suzuki 2016-03-06 08:26:29 PST
Comment on attachment 273136 [details]
Patch

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

Added comment.

> Source/JavaScriptCore/runtime/StringObject.cpp:72
> +    return JSObject::put(cell, exec, propertyName, value, slot);

Later, in Reflect.set patch, we return boolean from ::put, ::putByIndex. So now, we take the style like above.
Comment 3 Yusuke Suzuki 2016-03-06 08:29:03 PST
Comment on attachment 273136 [details]
Patch

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

> Source/JavaScriptCore/runtime/StringObject.cpp:113
> +        return validateAndApplyPropertyDescriptor(exec, nullptr, propertyName, isExtensible, descriptor, isCurrentDefined, current, throwException);

When passing nullptr to validateAndApplyPropertyDescriptor, it just performs validation.
This is OK since we know any descriptor cannot change the result.
All we need is the returned value of the validation phase.

And we note that this path is very very rare path. So performing slow / generic operation here causes no problem.
Comment 4 Geoffrey Garen 2016-03-07 09:30:57 PST
Comment on attachment 273136 [details]
Patch

r=me
Comment 5 WebKit Commit Bot 2016-03-07 10:21:42 PST
Comment on attachment 273136 [details]
Patch

Clearing flags on attachment: 273136

Committed r197684: <http://trac.webkit.org/changeset/197684>
Comment 6 WebKit Commit Bot 2016-03-07 10:21:47 PST
All reviewed patches have been landed.  Closing bug.