Changeset 53277 in webkit

Timestamp:

Jan 14, 2010 11:29:51 AM (14 years ago)

Author:

Adam Roben

Message:

Treat all synchronous loads equally in FrameLoader::loadSubframe

Only loads of the empty URL or about:blank were being treated as
synchronous loads. But other loads can be synchronous (e.g., when we
receive a null ResourceRequest from requestFromDelegate or when a
policy decision of "ignore" is made). We now treat those loads the
same way we treated empty URLs and about:blank.

Fixes <rdar://problem/7533333> <​http://webkit.org/b/33533>
window.onload never fires if page contains an <iframe> with a bad
scheme or whose load is cancelled by returning null from resource load
delegate's willSendRequest

Tests: fast/loader/onload-bad-scheme-for-frame.html

fast/loader/onload-policy-ignore-for-frame.html
fast/loader/onload-willSendRequest-null-for-frame.html

Reviewed by Brady Eidson.

WebCore:

• loader/FrameLoader.cpp:

(WebCore::FrameLoader::loadSubframe):

• Detect synchronous loads by checking the subframe's loader's state, rather than by checking its URL
• Removed unnecessary call to completed(), since checkCompleted() will call completed() if needed (the call to completed() was added first and wasn't removed when the call to checkCompleted() was added in r8316)
• Added more comments about the strange thing this function does with the subframe's loader

WebKitTools:

Add LayoutTestController API to force
-webView:resource:willSendRequest:: to return null

• DumpRenderTree/LayoutTestController.cpp:

(LayoutTestController::LayoutTestController): Initialize new member.
(setWillSendRequestReturnsNullCallback): Call through to
LayoutTestController.
(LayoutTestController::staticFunctions): Added new function.

• DumpRenderTree/LayoutTestController.h: Added

m_willSendRequestReturnsNull.
(LayoutTestController::willSendRequestReturnsNull):
(LayoutTestController::setWillSendRequestReturnsNull):
Added standard accessors.

• DumpRenderTree/mac/ResourceLoadDelegate.mm:

(-[ResourceLoadDelegate webView:resource:willSendRequest:redirectResponse:fromDataSource:]):

• DumpRenderTree/win/ResourceLoadDelegate.cpp:

(ResourceLoadDelegate::willSendRequest):
Return null if LayoutTestController says to.

LayoutTests:

Add tests that show that onload still fires in various situations with
subframes

• fast/loader/onload-bad-scheme-for-frame-expected.txt: Added.
• fast/loader/onload-bad-scheme-for-frame.html: Added.

Tests that onload still fires when a subframe with an unrecognized
scheme is in the page.

• fast/loader/onload-policy-ignore-for-frame-expected.txt: Added.
• fast/loader/onload-policy-ignore-for-frame.html: Added.

Tests that onload still fires when the policy delegate says to ignore
the initial load of a subframe.

• fast/loader/onload-willSendRequest-null-for-frame-expected.txt: Added.
• fast/loader/onload-willSendRequest-null-for-frame.html: Added.

Tests that onload still fires when the resource load delegate returns
null from willSendRequest for the initial load of a subframe.

• http/tests/loading/bad-scheme-subframe-expected.txt: Updated result

to show that we now fire onload for the main frame in this case. (This
is essentially the same as onload-bad-scheme-for-frame.html, but it
still seems useful to have that new, more explicit, test.)

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/loader/onload-bad-scheme-for-frame-expected.txt (added)
• LayoutTests/fast/loader/onload-bad-scheme-for-frame.html (added)
• LayoutTests/fast/loader/onload-policy-ignore-for-frame-expected.txt (added)
• LayoutTests/fast/loader/onload-policy-ignore-for-frame.html (added)
• LayoutTests/fast/loader/onload-willSendRequest-null-for-frame-expected.txt (added)
• LayoutTests/fast/loader/onload-willSendRequest-null-for-frame.html (added)
• LayoutTests/http/tests/loading/bad-scheme-subframe-expected.txt (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/loader/FrameLoader.cpp (modified) (2 diffs)
• WebKitTools/ChangeLog (modified) (1 diff)
• WebKitTools/DumpRenderTree/LayoutTestController.cpp (modified) (3 diffs)
• WebKitTools/DumpRenderTree/LayoutTestController.h (modified) (2 diffs)
• WebKitTools/DumpRenderTree/mac/ResourceLoadDelegate.mm (modified) (1 diff)
• WebKitTools/DumpRenderTree/win/ResourceLoadDelegate.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ -8 +8 @@
         * fast/frames/cached-frame-counter-expected.txt: Added.
         * fast/frames/cached-frame-counter.html: Added.
+
+2010-01-14  Adam Roben  <aroben@apple.com>
+
+        Add tests that show that onload still fires in various situations with
+        subframes
+
+        Tests for <rdar://problem/7533333> <http://webkit.org/b/33533>
+        window.onload never fires if page contains an <iframe> with a bad
+        scheme or whose load is cancelled by returning null from resource load
+        delegate's willSendRequest
+
+        Reviewed by Brady Eidson.
+
+        * fast/loader/onload-bad-scheme-for-frame-expected.txt: Added.
+        * fast/loader/onload-bad-scheme-for-frame.html: Added.
+        Tests that onload still fires when a subframe with an unrecognized
+        scheme is in the page.
+
+        * fast/loader/onload-policy-ignore-for-frame-expected.txt: Added.
+        * fast/loader/onload-policy-ignore-for-frame.html: Added.
+        Tests that onload still fires when the policy delegate says to ignore
+        the initial load of a subframe.
+
+        * fast/loader/onload-willSendRequest-null-for-frame-expected.txt: Added.
+        * fast/loader/onload-willSendRequest-null-for-frame.html: Added.
+        Tests that onload still fires when the resource load delegate returns
+        null from willSendRequest for the initial load of a subframe.
+
+        * http/tests/loading/bad-scheme-subframe-expected.txt: Updated result
+        to show that we now fire onload for the main frame in this case. (This
+        is essentially the same as onload-bad-scheme-for-frame.html, but it
+        still seems useful to have that new, more explicit, test.)
 
 2010-01-14  Adam Roben  <aroben@apple.com>

trunk/LayoutTests/http/tests/loading/bad-scheme-subframe-expected.txt

@@ -2 +2 @@
 main frame - didCommitLoadForFrame
 main frame - didFinishDocumentLoadForFrame
+main frame - didHandleOnloadEventsForFrame
 main frame - didFinishLoadForFrame
 This is a test of load callbacks. It is only useful inside the regression test tool.

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-01-14  Adam Roben  <aroben@apple.com>
+
+        Treat all synchronous loads equally in FrameLoader::loadSubframe
+
+        Only loads of the empty URL or about:blank were being treated as
+        synchronous loads. But other loads can be synchronous (e.g., when we
+        receive a null ResourceRequest from requestFromDelegate or when a
+        policy decision of "ignore" is made). We now treat those loads the
+        same way we treated empty URLs and about:blank.
+
+        Fixes <rdar://problem/7533333> <http://webkit.org/b/33533>
+        window.onload never fires if page contains an <iframe> with a bad
+        scheme or whose load is cancelled by returning null from resource load
+        delegate's willSendRequest
+
+        Tests: fast/loader/onload-bad-scheme-for-frame.html
+               fast/loader/onload-policy-ignore-for-frame.html
+               fast/loader/onload-willSendRequest-null-for-frame.html
+
+        Reviewed by Brady Eidson.
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::loadSubframe):
+          - Detect synchronous loads by checking the subframe's loader's
+            state, rather than by checking its URL
+          - Removed unnecessary call to completed(), since checkCompleted()
+            will call completed() if needed (the call to completed() was added
+            first and wasn't removed when the call to checkCompleted() was
+            added in r8316)
+          - Added more comments about the strange thing this function does
+            with the subframe's loader
+
 2010-01-14  Diego Gonzalez  <diego.gonzalez@openbossa.org>
 

trunk/WebCore/loader/FrameLoader.cpp

@@ -404 +404 @@
     }
     
