Commit 07fcf075 authored by Danyao Wang's avatar Danyao Wang Committed by Commit Bot

[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: default avatarKurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577534}
parent dccb439b
...@@ -73,6 +73,11 @@ class NavigationContextImpl : public NavigationContext { ...@@ -73,6 +73,11 @@ class NavigationContextImpl : public NavigationContext {
bool IsLoadingErrorPage() const; bool IsLoadingErrorPage() const;
void SetLoadingErrorPage(bool is_loading_error_page); 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: private:
NavigationContextImpl(WebState* web_state, NavigationContextImpl(WebState* web_state,
const GURL& url, const GURL& url,
...@@ -95,6 +100,7 @@ class NavigationContextImpl : public NavigationContext { ...@@ -95,6 +100,7 @@ class NavigationContextImpl : public NavigationContext {
int navigation_item_unique_id_ = -1; int navigation_item_unique_id_ = -1;
WKNavigationType wk_navigation_type_ = WKNavigationTypeOther; WKNavigationType wk_navigation_type_ = WKNavigationTypeOther;
bool is_loading_error_page_ = false; bool is_loading_error_page_ = false;
bool is_native_content_presented_ = false;
DISALLOW_COPY_AND_ASSIGN(NavigationContextImpl); DISALLOW_COPY_AND_ASSIGN(NavigationContextImpl);
}; };
......
...@@ -158,6 +158,15 @@ void NavigationContextImpl::SetLoadingErrorPage(bool is_loading_error_page) { ...@@ -158,6 +158,15 @@ void NavigationContextImpl::SetLoadingErrorPage(bool is_loading_error_page) {
is_loading_error_page_ = is_loading_error_page; 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, NavigationContextImpl::NavigationContextImpl(WebState* web_state,
const GURL& url, const GURL& url,
bool has_user_gesture, bool has_user_gesture,
......
...@@ -568,14 +568,13 @@ NSError* WKWebViewErrorWithSource(NSError* error, WKWebViewErrorSource source) { ...@@ -568,14 +568,13 @@ NSError* WKWebViewErrorWithSource(NSError* error, WKWebViewErrorSource source) {
- (void)updateCurrentBackForwardListItemHolder; - (void)updateCurrentBackForwardListItemHolder;
// Presents native content using the native controller for |item| without // Presents native content using the native controller for |item| without
// notifying WebStateObservers. This is used when loading native view for a new // notifying WebStateObservers. This method does not modify the underlying web
// navigation to avoid the delay introduced by placeholder navigation. // 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; - (void)presentNativeContentForNavigationItem:(web::NavigationItem*)item;
// Presents native content using the native controller for |item| and notifies // Notifies WebStateObservers the completion of this navigation.
// WebStateObservers the completion of this navigation. This method does not - (void)didLoadNativeContentForNavigationItem:(web::NavigationItem*)item;
// modify the underlying web view. It simply covers the web view with the native
// content.
- (void)loadNativeContentForNavigationItem:(web::NavigationItem*)item;
// Loads a blank page directly into WKWebView as a placeholder for a Native View // 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>. // or WebUI URL. This page has the URL about:blank?for=<encoded original URL>.
// The completion handler is called in the |webView:didFinishNavigation| // The completion handler is called in the |webView:didFinishNavigation|
...@@ -1799,12 +1798,15 @@ registerLoadRequestForURL:(const GURL&)requestURL ...@@ -1799,12 +1798,15 @@ registerLoadRequestForURL:(const GURL&)requestURL
if (!web::GetWebClient()->IsSlimNavigationManagerEnabled()) { if (!web::GetWebClient()->IsSlimNavigationManagerEnabled()) {
// Free the web view. // Free the web view.
[self removeWebView]; [self removeWebView];
[self loadNativeContentForNavigationItem:self.currentNavItem]; [self presentNativeContentForNavigationItem:self.currentNavItem];
[self didLoadNativeContentForNavigationItem:self.currentNavItem];
} else { } else {
// Just present the native view now. Leave the rest of native content load // Just present the native view now. Leave the rest of native content load
// until the placeholder navigation finishes. // until the placeholder navigation finishes.
[self presentNativeContentForNavigationItem:self.currentNavItem]; [self presentNativeContentForNavigationItem:self.currentNavItem];
[self loadPlaceholderInWebViewForURL:self.currentNavItem->GetVirtualURL()]; web::NavigationContextImpl* context = [self
loadPlaceholderInWebViewForURL:self.currentNavItem->GetVirtualURL()];
context->SetIsNativeContentPresented(true);
} }
} }
...@@ -1826,9 +1828,7 @@ registerLoadRequestForURL:(const GURL&)requestURL ...@@ -1826,9 +1828,7 @@ registerLoadRequestForURL:(const GURL&)requestURL
} }
} }
- (void)loadNativeContentForNavigationItem:(web::NavigationItem*)item { - (void)didLoadNativeContentForNavigationItem:(web::NavigationItem*)item {
[self presentNativeContentForNavigationItem:item];
const GURL targetURL = item ? item->GetURL() : GURL::EmptyGURL(); const GURL targetURL = item ? item->GetURL() : GURL::EmptyGURL();
const web::Referrer referrer; const web::Referrer referrer;
std::unique_ptr<web::NavigationContextImpl> navigationContext = std::unique_ptr<web::NavigationContextImpl> navigationContext =
...@@ -4900,7 +4900,12 @@ registerLoadRequestForURL:(const GURL&)requestURL ...@@ -4900,7 +4900,12 @@ registerLoadRequestForURL:(const GURL&)requestURL
} }
if ([self shouldLoadURLInNativeView:item->GetURL()]) { 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) { } else if (isWebUIURL) {
DCHECK(_webUIManager); DCHECK(_webUIManager);
[_webUIManager loadWebUIForURL:item->GetURL()]; [_webUIManager loadWebUIForURL:item->GetURL()];
......
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