New issue
Advanced search Search tips

Issue 759943 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

FATAL:web_state_impl.mm(662)] Check failed: equalURLs.

Project Member Reported by justincohen@chromium.org, Aug 29 2017

Issue description

- Visit washingtonpost.com
- Tap on top left hamburger icon
- Tap local

currentURLWithTrustLevel: https://www.washingtonpost.com/local/
GetLastCommittedURL: https://www.washingtonpost.com/local/?utm_term=.40381abbcd2a

#0  0x0000000105798468 in base::debug::BreakDebugger() at /base/debug/debugger_posix.cc:258
#1  0x00000001057ed800 in logging::LogMessage::~LogMessage() at /base/logging.cc:791
#2  0x00000001057ea9cc in logging::LogMessage::~LogMessage() at /base/logging.cc:554
#3  0x000000010472eae8 in web::WebStateImpl::GetCurrentURL(web::URLVerificationTrustLevel*) const at /ios/web/web_state/web_state_impl.mm:662
#4  0x0000000104afc248 in (anonymous namespace)::GetPageURLAndCheckTrustLevel(web::WebState*, GURL*) at /ios/chrome/browser/passwords/password_controller.mm:255
#5  0x0000000104afc00c in ::-[PasswordController webState:didLoadPageWithSuccess:](web::WebState *, BOOL) at /ios/chrome/browser/passwords/password_controller.mm:399
#6  0x000000010473c4cc in web::WebStateObserverBridge::PageLoaded(web::PageLoadCompletionStatus) at /ios/web/web_state/web_state_observer_bridge.mm:79
#7  0x0000000104729f8c in web::WebStateImpl::OnPageLoaded(GURL const&, bool) at /ios/web/web_state/web_state_impl.mm:260
#8  0x00000001046da6f4 in ::-[CRWWebController didFinishWithURL:loadSuccess:](const GURL &, BOOL) at /ios/web/web_state/ui/crw_web_controller.mm:2006
#9  0x00000001046da154 in ::-[CRWWebController loadCompleteWithSuccess:forNavigation:](BOOL, WKNavigation *) at /ios/web/web_state/ui/crw_web_controller.mm:1972
#10 0x00000001046d9a54 in ::-[CRWWebController didFinishNavigation:](WKNavigation *) at /ios/web/web_state/ui/crw_web_controller.mm:1941
#11 0x00000001046e36b4 in ::__70-[CRWWebController handleWindowHistoryDidReplaceStateMessage:context:]_block_invoke(id, NSError *) at /ios/web/web_state/ui/crw_web_controller.mm:2644

 
Labels: -Restrict-View-Google

Comment 2 by sczs@chromium.org, Aug 30 2017

Cc: -eugene...@chromium.org
Owner: eugene...@chromium.org
Status: Assigned (was: Untriaged)
eugenebut@ could you PTAL.
Cc: danyao@chromium.org
Components: UI>Browser>Navigation
Labels: -Pri-2 Proj-WKBackForwardList Pri-3
This bug happens because LastCommittedURL is different from _documentURL. The latter is  updated after some delay due to the way how replaceState API is implemented on iOS. 

The issue does not cause any functional or security impact because replaceState can not change the origin and the URLs are only different by query.

Justin, this will be fixed by Danyao's WKBackForwardList refactoring which will probably be finished in Q4. Please let me know if this bug bothers you and I will relax DCHECK by ignoring query part of the URL (we already ignore fragment for the purpose of DCHECK).
I find that debug builds 'crash' all the time.  When I check to see what the crash is it's almost always been this DCHECK.

I understand if you'd like to keep it, but if a lot of people are hitting it, and we know the fix, I'd prefer to have it removed.

Thanks!
Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 13 2017

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

commit 2d8340a4ff9a9dfa7070f07d9703837d35d08dad
Author: Eugene But <eugenebut@google.com>
Date: Wed Sep 13 16:59:47 2017

Use URL origins for Web.CurrentURLEqualsLastCommittedURL metric.

Metric and corresponding DCHECK was added to understand if
WebStateImpl::GetCurrentURL() calls can safely be replaced with
GetLastCommittedURL().

There are cases when these 2 URLs (current, last committed) get out of
sync, however it's not an issue as long as both URLs have the same
origin.

This CL relaxes DCHECK and metric to flag the URL difference only if the
origins of these 2 URLs (current, last committed) are different.

Bug:  759943 
Change-Id: I9f866e5d76253dece43fde55529e1b0633d4e09c
Reviewed-on: https://chromium-review.googlesource.com/660693
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501665}
[modify] https://crrev.com/2d8340a4ff9a9dfa7070f07d9703837d35d08dad/ios/web/web_state/web_state_impl.mm
[modify] https://crrev.com/2d8340a4ff9a9dfa7070f07d9703837d35d08dad/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Started)

Sign in to add a comment