+    // All new frames will have m_isComplete set to true at this point due to synchronously loading
+    // an empty document in FrameLoader::init(). But many frames will now be starting an
+    // asynchronous load of url, so we set m_isComplete to false and then check if the load is
+    // actually completed below. (Note that we set m_isComplete to false even for synchronous
+    // loads, so that checkCompleted() below won't bail early.)
+    // FIXME: Can we remove this entirely? m_isComplete normally gets set to false when a load is committed.
     frame->loader()->m_isComplete = false;
    
@@ -413 +419 @@
     checkCallImplicitClose();
     
+    // Some loads are performed synchronously (e.g., about:blank and loads
+    // cancelled by returning a null ResourceRequest from requestFromDelegate).
     // In these cases, the synchronous load would have finished
     // before we could connect the signals, so make sure to send the 
-    // completed() signal for the child by hand
+    // completed() signal for the child by hand and mark the load as being
+    // complete.
     // FIXME: In this case the Frame will have finished loading before 
     // it's being added to the child list. It would be a good idea to
     // create the child first, then invoke the loader separately.
-    if (url.isEmpty() || url == blankURL()) {
-        frame->loader()->completed();
+    if (frame->loader()->state() == FrameStateComplete)
         frame->loader()->checkCompleted();
-    }
 
     return frame.get();

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-01-14  Adam Roben  <aroben@apple.com>
+
+        Add LayoutTestController API to force
+        -webView:resource:willSendRequest:: to return null
+
+        Enables tests for <rdar://problem/7533333> <http://webkit.org/b/33533>
+        window.onload never fires if page contains an <iframe> with a bad
+        scheme or whose load is cancelled by returning null from resource load
+        delegate's willSendRequest
+
+        Reviewed by Brady Eidson.
+
+        * DumpRenderTree/LayoutTestController.cpp:
+        (LayoutTestController::LayoutTestController): Initialize new member.
+        (setWillSendRequestReturnsNullCallback): Call through to
+        LayoutTestController.
+        (LayoutTestController::staticFunctions): Added new function.
+
+        * DumpRenderTree/LayoutTestController.h: Added
+        m_willSendRequestReturnsNull.
+        (LayoutTestController::willSendRequestReturnsNull):
+        (LayoutTestController::setWillSendRequestReturnsNull):
+        Added standard accessors.
+
+        * DumpRenderTree/mac/ResourceLoadDelegate.mm:
+        (-[ResourceLoadDelegate webView:resource:willSendRequest:redirectResponse:fromDataSource:]):
+        * DumpRenderTree/win/ResourceLoadDelegate.cpp:
+        (ResourceLoadDelegate::willSendRequest):
+        Return null if LayoutTestController says to.
+
 2010-01-14  Kevin Ollivier  <kevino@theolliviers.com>
 

