Bug 193603

Summary: [JSC] Invalidate old scope operations using global lexical binding epoch
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, guijemont, guijemont+jsc-armv7-ews, keith_miller, mark.lam, msaboff, sbarati, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing
commit-queue: commit-queue-
Patch for landing
none
Patch
none
Follow-up none

Description Yusuke Suzuki 2019-01-18 18:23:13 PST
[JSC] Invalidate old scope operations using global lexical binding epoch
Comment 1 Yusuke Suzuki 2019-01-18 18:49:15 PST
Created attachment 359573 [details]
Patch
Comment 2 EWS Watchlist 2019-01-18 18:51:12 PST
Attachment 359573 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:142:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:204:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Yusuke Suzuki 2019-01-18 18:58:25 PST
Created attachment 359575 [details]
Patch
Comment 4 Yusuke Suzuki 2019-01-18 18:59:22 PST
<rdar://problem/47380869>
Comment 5 EWS Watchlist 2019-01-18 19:00:28 PST
Attachment 359575 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:142:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:204:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Yusuke Suzuki 2019-01-18 19:00:49 PST
Created attachment 359576 [details]
Patch
Comment 7 EWS Watchlist 2019-01-18 19:02:50 PST
Attachment 359576 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:142:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:204:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Yusuke Suzuki 2019-01-18 19:13:37 PST
Created attachment 359578 [details]
Patch
Comment 9 EWS Watchlist 2019-01-18 19:16:14 PST
Attachment 359578 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:142:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:204:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Yusuke Suzuki 2019-01-18 19:20:34 PST
Created attachment 359579 [details]
Patch
Comment 11 EWS Watchlist 2019-01-18 19:22:48 PST
Attachment 359579 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:142:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:204:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Yusuke Suzuki 2019-01-18 19:25:26 PST
Created attachment 359580 [details]
Patch
Comment 13 EWS Watchlist 2019-01-18 19:28:09 PST
Attachment 359580 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:142:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:204:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 EWS Watchlist 2019-01-19 15:29:46 PST
Comment on attachment 359580 [details]
Patch

Attachment 359580 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/10813221

New failing tests:
stress/scope-operation-cache-global-property-before-deleting.js.ftl-eager-no-cjit-b3o1
stress/let-lexical-binding-shadow-existing-global-property-ftl.js.ftl-eager-no-cjit-b3o1
stress/const-lexical-binding-shadow-existing-global-property-ftl.js.dfg-eager-no-cjit-validate
stress/const-lexical-binding-shadow-existing-global-property-ftl.js.no-llint
stress/scope-operation-cache-global-property-before-deleting.js.no-llint
stress/let-lexical-binding-shadow-existing-global-property-ftl.js.ftl-eager-no-cjit
stress/scope-operation-cache-global-property-before-deleting.js.ftl-eager-no-cjit
stress/const-lexical-binding-shadow-existing-global-property-ftl.js.ftl-eager-no-cjit-b3o1
stress/let-lexical-binding-shadow-existing-global-property-ftl.js.dfg-eager-no-cjit-validate
stress/let-lexical-binding-shadow-existing-global-property-ftl.js.no-llint
stress/scope-operation-cache-global-property-before-deleting.js.dfg-eager-no-cjit-validate
apiTests
Comment 15 Yusuke Suzuki 2019-01-19 16:31:35 PST
Created attachment 359625 [details]
Patch
Comment 16 EWS Watchlist 2019-01-19 16:33:39 PST
Attachment 359625 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:142:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:204:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 Saam Barati 2019-01-20 15:18:43 PST
Comment on attachment 359625 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGDesiredGlobalProperties.cpp:52
> +        property.globalObject()->ensureReferencedPropertyWatchpointSet(uid).fireAll(vm, "Lexical binding shadows the existing global properties");

"shadows the existing global properties" => "shadows an existing global property"

> Source/JavaScriptCore/dfg/DFGGraph.cpp:1065
> +    // but this is not done yet since we do not execute this op again. Just emitting ForceOSRExit to update the metadata when it reaches to this code.

"but this is not done yet since we do not execute this op again" => something like... "but we still have stale metadata here since we have not yet executed this bytecode operation since the invalidation"

> Source/JavaScriptCore/jit/JITPropertyAccess.cpp:815
>          JSScope** constantScopeSlot = metadata.m_constantScope.slot();
>          emitVarInjectionCheck(needsVarInjectionChecks(resolveType));
>          loadPtr(constantScopeSlot, regT0);

I don't think this code makes sense any more. I think it just works if you call emitCode(resolveType) since that will do the epoch check.

> Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:1072
> +    case GlobalPropertyWithVarInjectionChecks: // Global Lexical Binding Epoch is changed. Update op_resolve_scope.

Don't think this comment adds much.

> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:1889
> +WatchpointSet* JSGlobalObject::getReferencedPropertyWatchpointSetConcurrently(UniquedStringImpl* uid)

Why call this "concurrently" if that's the only option? Seems like we just call it with the old name.

> Source/JavaScriptCore/runtime/Options.h:511
> +    v(unsigned, thresholdForGlobalLexicalBindingEpoch, UINT_MAX, Normal, "Threshold for global lexical binding epoch. If the epoch reaches to this value, CodeBlock metadata for scope operations will be revised globally. It needs to be greater than 1.") \

Let's add a RELEASE_ASSERT in Options.cpp or just bump it to UINT_MAX if this is zero. I think I prefer the latter solution

> Source/JavaScriptCore/runtime/ProgramExecutable.cpp:210
> +            // So that we do not create WatchpointSet here. DFG will create if necessary on the main thread.

And it will only create it if the global lexical environment binding doesn't exist, which is why this code works.

^ I'd add something like that to this comment.
Comment 18 Yusuke Suzuki 2019-01-20 16:08:51 PST
Comment on attachment 359625 [details]
Patch

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

>> Source/JavaScriptCore/dfg/DFGDesiredGlobalProperties.cpp:52
>> +        property.globalObject()->ensureReferencedPropertyWatchpointSet(uid).fireAll(vm, "Lexical binding shadows the existing global properties");
> 
> "shadows the existing global properties" => "shadows an existing global property"

Fixed.

>> Source/JavaScriptCore/dfg/DFGGraph.cpp:1065
>> +    // but this is not done yet since we do not execute this op again. Just emitting ForceOSRExit to update the metadata when it reaches to this code.
> 
> "but this is not done yet since we do not execute this op again" => something like... "but we still have stale metadata here since we have not yet executed this bytecode operation since the invalidation"

Sounds nice. Fixed.

>> Source/JavaScriptCore/jit/JITPropertyAccess.cpp:815
>>          loadPtr(constantScopeSlot, regT0);
> 
> I don't think this code makes sense any more. I think it just works if you call emitCode(resolveType) since that will do the epoch check.

OK, use emitCode to construct this code path. We still need to switch the code based on metadata.resolveType since op_resolve_scope+GlobalProperty would be updated to op_resolve_scope+GlobalLexicalVar.

>> Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:1072
>> +    case GlobalPropertyWithVarInjectionChecks: // Global Lexical Binding Epoch is changed. Update op_resolve_scope.
> 
> Don't think this comment adds much.

Dropped.

>> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:1889
>> +WatchpointSet* JSGlobalObject::getReferencedPropertyWatchpointSetConcurrently(UniquedStringImpl* uid)
> 
> Why call this "concurrently" if that's the only option? Seems like we just call it with the old name.

OK, fixed.

>> Source/JavaScriptCore/runtime/Options.h:511
>> +    v(unsigned, thresholdForGlobalLexicalBindingEpoch, UINT_MAX, Normal, "Threshold for global lexical binding epoch. If the epoch reaches to this value, CodeBlock metadata for scope operations will be revised globally. It needs to be greater than 1.") \
> 
> Let's add a RELEASE_ASSERT in Options.cpp or just bump it to UINT_MAX if this is zero. I think I prefer the latter solution

OK, added correctOptions() function which corrects option values if it is out of range.

>> Source/JavaScriptCore/runtime/ProgramExecutable.cpp:210
>> +            // So that we do not create WatchpointSet here. DFG will create if necessary on the main thread.
> 
> And it will only create it if the global lexical environment binding doesn't exist, which is why this code works.
> 
> ^ I'd add something like that to this comment.

Added it. DFG will create not-invalidated watchpoint set only if the global lexical environment binding doesn't exist. (to avoid compile-and-fail loop, DFG also creates invalidated watchpoint set if it attempted to watch the GlobalProperty that is shadowed).
Comment 19 Yusuke Suzuki 2019-01-20 16:09:58 PST
Created attachment 359657 [details]
Patch
Comment 20 EWS Watchlist 2019-01-20 16:11:40 PST
Attachment 359657 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:142:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:204:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/Options.cpp:382:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 3 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 Saam Barati 2019-01-20 16:45:54 PST
Comment on attachment 359657 [details]
Patch

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

> Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:818
>          JSScope** constantScopeSlot = metadata.m_constantScope.slot();

might as well do the same thing here
Comment 22 Saam Barati 2019-01-20 16:46:09 PST
r=me
Comment 23 Yusuke Suzuki 2019-01-20 16:46:59 PST
Comment on attachment 359657 [details]
Patch

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

>> Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:818
>>          JSScope** constantScopeSlot = metadata.m_constantScope.slot();
> 
> might as well do the same thing here

Oops, right. Fixed.
Comment 24 Yusuke Suzuki 2019-01-20 16:50:50 PST
Created attachment 359661 [details]
Patch for landing
Comment 25 EWS Watchlist 2019-01-20 16:53:32 PST
Attachment 359661 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:142:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:204:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/Options.cpp:382:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 3 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 26 WebKit Commit Bot 2019-01-20 16:55:03 PST
Comment on attachment 359661 [details]
Patch for landing

Rejecting attachment 359661 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 359661, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in JSTests/ChangeLog contains OOPS!.

Full output: https://webkit-queues.webkit.org/results/10823482
Comment 27 Yusuke Suzuki 2019-01-20 17:01:08 PST
Created attachment 359662 [details]
Patch for landing
Comment 28 EWS Watchlist 2019-01-20 17:02:56 PST
Attachment 359662 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:142:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:204:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/Options.cpp:382:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 3 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 29 WebKit Commit Bot 2019-01-20 17:39:12 PST
Comment on attachment 359662 [details]
Patch for landing

Clearing flags on attachment: 359662

Committed r240220: <https://trac.webkit.org/changeset/240220>
Comment 30 Yusuke Suzuki 2019-01-21 21:28:41 PST
Committed r240248: <https://trac.webkit.org/changeset/240248>
Comment 31 Yusuke Suzuki 2019-01-21 22:36:22 PST
Comment on attachment 359657 [details]
Patch

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

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:2163
> +        loadi OpResolveScope::Metadata::m_globalLexicalBindingEpoch[globalObject], scratch

This was wrong. It should be `t5` for metadata table here.
Comment 32 Yusuke Suzuki 2019-01-21 22:42:26 PST
Reopening to attach new patch.
Comment 33 Yusuke Suzuki 2019-01-21 22:42:27 PST
Created attachment 359720 [details]
Patch
Comment 34 Yusuke Suzuki 2019-01-21 22:44:07 PST
Comment on attachment 359720 [details]
Patch

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

Annotated the changes from the previous one.

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:629
> +                    metadata.m_globalLexicalBindingEpoch = m_globalObject->globalLexicalBindingEpoch();

Set the current epoch, since JSScope::abstractResolve already ensured this is correct.

> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:2105
> +        loadi OpResolveScope::Metadata::m_globalLexicalBindingEpoch[t5], scratch

Ditto.

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:2163
> +        loadi OpResolveScope::Metadata::m_globalLexicalBindingEpoch[t5], scratch

This `t5` was previously `globalObject`, but it was wrong.
Comment 35 EWS Watchlist 2019-01-21 22:44:23 PST
Attachment 359720 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:142:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/CommonSlowPaths.h:204:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/Options.cpp:382:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 3 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 36 Yusuke Suzuki 2019-01-22 00:16:52 PST
                                           ToT                     Patched
