WebKit Bugzilla
Attachment 347674 Details for
Bug 188805
: Remove unused shouldKeepCurrentBackForwardListItemInList check
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-188805-20180821131019.patch (text/plain), 18.53 KB, created by
Alex Christensen
on 2018-08-21 13:10:20 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Alex Christensen
Created:
2018-08-21 13:10:20 PDT
Size:
18.53 KB
patch
obsolete
>Index: Source/WebKit/ChangeLog >=================================================================== >--- Source/WebKit/ChangeLog (revision 235123) >+++ Source/WebKit/ChangeLog (working copy) >@@ -1,3 +1,25 @@ >+2018-08-21 Alex Christensen <achristensen@webkit.org> >+ >+ Remove unused shouldKeepCurrentBackForwardListItemInList check >+ https://bugs.webkit.org/show_bug.cgi?id=188805 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ The check was only done in WKPageLoaderClient, and nobody implements the check. >+ It was not needed to put in WKPageNavigationClient or WKNavigationDelegate, so let's remove it! >+ >+ * UIProcess/API/APILoaderClient.h: >+ (API::LoaderClient::didChangeBackForwardList): >+ (API::LoaderClient::shouldKeepCurrentBackForwardListItemInList): Deleted. >+ * UIProcess/API/C/WKPage.cpp: >+ (WKPageSetPageLoaderClient): >+ * UIProcess/WebBackForwardList.cpp: >+ (WebKit::WebBackForwardList::addItem): >+ (WebKit::WebBackForwardList::goToItem): >+ * UIProcess/WebPageProxy.cpp: >+ (WebKit::WebPageProxy::shouldKeepCurrentBackForwardListItemInList): Deleted. >+ * UIProcess/WebPageProxy.h: >+ > 2018-08-21 Daniel Bates <dabates@apple.com> > > Replace TextCheckingTypeMask with OptionSet >Index: Source/WebKit/UIProcess/WebBackForwardList.cpp >=================================================================== >--- Source/WebKit/UIProcess/WebBackForwardList.cpp (revision 235090) >+++ Source/WebKit/UIProcess/WebBackForwardList.cpp (working copy) >@@ -141,11 +141,8 @@ void WebBackForwardList::addItem(Ref<Web > ASSERT(m_entries.isEmpty()); > m_currentIndex = 0; > m_hasCurrentIndex = true; >- } else { >- shouldKeepCurrentItem = m_page->shouldKeepCurrentBackForwardListItemInList(m_entries[m_currentIndex]); >- if (shouldKeepCurrentItem) >- m_currentIndex++; >- } >+ } else >+ m_currentIndex++; > > auto* newItemPtr = newItem.ptr(); > if (!shouldKeepCurrentItem) { >@@ -198,32 +195,12 @@ void WebBackForwardList::goToItem(WebBac > // If we're going to an item different from the current item, ask the client if the current > // item should remain in the list. > auto& currentItem = m_entries[m_currentIndex]; >- bool shouldKeepCurrentItem = true; >- if (currentItem.ptr() != &item) { >+ if (currentItem.ptr() != &item) > m_page->recordAutomaticNavigationSnapshot(); >- shouldKeepCurrentItem = m_page->shouldKeepCurrentBackForwardListItemInList(m_entries[m_currentIndex]); >- } >- >- // If the client said to remove the current item, remove it and then update the target index. >- Vector<Ref<WebBackForwardListItem>> removedItems; >- if (!shouldKeepCurrentItem) { >- removedItems.append(currentItem.copyRef()); >- m_entries.remove(m_currentIndex); >- targetIndex = notFound; >- for (size_t i = 0; i < m_entries.size(); ++i) { >- if (m_entries[i].ptr() == &item) { >- targetIndex = i; >- break; >- } >- } >- ASSERT(targetIndex != notFound); >- } > > m_currentIndex = targetIndex; > > LOG(BackForward, "(Back/Forward) WebBackForwardList %p going to item %s, is now at index %zu", this, item.itemID().logString(), targetIndex); >- >- m_page->didChangeBackForwardList(nullptr, WTFMove(removedItems)); > } > > WebBackForwardListItem* WebBackForwardList::currentItem() const >Index: Source/WebKit/UIProcess/WebPageProxy.cpp >=================================================================== >--- Source/WebKit/UIProcess/WebPageProxy.cpp (revision 235090) >+++ Source/WebKit/UIProcess/WebPageProxy.cpp (working copy) >@@ -1290,13 +1290,6 @@ void WebPageProxy::willGoToBackForwardLi > } > } > >-bool WebPageProxy::shouldKeepCurrentBackForwardListItemInList(WebBackForwardListItem& item) >-{ >- PageClientProtector protector(m_pageClient); >- >- return m_loaderClient->shouldKeepCurrentBackForwardListItemInList(*this, item); >-} >- > bool WebPageProxy::canShowMIMEType(const String& mimeType) > { > if (MIMETypeRegistry::canShowMIMEType(mimeType)) >Index: Source/WebKit/UIProcess/WebPageProxy.h >=================================================================== >--- Source/WebKit/UIProcess/WebPageProxy.h (revision 235090) >+++ Source/WebKit/UIProcess/WebPageProxy.h (working copy) >@@ -472,8 +472,6 @@ public: > void didChangeBackForwardList(WebBackForwardListItem* addedItem, Vector<Ref<WebBackForwardListItem>>&& removed); > void willGoToBackForwardListItem(const WebCore::BackForwardItemIdentifier&, bool inPageCache, const UserData&); > >- bool shouldKeepCurrentBackForwardListItemInList(WebBackForwardListItem&); >- > bool willHandleHorizontalScrollEvents() const; > > void updateWebsitePolicies(WebsitePoliciesData&&); >Index: Source/WebKit/UIProcess/API/APILoaderClient.h >=================================================================== >--- Source/WebKit/UIProcess/API/APILoaderClient.h (revision 235090) >+++ Source/WebKit/UIProcess/API/APILoaderClient.h (working copy) >@@ -90,7 +90,6 @@ public: > virtual bool processDidCrash(WebKit::WebPageProxy&) { return false; } > > virtual void didChangeBackForwardList(WebKit::WebPageProxy&, WebKit::WebBackForwardListItem*, Vector<Ref<WebKit::WebBackForwardListItem>>&&) { } >- virtual bool shouldKeepCurrentBackForwardListItemInList(WebKit::WebPageProxy&, WebKit::WebBackForwardListItem&) { return true; } > virtual void willGoToBackForwardListItem(WebKit::WebPageProxy&, WebKit::WebBackForwardListItem&, API::Object*) { } > > virtual void didNavigateWithNavigationData(WebKit::WebPageProxy&, const WebKit::WebNavigationDataStore&, WebKit::WebFrameProxy&) { } >Index: Source/WebKit/UIProcess/API/C/WKPage.cpp >=================================================================== >--- Source/WebKit/UIProcess/API/C/WKPage.cpp (revision 235090) >+++ Source/WebKit/UIProcess/API/C/WKPage.cpp (working copy) >@@ -1042,6 +1042,7 @@ void WKPageSetPageLoaderClient(WKPageRef > explicit LoaderClient(const WKPageLoaderClientBase* client) > { > initialize(client); >+ ASSERT(!m_client.shouldKeepCurrentBackForwardListItemInList); > } > > private: >@@ -1248,14 +1249,6 @@ void WKPageSetPageLoaderClient(WKPageRef > m_client.didChangeBackForwardList(toAPI(&page), toAPI(addedItem), toAPI(removedItemsArray.get()), m_client.base.clientInfo); > } > >- bool shouldKeepCurrentBackForwardListItemInList(WebKit::WebPageProxy& page, WebKit::WebBackForwardListItem& item) override >- { >- if (!m_client.shouldKeepCurrentBackForwardListItemInList) >- return true; >- >- return m_client.shouldKeepCurrentBackForwardListItemInList(toAPI(&page), toAPI(&item), m_client.base.clientInfo); >- } >- > void willGoToBackForwardListItem(WebPageProxy& page, WebBackForwardListItem& item, API::Object* userData) override > { > if (m_client.willGoToBackForwardListItem) >Index: Tools/ChangeLog >=================================================================== >--- Tools/ChangeLog (revision 235131) >+++ Tools/ChangeLog (working copy) >@@ -1,3 +1,13 @@ >+2018-08-21 Alex Christensen <achristensen@webkit.org> >+ >+ Remove unused shouldKeepCurrentBackForwardListItemInList check >+ https://bugs.webkit.org/show_bug.cgi?id=188805 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: >+ * TestWebKitAPI/Tests/WebKit/ShouldKeepCurrentBackForwardListItemInList.cpp: Removed. >+ > 2018-08-21 Andy VanWagoner <andy@vanwagoner.family> > > Unreviewed, add myself to committers list. >Index: Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj >=================================================================== >--- Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (revision 235090) >+++ Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (working copy) >@@ -534,7 +534,6 @@ > 7CCE7F111A411AE600447C4C /* RestoreSessionStateContainingFormData.cpp in Sources */ = {isa = PBXBuildFile; fileRef = C0ADBE8212FCA6AA00D2C129 /* RestoreSessionStateContainingFormData.cpp */; }; > 7CCE7F121A411AE600447C4C /* ScrollPinningBehaviors.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2D640B5417875DFF00BFAF99 /* ScrollPinningBehaviors.cpp */; }; > 7CCE7F131A411AE600447C4C /* ShouldGoToBackForwardListItem.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 51FCF7981534AC6D00104491 /* ShouldGoToBackForwardListItem.cpp */; }; >- 7CCE7F141A411AE600447C4C /* ShouldKeepCurrentBackForwardListItemInList.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 51E5C7041919EA5F00D8B3E1 /* ShouldKeepCurrentBackForwardListItemInList.cpp */; }; > 7CCE7F151A411AE600447C4C /* SpacebarScrolling.cpp in Sources */ = {isa = PBXBuildFile; fileRef = C02B77F1126612140026BF0F /* SpacebarScrolling.cpp */; }; > 7CCE7F161A411AE600447C4C /* TerminateTwice.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 1AE72F47173EB214006362F0 /* TerminateTwice.cpp */; }; > 7CCE7F171A411AE600447C4C /* UserMedia.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4A410F4B19AF7BD6002EBAB5 /* UserMedia.cpp */; }; >@@ -1499,7 +1498,6 @@ > 51CD1C711B38D48400142CA5 /* modal-alerts-in-new-about-blank-window.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "modal-alerts-in-new-about-blank-window.html"; sourceTree = "<group>"; }; > 51D124971E763AF8002B2820 /* WKHTTPCookieStore.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKHTTPCookieStore.mm; sourceTree = "<group>"; }; > 51DB16CD1F085047001FA4C5 /* WebViewIconLoading.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = WebViewIconLoading.mm; sourceTree = "<group>"; }; >- 51E5C7041919EA5F00D8B3E1 /* ShouldKeepCurrentBackForwardListItemInList.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ShouldKeepCurrentBackForwardListItemInList.cpp; sourceTree = "<group>"; }; > 51E6A8921D2F1BEC00C004B6 /* LocalStorageClear.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = LocalStorageClear.mm; sourceTree = "<group>"; }; > 51E6A8951D2F1C7700C004B6 /* LocalStorageClear.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = LocalStorageClear.html; sourceTree = "<group>"; }; > 51E780361919AFF8001829A2 /* simple2.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = simple2.html; sourceTree = "<group>"; }; >@@ -2938,7 +2936,6 @@ > 2D640B5417875DFF00BFAF99 /* ScrollPinningBehaviors.cpp */, > 51FCF7981534AC6D00104491 /* ShouldGoToBackForwardListItem.cpp */, > 51FCF7971534AC6D00104491 /* ShouldGoToBackForwardListItem_Bundle.cpp */, >- 51E5C7041919EA5F00D8B3E1 /* ShouldKeepCurrentBackForwardListItemInList.cpp */, > C02B77F1126612140026BF0F /* SpacebarScrolling.cpp */, > 76734997193016DC00E44DF9 /* StopLoadingDuringDidFailProvisionalLoad.cpp */, > 7673499A1930182E00E44DF9 /* StopLoadingDuringDidFailProvisionalLoad_bundle.cpp */, >@@ -3947,7 +3944,6 @@ > A17991881E1C994E00A505ED /* SharedBuffer.mm in Sources */, > A179918B1E1CA24100A505ED /* SharedBufferTest.cpp in Sources */, > 7CCE7F131A411AE600447C4C /* ShouldGoToBackForwardListItem.cpp in Sources */, >- 7CCE7F141A411AE600447C4C /* ShouldKeepCurrentBackForwardListItemInList.cpp in Sources */, > 37BCA61C1B596BA9002012CA /* ShouldOpenExternalURLsInNewWindowActions.mm in Sources */, > 7C83E0C51D0A654600FEBCF3 /* ShrinkToFit.mm in Sources */, > 7CCE7ECD1A411A7E00447C4C /* SimplifyMarkup.mm in Sources */, >Index: Tools/TestWebKitAPI/Tests/WebKit/ShouldKeepCurrentBackForwardListItemInList.cpp >=================================================================== >--- Tools/TestWebKitAPI/Tests/WebKit/ShouldKeepCurrentBackForwardListItemInList.cpp (revision 235090) >+++ Tools/TestWebKitAPI/Tests/WebKit/ShouldKeepCurrentBackForwardListItemInList.cpp (nonexistent) >@@ -1,163 +0,0 @@ >-/* >- * Copyright (C) 2014 Apple Inc. All rights reserved. >- * >- * Redistribution and use in source and binary forms, with or without >- * modification, are permitted provided that the following conditions >- * are met: >- * 1. Redistributions of source code must retain the above copyright >- * notice, this list of conditions and the following disclaimer. >- * 2. Redistributions in binary form must reproduce the above copyright >- * notice, this list of conditions and the following disclaimer in the >- * documentation and/or other materials provided with the distribution. >- * >- * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' >- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, >- * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR >- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS >- * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR >- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF >- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS >- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN >- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) >- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF >- * THE POSSIBILITY OF SUCH DAMAGE. >- */ >- >-#include "config.h" >- >-#if WK_HAVE_C_SPI >- >-#include "PlatformUtilities.h" >-#include "PlatformWebView.h" >-#include "Test.h" >- >-// This test navigates from simple.html, to simple2.html, to simple3.html >-// When navigating from simple2 to simple3, it disallows the simple2 back/forward list item from staying in the list >-// It then navigates back from simple3, expecting to land at simple. >- >-namespace TestWebKitAPI { >- >-static bool finished = false; >-static bool successfulSoFar = true; >-static int navigationNumber = 0; >- >-static bool itemURLLastComponentIsString(WKBackForwardListItemRef item, const char* string) >-{ >- WKRetainPtr<WKURLRef> url = adoptWK(WKBackForwardListItemCopyURL(item)); >- WKRetainPtr<WKStringRef> path = adoptWK(WKURLCopyLastPathComponent(url.get())); >- >- return WKStringIsEqualToUTF8CString(path.get(), string); >-} >- >-static void didFinishLoadForFrame(WKPageRef page, WKFrameRef frame, WKTypeRef, const void*) >-{ >- // Only mark finished when the main frame loads >- if (!WKFrameIsMainFrame(frame)) >- return; >- >- finished = true; >- navigationNumber++; >- >- WKBackForwardListRef list = WKPageGetBackForwardList(page); >- WKBackForwardListItemRef currentItem = WKBackForwardListGetCurrentItem(list); >- WKBackForwardListItemRef backItem = WKBackForwardListGetBackItem(list); >- WKBackForwardListItemRef forwardItem = WKBackForwardListGetForwardItem(list); >- unsigned forwardCount = WKBackForwardListGetForwardListCount(list); >- >- // This test should never have a forward list. >- if (forwardCount) >- successfulSoFar = false; >- >- if (navigationNumber == 1) { >- // We've only performed 1 navigation, we should only have a current item. >- if (!currentItem || !itemURLLastComponentIsString(currentItem, "simple.html")) >- successfulSoFar = false; >- if (backItem || forwardItem) >- successfulSoFar = false; >- } else if (navigationNumber == 2) { >- // On the second navigation, simple2 should be current and simple should be the back item. >- if (!currentItem || !itemURLLastComponentIsString(currentItem, "simple2.html")) >- successfulSoFar = false; >- if (!backItem || !itemURLLastComponentIsString(backItem, "simple.html")) >- successfulSoFar = false; >- if (forwardItem) >- successfulSoFar = false; >- } else if (navigationNumber == 3) { >- // On the third navigation the item for simple2 should have been removed. >- // So simple3 should be current and simple should still be the back item. >- if (!currentItem || !itemURLLastComponentIsString(currentItem, "simple3.html")) >- successfulSoFar = false; >- if (!backItem || !itemURLLastComponentIsString(backItem, "simple.html")) >- successfulSoFar = false; >- if (forwardItem) >- successfulSoFar = false; >- } else if (navigationNumber == 4) { >- // After the fourth navigation (which was a "back" navigation), the item for simple3 should have been removed. >- // So simple should be current and there should be no other items. >- if (!currentItem || !itemURLLastComponentIsString(currentItem, "simple.html")) >- successfulSoFar = false; >- if (backItem || forwardItem) >- successfulSoFar = false; >- } >-} >- >-static void willGoToBackForwardListItem(WKPageRef, WKBackForwardListItemRef item, WKTypeRef userData, const void*) >-{ >- if (!itemURLLastComponentIsString(item, "simple.html")) >- successfulSoFar = false; >-} >- >-static bool shouldKeepCurrentBackForwardListItemInList(WKPageRef page, WKBackForwardListItemRef item, const void*) >-{ >- // We make sure the item for "simple2.html" is removed when we navigate to "simple3.html" >- // We also want to make sure the item for "simple3.html" is removed when we go back to "simple.html" >- // So we only want to keep "simple.html" >- return itemURLLastComponentIsString(item, "simple.html"); >-} >- >-static void setPageLoaderClient(WKPageRef page) >-{ >- WKPageLoaderClientV5 loaderClient; >- memset(&loaderClient, 0, sizeof(loaderClient)); >- >- loaderClient.base.version = 5; >- loaderClient.didFinishLoadForFrame = didFinishLoadForFrame; >- loaderClient.shouldKeepCurrentBackForwardListItemInList = shouldKeepCurrentBackForwardListItemInList; >- loaderClient.willGoToBackForwardListItem = willGoToBackForwardListItem; >- >- WKPageSetPageLoaderClient(page, &loaderClient.base); >-} >- >-TEST(WebKit, ShouldKeepCurrentBackForwardListItemInList) >-{ >- WKRetainPtr<WKContextRef> context(AdoptWK, WKContextCreate()); >- >- PlatformWebView webView(context.get()); >- setPageLoaderClient(webView.page()); >- >- WKPageLoadURL(webView.page(), adoptWK(Util::createURLForResource("simple", "html")).get()); >- Util::run(&finished); >- EXPECT_EQ(successfulSoFar, true); >- >- finished = false; >- WKPageLoadURL(webView.page(), adoptWK(Util::createURLForResource("simple2", "html")).get()); >- Util::run(&finished); >- EXPECT_EQ(successfulSoFar, true); >- >- finished = false; >- WKPageLoadURL(webView.page(), adoptWK(Util::createURLForResource("simple3", "html")).get()); >- Util::run(&finished); >- EXPECT_EQ(successfulSoFar, true); >- >- finished = false; >- WKPageGoBack(webView.page()); >- Util::run(&finished); >- >- EXPECT_EQ(successfulSoFar, true); >- EXPECT_EQ(navigationNumber, 4); >-} >- >-} // namespace TestWebKitAPI >- >- >-#endif
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=347674">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Flags:
aestes
:
review+
ews-watchlist
:
commit-queue-
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 188805
: 347674 |
347692