New issue
Advanced search Search tips

Issue 798836 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Task

Blocking:
issue 734150



Sign in to add a comment

RequestDesktopMobileSiteTestCase fails with WKBasedNavigationManager

Project Member Reported by danyao@chromium.org, Jan 3 2018

Issue description

A number of RequestDesktopMobileSiteTestCase egtests fail with the following error when using WKBasedNavigationManager:

../../ios/chrome/test/earl_grey/chrome_earl_grey.mm:184: error: -[RequestDesktopMobileSiteTestCase testRequestDesktopSiteDoesNotPropagateToNewTab] : Exception: AssertionFailedException
Exception Name: AssertionFailedException
Exception Reason: (([condition waitWithTimeout:testing::kWaitForUIElementTimeout]) is true) failed
Exception Details: Failed waiting for web view containing Desktop
Bundle ID: org.chromium.gtest.generic-unit-test
Stack Trace: (
	0   EarlGrey                            0x000000010dd565e6 -[GREYDefaultFailureHandler handleException:details:] + 1270
	1   ios_chrome_ui_egtests               0x0000000102a07818 +[ChromeEarlGrey waitForWebViewContainingText:] + 2344
	2   ios_chrome_ui_egtests               0x0000000102b845bc -[RequestDesktopMobileSiteTestCase testRequestDesktopSiteDoesNotPropagateToNewTab] + 2332

Affected test cases:
RequestDesktopMobileSiteTestCase testRequestDesktopSiteDoesNotPropagateToNewTab
RequestDesktopMobileSiteTestCase testRequestDesktopSitePropagatesToNextNavigations
RequestDesktopMobileSiteTestCase testRequestDesktopSiteGoBackToMobile
RequestDesktopMobileSiteTestCase testRequestMobileSiteGoBackToDesktop
RequestDesktopMobileSiteTestCase testAppVersionJSAPIWithDesktopUserAgent
RequestDesktopMobileSiteTestCase testAppVersionJSAPIWithMobileUserAgent
RequestDesktopMobileSiteTestCase testAppVersionJSAPIWithMobileUserAgent

Example test output:
https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.fyi%2Fios-simulator%2F13854%2F%2B%2Frecipes%2Fsteps%2Fios_chrome_ui_egtests__iPhone_6s_iOS_11.0_%2F0%2Flogs%2FRequestDesktopMobileSiteTestCase__x2f_testRequestMobileSitePropagatesToNextNavigations%2F0

 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d6704fdd14fd7b682858f14a36bfc5f9703922c8

commit d6704fdd14fd7b682858f14a36bfc5f9703922c8
Author: Danyao Wang <danyao@google.com>
Date: Fri Feb 16 02:31:13 2018

[Nav Experiment] Move ReloadWithUserAgentType into NavigationManager.

This is a code cleanup before changing ReloadWithUserAgentType() to work
also with WKBasedNavigationManager, which would not be appropriate to
add to tab.mm.

This CL also fixed an apparent bug where IsItemRedirectItem() in
tab.mm seemed to have always used the wrong polarity in detecting
redirect items.

Bug:  798836 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Id39b3788a566fc8b188bdceb3a02803e5e182364
Reviewed-on: https://chromium-review.googlesource.com/920101
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537113}
[modify] https://crrev.com/d6704fdd14fd7b682858f14a36bfc5f9703922c8/ios/chrome/browser/tabs/tab.mm
[modify] https://crrev.com/d6704fdd14fd7b682858f14a36bfc5f9703922c8/ios/chrome/browser/web/BUILD.gn
[delete] https://crrev.com/b825a4218938f22c9e685dabfff64b0957d3e420/ios/chrome/browser/web/navigation_manager_util.h
[delete] https://crrev.com/b825a4218938f22c9e685dabfff64b0957d3e420/ios/chrome/browser/web/navigation_manager_util.mm
[delete] https://crrev.com/b825a4218938f22c9e685dabfff64b0957d3e420/ios/chrome/browser/web/navigation_manager_util_unittest.mm
[modify] https://crrev.com/d6704fdd14fd7b682858f14a36bfc5f9703922c8/ios/web/navigation/legacy_navigation_manager_impl.h
[modify] https://crrev.com/d6704fdd14fd7b682858f14a36bfc5f9703922c8/ios/web/navigation/legacy_navigation_manager_impl.mm
[modify] https://crrev.com/d6704fdd14fd7b682858f14a36bfc5f9703922c8/ios/web/navigation/navigation_manager_impl.h
[modify] https://crrev.com/d6704fdd14fd7b682858f14a36bfc5f9703922c8/ios/web/navigation/navigation_manager_impl.mm
[modify] https://crrev.com/d6704fdd14fd7b682858f14a36bfc5f9703922c8/ios/web/navigation/navigation_manager_impl_unittest.mm
[modify] https://crrev.com/d6704fdd14fd7b682858f14a36bfc5f9703922c8/ios/web/public/navigation_manager.h
[modify] https://crrev.com/d6704fdd14fd7b682858f14a36bfc5f9703922c8/ios/web/public/test/fakes/test_navigation_manager.h
[modify] https://crrev.com/d6704fdd14fd7b682858f14a36bfc5f9703922c8/ios/web/public/test/fakes/test_navigation_manager.mm
[modify] https://crrev.com/d6704fdd14fd7b682858f14a36bfc5f9703922c8/ios/web/web_state/navigation_and_load_callbacks_inttest.mm

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/24290122602dfb9c689bd044c23f6ee9d1ecf188

