Restore navigation history when web view is created after WebStateImpl::SetWebUsageEnabled(false) |
|||||
Issue descriptionWebStateImpl::SetWebUsageEnabled(false) forcibly destroys the WKWebView because this is the only way to clear visited link data. From eugenebut@: Visited links are not cleared with [WKWebsiteDataStore removeDataOfTypes:modifiedSince:] unless all web views and their configuration are deallocated (crbug.com/557963). SetWebUsageEnabled had somewhat a different purpose in UIWebView world. I will try to kill SetWebUsageEnabled API in 2018 with special purpose API for browsing data clearing. See also eugenebut@ Oct 27 comment here: https://chromium-review.googlesource.com/c/chromium/src/+/742482/2/ios/web/web_state/ui/crw_web_controller.mm#1091 For the time being, we need to implement history restoration when a new WKWebView is created in SetWebUsageEnabled(true) after SetWebUsageEnabled(false).
,
Nov 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a5a8c6ecf4f93a9b0ba4975416e431af2a4b8827 commit a5a8c6ecf4f93a9b0ba4975416e431af2a4b8827 Author: Danyao Wang <danyao@chromium.org> Date: Mon Nov 27 23:51:34 2017 [Nav Experiment] Restore session history after reenable web usage. The last use case of SetWebUsageEnabled(false) is to clear visited site data. It does so by removing the WKWebView due to a bug in WebKit (see crbug.com/557963). This breaks in WKBasedNavigationManager because it relies on WKWebView as the source of truth for session history. This CL fixes the issue by temporarily caching the session history in WebState and restoring it into the navigation manager when web usage is re-enabled. This fixes the LoadIfNecessaryTest.DisableAndReenableWebUsage test. Bug: 781916 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I6f7db84504603e624f5befa66ccb41c10433dd69 Reviewed-on: https://chromium-review.googlesource.com/789791 Commit-Queue: Danyao Wang <danyao@chromium.org> Reviewed-by: Eugene But <eugenebut@chromium.org> Cr-Commit-Position: refs/heads/master@{#519458} [modify] https://crrev.com/a5a8c6ecf4f93a9b0ba4975416e431af2a4b8827/ios/web/web_state/web_state_impl.h [modify] https://crrev.com/a5a8c6ecf4f93a9b0ba4975416e431af2a4b8827/ios/web/web_state/web_state_impl.mm
,
Nov 28 2017
,
Nov 30 2017
,
Nov 30 2017
This use case is not fully fixed. ios_chrome_integration_egtest surfaced another use case: TabUsageRecorderTestCase/testColdLaunchReloadCount failed with the following error: [1130/061558.733202:FATAL:wk_based_navigation_manager_impl.mm(364)] Check failed: GetItemCount() == 0 && !GetPendingItem(). 0 ios_chrome_integration_egtests 0x000000010b4efded base::debug::StackTrace::StackTrace(unsigned long) + 157 1 ios_chrome_integration_egtests 0x000000010b4efe2d base::debug::StackTrace::StackTrace(unsigned long) + 29 2 ios_chrome_integration_egtests 0x000000010b4ee89c base::debug::StackTrace::StackTrace() + 28 3 ios_chrome_integration_egtests 0x000000010b5511cf logging::LogMessage::~LogMessage() + 479 4 ios_chrome_integration_egtests 0x000000010b54eb75 logging::LogMessage::~LogMessage() + 21 5 ios_chrome_integration_egtests 0x000000010a3cf759 web::WKBasedNavigationManagerImpl::Restore(int, std::__1::vector<std::__1::unique_ptr<web::NavigationItem, std::__1::default_delete<web::NavigationItem> >, std::__1::allocator<std::__1::unique_ptr<web::NavigationItem, std::__1::default_delete<web::NavigationItem> > > >) + 297 6 ios_chrome_integration_egtests 0x000000010a3c9cdb web::SessionStorageBuilder::ExtractSessionState(web::WebStateImpl*, CRWSessionStorage*) const + 2699 7 ios_chrome_integration_egtests 0x000000010a38644e web::WebStateImpl::RestoreSessionStorage(CRWSessionStorage*) + 46 8 ios_chrome_integration_egtests 0x000000010a38fcb9 web::WebStateImpl::SetWebUsageEnabled(bool) + 233 9 ios_chrome_integration_egtests 0x000000010b2b66e4 -[TabModel setWebUsageEnabled:] + 244 10 ios_chrome_integration_egtests 0x000000010a92dfc9 chrome_test_util::EvictOtherTabModelTabs() + 617 11 ios_chrome_integration_egtests 0x000000010b48cc4a tab_usage_recorder_test_util::OpenNewIncognitoTabUsingUIAndEvictMainTabs() + 2586 12 ios_chrome_integration_egtests 0x000000010b46d163 -[TabUsageRecorderTestCase testColdLaunchReloadCount] + 2243 It looks like pending_item_ is not null when Restore() is called.
,
Dec 1 2017
Found a third issue introduced in https://chromium-review.googlesource.com/789791: session storage should only be cached on SetWebUsageEnabled(false). The submitted code incorrectly cache session in SetWebUsageEnabled(true) if no session is cached so far. This corrupts navigation history after two consecutive calls to SetWebUsageenabled(true).
,
Dec 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9bf50002006155105aca7c9033995841d44f6bd6 commit 9bf50002006155105aca7c9033995841d44f6bd6 Author: Danyao Wang <danyao@chromium.org> Date: Fri Dec 01 23:40:57 2017 [Nav Bug] Only cache session when disabling web usage and add inttest. This is a logic bug introduced in crrev.com/c/789791. If SetWebUsageEnabled(true) is called twice consecutively, the session at the time of the first call will incorrectly be restored during the second call. The navigation history between the two calls will be lost. This was exposed in TabUsageRecorderTestCase/testColdLaunchReloadCount egtest. Bug: 781916 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I6f7226fb42ba7d647b9229514a574afc57703ae3 Reviewed-on: https://chromium-review.googlesource.com/803740 Reviewed-by: Eugene But <eugenebut@chromium.org> Commit-Queue: Danyao Wang <danyao@chromium.org> Cr-Commit-Position: refs/heads/master@{#521123} [modify] https://crrev.com/9bf50002006155105aca7c9033995841d44f6bd6/ios/web/web_state/navigation_and_load_callbacks_inttest.mm [modify] https://crrev.com/9bf50002006155105aca7c9033995841d44f6bd6/ios/web/web_state/web_state_impl.mm
,
Dec 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3dcc03f43d97bb368eb872fcdadfc63b56a4df91 commit 3dcc03f43d97bb368eb872fcdadfc63b56a4df91 Author: Danyao Wang <danyao@chromium.org> Date: Mon Dec 04 17:02:20 2017 [Nav Experiment] Reset navigation manager states in Restore(). Current implementation of WKBasedNavigationManagerImpl::Restore() does not reset the internal previous item, pending item and last committed item index caches. This leaves the navigation manager in a corrupted state and is the cause of several egtest failures. Added unit test to cover this case. Bug: 781916 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I5d1777dfc5f9625c0e3ab6ae352cf82d60201834 Reviewed-on: https://chromium-review.googlesource.com/803637 Reviewed-by: Eugene But <eugenebut@chromium.org> Commit-Queue: Danyao Wang <danyao@chromium.org> Cr-Commit-Position: refs/heads/master@{#521369} [modify] https://crrev.com/3dcc03f43d97bb368eb872fcdadfc63b56a4df91/ios/web/navigation/navigation_manager_delegate.h [modify] https://crrev.com/3dcc03f43d97bb368eb872fcdadfc63b56a4df91/ios/web/navigation/navigation_manager_impl_unittest.mm [modify] https://crrev.com/3dcc03f43d97bb368eb872fcdadfc63b56a4df91/ios/web/navigation/wk_based_navigation_manager_impl.mm [modify] https://crrev.com/3dcc03f43d97bb368eb872fcdadfc63b56a4df91/ios/web/navigation/wk_based_navigation_manager_impl_unittest.mm [modify] https://crrev.com/3dcc03f43d97bb368eb872fcdadfc63b56a4df91/ios/web/test/fakes/fake_navigation_manager_delegate.h [modify] https://crrev.com/3dcc03f43d97bb368eb872fcdadfc63b56a4df91/ios/web/test/fakes/fake_navigation_manager_delegate.mm [modify] https://crrev.com/3dcc03f43d97bb368eb872fcdadfc63b56a4df91/ios/web/web_state/ui/crw_web_controller.h [modify] https://crrev.com/3dcc03f43d97bb368eb872fcdadfc63b56a4df91/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/3dcc03f43d97bb368eb872fcdadfc63b56a4df91/ios/web/web_state/web_state_impl.h [modify] https://crrev.com/3dcc03f43d97bb368eb872fcdadfc63b56a4df91/ios/web/web_state/web_state_impl.mm
,
Dec 4 2017
TabUsageRecorderTestCase/testColdLaunchReloadCount now passes.
,
Dec 4 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bobsirip...@gmail.com
, Nov 7 2017