Commit ceff1a21 authored by Justin Cohen's avatar Justin Cohen Committed by Commit Bot

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/+/1833743Reviewed-by: default avatarJustin Cohen <justincohen@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701746}
parent 71676a70
......@@ -71,11 +71,10 @@ class WebState;
// Returns whether |webState| is the WebState used for pre-rendering.
- (BOOL)isWebStatePrerendered:(web::WebState*)webState;
// Returns the currently prerendered WebState, or nil if it was not created
// while |oldWebState| was active, or nil if none exists. After this method is
// called, the PrerenderController reverts to a non-prerendering state.
- (std::unique_ptr<web::WebState>)releasePrerenderContentsForWebState:
(web::WebState*)oldWebState;
// Returns the currently prerendered WebState, or nil if none exists. After
// this method is called, the PrerenderController reverts to a non-prerendering
// state.
- (std::unique_ptr<web::WebState>)releasePrerenderContents;
@end
......
......@@ -32,7 +32,6 @@
#import "ios/web/public/navigation/web_state_policy_decider_bridge.h"
#include "ios/web/public/thread/web_thread.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_observer_bridge.h"
#import "ios/web/web_state/ui/crw_web_controller.h"
......@@ -187,10 +186,6 @@ class PreloadJavaScriptDialogPresenter : public web::JavaScriptDialogPresenter {
// there is no prerender scheduled.
@property(nonatomic, readonly) const GURL& scheduledURL;
// Contains the original webState that, for SlimNav, session history will be
// created from.
@property(nonatomic, assign) web::WebState* webStateToReplace;
// Whether or not the preference is enabled.
@property(nonatomic, getter=isPreferenceEnabled) BOOL preferenceEnabled;
......@@ -304,9 +299,6 @@ class PreloadJavaScriptDialogPresenter : public web::JavaScriptDialogPresenter {
}
[self removeScheduledPrerenderRequests];
self.webStateToReplace = [self.delegate webStateToReplace];
_scheduledRequest =
std::make_unique<PrerenderRequest>(url, transition, referrer);
......@@ -329,13 +321,10 @@ class PreloadJavaScriptDialogPresenter : public web::JavaScriptDialogPresenter {
return webState && _webState.get() == webState;
}
- (std::unique_ptr<web::WebState>)releasePrerenderContentsForWebState:
(web::WebState*)oldWebState {
if (!_webState || oldWebState != self.webStateToReplace ||
_webState->GetNavigationManager()->IsRestoreSessionInProgress())
- (std::unique_ptr<web::WebState>)releasePrerenderContents {
if (!_webState)
return nullptr;
self.webStateToReplace = nullptr;
self.successfulPrerendersPerSessionCount++;
[self recordReleaseMetrics];
[self removeScheduledPrerenderRequests];
......@@ -512,13 +501,7 @@ class PreloadJavaScriptDialogPresenter : public web::JavaScriptDialogPresenter {
}
web::WebState::CreateParams createParams(self.browserState);
if (web::GetWebClient()->IsSlimNavigationManagerEnabled()) {
_webState = web::WebState::CreateWithStorageSession(
createParams, self.webStateToReplace->BuildSessionStorage());
} else {
_webState = web::WebState::Create(createParams);
}
_webState = web::WebState::Create(createParams);
// Add the preload controller as a policyDecider before other tab helpers, so
// that it can block the navigation if needed before other policy deciders
// execute thier side effects (eg. AppLauncherTabHelper launching app).
......@@ -580,7 +563,6 @@ class PreloadJavaScriptDialogPresenter : public web::JavaScriptDialogPresenter {
self.prerenderedURL = GURL();
self.startTime = base::TimeTicks();
self.webStateToReplace = nullptr;
}
#pragma mark - Notification Helpers
......
......@@ -12,10 +12,6 @@
// A protocol implemented by a delegate of PreloadController
@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.
- (BOOL)preloadShouldUseDesktopUserAgent;
......
......@@ -116,7 +116,7 @@ TEST_F(PreloadControllerTest, DontPreloadNonWebURLs) {
referrer:kReferrer
transition:kTransition
immediately:YES];
EXPECT_FALSE([controller_ releasePrerenderContentsForWebState:nil]);
EXPECT_FALSE([controller_ releasePrerenderContents]);
// Attempt to prerender the NTP and verify that no WebState was created
// to preload.
......@@ -124,7 +124,7 @@ TEST_F(PreloadControllerTest, DontPreloadNonWebURLs) {
referrer:kReferrer
transition:kTransition
immediately:YES];
EXPECT_FALSE([controller_ releasePrerenderContentsForWebState:nil]);
EXPECT_FALSE([controller_ releasePrerenderContents]);
// Attempt to prerender the flags UI and verify that no WebState was created
// to preload.
......@@ -132,7 +132,7 @@ TEST_F(PreloadControllerTest, DontPreloadNonWebURLs) {
referrer:kReferrer
transition:kTransition
immediately:YES];
EXPECT_FALSE([controller_ releasePrerenderContentsForWebState:nil]);
EXPECT_FALSE([controller_ releasePrerenderContents]);
}
TEST_F(PreloadControllerTest, TestIsPrerenderingEnabled_preloadAlways) {
......
......@@ -65,6 +65,11 @@ std::unique_ptr<net::test_server::HttpResponse> StandardResponse(
@"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];
// Set server up.
int visitCounter = 0;
......
......@@ -39,6 +39,13 @@ void PrerenderService::StartPrerender(const GURL& url,
const web::Referrer& referrer,
ui::PageTransition transition,
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
referrer:referrer
transition:transition
......@@ -55,18 +62,12 @@ bool PrerenderService::MaybeLoadPrerenderedURL(
return false;
}
web::WebState* web_state = web_state_list->GetActiveWebState();
std::unique_ptr<web::WebState> new_web_state =
[controller_ releasePrerenderContentsForWebState:web_state];
if (!new_web_state) {
CancelPrerender();
return false;
}
[controller_ releasePrerenderContents];
DCHECK_NE(WebStateList::kInvalidIndex, web_state_list->active_index());
web::NavigationManager* active_navigation_manager =
web_state->GetNavigationManager();
web_state_list->GetActiveWebState()->GetNavigationManager();
int lastIndex = active_navigation_manager->GetLastCommittedItemIndex();
UMA_HISTOGRAM_COUNTS_100("Prerender.PrerenderLoadedOnIndex", lastIndex);
......@@ -78,13 +79,8 @@ bool PrerenderService::MaybeLoadPrerenderedURL(
web::NavigationManager* new_navigation_manager =
new_web_state->GetNavigationManager();
bool slim_navigation_manager_enabled =
web::GetWebClient()->IsSlimNavigationManagerEnabled();
if (new_navigation_manager->CanPruneAllButLastCommittedItem() ||
slim_navigation_manager_enabled) {
if (!slim_navigation_manager_enabled) {
new_navigation_manager->CopyStateFromAndPrune(active_navigation_manager);
}
if (new_navigation_manager->CanPruneAllButLastCommittedItem()) {
new_navigation_manager->CopyStateFromAndPrune(active_navigation_manager);
loading_prerender_ = true;
web_state_list->ReplaceWebStateAt(web_state_list->active_index(),
std::move(new_web_state));
......
......@@ -4757,10 +4757,6 @@ NSString* const kBrowserViewControllerSnackbarCategory =
return [self userAgentType] == web::UserAgentType::DESKTOP;
}
- (web::WebState*)webStateToReplace {
return self.currentWebState;
}
#pragma mark - NetExportTabHelperDelegate
- (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