Bug 99265

Summary: [EFL][WK2] Display page favicons in MiniBrowser
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit EFLAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: gyuyoung.kim, kenneth, laszlo.gombos, lucas.de.marchi, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 99087    
Bug Blocks:    
Attachments:
Description Flags
Patch
gyuyoung.kim: commit-queue-
Patch
gyuyoung.kim: review+
Patch for landing
cdumez: commit-queue-
Patch for landing none

Chris Dumez
Reported 2012-10-14 00:33:13 PDT
We should use the new Favicon API from Bug 99087 and display page favicons in MiniBrowser.
Attachments
Patch (3.75 KB, patch)
2012-10-14 06:22 PDT, Chris Dumez 		gyuyoung.kim: commit-queue-
DetailsFormatted DiffDiff
Patch (3.75 KB, patch)
2012-10-15 13:27 PDT, Chris Dumez 		gyuyoung.kim: review+
DetailsFormatted DiffDiff
Patch for landing (3.81 KB, patch)
2012-10-15 22:43 PDT, Chris Dumez 		cdumez: commit-queue-
DetailsFormatted DiffDiff
Patch for landing (3.81 KB, patch)
2012-10-15 23:10 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2012-10-14 06:22:00 PDT
Created attachment 168582 [details] Patch
Gyuyoung Kim
Comment 2 2012-10-14 06:26:57 PDT
Comment on attachment 168582 [details] Patch Attachment 168582 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/14311012
Chris Dumez
Comment 3 2012-10-15 13:27:27 PDT
Created attachment 168769 [details] Patch
Gyuyoung Kim
Comment 4 2012-10-15 18:12:28 PDT
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 5 2012-10-15 21:55:16 PDT
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
Comment 6 2012-10-15 22:43:39 PDT
Created attachment 168855 [details] Patch for landing Could someone please cq+ ?
Gyuyoung Kim
Comment 7 2012-10-15 22:59:02 PDT
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 8 2012-10-15 23:04:16 PDT
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
Comment 9 2012-10-15 23:10:35 PDT
Created attachment 168864 [details] Patch for landing Fixed wrong unref (Thanks Gyuyoung).
WebKit Review Bot
Comment 10 2012-10-15 23:31:07 PDT
Comment on attachment 168864 [details] Patch for landing Clearing flags on attachment: 168864 Committed r131416: <http://trac.webkit.org/changeset/131416>
WebKit Review Bot
Comment 11 2012-10-15 23:31:11 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.