Changeset 26074 in webkit

Timestamp:

Oct 5, 2007 5:54:00 PM (17 years ago)

Author:

ggaren

Message:

JavaScriptCore:

Reviewed by Sam Weinig.

Added JSObject::removeDirect, to support the fix for
<rdar://problem/5522487> REGRESSION: With JavaScript disabled, any
page load causes a crash in PropertyMap::put

• kjs/object.cpp: (KJS::JSObject::removeDirect):
• kjs/object.h:

WebCore:

Reviewed by Sam Weinig.

New fix for <rdar://problem/5522487> REGRESSION: With JavaScript
disabled, any page load causes a crash in PropertyMap::put

Explicitly remove the "document" property from the window. The old
solution would leave a stale "document" property around after JavaScript
was re-enabled.

The architecture for disabling JavaScript could use some consolidation.
It seems wrong that a script proxy even exists when JavaScript is
disabled. It also seems wrong that so many individual call sites are
responsible for checking whether JavaScript is enabled. I've filed a
bug about this: ​http://bugs.webkit.org/show_bug.cgi?id=15385.

• bindings/js/kjs_proxy.cpp: (WebCore::KJSProxy::clearDocumentWrapper):
• bindings/js/kjs_proxy.h:
• page/Frame.cpp: (WebCore::Frame::setDocument):

Location:

trunk

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/JavaScriptCore.exp (modified) (5 diffs)
• JavaScriptCore/kjs/object.cpp (modified) (1 diff)
• JavaScriptCore/kjs/object.h (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/bindings/js/kjs_proxy.cpp (modified) (1 diff)
• WebCore/bindings/js/kjs_proxy.h (modified) (1 diff)
• WebCore/page/Frame.cpp (modified) (1 diff)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2007-10-05  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Sam Weinig.
+        
+        Added JSObject::removeDirect, to support the fix for 
+        <rdar://problem/5522487> REGRESSION: With JavaScript disabled, any 
+        page load causes a crash in PropertyMap::put
+
+        * kjs/object.cpp:
+        (KJS::JSObject::removeDirect):
+        * kjs/object.h:
+
 2007-10-04  Mark Rowe  <mrowe@apple.com>
 

trunk/JavaScriptCore/JavaScriptCore.exp

@@ -1 @@
-_jscore_collector_introspection
-_jscore_fastmalloc_introspection
 _JSCheckScriptSyntax
 _JSClassCreate
@@ -143 +141 @@
 __ZN3KJS13SavedBuiltinsC1Ev
 __ZN3KJS13SavedBuiltinsD1Ev
+__ZN3KJS13jsOwnedStringERKNS_7UStringE
 __ZN3KJS14StringInstance14deletePropertyEPNS_9ExecStateERKNS_10IdentifierE
 __ZN3KJS14StringInstance16getPropertyNamesEPNS_9ExecStateERNS_17PropertyNameArrayE
@@ -198 +197 @@
 __ZN3KJS8DebuggerD2Ev
 __ZN3KJS8JSObject11hasInstanceEPNS_9ExecStateEPNS_7JSValueE
+__ZN3KJS8JSObject12removeDirectERKNS_10IdentifierE
 __ZN3KJS8JSObject14callAsFunctionEPNS_9ExecStateEPS0_RKNS_4ListE
 __ZN3KJS8JSObject14deletePropertyEPNS_9ExecStateERKNS_10IdentifierE
@@ -214 +214 @@
 __ZN3KJS8jsStringEPKc
 __ZN3KJS8jsStringERKNS_7UStringE
-__ZN3KJS13jsOwnedStringERKNS_7UStringE
 __ZN3KJS9Collector15numInterpretersEv
 __ZN3KJS9Collector15recordExtraCostEm
@@ -283 +282 @@
 __ZTVN3KJS19InternalFunctionImpE
 __ZTVN3KJS8JSObjectE
+_jscore_collector_introspection
+_jscore_fastmalloc_introspection
 _kJSClassDefinitionEmpty
 _kjs_pcre_compile

trunk/JavaScriptCore/kjs/object.cpp

@@ -559 +559 @@
 }
 
+void JSObject::removeDirect(const Identifier &propertyName)
+{
+    _prop.remove(propertyName);
+}
+
 void JSObject::putDirectFunction(InternalFunctionImp* func, int attr)
 {

trunk/JavaScriptCore/kjs/object.h

@@ -436 +436 @@
     void putDirect(const Identifier &propertyName, JSValue *value, int attr = 0);
     void putDirect(const Identifier &propertyName, int value, int attr = 0);
-
+    void removeDirect(const Identifier &propertyName);
+    
     // convenience to add a function property under the function's own built-in name
     void putDirectFunction(InternalFunctionImp*, int attr = 0);

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-10-05  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Sam Weinig.
+        
+        New fix for <rdar://problem/5522487> REGRESSION: With JavaScript 
+        disabled, any page load causes a crash in PropertyMap::put
+        
+        Explicitly remove the "document" property from the window. The old 
+        solution would leave a stale "document" property around after JavaScript
+        was re-enabled.
+
+        The architecture for disabling JavaScript could use some consolidation. 
+        It seems wrong that a script proxy even exists when JavaScript is 
+        disabled. It also seems wrong that so many individual call sites are 
+        responsible for checking whether JavaScript is enabled. I've filed a 
+        bug about this: http://bugs.webkit.org/show_bug.cgi?id=15385.
+
+        * bindings/js/kjs_proxy.cpp:
+        (WebCore::KJSProxy::clearDocumentWrapper):
+        * bindings/js/kjs_proxy.h:
+        * page/Frame.cpp:
+        (WebCore::Frame::setDocument):
+
 2007-10-05  Jon Honeycutt  <jhoneycutt@apple.com>
 

trunk/WebCore/bindings/js/kjs_proxy.cpp

@@ -168 +168 @@
 }
     
-void KJSProxy::updateDocumentWrapper() 
+void KJSProxy::clearDocumentWrapper() 
 {
-    Settings* settings = m_frame->settings();
-    if (!settings || !settings->isJavaScriptEnabled())
-        return;
-
-    if (!m_script || !m_frame->document())
+    if (!m_script)
         return;
 
     JSLock lock;
-    // this will update 'document' property to point to the current document
-    toJS(m_script->globalExec(), m_frame->document());
+    m_script->globalObject()->removeDirect("document");
 }
 

trunk/WebCore/bindings/js/kjs_proxy.h

@@ -56 +56 @@
     bool haveInterpreter() const { return m_script; }
     
-    void updateDocumentWrapper();
+    void clearDocumentWrapper();
 
 private:

trunk/WebCore/page/Frame.cpp

@@ -281 +281 @@
         d->m_doc->attach();
     
-    if (d->m_jscript && d->m_doc)
-        d->m_jscript->updateDocumentWrapper();
+    // Remove the cached 'document' property, which is now stale.
+    if (d->m_jscript)
+        d->m_jscript->clearDocumentWrapper();
 }