Commit 6836cfc5 authored by Justin Cohen's avatar Justin Cohen Committed by Commit Bot

Reland "[ios] Enable preload with slim-navigation-manager."

This reverts commit ceff1a21.

Reason for revert: Fixed.

Original change's description:
> Revert "[ios] Enable preload with slim-navigation-manager."
>
> This reverts commit 101b5ae0.
>
> Reason for revert: Needs some more work, some last minute
> refactoring introduced some crashes.
>
> Original change's description:
> > [ios] Enable preload with slim-navigation-manager.
> >
> > Previously, preload required CopyStateFromAndPrune and
> > CanPruneAllButLastCommittedItem within NavigationManager, but those
> > are not supported with SlimNav. Here, preload is implemented by
> > creating a preload WebState with navigation history by restoring the
> > current WebState.
> >
> > The performance of this implementation depends on Slim
> > Navigation Manager's session restoration performance which can be
> > measured with existing kRestoreNavigationTime metric.
> >
> > Original implementation of this CL was written by eugenebut@,
> > simply updated in this resurrected CL.
> >
> > Bug: 834116
> > Change-Id: I86c9ecf7e3ed213ebaeddce6264dff3bbb8a8c94
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1815702
> > Commit-Queue: Justin Cohen <justincohen@chromium.org>
> > Reviewed-by: Rohit Rao <rohitrao@chromium.org>
> > Reviewed-by: Eugene But <eugenebut@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#701228}
>
> TBR=rohitrao@chromium.org,justincohen@chromium.org,eugenebut@chromium.org,gambard@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 834116
> Change-Id: I16beb96bc2d1eab9a9c36a8e03728fd3cc23f035
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1833743
> Reviewed-by: Justin Cohen <justincohen@chromium.org>
> Commit-Queue: Justin Cohen <justincohen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#701746}