trunk/WebKitTools/DumpRenderTree/LayoutTestController.cpp

@@ -67 +67 @@
     , m_testRepaintSweepHorizontally(false)
     , m_waitToDump(false)
+    , m_willSendRequestReturnsNull(false)
     , m_willSendRequestReturnsNullOnRedirect(false)
     , m_windowIsKey(true)
@@ -908 +909 @@
     LayoutTestController* controller = static_cast<LayoutTestController*>(JSObjectGetPrivate(thisObject));
     controller->setUserStyleSheetLocation(path.get());
+
+    return JSValueMakeUndefined(context);
+}
+
+static JSValueRef setWillSendRequestReturnsNullCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
+{
+    // Has cross-platform implementation
+    if (argumentCount < 1)
+        return JSValueMakeUndefined(context);
+
+    LayoutTestController* controller = static_cast<LayoutTestController*>(JSObjectGetPrivate(thisObject));
+    controller->setWillSendRequestReturnsNull(JSValueToBoolean(context, arguments[0]));
 
     return JSValueMakeUndefined(context);
@@ -1308 +1321 @@
         { "setUserStyleSheetEnabled", setUserStyleSheetEnabledCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "setUserStyleSheetLocation", setUserStyleSheetLocationCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
+        { "setWillSendRequestReturnsNull", setWillSendRequestReturnsNullCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "setWillSendRequestReturnsNullOnRedirect", setWillSendRequestReturnsNullOnRedirectCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "setWindowIsKey", setWindowIsKeyCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },

trunk/WebKitTools/DumpRenderTree/LayoutTestController.h

@@ -177 +177 @@
     void waitToDumpWatchdogTimerFired();
 
+    bool willSendRequestReturnsNull() const { return m_willSendRequestReturnsNull; }
+    void setWillSendRequestReturnsNull(bool returnsNull) { m_willSendRequestReturnsNull = returnsNull; }
+
     bool willSendRequestReturnsNullOnRedirect() const { return m_willSendRequestReturnsNullOnRedirect; }
     void setWillSendRequestReturnsNullOnRedirect(bool returnsNull) { m_willSendRequestReturnsNullOnRedirect = returnsNull; }
@@ -250 +253 @@
     bool m_testRepaintSweepHorizontally;
     bool m_waitToDump; // True if waitUntilDone() has been called, but notifyDone() has not yet been called.
+    bool m_willSendRequestReturnsNull;
     bool m_willSendRequestReturnsNullOnRedirect;
     bool m_windowIsKey;

trunk/WebKitTools/DumpRenderTree/mac/ResourceLoadDelegate.mm

@@ -130 +130 @@
     }
 
+    if (!done && gLayoutTestController->willSendRequestReturnsNull())
+        return nil;
+
     if (!done && gLayoutTestController->willSendRequestReturnsNullOnRedirect() && redirectResponse) {
         printf("Returning null for this redirect\n");

trunk/WebKitTools/DumpRenderTree/win/ResourceLoadDelegate.cpp

@@ -244 +244 @@
     }
 
+    if (!done && gLayoutTestController->willSendRequestReturnsNull()) {
+        *newRequest = 0;
+        return S_OK;
+    }
+
     if (!done && gLayoutTestController->willSendRequestReturnsNullOnRedirect() && redirectResponse) {
         printf("Returning null for this redirect\n");