SunSpider:
   3d-cube                            4.6991+-0.0880     ?      4.7298+-0.0750        ?
   3d-morph                           5.3163+-0.0393            5.2699+-0.0391
   3d-raytrace                        4.9223+-0.0675     ?      4.9307+-0.0567        ?
   access-binary-trees                2.0875+-0.0325     ?      2.1027+-0.0332        ?
   access-fannkuch                    5.1875+-0.1310            5.0740+-0.1209          might be 1.0224x faster
   access-nbody                       2.8640+-0.0419     ?      2.8751+-0.0358        ?
   access-nsieve                      2.6054+-0.1617            2.5496+-0.1412          might be 1.0219x faster
   bitops-3bit-bits-in-byte           1.4706+-0.0284     ?      1.4765+-0.0285        ?
   bitops-bits-in-byte                2.7107+-0.0381            2.6668+-0.0329          might be 1.0165x faster
   bitops-bitwise-and                 2.3497+-0.0372     ?      2.3927+-0.0334        ? might be 1.0183x slower
   bitops-nsieve-bits                 3.6324+-0.0260            3.6200+-0.0297
   controlflow-recursive              2.3468+-0.0371     ?      2.3478+-0.0337        ?
   crypto-aes                         4.1436+-0.0492            4.1068+-0.0515
   crypto-md5                         2.7557+-0.0348     ?      2.7578+-0.0387        ?
   crypto-sha1                        3.0644+-0.1375            2.9451+-0.0418          might be 1.0405x faster
   date-format-tofte                  7.0749+-0.0944            7.0373+-0.0595
   date-format-xparb                  5.7545+-0.0401     ?      5.8003+-0.0873        ?
   math-cordic                        3.0190+-0.0345            3.0072+-0.0342
   math-partial-sums                  4.2980+-0.0398     ?      4.3634+-0.0377        ? might be 1.0152x slower
   math-spectral-norm                 2.0313+-0.0342     ?      2.0389+-0.0312        ?
   regexp-dna                         7.5687+-0.0900     ?      7.6086+-0.1278        ?
   string-base64                      4.2967+-0.0419     ?      4.3243+-0.0527        ?
   string-fasta                       6.2167+-0.0621     ?      6.2472+-0.1422        ?
   string-tagcloud                    8.1186+-0.0805            8.1171+-0.0849
   string-unpack-code                16.7391+-0.2687           16.6323+-0.2328
   string-validate-input              4.9366+-0.0780     ?      4.9824+-0.0758        ?

   <arithmetic>                       4.6235+-0.0195            4.6155+-0.0139          might be 1.0017x faster

                                           ToT                     Patched
Octane:
   encrypt                           0.14941+-0.00059    ?     0.14975+-0.00064       ?
   decrypt                           2.95386+-0.01838    ?     2.96075+-0.01959       ?
   deltablue                x2       0.12588+-0.00362    ?     0.12637+-0.00306       ?
   earley                            0.21826+-0.00137          0.21666+-0.00097
   boyer                             3.58181+-0.02607          3.56802+-0.02056
   navier-stokes            x2       4.22469+-0.06589          4.18124+-0.02478         might be 1.0104x faster
   raytrace                 x2       0.73508+-0.00516          0.73258+-0.00245
   richards                 x2       0.08183+-0.00103          0.08158+-0.00079
   splay                    x2       0.22986+-0.00233          0.22856+-0.00127
   regexp                   x2      12.87470+-0.11864         12.73747+-0.10437         might be 1.0108x faster
   pdfjs                    x2      26.95156+-0.24909    ?    27.16141+-0.23212       ?
   mandreel                 x2      33.07604+-0.13692    ?    33.43279+-0.34674       ? might be 1.0108x slower
   gbemu                    x2      23.06035+-0.16657    ?    23.27870+-0.26941       ?
   closure                           0.40390+-0.00103    ?     0.40686+-0.00300       ?
   jquery                            5.28154+-0.07343          5.24535+-0.02954
   box2d                    x2       6.67657+-0.03759          6.65511+-0.03653
   zlib                     x2     269.18828+-5.31932        266.83823+-1.74256
   typescript               x2     486.49224+-4.01904    ?   492.18654+-9.78357       ? might be 1.0117x slower
   splay-latency                     1.78478+-0.02078    ?     1.79133+-0.02318       ?

   <geometric>                       3.97328+-0.01269          3.97296+-0.01141         might be 1.0001x faster

                                           ToT                     Patched
Geomean of preferred means:
   <scaled-result>                   13.5534+-0.0363           13.5414+-0.0329          might be 1.0009x faster
Comment 37 WebKit Commit Bot 2019-01-22 09:48:14 PST
Comment on attachment 359720 [details]
Patch

Clearing flags on attachment: 359720

Committed r240254: <https://trac.webkit.org/changeset/240254>
Comment 38 WebKit Commit Bot 2019-01-22 09:48:16 PST
All reviewed patches have been landed.  Closing bug.
Comment 39 Yusuke Suzuki 2019-01-22 21:12:31 PST
Reopening to attach new patch.
Comment 40 Yusuke Suzuki 2019-01-22 21:12:32 PST
Created attachment 359846 [details]
Follow-up
Comment 41 Yusuke Suzuki 2019-01-22 22:18:28 PST
(In reply to Yusuke Suzuki from comment #40)
> Created attachment 359846 [details]
> Follow-up

Discussed with Saam offline, and we agree that this is OK.
Comment 42 Yusuke Suzuki 2019-01-22 22:21:46 PST
Committed r240329: <https://trac.webkit.org/changeset/240329>