Change-Id: I52889208ed3a84c08c66742e72b524adce0560af
Bug: 834116
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1833706Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Reviewed-by: default avatarRohit Rao <rohitrao@chromium.org>
Auto-Submit: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702004}
parent cee6cec0
...@@ -32,6 +32,7 @@ ...@@ -32,6 +32,7 @@
#import "ios/web/public/navigation/web_state_policy_decider_bridge.h" #import "ios/web/public/navigation/web_state_policy_decider_bridge.h"
#include "ios/web/public/thread/web_thread.h" #include "ios/web/public/thread/web_thread.h"
#import "ios/web/public/ui/java_script_dialog_presenter.h" #import "ios/web/public/ui/java_script_dialog_presenter.h"
#include "ios/web/public/web_client.h"
#import "ios/web/public/web_state.h" #import "ios/web/public/web_state.h"
#import "ios/web/public/web_state_observer_bridge.h" #import "ios/web/public/web_state_observer_bridge.h"
#import "ios/web/web_state/ui/crw_web_controller.h" #import "ios/web/web_state/ui/crw_web_controller.h"
...@@ -322,7 +323,8 @@ class PreloadJavaScriptDialogPresenter : public web::JavaScriptDialogPresenter { ...@@ -322,7 +323,8 @@ class PreloadJavaScriptDialogPresenter : public web::JavaScriptDialogPresenter {
} }
- (std::unique_ptr<web::WebState>)releasePrerenderContents { - (std::unique_ptr<web::WebState>)releasePrerenderContents {
if (!_webState) if (!_webState ||
_webState->GetNavigationManager()->IsRestoreSessionInProgress())
return nullptr; return nullptr;
self.successfulPrerendersPerSessionCount++; self.successfulPrerendersPerSessionCount++;
...@@ -495,13 +497,20 @@ class PreloadJavaScriptDialogPresenter : public web::JavaScriptDialogPresenter { ...@@ -495,13 +497,20 @@ class PreloadJavaScriptDialogPresenter : public web::JavaScriptDialogPresenter {
self.prerenderedURL = self.scheduledURL; self.prerenderedURL = self.scheduledURL;
std::unique_ptr<PrerenderRequest> request = std::move(_scheduledRequest); std::unique_ptr<PrerenderRequest> request = std::move(_scheduledRequest);
if (!self.prerenderedURL.is_valid()) { web::WebState* webStateToReplace = [self.delegate webStateToReplace];
if (!self.prerenderedURL.is_valid() || !webStateToReplace) {
[self destroyPreviewContents]; [self destroyPreviewContents];
return; return;
} }
web::WebState::CreateParams createParams(self.browserState); web::WebState::CreateParams createParams(self.browserState);
_webState = web::WebState::Create(createParams); if (web::GetWebClient()->IsSlimNavigationManagerEnabled()) {
_webState = web::WebState::CreateWithStorageSession(
createParams, webStateToReplace->BuildSessionStorage());
} else {
_webState = web::WebState::Create(createParams);
}
// Add the preload controller as a policyDecider before other tab helpers, so // Add the preload controller as a policyDecider before other tab helpers, so
// that it can block the navigation if needed before other policy deciders // that it can block the navigation if needed before other policy deciders
// execute thier side effects (eg. AppLauncherTabHelper launching app). // execute thier side effects (eg. AppLauncherTabHelper launching app).
......
...@@ -12,6 +12,10 @@ ...@@ -12,6 +12,10 @@
// A protocol implemented by a delegate of PreloadController // A protocol implemented by a delegate of PreloadController
@protocol PreloadControllerDelegate @protocol PreloadControllerDelegate
// WebState from which preload controller should copy the session history.
// This web state will be replaced on successful preload.
- (web::WebState*)webStateToReplace;
// Should preload controller request a desktop site. // Should preload controller request a desktop site.
- (BOOL)preloadShouldUseDesktopUserAgent; - (BOOL)preloadShouldUseDesktopUserAgent;
......
...@@ -65,11 +65,6 @@ std::unique_ptr<net::test_server::HttpResponse> StandardResponse( ...@@ -65,11 +65,6 @@ std::unique_ptr<net::test_server::HttpResponse> StandardResponse(
@"Disabled for iPad due to alternate letters educational screen."); @"Disabled for iPad due to alternate letters educational screen.");
} }
if ([ChromeEarlGrey isSlimNavigationManagerEnabled]) {
// TODO(crbug.com/834116): Fix and enable this test.
EARL_GREY_TEST_DISABLED(@"Prerender is not supported by slim-nav yet.");
}
[ChromeEarlGrey clearBrowsingHistory]; [ChromeEarlGrey clearBrowsingHistory];
// Set server up. // Set server up.
int visitCounter = 0; int visitCounter = 0;
......
...@@ -39,13 +39,6 @@ void PrerenderService::StartPrerender(const GURL& url, ...@@ -39,13 +39,6 @@ void PrerenderService::StartPrerender(const GURL& url,
const web::Referrer& referrer, const web::Referrer& referrer,
ui::PageTransition transition, ui::PageTransition transition,
bool immediately) { bool immediately) {
// PrerenderService is not compatible with WKBasedNavigationManager because it
// loads the URL in a new WKWebView, which doesn't have the current session
// history. TODO(crbug.com/814789): decide whether PrerenderService needs to
// be supported after evaluating the performance impact in Finch experiment.
if (web::GetWebClient()->IsSlimNavigationManagerEnabled())
return;
[controller_ prerenderURL:url [controller_ prerenderURL:url
referrer:referrer referrer:referrer
transition:transition transition:transition
...@@ -64,6 +57,11 @@ bool PrerenderService::MaybeLoadPrerenderedURL( ...@@ -64,6 +57,11 @@ bool PrerenderService::MaybeLoadPrerenderedURL(
std::unique_ptr<web::WebState> new_web_state = std::unique_ptr<web::WebState> new_web_state =
[controller_ releasePrerenderContents]; [controller_ releasePrerenderContents];
if (!new_web_state) {
CancelPrerender();
return false;
}
DCHECK_NE(WebStateList::kInvalidIndex, web_state_list->active_index()); DCHECK_NE(WebStateList::kInvalidIndex, web_state_list->active_index());
web::NavigationManager* active_navigation_manager = web::NavigationManager* active_navigation_manager =
...@@ -79,8 +77,13 @@ bool PrerenderService::MaybeLoadPrerenderedURL( ...@@ -79,8 +77,13 @@ bool PrerenderService::MaybeLoadPrerenderedURL(
web::NavigationManager* new_navigation_manager = web::NavigationManager* new_navigation_manager =
new_web_state->GetNavigationManager(); new_web_state->GetNavigationManager();
if (new_navigation_manager->CanPruneAllButLastCommittedItem()) { bool slim_navigation_manager_enabled =
new_navigation_manager->CopyStateFromAndPrune(active_navigation_manager); web::GetWebClient()->IsSlimNavigationManagerEnabled();
if (new_navigation_manager->CanPruneAllButLastCommittedItem() ||
slim_navigation_manager_enabled) {
if (!slim_navigation_manager_enabled) {
new_navigation_manager->CopyStateFromAndPrune(active_navigation_manager);
}
loading_prerender_ = true; loading_prerender_ = true;
web_state_list->ReplaceWebStateAt(web_state_list->active_index(), web_state_list->ReplaceWebStateAt(web_state_list->active_index(),
std::move(new_web_state)); std::move(new_web_state));
......
...@@ -4757,6 +4757,10 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -4757,6 +4757,10 @@ NSString* const kBrowserViewControllerSnackbarCategory =
return [self userAgentType] == web::UserAgentType::DESKTOP; return [self userAgentType] == web::UserAgentType::DESKTOP;
} }
- (web::WebState*)webStateToReplace {
return self.currentWebState;
}
#pragma mark - NetExportTabHelperDelegate #pragma mark - NetExportTabHelperDelegate
- (void)netExportTabHelper:(NetExportTabHelper*)tabHelper - (void)netExportTabHelper:(NetExportTabHelper*)tabHelper
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment