Bug 63945 – [WK2] IconDatabase: Add a way to notify when icon data is available

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Carlos Garcia Campos]

Currently WebIconDatabaseClient uses a single callback, didChangeIconForPageURL(), called when the icon has changed in the database. WebIconDatabase uses that callback when the icon data is available too. The problem is that didImportIconDataForPageURL() is never called in WebKit2 when the icon is already in the database, because when imported the list of pages interested in icons is empty. In Webkit1 we use FrameLoaderClient::dispatchDidReceiveIcon(), called when the icon has just been loaded or when it was in the database and the data was already available, and FrameLoaderClient::registerForIconNotification() called when the icon was in the database but the data wasn't available yet. Both methods are currently unimplemented in WebKit2, but I'm not sure the same approach of WebKit1 is the right one for WebKit2.

[Carlos Garcia Campos]

A possible solution would be to add a client callback to notify when the icon data is available (and imageForPageURL() will return a valid image) called from both didImportIconDataForPageURL() and setIconDataForIconURL()

[Mario Sanchez Prada]

Created attachment 163129 [details]
Patch proposal

Attaching a patch proposal for this issue.

The current patch is what I'm using now in a local branch to implement the favicons API for WebKit2GTK+, and has been apparently working fine (at least as good as I would expect) for the last week.

[WebKit Review Bot]

Attachment 163129 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1
Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:44:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mario Sanchez Prada]

(In reply to comment #3)
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1
> Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:44:  Alphabetical sorting problem.  [build/include_order] [4]
> Total errors found: 1 in 8 files

Oops! Sorry about this. I will fix it in a follow-up patch, but first I'd say it would be better to concentrate on reviewing the actual code :-)

[Mario Sanchez Prada]

Created attachment 163598 [details]
Patch proposal

Uploading a new patch here fixing the code style issue.

[Mario Sanchez Prada]

Ping reviewers?

[Mario Sanchez Prada]

Committed r129742: <http://trac.webkit.org/changeset/129742>

[Alexey Proskuryakov]

This broke nightly builds, bug 97784.

[Mario Sanchez Prada]

Committed r129789: <http://trac.webkit.org/changeset/129789>