Changeset 13568 in webkit for trunk/JavaScriptCore/kjs

Timestamp:

Mar 29, 2006, 6:39:24 PM (19 years ago)

Author:

ggaren

Message:

JavaScriptCore:

Reviewed by Darin.

• JavaScriptCore side of fix for <rdar://problem/4308243> 8F36 Regression: crash in malloc_consolidate if you use a .PAC file

The crash was a result of threaded deallocation of thread-unsafe
objects. Pure JS objects are thread-safe because all JS execution
is synchronized through JSLock. However, JS objects that wrap WebCore
objects are thread-unsafe because JS and WebCore execution are not
synchronized. That unsafety comes into play when the collector
deallocates a JS object that wraps a WebCore object, thus causing the
WebCore object to be deallocated.

The solution here is to have each JSCell know whether it is safe to
collect on a non-main thread, and to avoid collecting unsafe cells
when on a non-main thread.

We don't have a way to test PAC files yet, so there's no test
attached to this patch.

• kjs/collector.cpp: (KJS::Collector::collect):

(1) Added the test "currentThreadIsMainThread

imp->m_destructorIsThreadSafe".

• kjs/protect.h: (KJS::gcProtectNullTolerant): (KJS::gcUnprotectNullTolerant):
• kjs/value.h: (KJS::JSCell::JSCell): The bools here must be bitfields, otherwise m_destructorIsThreadSafe becomes another whole word, ruining the collector optimizations we've made based on the size of a JSObject.
• kxmlcore/FastMalloc.cpp: (KXMLCore::currentThreadIsMainThread): (KXMLCore::fastMallocRegisterThread):
• kxmlcore/FastMalloc.h:

WebCore:

Reviewed by Hyatt.

• Fix for ​http://bugzilla.opendarwin.org/show_bug.cgi?id=6986 Switch to use new text field implementation for <input type="text">

• css/html4.css: Added default style info for new text fields.
• rendering/RenderTextField.cpp: (WebCore::RenderTextField::createDivStyle): Added an extra 1px of padding on the left & right to match Win IE & the latest Mozilla. (WebCore::RenderTextField::updateFromElement): Removed some outdated comments. Cleaned up the way we add text nodes to the div. (WebCore::RenderTextField::setSelectionStart): Tweaked selection code to better match Mozilla behavior. (WebCore::RenderTextField::setSelectionEnd): ditto. (WebCore::RenderTextField::select): Cleaned this up by having it call setSelectionRange. (WebCore::RenderTextField::setSelectionRange): Calls updateLayout now in case this is called in an onload handler, and no other layout has occurred. (WebCore::RenderTextField::calcMinMaxWidth): Use floatWidth to calculate the width of the "0" character.
• rendering/RenderTheme.cpp: (WebCore::RenderTheme::isControlStyled): If the text field's specified border is different from the default border, then treat the control as styled, so the engine knows to turn off the aqua appearance.
• rendering/RenderThemeMac.mm: (WebCore::RenderThemeMac::paintTextField): return false so the engine knows not to try to draw the border. (WebCore::RenderThemeMac::adjustTextFieldStyle): text field style info has been moved to html4.css. We also add intrinsic margins here if the font size is large enough.
• html/HTMLTextFieldInnerElement.cpp: (WebCore::HTMLTextFieldInnerElement::defaultEventHandler): No longer check for appearance. All text fields with m_type == TEXT will use the new implementation.
• html/HTMLInputElement.cpp: (WebCore::HTMLInputElement::isKeyboardFocusable): ditto. (WebCore::HTMLInputElement::focus): ditto. (WebCore::HTMLInputElement::selectionStart): ditto. (WebCore::HTMLInputElement::selectionEnd): ditto. (WebCore::HTMLInputElement::setSelectionStart): ditto. (WebCore::HTMLInputElement::setSelectionEnd): ditto. (WebCore::HTMLInputElement::select): ditto. (WebCore::HTMLInputElement::setSelectionRange): ditto. (WebCore::HTMLInputElement::createRenderer): ditto. (WebCore::HTMLInputElement::defaultEventHandler): ditto. (WebCore::HTMLInputElement::isMouseFocusable): Added. Old text fields relied on the widget to provide a focus policy. A text field that is focusable should be mouse focusable, and shouldn't need to ask the base class.
• html/HTMLInputElement.h: Added isMouseFocusable.
• html/HTMLGenericFormElement.cpp: (WebCore::HTMLGenericFormElement::isMouseFocusable): Removed specific text field code since that is now done in HTMLInputElement::isMouseFocusable.
• dom/Document.cpp: (WebCore::Document::clearSelectionIfNeeded): Check that the new selection is does not have a shadowAncestorNode that is focused.

