Bug 172575 - Properties set on window.customElements can disappear due to GC
Summary: Properties set on window.customElements can disappear due to GC
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari 10
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks: 154907 188353
  Show dependency treegraph
 
Reported: 2017-05-24 20:35 PDT by Russell Bicknell
Modified: 2018-08-06 16:30 PDT (History)
16 users (show)

See Also:


Attachments
test.html (908 bytes, text/html)
2017-05-24 20:36 PDT, Russell Bicknell
no flags Details
index.html (908 bytes, text/html)
2017-05-24 20:36 PDT, Russell Bicknell
no flags Details
test.html (375 bytes, text/html)
2017-05-24 20:39 PDT, Russell Bicknell
no flags Details
Fixes the bug (8.38 KB, patch)
2018-08-03 21:13 PDT, Ryosuke Niwa
sbarati: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Russell Bicknell 2017-05-24 20:35:52 PDT
Occasionally, properties that are set on window.customElements very early in the lifetime of the page are removed by garbage collection. To repro, put the attached files in the same directory and open 'index.html'. You may need to adjust the number of objects created in 'test.html' to balance the repro rate with the time per run. You can also trigger this (at a reduced rate) by putting an external script that spins for 10-20ms between the write / read instead of creating objects. This seems to affect 10, TP, and Nightly.
Comment 1 Russell Bicknell 2017-05-24 20:36:19 PDT
Created attachment 311187 [details]
test.html
Comment 2 Russell Bicknell 2017-05-24 20:36:37 PDT
Created attachment 311188 [details]
index.html
Comment 3 Russell Bicknell 2017-05-24 20:38:54 PDT
Comment on attachment 311187 [details]
test.html

<script>
  // Uncomment this section to prevent the bug.
  /*
  Object.defineProperty(window, 'customElements', {
    value: window.customElements,
  });
  */

  window.customElements.prop = 1234;

  const a = [];
  for (let i = 0; i < 1000000; i++) {
    a.push({});
  }

  const pass = window.customElements.prop === 1234;
  window.parent.postMessage(pass, '*');
</script>
Comment 4 Russell Bicknell 2017-05-24 20:39:27 PDT
Created attachment 311189 [details]
test.html
Comment 5 Russell Bicknell 2017-05-24 20:46:31 PDT
This seems like it's probably related to 171567.
Comment 6 Radar WebKit Bug Importer 2017-05-27 00:13:35 PDT
<rdar://problem/32440668>
Comment 7 Ryosuke Niwa 2017-05-27 00:22:43 PDT
(In reply to Russell Bicknell from comment #5)
> This seems like it's probably related to 171567.

It's not.
Comment 8 Ryosuke Niwa 2018-08-03 21:13:14 PDT
Created attachment 346581 [details]
Fixes the bug
Comment 9 Ryosuke Niwa 2018-08-04 02:02:46 PDT
Committed r234578: <https://trac.webkit.org/changeset/234578>
Comment 10 Darin Adler 2018-08-05 15:15:04 PDT
Comment on attachment 346581 [details]
Fixes the bug

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

> Source/WebCore/dom/CustomElementRegistry.idl:29
>      EnabledAtRuntime=CustomElements,
> -    ImplementationLacksVTable,
>      JSGenerateToNativeObject,
> +    GenerateIsReachable=ImplScriptExecutionContext

We’ve been sorting these alphabetically and putting commas on every line. So this should be a line higher and have a comma.
Comment 11 Ryosuke Niwa 2018-08-05 17:45:14 PDT
Sure. Fixed it in r234585.
Comment 12 Truitt Savell 2018-08-06 09:38:20 PDT
Looks like the new test fast/custom-elements/custom-element-registry-wrapper-should-stay-alive.html

is a constant timeout on all debug platforms. 

Test History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fcustom-elements%2Fcustom-element-registry-wrapper-should-stay-alive.html
Comment 13 Ryosuke Niwa 2018-08-06 10:20:41 PDT
Yeah, noticed. Will look into it today.
Comment 14 Ryosuke Niwa 2018-08-06 13:30:26 PDT
The test failure is tracked by https://bugs.webkit.org/show_bug.cgi?id=188353.