76009 2012-01-10 17:02:58 -0800 Unlimited history items saved in WebProcessProxy 2021-03-31 09:05:24 -0700 1 1 1 Unclassified WebKit History 528+ (Nightly build) All All UNCONFIRMED P2 Normal --- 0 eglerik webkit-unassigned beidson darin jjjaquanrice sam webkit.review.bot oldest_to_newest 533497 0 eglerik 2012-01-10 17:02:58 -0800 m_backForwardListItemMap in WebProcessProxy can accumulate WebBackForwardListItem elements until WebKit runs out of available memory. There is a maximum capacity enforced on the WebBackForwardList class, but the same history items saved in the WebProcessProxy have no such cap. Steps to Reproduce: 1) Go to any webpage 2) Go to a different webpage 3) Go back to 1) and repeat Steps The size of WebProcessProxy::m_backForwardListItemMap grows with each new site visited, while the size of WebBackForwardList::m_entries stops at "DefaultCapacity" defined in WebBackForwardList.cpp. Produced bug on Windows and Mac OSX SVN Revision: 104601 I plan to submit a patch shortly. 534444 1 122139 eglerik 2012-01-11 17:29:09 -0800 Created attachment 122139 Patch 538833 2 122139 eglerik 2012-01-19 11:13:49 -0800 Comment on attachment 122139 Patch This is a simple fix: - One new function that simply calls HashMap's remove function. - The call of the new function where the same history element is currently being cleaned up for a separate copy of the data. 695255 3 darin 2012-08-14 12:36:24 -0700 Sam, Brady, I remember discussing this a while back. I thought that there was a reason that the history item could still matter even if it’s not still in the back/forward list. For example, a client might still be holding a reference to it. 695310 4 beidson 2012-08-14 13:10:53 -0700 (In reply to comment #3) > Sam, Brady, I remember discussing this a while back. I thought that there was a reason that the history item could still matter even if it’s not still in the back/forward list. For example, a client might still be holding a reference to it. I'm not sure. A glance at the code implies that if the client is correctly reff'ing the WKBackForwardListItem, removing it from this map won't destroy it. 698459 5 eglerik 2012-08-17 10:19:12 -0700 (In reply to comment #4) Currently, the only function that exposes m_backForwardListItemMap items outside of WebProcessProxy is WebProcessProxy::webBackForwardItem(uint64_t itemID). This is only interfaced with by the WebPageProxy class via the following 4 functions: WebPageProxy::shouldGoToBackForwardListItem(uint64_t itemID, ... WebPageProxy::willGoToBackForwardListItem(uint64_t itemID, ... WebPageProxy::backForwardAddItem(uint64_t itemID) WebPageProxy::backForwardGoToItem(uint64_t itemID, ... All the code paths I see for these functions get their itemID from WebBackForwardListProxy::historyItemToIDMap() WebBackForwardListProxy::historyItemToIDMap() enforces the capacity of history items via WebPageProxy::backForwardRemovedItem(uint64_t itemID), which is where I'm suggesting WebProcessProxy::m_backForwardListItemMap should also maintain the history capacity by removing the same items. call path for WebPageProxy::backForwardRemovedItem to WebBackForwardListProxy::removeItem: -------------------------------------------------------------------------- WebPageProxy::backForwardRemovedItem(uint64_t itemID) calls: process()->send(Messages::WebPage::DidRemoveBackForwardItem(itemID), m_pageID); WebPage::didRemoveBackForwardItem(uint64_t itemID) calls: WebBackForwardListProxy::removeItem(itemID); WebBackForwardListProxy::removeItem(itemID) removes itemID from historyItemToIDMap() 807760 6 122139 webkit.review.bot 2013-01-15 15:41:53 -0800 Comment on attachment 122139 Patch Rejecting attachment 122139 from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: xy.cpp Hunk #1 FAILED at 3415. 1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit2/UIProcess/WebPageProxy.cpp.rej patching file Source/WebKit2/UIProcess/WebProcessProxy.cpp Hunk #1 succeeded at 198 (offset 3 lines). patching file Source/WebKit2/UIProcess/WebProcessProxy.h Hunk #1 succeeded at 89 with fuzz 1 (offset 1 line). Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Darin Adler']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/15914026 122139 2012-01-11 17:29:09 -0800 2013-01-15 15:41:53 -0800 Patch bug-76009-20120111172908.patch text/plain 2788 eglerik SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwNDc3MykKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDEyLTAxLTExICBFcmlrIEhp bGwgIDxlZ2xlcmlrQGdtYWlsLmNvbT4KKworICAgICAgICBSZW1vdmVkIGhpc3RvcnkgaXRlbXMg ZnJvbSBXZWJQYWdlUHJveHkgdGhhdCB3ZXJlIG5vIGxvbmdlciBuZWVkZWQuCisKKyAgICAgICAg QnVnIDc2MDA5OiBVbmxpbWl0ZWQgaGlzdG9yeSBpdGVtcyBzYXZlZCBpbiBXZWJQcm9jZXNzUHJv eHkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTc2MDA5 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBVSVBy b2Nlc3MvV2ViUGFnZVByb3h5LmNwcDoKKyAgICAgICAgKFdlYktpdDo6V2ViUGFnZVByb3h5Ojpi YWNrRm9yd2FyZFJlbW92ZWRJdGVtKTogY2FsbCByZW1vdmVJdGVtIGZ1bmN0aW9uIGZyb20gV2Vi UHJvY2Vzc1Byb3h5CisgICAgICAgICogVUlQcm9jZXNzL1dlYlByb2Nlc3NQcm94eS5jcHA6IGFk ZGVkIGZ1bmN0aW9uIHRvIHJlbW92ZSBoaXN0b3J5IGl0ZW1zIGZyb20gbV9iYWNrRm9yd2FyZExp c3RJdGVtTWFwCisgICAgICAgIChXZWJLaXQ6OldlYlByb2Nlc3NQcm94eTo6cmVtb3ZlQmFja0Zv cndhcmRJdGVtKToKKyAgICAgICAgKiBVSVByb2Nlc3MvV2ViUHJvY2Vzc1Byb3h5Lmg6IHJlbW92 ZUJhY2tGb3J3YXJkSXRlbSBkZWNsYXJhdGlvbiBhZGRlZCAKKwogMjAxMi0wMS0xMSAgQmV0aCBE YWtpbiAgPGJkYWtpbkBhcHBsZS5jb20+CiAKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTc1OTA0CkluZGV4OiBTb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3Mv V2ViUGFnZVByb3h5LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3Mv V2ViUGFnZVByb3h5LmNwcAkocmV2aXNpb24gMTA0NjAxKQorKysgU291cmNlL1dlYktpdDIvVUlQ cm9jZXNzL1dlYlBhZ2VQcm94eS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTM0MTUsNiArMzQxNSw3 IEBAIHZvaWQgV2ViUGFnZVByb3h5OjpkaWRGaW5pc2hMb2FkaW5nRGF0YUYKIHZvaWQgV2ViUGFn ZVByb3h5OjpiYWNrRm9yd2FyZFJlbW92ZWRJdGVtKHVpbnQ2NF90IGl0ZW1JRCkKIHsKICAgICBw cm9jZXNzKCktPnNlbmQoTWVzc2FnZXM6OldlYlBhZ2U6OkRpZFJlbW92ZUJhY2tGb3J3YXJkSXRl bShpdGVtSUQpLCBtX3BhZ2VJRCk7CisgICAgcHJvY2VzcygpLT5yZW1vdmVCYWNrRm9yd2FyZEl0 ZW0oaXRlbUlEKTsKIH0KIAogdm9pZCBXZWJQYWdlUHJveHk6OmJlZ2luUHJpbnRpbmcoV2ViRnJh bWVQcm94eSogZnJhbWUsIGNvbnN0IFByaW50SW5mbyYgcHJpbnRJbmZvKQpJbmRleDogU291cmNl L1dlYktpdDIvVUlQcm9jZXNzL1dlYlByb2Nlc3NQcm94eS5jcHAKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL1dlYktpdDIvVUlQcm9jZXNzL1dlYlByb2Nlc3NQcm94eS5jcHAJKHJldmlzaW9uIDEwNDYw MSkKKysrIFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJQcm9jZXNzUHJveHkuY3BwCSh3b3Jr aW5nIGNvcHkpCkBAIC0xOTUsNiArMTk1LDExIEBAIFdlYkJhY2tGb3J3YXJkTGlzdEl0ZW0qIFdl YlByb2Nlc3NQcm94eToKICAgICByZXR1cm4gbV9iYWNrRm9yd2FyZExpc3RJdGVtTWFwLmdldChp dGVtSUQpLmdldCgpOwogfQogCit2b2lkIFdlYlByb2Nlc3NQcm94eTo6cmVtb3ZlQmFja0Zvcndh cmRJdGVtKHVpbnQ2NF90IGl0ZW1JRCkKK3sKKyAgICBtX2JhY2tGb3J3YXJkTGlzdEl0ZW1NYXAu cmVtb3ZlKGl0ZW1JRCk7Cit9CisKIHZvaWQgV2ViUHJvY2Vzc1Byb3h5OjpyZWdpc3Rlck5ld1dl YkJhY2tGb3J3YXJkTGlzdEl0ZW0oV2ViQmFja0ZvcndhcmRMaXN0SXRlbSogaXRlbSkKIHsKICAg ICAvLyBUaGlzIGl0ZW0gd2FzIGp1c3QgY3JlYXRlZCBieSB0aGUgVUlQcm9jZXNzIGFuZCBpcyBi ZWluZyBhZGRlZCB0byB0aGUgbWFwIGZvciB0aGUgZmlyc3QgdGltZQpJbmRleDogU291cmNlL1dl YktpdDIvVUlQcm9jZXNzL1dlYlByb2Nlc3NQcm94eS5oCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9X ZWJLaXQyL1VJUHJvY2Vzcy9XZWJQcm9jZXNzUHJveHkuaAkocmV2aXNpb24gMTA0NjAxKQorKysg U291cmNlL1dlYktpdDIvVUlQcm9jZXNzL1dlYlByb2Nlc3NQcm94eS5oCSh3b3JraW5nIGNvcHkp CkBAIC04OCw2ICs4OCw3IEBAIHB1YmxpYzoKICAgICB2b2lkIHJlbW92ZVdlYlBhZ2UodWludDY0 X3QgcGFnZUlEKTsKIAogICAgIFdlYkJhY2tGb3J3YXJkTGlzdEl0ZW0qIHdlYkJhY2tGb3J3YXJk SXRlbSh1aW50NjRfdCBpdGVtSUQpIGNvbnN0OworICAgIHZvaWQgcmVtb3ZlQmFja0ZvcndhcmRJ dGVtKHVpbnQ2NF90IGl0ZW1JRCk7CiAKICAgICBSZXNwb25zaXZlbmVzc1RpbWVyKiByZXNwb25z aXZlbmVzc1RpbWVyKCkgeyByZXR1cm4gJm1fcmVzcG9uc2l2ZW5lc3NUaW1lcjsgfQogCg==