Changeset 18399 in webkit

Timestamp:

Dec 22, 2006 4:19:06 PM (17 years ago)

Author:

ggaren

Message:

LayoutTests:

Reviewed by Brady Eidson.

Test for <rdar://problem/4871518> Leopard9A321: Crash visiting www.audible.com
(WebCore::FrameLoader::loadSubframe)

Tests an onload handler for a synchronously loaded iframe removing the iframe
from the document.

• fast/frames/onload-remove-iframe-crash-expected.txt: Added.
• fast/frames/onload-remove-iframe-crash.html: Added.

WebCore:

Reviewed by Brady Eidson.

Fixed <rdar://problem/4871518> Leopard9A321: Crash visiting www.audible.com
(WebCore::FrameLoader::loadSubframe)

• loader/mac/FrameLoaderMac.mm: (WebCore::FrameLoader::createFrame): Updated to reflect the fact that createChildFrameNamed: now returns a WebCore::Frame* instead of a WebCoreFrameBridge *.
• page/mac/WebCoreFrameBridge.h:

WebKit:

Reviewed by Brady Eidson.

Fixed <rdar://problem/4871518> Leopard9A321: Crash visiting www.audible.com
(WebCore::FrameLoader::loadSubframe)

• WebCoreSupport/WebFrameBridge.mm: (-[WebFrameBridge createChildFrameNamed:withURL:referrer:ownerElement:allowsScrolling:marginWidth:marginHeight:]):
• The fix:

Changed to re-fetch the child frame we're trying to load before returning it,
since its onload handler may have removed it from the document. This
allows us to treat a removed frame like a frame that never loaded.

• Plus some cleanup:
  • Changed to return a WebCore::Frame* instead of a WebFrameBridge *, to simplify some code.
  • Grouped ObjC objects by usage, and moved calls to -release so that they immediately follow the calls that retain.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/frames/onload-remove-iframe-crash-expected.txt (added)