commit 24290122602dfb9c689bd044c23f6ee9d1ecf188
Author: Danyao Wang <danyao@google.com>
Date: Fri Feb 16 05:14:32 2018

[Nav Experiment] Fix Request Desktop Page for WKBasedNavigationManager.

With LegacyNavigationManager, Tab calls |requirePageReconstruction|
to free the WKWebView so that it'll be recreated with the user agent of
the newest navigation item. This approach doesn't work for
WKBasedNavigationManager because deleting the WKWebView would require
reconstruction of history.

Luckily with iOS9+, WKWebView.customUserAgent can be updated on an
existing WKWebView and will cause the new header to be sent to the
server. This CL uses this mechanism to update the UA before loading
a request based on the current NavigationItem.

This alone only reloads the visible page with the new UA under
WKBasedNavigationManager because there is no URL change. So to create
a back/forward entry, the URL is reloaded using restore_session.html
with a client-side redirect.

This CL fixes RequestDesktopMobileSiteTestCase for new nav manager.

Bug:  798836 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I7f23232b10a7c83db16f545cd29445a514eaa57c
Reviewed-on: https://chromium-review.googlesource.com/919175
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537209}
[modify] https://crrev.com/24290122602dfb9c689bd044c23f6ee9d1ecf188/ios/web/navigation/BUILD.gn
[modify] https://crrev.com/24290122602dfb9c689bd044c23f6ee9d1ecf188/ios/web/navigation/navigation_manager_impl.mm
[modify] https://crrev.com/24290122602dfb9c689bd044c23f6ee9d1ecf188/ios/web/navigation/wk_based_restore_session_util.h
[modify] https://crrev.com/24290122602dfb9c689bd044c23f6ee9d1ecf188/ios/web/navigation/wk_based_restore_session_util.mm
[modify] https://crrev.com/24290122602dfb9c689bd044c23f6ee9d1ecf188/ios/web/navigation/wk_based_restore_session_util_unittest.mm
[modify] https://crrev.com/24290122602dfb9c689bd044c23f6ee9d1ecf188/ios/web/web_state/ui/crw_web_controller.mm

Comment 3 by danyao@chromium.org, Feb 16 2018

Status: Fixed (was: Available)

Sign in to add a comment