Bug 185322 – ASSERT(!childItemWithTarget(child->target())) is hit in HistoryItem::addChildItem()

• Status: RESOLVED
• Resolution: FIXED
• Component: History

[Chris Dumez]

ASSERT(!childItemWithTarget(child->target())) is hit in HistoryItem::addChildItem():
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x000000014faad070 WTFCrash + 16 (Assertions.cpp:261)
1   com.apple.WebCore             	0x000000014213064d WebCore::HistoryItem::addChildItem(WTF::Ref<WebCore::HistoryItem, WTF::DumbPtrTraits<WebCore::HistoryItem> >&&) + 253 (HistoryItem.cpp:316)
2   com.apple.WebCore             	0x000000014264c7c0 WebCore::HistoryController::createItemTree(WebCore::Frame&, bool) + 416 (HistoryController.cpp:718)
3   com.apple.WebCore             	0x0000000142649990 WebCore::HistoryController::updateBackForwardListClippedAtTarget(bool) + 208 (HistoryController.cpp:821)
4   com.apple.WebCore             	0x000000014264ae7f WebCore::HistoryController::updateForStandardLoad(WebCore::HistoryController::HistoryUpdateType) + 335 (HistoryController.cpp:392)
5   com.apple.WebCore             	0x000000014262879a WebCore::FrameLoader::transitionToCommitted(WebCore::CachedPage*) + 1082 (FrameLoader.cpp:2069)
6   com.apple.WebCore             	0x000000014262787e WebCore::FrameLoader::commitProvisionalLoad() + 2142 (FrameLoader.cpp:1892)
7   com.apple.WebCore             	0x00000001425d095c WebCore::DocumentLoader::commitIfReady() + 60 (DocumentLoader.cpp:356)
8   com.apple.WebCore             	0x00000001425d6aac WebCore::DocumentLoader::commitLoad(char const*, int) + 76 (DocumentLoader.cpp:958)
9   com.apple.WebCore             	0x00000001425d6a4f WebCore::DocumentLoader::dataReceived(char const*, int) + 511 (DocumentLoader.cpp:1107)
10  com.apple.WebCore             	0x00000001425d7204 WebCore::DocumentLoader::dataReceived(WebCore::CachedResource&, char const*, int) + 116 (DocumentLoader.cpp:1080)

The unique frame names we generate are based on their position inside their parent and is not properly updated when the tree structure changes. As a result, we can end up with duplicate uniqueFrameNames and hit this assertion.

[Chris Dumez]

The issue seems to be:
1. We have a top frame with 8 subframes
-> Frame names are frame0 to frame7 based on their position in their parent
2. Then a new frame is inserted *before* the last one
-> it gets frame7 name and existing frame7 is NOT renamed to frame8. Therfore, we end up with a conflict.

[Chris Dumez]

Created attachment 339621 [details]
Patch

[EWS Watchlist]

Comment on attachment 339621 [details]
Patch

Attachment 339621 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/7571797

New failing tests:
webarchive/loading/object.html

[EWS Watchlist]

Created attachment 339624 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 339621 [details]
Patch

Attachment 339621 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/7571830

New failing tests:
webarchive/test-duplicate-resources.html
webarchive/adopt-attribute-styled-body-webarchive.html

[EWS Watchlist]

Created attachment 339628 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[Chris Dumez]

Created attachment 339630 [details]
Patch

[Geoffrey Garen]

Comment on attachment 339630 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=339630&action=review

r=me

> Source/WebCore/ChangeLog:10
> +        and for things like restoring form state from an HistoryItem.

a HistoryItem

> Source/WebCore/ChangeLog:15
> +        would not take care of updating existing Frames's unique name on frame tree mutation.

Frames'

[WebKit Commit Bot]

Comment on attachment 339630 [details]
Patch

Rejecting attachment 339630 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 339630, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
fs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 231439 = ee4ed66a0af3fd7fb8e7ee50defa9539e558fb05
r231440 = 9d20703ea1ea299423fc94fa79a632ba25479acd
r231441 = 9ab072da44e00798d9743d500585940a3f4332f9
r231442 = 79933c0d14e2a3dbc9f8abe637575ac1ef9362bc
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/7596117

[Chris Dumez]

Created attachment 339736 [details]
Patch

[Chris Dumez]

Comment on attachment 339736 [details]
Patch

Clearing flags on attachment: 339736

Committed r231450: <https://trac.webkit.org/changeset/231450>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/40031223>

[Ryan Haddad]

This change appears to have caused performance test failures. Dromaeo and Speedometer fail with ERROR: frame "<!--frame7-->" - has 1 onunload handler(s)
https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK2%20%28Perf%29/builds/772

[Chris Dumez]

(In reply to Ryan Haddad from comment #14)
> This change appears to have caused performance test failures. Dromaeo and
> Speedometer fail with ERROR: frame "<!--frame7-->" - has 1 onunload
> handler(s)
> https://build.webkit.org/builders/
> Apple%20Sierra%20Release%20WK2%20%28Perf%29/builds/772

Looking.

[Chris Dumez]

(In reply to Chris Dumez from comment #15)
> (In reply to Ryan Haddad from comment #14)
> > This change appears to have caused performance test failures. Dromaeo and
> > Speedometer fail with ERROR: frame "<!--frame7-->" - has 1 onunload
> > handler(s)
> > https://build.webkit.org/builders/
> > Apple%20Sierra%20Release%20WK2%20%28Perf%29/builds/772
> 
> Looking.

Oh, Tools/Scripts/webkitpy/performance_tests/perftest.py needs updating for new naming. Will land a fix shortly.

[Chris Dumez]

(In reply to Chris Dumez from comment #16)
> (In reply to Chris Dumez from comment #15)
> > (In reply to Ryan Haddad from comment #14)
> > > This change appears to have caused performance test failures. Dromaeo and
> > > Speedometer fail with ERROR: frame "<!--frame7-->" - has 1 onunload
> > > handler(s)
> > > https://build.webkit.org/builders/
> > > Apple%20Sierra%20Release%20WK2%20%28Perf%29/builds/772
> > 
> > Looking.
> 
> Oh, Tools/Scripts/webkitpy/performance_tests/perftest.py needs updating for
> new naming. Will land a fix shortly.

<https://trac.webkit.org/changeset/231474>