Bug 132805 – Need function to copy favicon data without image conversion.

• Component: WebKit2
• Status: RESOLVED FIXED

[Gordon Sheridan]

With WKIconDatabaseSetIconDataForIconURL(), we have a function that can *set* the data for an icon in the destination database.

In order to copy a favicon between icon databases, without potential image conversion, we need a function that can get a copy of the data for an icon in the source database.

[Gordon Sheridan]

Created attachment 231264 [details]
Patch

[Darin Adler]

Comment on attachment 231264 [details]
Patch

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

> Source/WebKit2/UIProcess/API/C/WKIconDatabase.cpp:77
> +    WebCore::Image *image = toImpl(iconDatabaseRef)->imageForPageURL(toWTFString(pageURL));

Formatted wrong. Should be Image* rather than Image * with a space.

> Source/WebKit2/UIProcess/API/C/WKIconDatabase.cpp:79
> +        return 0;

Should be nullptr.

> Source/WebKit2/UIProcess/API/C/WKIconDatabase.cpp:81
> +    WebCore::SharedBuffer *iconData = image->data();

Formatted wrong. Should be SharedBuffer* rather than SharedBuffer * with a space.

[Gordon Sheridan]

Thanks Darin!

I'll update it and post a new patch.

[Gordon Sheridan]

Created attachment 231265 [details]
Patch

[Gordon Sheridan]

Comment on attachment 231265 [details]
Patch

I need to update the patch handle when there is no image data.

[Gordon Sheridan]

Created attachment 231268 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 231268 [details]
Patch

Clearing flags on attachment: 231268

Committed r168609: <http://trac.webkit.org/changeset/168609>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 231268 [details]
Patch

There is one thing wrong with this patch that I did not notice until after it was landed.

Generally, API files are not supposed to have substantive code in them. They just have bindings.

So this patch should have added a copyIconDataForPageURL member function to WebIconDatabase, rather than putting the code into the bindings file itself. Then the bindings file would just have the binding.

[Gordon Sheridan]

I can submit a new patch. Can this bug be reopened for that, or should I create a new bug?

[Darin Adler]

(In reply to comment #10)
> I can submit a new patch. Can this bug be reopened for that, or should I create a new bug?

I took care of it and did some other icon database improvements. It’s in bug 132812.