New issue
Advanced search Search tips

Issue 798832 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 734150



Sign in to add a comment

windowID injection timed out in egtest with WKBasedNavigationManager

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

Issue description

A number of egtests fail with the following error when new navigation manager is turned on:

ActivityServiceControllerTestCase/testActivityServiceControllerIsDisabled:
../../ios/web/public/test/earl_grey/js_test_util.mm:55: error: -[ActivityServiceControllerTestCase testActivityServiceControllerIsDisabled] : Exception: AssertionFailedException
Exception Name: AssertionFailedException
Exception Reason: ((is_timeout) is false) failed
Exception Details: windowID injection timed out
Bundle ID: org.chromium.gtest.generic-unit-test
Stack Trace: (
	0   EarlGrey                            0x000000010dd565e6 -[GREYDefaultFailureHandler handleException:details:] + 1270
	1   ios_chrome_ui_egtests               0x0000000102a14ce8 _ZN3web25WaitUntilWindowIdInjectedEPNS_8WebStateE + 2488
	2   ios_chrome_ui_egtests               0x0000000102a04812 +[ChromeEarlGrey loadURL:] + 98
	3   ios_chrome_ui_egtests               0x0000000102a6ff26 -[ActivityServiceControllerTestCase testActivityServiceControllerIsDisabled] + 118

The failure is triggered when calling [ChromeEarlGrey loadURL:] on a URL that requires either WebUI or native controller. |loadURL:| calls web::WaitUntilWindowIdInjected(webState) which tries to evaluate some JavaScript in the web view. Under new navigation manager, the javascript is evaluated in the placeholder about:blank page. Because Window ID is not injected into the placeholder page (which is probably a bug), loadURL never completes.

The following tests are affected:

ios_chrome_ui_egtests:
ActivityServiceControllerTestCase testActivityServiceControllerIsDisabled
ToolbarTestCase testTypeJavaScriptIntoOmniboxWithWebUIPage
ToolbarTestCase testToolbarUI
FullscreenTestCase testChromeToChromeURLKeepsHeaderOnScreen
WebUITestCase testVersion
WebUITestCase testChromeURLNavigateToTerms
WebUITestCase testChromeURLBackAndForwardNavigation
WebUITestCase testChromeURLsLoadWithoutError
WebUITestCase testChromeURLBackNavigationFromAnchorClick




 
More test cases:

ios_chrome_integration_egtests:
FeatureEngagementTestCase testNewTabTipPromoShouldShow

ios_chrome_web_egtests:
NavigationTestCase testHistoryForwardToErrorPage
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 3 2018

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

commit 8df7c018a5c81f3aec41b215b1faea1aa16963e8
Author: Danyao Wang <danyao@chromium.org>
Date: Wed Jan 03 22:45:09 2018

[Nav Experiment] Set back-forward bit in NavigationItem transition type.

Fix a bug where WKBasedNavigationManager is not setting the
ui::PAGE_TRANSITION_FORWARD_BACK bit in an existing NavigationItem in
history navigation.

The legacy navigation manager doesn't set this bit for same-document
back-forward navigation. This seems to be a bug, but will be fixed in
a separate CL to make possible bisecting easier.

Bug:  798832 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I2967256a32027384e16b03c24fdc0ee899a3d61d
Reviewed-on: https://chromium-review.googlesource.com/849352
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526844}
[modify] https://crrev.com/8df7c018a5c81f3aec41b215b1faea1aa16963e8/ios/web/navigation/navigation_manager_impl_unittest.mm
[modify] https://crrev.com/8df7c018a5c81f3aec41b215b1faea1aa16963e8/ios/web/navigation/wk_based_navigation_manager_impl.mm

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 4 2018

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

commit 63b4eb1add4a939788be71bacd314013b123f6db
Author: Danyao Wang <danyao@chromium.org>
Date: Thu Jan 04 21:51:04 2018

[ios] Fix transition type for same-document back/forward navigation.

The ui::PAGE_TRANSITION_FORWARD_BACK bit has always been set for
different-document back/forward navigation. It's not clear why
same-document back/forward navigation should be treated differently.
Making this change matches the Blink behavior (so should be low risk)
and removes a special case.

Bug:  798832 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I39e7a28caa9069cbb12ebdc965918792b36b77b2
Reviewed-on: https://chromium-review.googlesource.com/847940
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527105}
[modify] https://crrev.com/63b4eb1add4a939788be71bacd314013b123f6db/ios/web/navigation/legacy_navigation_manager_impl.mm
[modify] https://crrev.com/63b4eb1add4a939788be71bacd314013b123f6db/ios/web/navigation/navigation_manager_impl_unittest.mm

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 10 2018

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

commit a765fd8da6c81b4d74468ac6adf4cb7fcfa48f7d
Author: Danyao Wang <danyao@chromium.org>
Date: Wed Jan 10 19:52:20 2018

[Nav Experiment] Remove placeholder URL shortcircuit in CRWWebController

So that windowID is injected into the placeholder page (which is
considered an HTML page). This is an invariant of
[ChromeEarlGrey loadURL:]

This change fixes the following egtest:
ActivityServiceControllerTestCase testActivityServiceControllerIsDisabled
WebUITestCase testVersion
ToolbarTestCase testTypeJavaScriptIntoOmniboxWithWebUIPage
ToolbarTestCase testToolbarUI

Bug:  798832 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If06049f63e60f2f7f3589d5d9938e45deae3fb0e
Reviewed-on: https://chromium-review.googlesource.com/858045
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528390}
[modify] https://crrev.com/a765fd8da6c81b4d74468ac6adf4cb7fcfa48f7d/ios/web/BUILD.gn
[modify] https://crrev.com/a765fd8da6c81b4d74468ac6adf4cb7fcfa48f7d/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/a765fd8da6c81b4d74468ac6adf4cb7fcfa48f7d/ios/web/web_state/web_state_impl.mm
[modify] https://crrev.com/a765fd8da6c81b4d74468ac6adf4cb7fcfa48f7d/ios/web/web_state/web_state_impl_unittest.mm

Comment 5 by danyao@chromium.org, Jan 12 2018

Status: Fixed (was: Assigned)

Sign in to add a comment