Bug 56320

Summary:

Remove IconDatabaseBase::defaultIcon() dependency from WebCore

Product:

WebKit

Reporter:

Brady Eidson <beidson>

Component:

WebCore Misc.

Assignee:

Brady Eidson <beidson>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, eric, webkit-ews, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch v1	beidson: commit-queue-
Patch v2 - Fix Qt also	beidson: commit-queue-
Patch v3 - "Better" Qt fix	andersca: review+, beidson: commit-queue-

Description Brady Eidson 2011-03-14 11:28:03 PDT

WebCore actually called IconDatabaseBase::defaultIcon() internally, just to support HistoryItem::icon(), which was only used to support API in a non-maintained WebKit port.

Let's remove that nonsense.

Comment 1 Brady Eidson 2011-03-14 11:41:15 PDT

Created attachment 85695 [details]
Patch v1

Comment 2 Early Warning System Bot 2011-03-14 11:52:13 PDT

Attachment 85695 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8174348

Comment 3 Brady Eidson 2011-03-14 12:05:15 PDT

Ahhah, qt is using it to!  New patch coming...

Comment 4 Brady Eidson 2011-03-14 12:05:48 PDT

Created attachment 85698 [details]
Patch v2 - Fix Qt also

Comment 5 WebKit Review Bot 2011-03-14 12:08:25 PDT

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebKit/qt/Api/qwebhistory.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 7 files


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

Comment 6 Early Warning System Bot 2011-03-14 12:18:23 PDT

Attachment 85698 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8174354

Comment 7 Brady Eidson 2011-03-14 12:25:38 PDT

Created attachment 85701 [details]
Patch v3 - "Better" Qt fix

Comment 8 Brady Eidson 2011-03-14 14:26:17 PDT

r81058

Comment 9 WebKit Review Bot 2011-03-14 14:39:10 PDT

http://trac.webkit.org/changeset/81058 might have broken EFL Linux Release (Build)

Comment 10 Brady Eidson 2011-03-14 14:39:48 PDT

(In reply to comment #9)
> http://trac.webkit.org/changeset/81058 might have broken EFL Linux Release (Build)

Sure would suck if it did, because the EFL EWS bot said it was fine!  :)

Comment 11 Brady Eidson 2011-03-14 14:41:37 PDT

(In reply to comment #10)
> (In reply to comment #9)
> > http://trac.webkit.org/changeset/81058 might have broken EFL Linux Release (Build)
> 
> Sure would suck if it did, because the EFL EWS bot said it was fine!  :)


It did - 
http://build.webkit.org/builders/EFL%20Linux%20Release%20%28Build%29/builds/9596/steps/compile-webkit/logs/stdio

What use is the EWS if it's not in line with what the build bot will see?

Comment 12 Adam Barth 2011-03-14 14:43:23 PDT

> What use is the EWS if it's not in line with what the build bot will see?

That's a good question.  We should follow up with the folks who run the EFL EWS bot.

Comment 13 Brady Eidson 2011-03-14 14:44:20 PDT

Attempted EFL build fix in r81060