• LayoutTests/fast/frames/onload-remove-iframe-crash.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/loader/mac/FrameLoaderMac.mm (modified) (1 diff)
• WebCore/page/mac/WebCoreFrameBridge.h (modified) (1 diff)
• WebKit/ChangeLog (modified) (1 diff)
• WebKit/WebCoreSupport/WebFrameBridge.mm (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2006-12-22  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Brady Eidson.
+        
+        Test for <rdar://problem/4871518> Leopard9A321: Crash visiting www.audible.com 
+        (WebCore::FrameLoader::loadSubframe)
+        
+        Tests an onload handler for a synchronously loaded iframe removing the iframe 
+        from the document.
+
+        * fast/frames/onload-remove-iframe-crash-expected.txt: Added.
+        * fast/frames/onload-remove-iframe-crash.html: Added.
+
 2006-12-22  Nikolas Zimmermann  <zimmermann@kde.org>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2006-12-22  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Brady Eidson.
+        
+        Fixed <rdar://problem/4871518> Leopard9A321: Crash visiting www.audible.com 
+        (WebCore::FrameLoader::loadSubframe)
+
+        * loader/mac/FrameLoaderMac.mm:
+        (WebCore::FrameLoader::createFrame): Updated to reflect the fact that
+        createChildFrameNamed: now returns a WebCore::Frame* instead of a 
+        WebCoreFrameBridge *.
+        * page/mac/WebCoreFrameBridge.h:
+
 2006-12-22  David Hyatt  <hyatt@apple.com>
 

trunk/WebCore/loader/mac/FrameLoaderMac.mm

@@ -1259 +1259 @@
     BEGIN_BLOCK_OBJC_EXCEPTIONS;
     
-    return [[Mac(m_frame)->bridge() createChildFrameNamed:name
-                                                             withURL:url.getNSURL()
-                                                            referrer:referrer 
-                                                          ownerElement:ownerElement
-                                                     allowsScrolling:allowsScrolling
-                                                         marginWidth:marginWidth
-                                                        marginHeight:marginHeight] _frame];
+    return [Mac(m_frame)->bridge() createChildFrameNamed:name
+                                                 withURL:url.getNSURL()
+                                                referrer:referrer 
+                                              ownerElement:ownerElement
+                                         allowsScrolling:allowsScrolling
+                                             marginWidth:marginWidth
+                                            marginHeight:marginHeight];
 
     END_BLOCK_OBJC_EXCEPTIONS;

trunk/WebCore/page/mac/WebCoreFrameBridge.h

@@ -279 +279 @@
 - (void)setStatusText:(NSString *)status;
 
-- (WebCoreFrameBridge *)createChildFrameNamed:(NSString *)frameName withURL:(NSURL *)URL referrer:(const WebCore::String&)referrer
+- (WebCore::Frame*)createChildFrameNamed:(NSString *)frameName withURL:(NSURL *)URL referrer:(const WebCore::String&)referrer
     ownerElement:(WebCore::HTMLFrameOwnerElement *)ownerElement allowsScrolling:(BOOL)allowsScrolling marginWidth:(int)width marginHeight:(int)height;
 

trunk/WebKit/ChangeLog

@@ +1 @@
+2006-12-22  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Brady Eidson.
+        
+        Fixed <rdar://problem/4871518> Leopard9A321: Crash visiting www.audible.com 
+        (WebCore::FrameLoader::loadSubframe)
+
+        * WebCoreSupport/WebFrameBridge.mm:
+        (-[WebFrameBridge createChildFrameNamed:withURL:referrer:ownerElement:allowsScrolling:marginWidth:marginHeight:]):
+        - The fix: 
+            Changed to re-fetch the child frame we're trying to load before returning it,
+            since its onload handler may have removed it from the document. This
+            allows us to treat a removed frame like a frame that never loaded.
+        
+        - Plus some cleanup:
+            - Changed to return a WebCore::Frame* instead of a WebFrameBridge *,
+            to simplify some code.
+            - Grouped ObjC objects by usage, and moved calls to -release so that they
+            immediately follow the calls that retain.
+
 2006-12-21  John Sullivan  <sullivan@apple.com>
 

trunk/WebKit/WebCoreSupport/WebFrameBridge.mm

@@ -380 +380 @@
 }
 
-- (WebCoreFrameBridge *)createChildFrameNamed:(NSString *)frameName 
+- (Frame*)createChildFrameNamed:(NSString *)frameName 
                                       withURL:(NSURL *)URL
                                      referrer:(const String&)referrer
@@ -390 +390 @@
     bool hideReferrer;
     if (!m_frame->loader()->canLoad(URL, referrer, hideReferrer))
-        return nil;
+        return 0;
 
     ASSERT(_frame);
@@ -396 +396 @@
     WebFrameView *childView = [[WebFrameView alloc] initWithFrame:NSMakeRect(0,0,0,0)];
     [childView setAllowsScrolling:allowsScrolling];
-    WebFrameBridge *newBridge = [[WebFrameBridge alloc] initSubframeWithOwnerElement:ownerElement frameName:frameName frameView:childView];
-    [_frame _addChild:[newBridge webFrame]];
-    [childView release];
-
     [childView _setMarginWidth:width];
     [childView _setMarginHeight:height];
 
+    WebFrameBridge *newBridge = [[WebFrameBridge alloc] initSubframeWithOwnerElement:ownerElement frameName:frameName frameView:childView];
+    [childView release];
+
+    if (!newBridge)
+        return 0;
+
+    [_frame _addChild:[newBridge webFrame]];
     [newBridge release];
 
-    if (!newBridge)
-        return nil;
-
     [_frame _loadURL:URL referrer:(hideReferrer ? String() : referrer) intoChild:[newBridge webFrame]];
 
-    return newBridge;
+    // Re-fetch the child frame, since its onload handler may have removed it from the document.
+    return m_frame->tree()->child(frameName);
 }