Changeset 12486 in webkit

Timestamp:

Jan 30, 2006 6:52:49 PM (18 years ago)

Author:

ggaren

Message:

Reviewed by mjs.

• Speculative fix for <rdar://problem/4135845> Crash executing cross-frame script on timeout in KJS::ScheduledAction::execute

If we ever get a reproducible case of 4135845, I'll add a test for it.

This is a re-working of Maciej's fix for 3157014 (circa 2003!). Since
you can't reliably predict what the state of the page will be when
a timer fires, I've made the timer responsbile for making sure that
everything is OK to execute.

I tested @ ​http://www.javascriptkit.com/script/cut3.shtml with various
combinations of reload, back, and regular navigations with JS enabled/
disabled to ensure that the previous crash didn't return. I also ran a
leaks test and discovered some, but none unique to this patch. (See
<rdar://problem/4427420> TOT REGRESSION: Leaks seen on page with
JavaScript timer.)

• khtml/ecma/kjs_window.cpp: (KJS::ScheduledAction::execute): Return early if there's no window object. (This happens when JavaScript is disabled.) (KJS::Window::retrieveWindow): Reversed a backwards ASSERT, increased prettiness. (The assert fired while I was testing. Not sure why we haven't seen it before.)
• page/Frame.cpp: (Frame::didOpenURL): Returned setting of JavaScript enabled/disabled preference to its rightful place. This introduces a new behavior: now, the unload event does not fire after you've disabled JavaScript. That seems like a good thing. (See <rdar://problem/4426506> Disabling JavaScript should immediately end JavaScript execution.) (Frame::begin): Ditto.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• khtml/ecma/kjs_window.cpp (modified) (2 diffs)
• khtml/ecma/kjs_window.h (modified) (1 diff)
• page/Frame.cpp (modified) (2 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2006-01-30  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by mjs.
+
+        - Speculative fix for <rdar://problem/4135845> Crash executing 
+        cross-frame script on timeout in KJS::ScheduledAction::execute
+
+        If we ever get a reproducible case of 4135845, I'll add a test for it.
+
+        This is a re-working of Maciej's fix for 3157014 (circa 2003!). Since
+        you can't reliably predict what the state of the page will be when
+        a timer fires, I've made the timer responsbile for making sure that
+        everything is OK to execute.
+
+        I tested @ http://www.javascriptkit.com/script/cut3.shtml with various
+        combinations of reload, back, and regular navigations with JS enabled/
+        disabled to ensure that the previous crash didn't return. I also ran a 
+        leaks test and discovered some, but none unique to this patch. (See 
+        <rdar://problem/4427420> TOT REGRESSION: Leaks seen on page with 
+        JavaScript timer.)
+
+        * khtml/ecma/kjs_window.cpp:
+        (KJS::ScheduledAction::execute): Return early if there's no window
+        object. (This happens when JavaScript is disabled.)
+        (KJS::Window::retrieveWindow): Reversed a backwards ASSERT, increased
+        prettiness. (The assert fired while I was testing. Not sure why we
+        haven't seen it before.)
+        * page/Frame.cpp:
+        (Frame::didOpenURL): Returned setting of JavaScript enabled/disabled
+        preference to its rightful place. This introduces a new behavior: now,
+        the unload event does not fire after you've disabled JavaScript. That 
+        seems like a good thing. (See <rdar://problem/4426506> Disabling 
+        JavaScript should immediately end JavaScript execution.)
+        (Frame::begin): Ditto.
+
 2006-01-30  Geoffrey Garen  <ggaren@apple.com>
 

trunk/WebCore/khtml/ecma/kjs_window.cpp

@@ -331 +331 @@
 }
 
-Window *Window::retrieveWindow(Frame *p)
-{
-  JSObject *obj = retrieve(p)->getObject();
-  // obj should never be null, except when javascript has been disabled in that frame.
-  ASSERT(obj || (p && p->jScriptEnabled()));
-  if (!obj) // JS disabled
-    return 0;
-  return static_cast<Window*>(obj);
+Window *Window::retrieveWindow(Frame *f)
+{
+    JSObject *o = retrieve(f)->getObject();
+
+    ASSERT(o || !f->jScriptEnabled());
+    return static_cast<Window *>(o);
 }
 
@@ -1836 +1834 @@
 void ScheduledAction::execute(Window *window)
 {
-    if (!window->m_frame)
+    if (!window->m_frame || !window->m_frame->jScript())
         return;
 
-    ScriptInterpreter *interpreter = window->interpreter();
+    ScriptInterpreter *interpreter = window->m_frame->jScript()->interpreter();
 
     interpreter->setProcessingTimerCallback(true);

trunk/WebCore/khtml/ecma/kjs_window.h

@@ -121 +121 @@
      * bindings.
      */
-    static JSValue *retrieve(Frame *p);
+    static JSValue *retrieve(Frame *);
     /**
      * Returns the Window object for a given HTML frame
      */
-    static Window *retrieveWindow(Frame *p);
+    static Window *retrieveWindow(Frame *);
     /**
      * returns a pointer to the Window object this javascript interpreting instance

trunk/WebCore/page/Frame.cpp

@@ -317 +317 @@
   }
 
-  // set the javascript flags according to the current url
+  d->m_bJScriptEnabled = d->m_settings->isJavaScriptEnabled(url.host());
   d->m_bJavaEnabled = d->m_settings->isJavaEnabled(url.host());
   d->m_bPluginsEnabled = d->m_settings->isPluginsEnabled(url.host());
@@ -755 +755 @@
 
   clear();
-
   partClearedInBegin();
-
-  // Only do this after clearing the part, so that JavaScript can
-  // clean up properly if it was on for the last load.
-  d->m_bJScriptEnabled = d->m_settings->isJavaScriptEnabled(url.host());
 
   d->m_bCleared = false;