Bug 99265 – [EFL][WK2] Display page favicons in MiniBrowser

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

[Chris Dumez]

We should use the new Favicon API from Bug 99087 and display page favicons in MiniBrowser.

[Chris Dumez]

Created attachment 168582 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 168582 [details]
Patch

Attachment 168582 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14311012

[Chris Dumez]

Created attachment 168769 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 168769 [details]
Patch

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

> Tools/MiniBrowser/efl/main.c:345
> +        evas_object_ref(icon);

You remove icon object without doing unref at 334 line. So, do you need to increase ref count here ?

[Chris Dumez]

Comment on attachment 168769 [details]
Patch

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

> Tools/MiniBrowser/efl/main.c:335
> +        evas_object_del(old_icon);

Right, I probably need to unref() here as well.

>> Tools/MiniBrowser/efl/main.c:345
>> +        evas_object_ref(icon);
> 
> You remove icon object without doing unref at 334 line. So, do you need to increase ref count here ?

Yes, the caller will unref() the icon after the callback is called so the callback should ref() it if it wishes to use / keep it.

[Chris Dumez]

Created attachment 168855 [details]
Patch for landing

Could someone please cq+ ?

[Gyuyoung Kim]

Comment on attachment 168855 [details]
Patch for landing

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

> Tools/MiniBrowser/efl/main.c:335
> +        evas_object_unref(icon);

Could you explain why you do unref *icon* instead of *old_icon* ?

[Chris Dumez]

Comment on attachment 168855 [details]
Patch for landing

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

>> Tools/MiniBrowser/efl/main.c:335
>> +        evas_object_unref(icon);
> 
> Could you explain why you do unref *icon* instead of *old_icon* ?

because I did not properly wake up yet :) Thanks for catching this.

[Chris Dumez]

Created attachment 168864 [details]
Patch for landing

Fixed wrong unref (Thanks Gyuyoung).

[WebKit Review Bot]

Comment on attachment 168864 [details]
Patch for landing

Clearing flags on attachment: 168864

Committed r131416: <http://trac.webkit.org/changeset/131416>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.