Location:

trunk/JavaScriptCore/kjs

Files:

• collector.cpp (modified) (4 diffs)
• object.h (modified) (3 diffs)
• protect.h (modified) (1 diff)
• value.h (modified) (3 diffs)

trunk/JavaScriptCore/kjs/collector.cpp

@@ -444 +444 @@
     InterpreterImp *scr = InterpreterImp::s_hook;
     do {
-      //fprintf( stderr, "Collector marking interpreter %p\n",(void*)scr);
       scr->mark();
       scr = scr->next;
@@ -461 +460 @@
   size_t numLiveObjects = heap.numLiveObjects;
 
+#if USE(MULTIPLE_THREADS)
+  bool currentThreadIsMainThread = !pthread_is_threaded_np() || pthread_main_np();
+#else
+  bool currentThreadIsMainThread = true;
+#endif
+  
   for (size_t block = 0; block < heap.usedBlocks; block++) {
     CollectorBlock *curBlock = heap.blocks[block];
@@ -474 +479 @@
         if (imp->m_marked) {
           imp->m_marked = false;
-        } else {
+        } else if (currentThreadIsMainThread || imp->m_destructorIsThreadSafe) {
           imp->~JSCell();
           --usedCells;
@@ -495 +500 @@
           if (imp->m_marked) {
             imp->m_marked = false;
-          } else {
+          } else if (currentThreadIsMainThread || imp->m_destructorIsThreadSafe) {
             imp->~JSCell();
             --usedCells;

trunk/JavaScriptCore/kjs/object.h

@@ -118 +118 @@
      * @param proto The prototype
      */
-    JSObject(JSObject *proto);
+    JSObject(JSObject *proto, bool destructorIsThreadSafe = true);
 
     /**
@@ -124 +124 @@
      * (that is, the ECMAScript "null" value, not a null object pointer).
      */
-    JSObject();
+    explicit JSObject(bool destructorIsThreadSafe = true);
 
     virtual void mark();
@@ -580 +580 @@
 JSObject *throwError(ExecState *, ErrorType);
 
-inline JSObject::JSObject(JSObject *proto)
-    : _proto(proto), _internalValue(0)
+inline JSObject::JSObject(JSObject *proto, bool destructorIsThreadSafe)
+    : JSCell(destructorIsThreadSafe)
+    , _proto(proto)
+    , _internalValue(0)
 {
     assert(proto);
 }
 
-inline JSObject::JSObject()
-    : _proto(jsNull()), _internalValue(0)
+inline JSObject::JSObject(bool destructorIsThreadSafe)
+    : JSCell(destructorIsThreadSafe)
+    , _proto(jsNull())
+    , _internalValue(0)
 {
 }

trunk/JavaScriptCore/kjs/protect.h

@@ -44 +44 @@
     inline void gcProtectNullTolerant(JSValue *val) 
     {
-        if (val) gcProtect(val);
+        if (val) 
+            gcProtect(val);
     }
 
     inline void gcUnprotectNullTolerant(JSValue *val) 
     {
-        if (val) gcUnprotect(val);
+        if (val) 
+            gcUnprotect(val);
     }
     

trunk/JavaScriptCore/kjs/value.h

@@ -122 +122 @@
     friend class GetterSetterImp;
 private:
-    JSCell();
+    explicit JSCell(bool destructorIsThreadSafe = true);
     virtual ~JSCell();
 public:
@@ -156 +156 @@
 
 private:
-    bool m_marked;
+    bool m_destructorIsThreadSafe : 1;
+    bool m_marked : 1;
 };
 
@@ -209 +210 @@
 }
 
-inline JSCell::JSCell()
-    : m_marked(false)
+inline JSCell::JSCell(bool destructorIsThreadSafe)
+    : m_destructorIsThreadSafe(destructorIsThreadSafe)
+    , m_marked(false)
 {
 }