Bug 148495 – DOM Structure and Constructor caches should be weak

• Status: NEW
• Component: WebCore JavaScript

[Andreas Kling]

I don't see why JSDOMGlobalObject's structures() and constructors() caches couldn't be WeakGCMaps instead of strongly owning HashMaps.

This would allow us to clean up things that are only used once, e.g by generic feature detection scripts.

[Andreas Kling]

Created attachment 259990 [details]
Patch

Let's see what EWS thinks..

[Andreas Kling]

Created attachment 259991 [details]
Patch

Let's see what EWS thinks (about a patch for *this* bug...)

[Build Bot]

Comment on attachment 259991 [details]
Patch

Attachment 259991 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/110859

New failing tests:
fast/workers/constructor-proto.html
fast/dom/gc-12.html
fast/dom/constructors-cached-navigate.html

[Build Bot]

Created attachment 259999 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 259991 [details]
Patch

Attachment 259991 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/110881

New failing tests:
fast/workers/constructor-proto.html
fast/dom/gc-12.html
fast/dom/constructors-cached-navigate.html

[Build Bot]

Created attachment 260000 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Andreas Kling]

Looks like there are two problems here:

- DOM Prototypes need to survive if they have custom properties, since losing those after a GC would be observable.

- Weak constructors seem to break instanceof somehow. I think it may have something to do with DOM constructor objects only setting the butterfly's "prototype" entry to the DOM prototype, but leaving ObjectPrototype in the Structure::storedPrototype() slot. Something's weird about this.

[Andreas Kling]

(In reply to comment #7)
> Looks like there are two problems here:
> 
> - DOM Prototypes need to survive if they have custom properties, since
> losing those after a GC would be observable.
> 
> - Weak constructors seem to break instanceof somehow. I think it may have
> something to do with DOM constructor objects only setting the butterfly's
> "prototype" entry to the DOM prototype, but leaving ObjectPrototype in the
> Structure::storedPrototype() slot. Something's weird about this.

lol all wrong

The real problem here was that DOM prototypes were retrieved through the DOM Structure cache, and when that was made weak, a GC could destroy e.g the Node Structure, making it impossible for the next caller who wanted the Node prototype to find the NodePrototype we had already instantiated.

Gonna break this into two patches so you can see what's going on better.

[Andreas Kling]

Created attachment 263242 [details]
Patch #1: Make DOM Structure and prototype caches weak

[Darin Adler]

Comment on attachment 263242 [details]
Patch #1: Make DOM Structure and prototype caches weak

Windows build failure is:

C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/PassRefPtr.h(42): error C2027: use of undefined type 'WebCore::DOMWrapperWorld' (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\bridge\runtime_root.cpp) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj]
  C:\cygwin\home\buildbot\WebKit\Source\WebCore\bindings\js\JSDOMGlobalObject.h(37): note: see declaration of 'WebCore::DOMWrapperWorld' (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\bridge\runtime_root.cpp)
  C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/RefPtr.h(59): note: see reference to function template instantiation 'void WTF::derefIfNotNull<T>(T *)' being compiled
          with
          [
              T=WebCore::DOMWrapperWorld
          ] (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\bridge\runtime_root.cpp)
  C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/RefPtr.h(59): note: while compiling class template member function 'WTF::RefPtr<WebCore::DOMWrapperWorld>::~RefPtr(void)' (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\bridge\runtime_root.cpp)
  C:\cygwin\home\buildbot\WebKit\Source\WebCore\bindings\js\JSDOMGlobalObject.h(91): note: see reference to function template instantiation 'WTF::RefPtr<WebCore::DOMWrapperWorld>::~RefPtr(void)' being compiled (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\bridge\runtime_root.cpp)
  C:\cygwin\home\buildbot\WebKit\Source\WebCore\bindings\js\JSDOMGlobalObject.h(89): note: see reference to class template instantiation 'WTF::RefPtr<WebCore::DOMWrapperWorld>' being compiled (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\bridge\runtime_root.cpp)
C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/PassRefPtr.h(42): error C2227: left of '->deref' must point to class/struct/union/generic type (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\bridge\runtime_root.cpp) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj]
C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/PassRefPtr.h(41): warning C4390: ';': empty controlled statement found; is this the intent? (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\bridge\runtime_root.cpp) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj]

[Geoffrey Garen]

> > - DOM Prototypes need to survive if they have custom properties, since
> > losing those after a GC would be observable.
> > 
> lol all wrong

But what is the answer to the problem of custom properties on prototypes disappearing?

[Andreas Kling]

(In reply to comment #11)
> > > - DOM Prototypes need to survive if they have custom properties, since
> > > losing those after a GC would be observable.
> > > 
> > lol all wrong
> 
> But what is the answer to the problem of custom properties on prototypes
> disappearing?

... lol even more wrong :|