[Nav Experiment] Remove duplicate presentation of native content.
Native content is typically presented after its placeholder navigation
to ensure correct ordering of WebStateObserver callbacks. However, for
UI smoothness when opening a new tab, native content is presented before
its placeholder navigation as well (http://crbug.com/819606). This CL
removes the duplicate presentation of native content after its
placeholder navigation, by storing a flag in NavigationContextImpl that
the native content is already presented.
Bug: 865422
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I71dc25f6a3284c34cf946800dee9bb4581506370
Reviewed-on: https://chromium-review.googlesource.com/1146777
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577534}
diff --git a/ios/web/web_state/navigation_context_impl.h b/ios/web/web_state/navigation_context_impl.h
index 0af55de..76e1034 100644
--- a/ios/web/web_state/navigation_context_impl.h
+++ b/ios/web/web_state/navigation_context_impl.h
@@ -73,6 +73,11 @@
bool IsLoadingErrorPage() const;
void SetLoadingErrorPage(bool is_loading_error_page);
+ // true if this navigation context is a placeholder navigation associated with
+ // a native view URL and the native content is already presented.
+ bool IsNativeContentPresented() const;
+ void SetIsNativeContentPresented(bool is_native_content_presented);
+
private:
NavigationContextImpl(WebState* web_state,
const GURL& url,
@@ -95,6 +100,7 @@
int navigation_item_unique_id_ = -1;
WKNavigationType wk_navigation_type_ = WKNavigationTypeOther;
bool is_loading_error_page_ = false;
+ bool is_native_content_presented_ = false;
DISALLOW_COPY_AND_ASSIGN(NavigationContextImpl);
};
diff --git a/ios/web/web_state/navigation_context_impl.mm b/ios/web/web_state/navigation_context_impl.mm
index f7c20a17..322929a 100644
--- a/ios/web/web_state/navigation_context_impl.mm
+++ b/ios/web/web_state/navigation_context_impl.mm
@@ -158,6 +158,15 @@
is_loading_error_page_ = is_loading_error_page;
}
+bool NavigationContextImpl::IsNativeContentPresented() const {
+ return is_native_content_presented_;
+}
+
+void NavigationContextImpl::SetIsNativeContentPresented(
+ bool is_native_content_presented) {
+ is_native_content_presented_ = is_native_content_presented;
+}
+
NavigationContextImpl::NavigationContextImpl(WebState* web_state,
const GURL& url,
bool has_user_gesture,
diff --git a/ios/web/web_state/ui/crw_web_controller.mm b/ios/web/web_state/ui/crw_web_controller.mm
index a4c59f1..1e71cff 100644
--- a/ios/web/web_state/ui/crw_web_controller.mm
+++ b/ios/web/web_state/ui/crw_web_controller.mm
@@ -568,14 +568,13 @@
- (void)updateCurrentBackForwardListItemHolder;
// Presents native content using the native controller for |item| without
-// notifying WebStateObservers. This is used when loading native view for a new
-// navigation to avoid the delay introduced by placeholder navigation.
+// notifying WebStateObservers. This method does not modify the underlying web
+// view. It simply covers the web view with the native content.
+// |-didLoadNativeContentForNavigationItem| must be called some time later
+// to notify WebStateObservers.
- (void)presentNativeContentForNavigationItem:(web::NavigationItem*)item;
-// Presents native content using the native controller for |item| and notifies
-// WebStateObservers the completion of this navigation. This method does not
-// modify the underlying web view. It simply covers the web view with the native
-// content.
-- (void)loadNativeContentForNavigationItem:(web::NavigationItem*)item;
+// Notifies WebStateObservers the completion of this navigation.
+- (void)didLoadNativeContentForNavigationItem:(web::NavigationItem*)item;
// Loads a blank page directly into WKWebView as a placeholder for a Native View
// or WebUI URL. This page has the URL about:blank?for=<encoded original URL>.
// The completion handler is called in the |webView:didFinishNavigation|
@@ -1799,12 +1798,15 @@
if (!web::GetWebClient()->IsSlimNavigationManagerEnabled()) {
// Free the web view.
[self removeWebView];
- [self loadNativeContentForNavigationItem:self.currentNavItem];
+ [self presentNativeContentForNavigationItem:self.currentNavItem];
+ [self didLoadNativeContentForNavigationItem:self.currentNavItem];
} else {
// Just present the native view now. Leave the rest of native content load
// until the placeholder navigation finishes.
[self presentNativeContentForNavigationItem:self.currentNavItem];
- [self loadPlaceholderInWebViewForURL:self.currentNavItem->GetVirtualURL()];
+ web::NavigationContextImpl* context = [self
+ loadPlaceholderInWebViewForURL:self.currentNavItem->GetVirtualURL()];
+ context->SetIsNativeContentPresented(true);
}
}
@@ -1826,9 +1828,7 @@
}
}
-- (void)loadNativeContentForNavigationItem:(web::NavigationItem*)item {
- [self presentNativeContentForNavigationItem:item];
-
+- (void)didLoadNativeContentForNavigationItem:(web::NavigationItem*)item {
const GURL targetURL = item ? item->GetURL() : GURL::EmptyGURL();
const web::Referrer referrer;
std::unique_ptr<web::NavigationContextImpl> navigationContext =
@@ -4900,7 +4900,12 @@
}
if ([self shouldLoadURLInNativeView:item->GetURL()]) {
- [self loadNativeContentForNavigationItem:item];
+ // Native content may have already been presented if this navigation is
+ // started in |-loadCurrentURLInNativeView|. If not, present it now.
+ if (!context || !context->IsNativeContentPresented()) {
+ [self presentNativeContentForNavigationItem:item];
+ }
+ [self didLoadNativeContentForNavigationItem:item];
} else if (isWebUIURL) {
DCHECK(_webUIManager);
[_webUIManager loadWebUIForURL:item->GetURL()];