Skip to content
Permalink
Browse files
Ignore history items added by JS without user interaction when naviga…
…tion back/forward via the WKWebView API

https://bugs.webkit.org/show_bug.cgi?id=241885
<rdar://94838657>

Reviewed by Geoffrey Garen.

Ignore history items added by JS without user interaction when navigation
back/forward via the WKWebView API. This is a behavior similar to the
intervention made in Chrome (https://bugs.chromium.org/p/chromium/issues/detail?id=907167)
to prevent websites from hijacking the back/forward list.

When an history item is added by JS via history.pushState() and without a user
gesture, we now set a flag on that HistoryItem to remember this. Later on, when
calling [WKWebView goBack] or [WKWebView goForward], we will skip the history
item that have this flag set. This behavior occurs behind a linked-on-after
check to reduce the compatibility risk.

Also, navigations via other means (e.g. via JavaScript) are not impacted and will
ignore this new flag.

* Source/WTF/wtf/cocoa/RuntimeApplicationChecksCocoa.h:
* Source/WebCore/history/HistoryItem.h:
(WebCore::HistoryItem::setWasCreatedByJSWithoutUserInteraction):
(WebCore::HistoryItem::wasCreatedByJSWithoutUserInteraction const):
* Source/WebCore/loader/HistoryController.cpp:
(WebCore::FrameLoader::HistoryController::pushState):
* Source/WebKit/Shared/SessionState.cpp:
(WebKit::PageState::encode const):
(WebKit::PageState::decode):
* Source/WebKit/Shared/SessionState.h:
* Source/WebKit/Shared/WebBackForwardListItem.h:
(WebKit::WebBackForwardListItem::wasCreatedByJSWithoutUserInteraction const):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::itemSkippingBackForwardItemsAddedByJSWithoutUserGesture):
(WebKit::WebPageProxy::goForward):
(WebKit::WebPageProxy::goBack):
* Source/WebKit/WebProcess/WebCoreSupport/SessionStateConversion.cpp:
(WebKit::toBackForwardListItemState):
* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/WebKit/WKBackForwardList.mm:
(TEST):
* Tools/TestWebKitAPI/cocoa/TestNavigationDelegate.h:
* Tools/TestWebKitAPI/cocoa/TestNavigationDelegate.mm:
(-[TestNavigationDelegate _webView:navigation:didSameDocumentNavigation:]):
(-[TestNavigationDelegate waitForDidFinishNavigationOrSameDocumentNavigation]):
(-[WKWebView _test_waitForDidFinishNavigationOrSameDocumentNavigation]):

