New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 748600 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

Chrome hang

Project Member Reported by a...@chromium.org, Jul 25 2017

Issue description

My Chrome Canary hung at:

content::WebContentsImpl::DidFinishNavigation(content::NavigationHandle*)
 sync_sessions::SyncSessionsWebContentsRouter::NotifyTabModified(content::WebContents*, bool)
  sync_sessions::SessionsSyncManager::OnLocalTabModified(sync_sessions::SyncedTabDelegate*)
   sync_sessions::SessionsSyncManager::TrackNavigationIds(sessions::SerializedNavigationEntry const&)
    syncer::UserEventSyncBridge::HandleGlobalIdChange(long long, long long)  (in Google Chrome Framework)

Full trace attached.
 
hang.txt
836 KB View Download

Comment 1 by s...@chromium.org, Jul 25 2017

Status: Started (was: Untriaged)

Comment 2 by a...@chromium.org, Jul 25 2017

This has happened again to me. Let me know if there is anything I can to do help figure this out.

Comment 3 by s...@chromium.org, Jul 25 2017

Fix posted https://chromium-review.googlesource.com/c/584554/

Sorry for the productivity loss, but thanks for the bug and stacktrace!

Comment 4 by s...@chromium.org, Jul 25 2017

Whoops, wrong link, Should have been https://chromium-review.googlesource.com/c/585332/ ... I'm having a day.
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 25 2017

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

commit b99830cc8f7df9632fc949daa233a78f824de61d
Author: Sky Malice <skym@chromium.org>
Date: Tue Jul 25 21:10:32 2017

[Sync] Fix iterator error in UserEventSyncBridge::HandleGlobalIdChange.

There were two mistakes in the previous code, and together they were
able to cause an infinite loop.

The first, and most egregious, was the iterating from
std::multimap::find() to multimap::end(). The intention was to only
iterate over the pairs with a given key, but instead all pairs were
iterated over with a key equal or greater. This did trigger the
DCHECK when we iterate onto an element with the wrong navigation_id,
but if DCHECKs were not on these events would incorrectly be
processed.

The second, and arguably not a bug if the former was working
correctly, was while this iter loop was happening, which deleted
items, we would call RecordUserEvent which would insert a new pair
into in_flight_nav_linked_events_ which we were iterating over. If
the new element had a key greater than or equal to our current iter
then we would encounter it. And thus we infinitely loop, erasing and
inserting what was essentially the same element over and over.

This fix removes both of these issues. multimap::equal_range() for
the former, and a temp vector for the latter.

Bug:  747621 ,  748600 
Change-Id: I2a48ee1f86c81eec787dfbf293ebabc239f11e5d
Reviewed-on: https://chromium-review.googlesource.com/585332
Reviewed-by: Patrick Noland <pnoland@chromium.org>
Commit-Queue: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489432}
[modify] https://crrev.com/b99830cc8f7df9632fc949daa233a78f824de61d/components/sync/user_events/user_event_sync_bridge.cc
[modify] https://crrev.com/b99830cc8f7df9632fc949daa233a78f824de61d/components/sync/user_events/user_event_sync_bridge_unittest.cc

Comment 6 by s...@chromium.org, Jul 25 2017

Status: Fixed (was: Started)

Sign in to add a comment