DCHECK hit in CRWWebController -webView:didReceiveAuthenticationChallenge:completionHandler: |
|||
Issue descriptionWith slim navigation manager enabled. What steps will reproduce the problem? (1) Open a NTP (2) Load a page (3) Interrupt the navigation by going back (before the end of the page load) What is the expected result? Everything should be fine. What happens instead? I hit the first DCHECK in . https://cs.chromium.org/chromium/src/ios/web/web_state/ui/crw_web_controller.mm?type=cs&sq=package:chromium&g=0&l=4917 I am able to reproduce it 1/5, but I hit it pretty often. Assigning to danyao@ as I hit it with the flag enabled.
,
Jul 24
,
Jul 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0f8a2964d432cf3a0cf213e35aaa26961eef20a8 commit 0f8a2964d432cf3a0cf213e35aaa26961eef20a8 Author: Danyao Wang <danyao@chromium.org> Date: Wed Jul 25 16:26:14 2018 [Nav Experiment] Remove DCHECK for incorrect assumption. Previously I assumed -webView:didReceivedAuthenticationChallenge callback is always called for the navigation associated with the current WKWebView.URL value. In testing, it turns out in back/forward navigation, this callback may be triggered for a subresource on the previous item after WKWebView.URL has already been updated to the new item. As a result, this DCHECK is incorrectly triggered when navigating back to NTP from a web page before the web page finishes loading completely. From code inspection, the implementation of this delegate call doesn't depend on states associated with self.currentNavItem. This scenario already exists between web pages. So the DCHECK is not protecting any meaningful state corruption. Bug: 864525 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I02a1678721e2db59ed1f288c9e4ec7139efc8632 Reviewed-on: https://chromium-review.googlesource.com/1147328 Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org> Commit-Queue: Danyao Wang <danyao@chromium.org> Cr-Commit-Position: refs/heads/master@{#577924} [modify] https://crrev.com/0f8a2964d432cf3a0cf213e35aaa26961eef20a8/ios/web/web_state/ui/crw_web_controller.mm
,
Jul 25
,
Jul 26
I found another DCHECK hit in |webView:didFailNavigation| that is wrong for a similar reason: https://cs.chromium.org/chromium/src/ios/web/web_state/ui/crw_web_controller.mm?l=4948
,
Jul 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c2c325c70e9688de84e5c5c32471ba711a2a18e7 commit c2c325c70e9688de84e5c5c32471ba711a2a18e7 Author: Danyao Wang <danyao@chromium.org> Date: Thu Jul 26 17:55:26 2018 [Nav Experiment] Update incorrect DCHECK assumption in didFailNavigation This DCHECK is meant to assert that placeholder navigation never fails. However, webView.URL may have reverted to the previous entry when a failure happens so is not always the URL that triggers the error. The correct approach is to check the error URL. Bug: 864525 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I4efb01041a39f87c4c1ac36115c93364bd41aff9 Reviewed-on: https://chromium-review.googlesource.com/1151391 Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org> Commit-Queue: Danyao Wang <danyao@chromium.org> Cr-Commit-Position: refs/heads/master@{#578359} [modify] https://crrev.com/c2c325c70e9688de84e5c5c32471ba711a2a18e7/ios/web/web_state/ui/crw_web_controller.mm |
|||
►
Sign in to add a comment |
|||
Comment 1 by danyao@chromium.org
, Jul 23