Canonical link: https://commits.webkit.org/251783@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295778 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
cdumez committed Jun 23, 2022
1 parent 6c074d1 commit 4a761ea1b444cb3c363b4dbd7bdee31429bd98cb
Showing 12 changed files with 189 additions and 2 deletions.
@@ -94,6 +94,7 @@ enum class SDKAlignedBehavior {
WebSQLDisabledByDefaultInLegacyWebKit,
WKContentViewDoesNotOverrideKeyCommands,
WKWebsiteDataStoreInitReturningNil,
UIBackForwardSkipsHistoryItemsWithoutUserGesture,

NumberOfBehaviors
};
@@ -209,6 +209,9 @@ class HistoryItem : public RefCounted<HistoryItem> {
void setWasRestoredFromSession(bool wasRestoredFromSession) { m_wasRestoredFromSession = wasRestoredFromSession; }
bool wasRestoredFromSession() const { return m_wasRestoredFromSession; }

void setWasCreatedByJSWithoutUserInteraction(bool wasCreatedByJSWithoutUserInteraction) { m_wasCreatedByJSWithoutUserInteraction = wasCreatedByJSWithoutUserInteraction; }
bool wasCreatedByJSWithoutUserInteraction() const { return m_wasCreatedByJSWithoutUserInteraction; }

#if !LOG_DISABLED
const char* logString() const;
#endif
@@ -246,6 +249,7 @@ class HistoryItem : public RefCounted<HistoryItem> {
bool m_lastVisitWasFailure { false };
bool m_isTargetItem { false };
bool m_wasRestoredFromSession { false };
bool m_wasCreatedByJSWithoutUserInteraction { false };
bool m_shouldRestoreScrollPosition { true };

// If two HistoryItems have the same item sequence number, then they are
@@ -859,6 +859,9 @@ void FrameLoader::HistoryController::pushState(RefPtr<SerializedScriptValue>&& s
ASSERT(page);

bool shouldRestoreScrollPosition = m_currentItem->shouldRestoreScrollPosition();

if (!UserGestureIndicator::processingUserGesture(m_frame.document()))
m_currentItem->setWasCreatedByJSWithoutUserInteraction(true);

// Get a HistoryItem tree for the current frame tree.
Ref<HistoryItem> topItem = m_frame.mainFrame().loader().history().createItemTree(m_frame, false);
@@ -186,6 +186,7 @@ void PageState::encode(IPC::Encoder& encoder) const
encoder << sessionStateObject->wireBytes();

encoder << shouldOpenExternalURLsPolicy;
encoder << wasCreatedByJSWithoutUserInteraction;
}

bool PageState::decode(IPC::Decoder& decoder, PageState& result)
@@ -216,6 +217,10 @@ bool PageState::decode(IPC::Decoder& decoder, PageState& result)
return false;

result.shouldOpenExternalURLsPolicy = *shouldOpenExternalURLsPolicy;

if (!decoder.decode(result.wasCreatedByJSWithoutUserInteraction))
return false;

return true;
}

@@ -136,6 +136,7 @@ struct PageState {
FrameState mainFrameState;
WebCore::ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy { WebCore::ShouldOpenExternalURLsPolicy::ShouldNotAllow };
RefPtr<WebCore::SerializedScriptValue> sessionStateObject;
bool wasCreatedByJSWithoutUserInteraction { false };
};

struct BackForwardListItemState {
@@ -68,6 +68,7 @@ class WebBackForwardListItem : public API::ObjectImpl<API::Object::Type::BackFor
const String& originalURL() const { return m_itemState.pageState.mainFrameState.originalURLString; }
const String& url() const { return m_itemState.pageState.mainFrameState.urlString; }
const String& title() const { return m_itemState.pageState.title; }
bool wasCreatedByJSWithoutUserInteraction() const { return m_itemState.pageState.wasCreatedByJSWithoutUserInteraction; }

const URL& resourceDirectoryURL() const { return m_resourceDirectoryURL; }
void setResourceDirectoryURL(URL&& url) { m_resourceDirectoryURL = WTFMove(url); }
@@ -1824,9 +1824,35 @@ void WebPageProxy::recordNavigationSnapshot(WebBackForwardListItem& item)
#endif
}

enum class NavigationDirection { Backward, Forward };
static WebBackForwardListItem* itemSkippingBackForwardItemsAddedByJSWithoutUserGesture(const WebBackForwardList& backForwardList, NavigationDirection direction)
{
auto delta = direction == NavigationDirection::Backward ? -1 : 1;
int itemIndex = delta;
auto* item = backForwardList.itemAtIndex(itemIndex);
if (!item)
return nullptr;

#if PLATFORM(COCOA)
if (!linkedOnOrAfterSDKWithBehavior(SDKAlignedBehavior::UIBackForwardSkipsHistoryItemsWithoutUserGesture))
return item;
#endif

auto* originalItem = item;
while (item->wasCreatedByJSWithoutUserInteraction()) {
itemIndex += delta;
item = backForwardList.itemAtIndex(itemIndex);
if (!item)
return originalItem;
RELEASE_LOG(Loading, "UI Navigation is skipping a WebBackForwardListItem because it was added by JavaScript without user interaction");
}
return item;
}

RefPtr<API::Navigation> WebPageProxy::goForward()
{
WebBackForwardListItem* forwardItem = m_backForwardList->forwardItem();
WEBPAGEPROXY_RELEASE_LOG(Loading, "goForward:");
auto* forwardItem = itemSkippingBackForwardItemsAddedByJSWithoutUserGesture(m_backForwardList, NavigationDirection::Forward);
if (!forwardItem)
return nullptr;

@@ -1835,7 +1861,8 @@ RefPtr<API::Navigation> WebPageProxy::goForward()

RefPtr<API::Navigation> WebPageProxy::goBack()
{
WebBackForwardListItem* backItem = m_backForwardList->backItem();
WEBPAGEPROXY_RELEASE_LOG(Loading, "goBack:");
auto* backItem = itemSkippingBackForwardItemsAddedByJSWithoutUserGesture(m_backForwardList, NavigationDirection::Backward);
if (!backItem)
return nullptr;

@@ -114,6 +114,7 @@ BackForwardListItemState toBackForwardListItemState(const WebCore::HistoryItem&
state.pageState.mainFrameState = toFrameState(historyItem);
state.pageState.shouldOpenExternalURLsPolicy = historyItem.shouldOpenExternalURLsPolicy();
state.pageState.sessionStateObject = historyItem.stateObject();
state.pageState.wasCreatedByJSWithoutUserInteraction = historyItem.wasCreatedByJSWithoutUserInteraction();
state.hasCachedPage = historyItem.isInBackForwardCache();
return state;
}
@@ -244,6 +244,7 @@
46C519E61D3563FD00DAA51A /* LocalStorageNullEntries.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 46C519E21D35629600DAA51A /* LocalStorageNullEntries.html */; };
46C519E71D3563FD00DAA51A /* LocalStorageNullEntries.localstorage in Copy Resources */ = {isa = PBXBuildFile; fileRef = 46C519E31D35629600DAA51A /* LocalStorageNullEntries.localstorage */; };
46C519E81D3563FD00DAA51A /* LocalStorageNullEntries.localstorage-shm in Copy Resources */ = {isa = PBXBuildFile; fileRef = 46C519E41D35629600DAA51A /* LocalStorageNullEntries.localstorage-shm */; };
46E45EE62863D3E200441B14 /* WKBackForwardList.mm in Sources */ = {isa = PBXBuildFile; fileRef = 1F83571A1D3FFB0E00E3967B /* WKBackForwardList.mm */; };
46E816F81E79E29C00375ADC /* RestoreStateAfterTermination.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46E816F71E79E29100375ADC /* RestoreStateAfterTermination.mm */; };
46F03C1C255B2D5A00AA51C5 /* audio-context-playing.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 46F03C1B255B2D3600AA51C5 /* audio-context-playing.html */; };
46FA2FEE23846CA5000CCB0C /* HTTPHeaderMap.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 46FA2FED23846C9A000CCB0C /* HTTPHeaderMap.cpp */; };
@@ -6040,6 +6041,7 @@
7CCE7F1C1A411AE600447C4C /* WillSendSubmitEvent.cpp in Sources */,
7CCE7ED81A411A7E00447C4C /* WillSendSubmitEvent.mm in Sources */,
7CCE7ED91A411A7E00447C4C /* WindowlessWebViewWithMedia.mm in Sources */,
46E45EE62863D3E200441B14 /* WKBackForwardList.mm in Sources */,
7CCE7F2E1A411B1000447C4C /* WKBrowsingContextGroupTest.mm in Sources */,
7CCE7F2F1A411B1000447C4C /* WKBrowsingContextLoadDelegateTest.mm in Sources */,
7CCE7F1D1A411AE600447C4C /* WKImageCreateCGImageCrash.cpp in Sources */,
@@ -33,6 +33,7 @@
#import <WebKit/WKWebViewPrivate.h>
#import <WebKit/_WKSessionState.h>
#import <wtf/RetainPtr.h>
#import <wtf/text/WTFString.h>

