WebCore IconDatabase retain counts can get unbalanced when WebContent process terminates

[Gordon Sheridan]

The WebCore IconDatabase lives in the UI process. The WebContent process sends RetainIconForPageURL/ReleaseIconForPageURL messages to the UI process via WebIconDatabaseProxy. When a page is closed, its associated HistoryItems are released, which in turn queue ReleaseIconForPageURL messages for their URLs. However, the process terminates before the messages are actually sent to the UI process, so the WebCore IconDatabase is not able to balance the retain counts.

[Gordon Sheridan]

Brady Eidson suggested WebProcessProxy the retain/releases in the UI process, and issue any remaining releases necessary when the associated process terminates. I expect to post a patch implementing this suggestion shortly.

[Gordon Sheridan]

Created attachment 250344 [details]
Patch to track IconDatabase retain counts for WebContent processes.

[Gordon Sheridan]

This is associated with rdar://problem/17388461.

[Brady Eidson]

Comment on attachment 250344 [details]
Patch to track IconDatabase retain counts for WebContent processes.

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

Looks great. Just a couple of tweaks needed.

> Source/WebKit2/UIProcess/WebProcessProxy.cpp:443
> +    if (!iconDatabase || pageURL.isNull())

Make this check isEmpty() instead of isNull()

> Source/WebKit2/UIProcess/WebProcessProxy.cpp:457
> +    if (!iconDatabase || pageURL.isNull())

Make this check isEmpty() instead of isNull()

> Source/WebKit2/UIProcess/WebProcessProxy.cpp:465
> +    --result->value;

If the new value is 0, we should remove the entry from the map.

Otherwise, a long lived WebProcess used for lots of navigations might build up a lot of leaked cruft inside this map.

[Gordon Sheridan]

Created attachment 250427 [details]
Updated patch to track IconDatabase retain counts for WebContent processes.

Updated patch to incorporate Brady's comments.

[WebKit Commit Bot]

Comment on attachment 250427 [details]
Updated patch to track IconDatabase retain counts for WebContent processes.

Clearing flags on attachment: 250427

Committed r182610: <http://trac.webkit.org/changeset/182610>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.