Bug 100379 – [Win] WebView didReceiveIcon delegate call doesn't have the icon in it

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

Pass HBITMAP favicon to load delegate.

Created attachment 170645 [details]
Patch as diff, output from svn-create-patch script.

[Alexey Proskuryakov]

<rdar://problem/5491010>

[Alexey Proskuryakov]

Would you be willing to submit a patch for inclusion in WebKit? Please follow the steps in <http://www.webkit.org/coding/contributing.html> - in particular, every patch needs a ChangeLog, and needs to be marked for review to be in review queue.

Yes, will submit a patch soon.

Created attachment 171249 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 171249 [details]
Patch

Brady should probably review this.

Any takers? :)

[Brent Fulgham]

Comment on attachment 171249 [details]
Patch

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

Looks good.  I think it would be simpler to create one IntSize and use it in both API calls that need it.  Can you make that small change and I'll r+ this?

> Source/WebKit/win/WebView.cpp:2842
> +        sz.cy = 16;

Why not just create the IntSize directly, then use it later?

> Source/WebKit/win/WebView.cpp:2844
> +        BitmapInfo bmInfo = BitmapInfo::create(IntSize(sz));

Use the IntSize here...

> Source/WebKit/win/WebView.cpp:2848
> +        Image* icon = iconDatabase().synchronousIconForPageURL(str, IntSize(sz));

... and here!

Created attachment 204703 [details]
Patch

Modifed patch after review.

(In reply to comment #8)

> Looks good.  I think it would be simpler to create one IntSize and use it in both API calls that need it.  Can you make that small change and I'll r+ this?

r+? ;)

[Brent Fulgham]

Comment on attachment 204703 [details]
Patch

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

Sorry -- I didn't notice that we would have to cast that sz variable from IntSize to SIZE. Please correct that and I'll r+ it (today!)

> Source/WebKit/win/WebView.cpp:2882
> +            icon->getHBITMAPOfSize(hBitmap, &(SIZE)sz);

This should be written as "&static_cast<SIZE>(sz)"

Created attachment 207339 [details]
Patch

[Brent Fulgham]

r=me

(In reply to comment #14)
> r=me

Thanks for reviewing!

[WebKit Commit Bot]

Comment on attachment 207339 [details]
Patch

Clearing flags on attachment: 207339

Committed r153059: <http://trac.webkit.org/changeset/153059>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.