static NSString *loadableURL1 = @"data:text/html,no%20error%20A";
static NSString *loadableURL2 = @"data:text/html,no%20error%20B";
@@ -309,3 +310,107 @@ - (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigati
EXPECT_STREQ([[list.currentItem URL] absoluteString].UTF8String, url3.absoluteString.UTF8String);
}

TEST(WKBackForwardList, BackForwardNavigationSkipsItemsWithoutUserGesture)
{
auto webView = adoptNS([[WKWebView alloc] init]);
NSURL *url1 = [[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
NSURL *url2 = [[NSBundle mainBundle] URLForResource:@"simple2" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
[webView loadRequest:[NSURLRequest requestWithURL:url1]];
[webView _test_waitForDidFinishNavigation];

[webView loadRequest:[NSURLRequest requestWithURL:url2]];
[webView _test_waitForDidFinishNavigation];

// Add back/forward list items without user gestures.
done = false;
[webView _evaluateJavaScriptWithoutUserGesture:@"history.pushState(null, document.title, location.pathname + '#a');" completionHandler:^(id, NSError *) {
done = true;
}];
TestWebKitAPI::Util::run(&done);
done = false;
[webView _evaluateJavaScriptWithoutUserGesture:@"history.pushState(null, document.title, location.pathname + '#b');" completionHandler:^(id, NSError *) {
done = true;
}];
TestWebKitAPI::Util::run(&done);
done = false;
[webView _evaluateJavaScriptWithoutUserGesture:@"history.pushState(null, document.title, location.pathname + '#c');" completionHandler:^(id, NSError *) {
done = true;
}];
TestWebKitAPI::Util::run(&done);

EXPECT_EQ([webView backForwardList].backList.count, 4U);
EXPECT_EQ([webView backForwardList].forwardList.count, 0U);

auto* lastURL = [webView URL];

// Going back should skip the back/forward list items without user gestures.
[webView goBack];
[webView _test_waitForDidFinishNavigationOrSameDocumentNavigation];

EXPECT_STREQ([webView URL].absoluteString.UTF8String, url1.absoluteString.UTF8String);

EXPECT_EQ([webView backForwardList].backList.count, 0U);
EXPECT_EQ([webView backForwardList].forwardList.count, 4U);

// Going forward should skip the back/forward list items without user gestures.
[webView goForward];
[webView _test_waitForDidFinishNavigationOrSameDocumentNavigation];

EXPECT_STREQ([webView URL].absoluteString.UTF8String, lastURL.absoluteString.UTF8String);

EXPECT_EQ([webView backForwardList].backList.count, 4U);
EXPECT_EQ([webView backForwardList].forwardList.count, 0U);

NSString *currentURLString = [webView URL].absoluteString;
NSString *expectedURLString = makeString(String([[NSBundle mainBundle] URLForResource:@"simple2" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"].absoluteString), "#c");
EXPECT_WK_STREQ(currentURLString, expectedURLString);

// Navigating via the JS API shouldn't skip those back/forward list items.
[webView _evaluateJavaScriptWithoutUserGesture:@"history.back();" completionHandler:^(id, NSError *) { }];
[webView _test_waitForDidFinishNavigationOrSameDocumentNavigation];

expectedURLString = makeString(String([[NSBundle mainBundle] URLForResource:@"simple2" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"].absoluteString), "#b");
EXPECT_WK_STREQ([webView URL].absoluteString.UTF8String, expectedURLString.UTF8String);
}

TEST(WKBackForwardList, BackForwardNavigationDoesNotSkipItemsWithUserGesture)
{
auto webView = adoptNS([[WKWebView alloc] init]);
NSURL *url1 = [[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
NSURL *url2 = [[NSBundle mainBundle] URLForResource:@"simple2" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
[webView loadRequest:[NSURLRequest requestWithURL:url1]];
[webView _test_waitForDidFinishNavigation];

[webView loadRequest:[NSURLRequest requestWithURL:url2]];
[webView _test_waitForDidFinishNavigation];

// Add back/forward list items without user gestures.
done = false;
[webView evaluateJavaScript:@"history.pushState(null, document.title, location.pathname + '#a');" completionHandler:^(id, NSError *) {
done = true;
}];
TestWebKitAPI::Util::run(&done);

auto* lastURL = [webView URL];
EXPECT_FALSE([lastURL isEqual:url2]);

[webView goBack];
[webView _test_waitForDidFinishNavigationOrSameDocumentNavigation];

EXPECT_STREQ([webView URL].absoluteString.UTF8String, url2.absoluteString.UTF8String);

[webView goBack];
[webView _test_waitForDidFinishNavigationOrSameDocumentNavigation];

EXPECT_STREQ([webView URL].absoluteString.UTF8String, url1.absoluteString.UTF8String);

[webView goForward];
[webView _test_waitForDidFinishNavigationOrSameDocumentNavigation];

EXPECT_STREQ([webView URL].absoluteString.UTF8String, url2.absoluteString.UTF8String);

[webView goForward];
[webView _test_waitForDidFinishNavigationOrSameDocumentNavigation];

EXPECT_STREQ([webView URL].absoluteString.UTF8String, lastURL.absoluteString.UTF8String);
}
@@ -37,13 +37,15 @@
@property (nonatomic, copy) void (^didStartProvisionalNavigation)(WKWebView *, WKNavigation *);
@property (nonatomic, copy) void (^didCommitNavigation)(WKWebView *, WKNavigation *);
@property (nonatomic, copy) void (^didFinishNavigation)(WKWebView *, WKNavigation *);
@property (nonatomic, copy) void (^didSameDocumentNavigation)(WKWebView *, WKNavigation *);
@property (nonatomic, copy) void (^renderingProgressDidChange)(WKWebView *, _WKRenderingProgressEvents);
@property (nonatomic, copy) void (^webContentProcessDidTerminate)(WKWebView *);
@property (nonatomic, copy) void (^didReceiveAuthenticationChallenge)(WKWebView *, NSURLAuthenticationChallenge *, void (^)(NSURLSessionAuthChallengeDisposition, NSURLCredential *));
@property (nonatomic, copy) void (^contentRuleListPerformedAction)(WKWebView *, NSString *, _WKContentRuleListAction *, NSURL *);

- (void)waitForDidStartProvisionalNavigation;
- (void)waitForDidFinishNavigation;
- (void)waitForDidFinishNavigationOrSameDocumentNavigation;
- (void)waitForDidFinishNavigationWithPreferences:(WKWebpagePreferences *)preferences;
- (NSError *)waitForDidFailProvisionalNavigation;

@@ -52,6 +54,7 @@
@interface WKWebView (TestWebKitAPIExtras)
- (void)_test_waitForDidStartProvisionalNavigation;
- (void)_test_waitForDidFinishNavigation;
- (void)_test_waitForDidFinishNavigationOrSameDocumentNavigation;
- (void)_test_waitForDidFinishNavigationWithPreferences:(WKWebpagePreferences *)preferences;
- (void)_test_waitForDidFinishNavigationWithoutPresentationUpdate;
- (void)_test_waitForDidFailProvisionalNavigation;
@@ -98,6 +98,12 @@ - (void)webView:(WKWebView *)webView didReceiveAuthenticationChallenge:(NSURLAut
completionHandler(NSURLSessionAuthChallengePerformDefaultHandling, nil);
}

- (void)_webView:(WKWebView *)webView navigation:(WKNavigation *)navigation didSameDocumentNavigation:(_WKSameDocumentNavigationType)navigationType
{
if (_didSameDocumentNavigation)
_didSameDocumentNavigation(webView, navigation);
}

- (void)waitForDidStartProvisionalNavigation
{
EXPECT_FALSE(self.didStartProvisionalNavigation);
@@ -126,6 +132,23 @@ - (void)waitForDidFinishNavigation
self.didFinishNavigation = nil;
}

- (void)waitForDidFinishNavigationOrSameDocumentNavigation
{
EXPECT_FALSE(self.didFinishNavigation);

__block bool finished = false;
self.didFinishNavigation = ^(WKWebView *, WKNavigation *) {
finished = true;
};
self.didSameDocumentNavigation = ^(WKWebView *, WKNavigation *) {
finished = true;
};

TestWebKitAPI::Util::run(&finished);

self.didFinishNavigation = nil;
}

- (void)waitForWebContentProcessDidTerminate
{
EXPECT_FALSE(self.webContentProcessDidTerminate);
@@ -242,6 +265,17 @@ - (void)_test_waitForDidFinishNavigation
#endif
}

- (void)_test_waitForDidFinishNavigationOrSameDocumentNavigation
{
EXPECT_FALSE(self.navigationDelegate);

auto navigationDelegate = adoptNS([[TestNavigationDelegate alloc] init]);
self.navigationDelegate = navigationDelegate.get();
[navigationDelegate waitForDidFinishNavigationOrSameDocumentNavigation];

self.navigationDelegate = nil;
}

- (void)_test_waitForWebContentProcessDidTerminate
{
EXPECT_FALSE(self.navigationDelegate);

0 comments on commit 4a761ea

Please sign in to comment.