Changeset 261761 in webkit

Timestamp:

May 15, 2020 2:31:22 PM (4 years ago)

Author:

commit-queue@webkit.org

Message:

Regression (iOS 13.4, r249142): WKWebView.loading is never reset after a Back navigation in an iframe
​https://bugs.webkit.org/show_bug.cgi?id=211449

Patch by Alex Christensen <​achristensen@webkit.org> on 2020-05-15
Reviewed by Geoffrey Garen.

Source/WebKit:

Always call clearPendingAPIRequest in WebPageProxy::didStartProvisionalLoadForFrameShared.
r249142 regressed this and caused more "loading appears to continue forever" bugs fixed in r249890.
This should fix both, by removing the problematic half of r249142.

Covered by an API test.

• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::didStartProvisionalLoadForFrameShared):

Tools:

• TestWebKitAPI/Tests/WebKitCocoa/Navigation.mm:

(-[LoadingObserver changesObserved]):
(-[LoadingObserver observeValueForKeyPath:ofObject:change:context:]):
(TEST):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/WebPageProxy.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/Navigation.mm (modified) (3 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-05-15  Alex Christensen  <achristensen@webkit.org>
+
+        Regression (iOS 13.4, r249142): WKWebView.loading is never reset after a Back navigation in an iframe
+        https://bugs.webkit.org/show_bug.cgi?id=211449
+
+        Reviewed by Geoffrey Garen.
+
+        Always call clearPendingAPIRequest in WebPageProxy::didStartProvisionalLoadForFrameShared.
+        r249142 regressed this and caused more "loading appears to continue forever" bugs fixed in r249890.
+        This should fix both, by removing the problematic half of r249142.
+
+        Covered by an API test.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::didStartProvisionalLoadForFrameShared):
+
 2020-05-15  Adrian Perez de Castro  <aperez@igalia.com>
 

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -4445 +4445 @@
 
     auto transaction = m_pageLoadState.transaction();
-    bool fromAlternateHTMLAPI = !unreachableURL.isEmpty() && unreachableURL == m_pageLoadState.pendingAPIRequestURL();
-    if (navigation || fromAlternateHTMLAPI)
-        m_pageLoadState.clearPendingAPIRequest(transaction);
+    m_pageLoadState.clearPendingAPIRequest(transaction);
 
     if (frame->isMainFrame()) {

trunk/Tools/ChangeLog

@@ +1 @@
+2020-05-15  Alex Christensen  <achristensen@webkit.org>
+
+        Regression (iOS 13.4, r249142): WKWebView.loading is never reset after a Back navigation in an iframe
+        https://bugs.webkit.org/show_bug.cgi?id=211449
+
+        Reviewed by Geoffrey Garen.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/Navigation.mm:
+        (-[LoadingObserver changesObserved]):
+        (-[LoadingObserver observeValueForKeyPath:ofObject:change:context:]):
+        (TEST):
+
 2020-05-15  Chris Dumez  <cdumez@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Navigation.mm

@@ -26 +26 @@
 #import "config.h"
 
+#import "HTTPServer.h"
+#import "PlatformUtilities.h"
+#import "Test.h"
+#import "TestUIDelegate.h"
+#import "TestURLSchemeHandler.h"
 #import <WebKit/WKBackForwardListPrivate.h>
 #import <WebKit/WKNavigationActionPrivate.h>
@@ -34 +39 @@
 #import <wtf/RetainPtr.h>
 #import <wtf/Vector.h>
-#import "PlatformUtilities.h"
-#import "Test.h"
-#import "TestURLSchemeHandler.h"
 
 static bool isDone;
@@ -672 +674 @@
     TestWebKitAPI::Util::run(&isDone);
 }
+
 #endif // PLATFORM(MAC)
+
+#if HAVE(NETWORK_FRAMEWORK)
+
+@interface LoadingObserver : NSObject
+@property (nonatomic, readonly) size_t changesObserved;
+@end
+
+@implementation LoadingObserver {
+    size_t _changesObserved;
+}
+
+- (size_t)changesObserved
+{
+    return _changesObserved;
+}
+
+- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context
+{
+    EXPECT_WK_STREQ(keyPath, "loading");
+    _changesObserved++;
+}
+
+@end
+
+TEST(WKNavigation, FrameBackLoading)
+{
+    using namespace TestWebKitAPI;
+    HTTPServer server({
+        { "/", { "<iframe src='frame1.html'></iframe>" } },
+        { "/frame1.html", { "<a href='frame2.html'>link</a>" } },
+        { "/frame2.html", { "<script>alert('frame2 loaded')</script>" } },
+    });
+    auto webView = [[WKWebView new] autorelease];
+    auto delegate = [[TestUIDelegate new] autorelease];
+    auto observer = [[LoadingObserver new] autorelease];
+    webView.UIDelegate = delegate;
+    [webView addObserver:observer forKeyPath:@"loading" options:NSKeyValueObservingOptionNew context:nil];
+    EXPECT_FALSE(webView.loading);
+    EXPECT_EQ(observer.changesObserved, 0u);
+    [webView loadRequest:server.request()];
+    EXPECT_TRUE(webView.loading);
+    EXPECT_EQ(observer.changesObserved, 1u);
+    while (observer.changesObserved < 2u)
+        Util::spinRunLoop();
+    EXPECT_FALSE(webView.loading);
+    EXPECT_EQ(observer.changesObserved, 2u);
+    EXPECT_FALSE(webView.canGoBack);
+    [webView evaluateJavaScript:@"document.querySelector('iframe').contentWindow.document.querySelector('a').click()" completionHandler:nil];
+    EXPECT_WK_STREQ([delegate waitForAlert], "frame2 loaded");
+    EXPECT_EQ(observer.changesObserved, 2u);
+    EXPECT_TRUE(webView.canGoBack);
+    [webView goBack];
+    while (observer.changesObserved < 3)
+        Util::spinRunLoop();
+    EXPECT_TRUE(webView.loading);
+    while (observer.changesObserved < 4)
+        Util::spinRunLoop();
+    EXPECT_FALSE(webView.loading);
+    [webView removeObserver:observer forKeyPath:@"loading"];
+
+}
+
+#endif // HAVE(NETWORK_FRAMEWORK)