[go: nahoru, domu]

[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()];