Bug 27007 – Build fixes when ICONDATABASE is disabled

• Component: Page Loading
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

Created attachment 32324 [details]
Patch v1

Build fixes when ICONDATABASE is disabled

Reviewed by NOBODY (OOPS!).

WebCore:

        * Configurations/WebCore.xcconfig: Exclude IconDatabase.cpp
        from being compiled when ICONDATABASE is disabled, and exclude
        IconDatabaseNone.cpp when ICONDATABASE is enabled.  Reworked
        definition of EXCLUDED_SOURCE_FILE_NAMES.
        * WebCore.xcodeproj/project.pbxproj: Added IconDatabaseNone.cpp
        to the project.
        * loader/icon/IconDatabase.h: Removed three public methods from
        #if ENABLE(ICONDATABASE)/#endif macro so that they may be
        stubbed out in IconDatabaseNone.cpp.
        * loader/icon/IconDatabaseNone.cpp:
        (WebCore::IconDatabase::importIconURLForPageURL): Added.
        (WebCore::IconDatabase::importIconDataForIconURL): Added.
        (WebCore::IconDatabase::shouldStopThreadActivity): Added.

WebKit/mac:

        * Misc/WebIconDatabase.mm:
        (defaultClient): Return 0 if ICONDATABASE is disabled.
        * WebCoreSupport/WebFrameLoaderClient.mm:
        (WebFrameLoaderClient::dispatchDidReceiveIcon): Commented out
        the body of the method if ICONDATABASE is disabled.
        (WebFrameLoaderClient::registerForIconNotification): Ditto.
        * WebCoreSupport/WebIconDatabaseClient.mm: Added #if
        ENABLE(ICONDATABASE)/#endif guard to source.
        * WebView/WebView.mm:
        (-[WebView setFrameLoadDelegate:]): Wrapped code that starts the
        icon database machinery in #if ENABLE(ICONDATABASE)/#endif.
        (-[WebView _registerForIconNotification:]): Wrapped method in
        #if ENABLE(ICONDATABASE)/#endif guard.
        (-[WebView _dispatchDidReceiveIconFromWebFrame:]): Ditto.
        * WebView/WebViewInternal.h: Wrapped methods in #if
        ENABLE(ICONDATABASE)/#endif guard.
        (-[WebView _registerForIconNotification:]):
        (-[WebView _dispatchDidReceiveIconFromWebFrame:]):

[David Kilzer (:ddkilzer)]

Comment on attachment 32324 [details]
Patch v1

Mark Rowe prefers #if macros around the IconDatabase.cpp and IconDatabaseNone.cpp source rather than futzing with the EXCLUDED_SOURCE_FILE_NAMES Xcode setting, so I will update the patch to do that.

[David Kilzer (:ddkilzer)]

Created attachment 32396 [details]
Patch v2


        <https://bugs.webkit.org/show_bug.cgi?id=27007>

        Reviewed by NOBODY (OOPS!).

WebCore:

        * WebCore.xcodeproj/project.pbxproj: Added IconDatabaseNone.cpp
        to the project.
        * loader/icon/IconDatabase.cpp: Added
        #if ENABLE(ICONDATABASE)/#endif macro guard.
        * loader/icon/IconDatabase.h: Removed three public methods from
        #if ENABLE(ICONDATABASE)/#endif macro so that they may be
        stubbed out in IconDatabaseNone.cpp.
        * loader/icon/IconDatabaseNone.cpp: Added
        #if !ENABLE(ICONDATABASE)/#endif macro guard.
        (WebCore::IconDatabase::importIconURLForPageURL): Added.
        (WebCore::IconDatabase::importIconDataForIconURL): Added.
        (WebCore::IconDatabase::shouldStopThreadActivity): Added.

WebKit/mac:

        * Misc/WebIconDatabase.mm:
        (defaultClient): Return 0 if ICONDATABASE is disabled.
        * WebCoreSupport/WebFrameLoaderClient.mm:
        (WebFrameLoaderClient::dispatchDidReceiveIcon): Commented out
        the body of the method if ICONDATABASE is disabled.
        (WebFrameLoaderClient::registerForIconNotification): Ditto.
        * WebCoreSupport/WebIconDatabaseClient.mm: Added #if
        ENABLE(ICONDATABASE)/#endif guard to source.
        * WebView/WebView.mm:
        (-[WebView setFrameLoadDelegate:]): Wrapped code that starts the
        icon database machinery in #if ENABLE(ICONDATABASE)/#endif.
        (-[WebView _registerForIconNotification:]): Wrapped method in
        #if ENABLE(ICONDATABASE)/#endif guard.
        (-[WebView _dispatchDidReceiveIconFromWebFrame:]): Ditto.
        * WebView/WebViewInternal.h: Wrapped methods in #if
        ENABLE(ICONDATABASE)/#endif guard.
        (-[WebView _registerForIconNotification:]):
        (-[WebView _dispatchDidReceiveIconFromWebFrame:]):---
 11 files changed, 97 insertions(+), 2 deletions(-)

[Eric Seidel (no email)]

Comment on attachment 32396 [details]
Patch v2

This seems like the wrong patch.

re: this patch.  I like the ability to detect bug URLs in ChangeLogs, but I don't think we should remove the ability to specify a bug?  Maybe we should... I guess that forces folks to add the bug url to the ChangeLog?

Also the changelog greping code shoudl be in its own function.

[David Kilzer (:ddkilzer)]

Comment on attachment 32396 [details]
Patch v2

(In reply to comment #3)
> (From update of attachment 32396 [details])
> This seems like the wrong patch.

bugzilla-tool FAIL.  Why did "post-commit" post a local diff?  *sigh*

> re: this patch.  I like the ability to detect bug URLs in ChangeLogs, but I
> don't think we should remove the ability to specify a bug?  Maybe we should...
> I guess that forces folks to add the bug url to the ChangeLog?

Exactly.

> Also the changelog greping code shoudl be in its own function.

Okay.  Will do that before posting the bugzilla-tool patch.

[David Kilzer (:ddkilzer)]

Created attachment 32404 [details]
Patch v3


        <https://bugs.webkit.org/show_bug.cgi?id=27007>

        Reviewed by NOBODY (OOPS!).

WebCore:

        * WebCore.xcodeproj/project.pbxproj: Added IconDatabaseNone.cpp
        to the project.
        * loader/icon/IconDatabase.cpp: Added
        #if ENABLE(ICONDATABASE)/#endif macro guard.
        * loader/icon/IconDatabase.h: Removed three public methods from
        #if ENABLE(ICONDATABASE)/#endif macro so that they may be
        stubbed out in IconDatabaseNone.cpp.
        * loader/icon/IconDatabaseNone.cpp: Added
        #if !ENABLE(ICONDATABASE)/#endif macro guard.
        (WebCore::IconDatabase::importIconURLForPageURL): Added.
        (WebCore::IconDatabase::importIconDataForIconURL): Added.
        (WebCore::IconDatabase::shouldStopThreadActivity): Added.

WebKit/mac:

        * Misc/WebIconDatabase.mm:
        (defaultClient): Return 0 if ICONDATABASE is disabled.
        * WebCoreSupport/WebFrameLoaderClient.mm:
        (WebFrameLoaderClient::dispatchDidReceiveIcon): Commented out
        the body of the method if ICONDATABASE is disabled.
        (WebFrameLoaderClient::registerForIconNotification): Ditto.
        * WebCoreSupport/WebIconDatabaseClient.mm: Added #if
        ENABLE(ICONDATABASE)/#endif guard to source.
        * WebView/WebView.mm:
        (-[WebView setFrameLoadDelegate:]): Wrapped code that starts the
        icon database machinery in #if ENABLE(ICONDATABASE)/#endif.
        (-[WebView _registerForIconNotification:]): Wrapped method in
        #if ENABLE(ICONDATABASE)/#endif guard.
        (-[WebView _dispatchDidReceiveIconFromWebFrame:]): Ditto.
        * WebView/WebViewInternal.h: Wrapped methods in #if
        ENABLE(ICONDATABASE)/#endif guard.
        (-[WebView _registerForIconNotification:]):
        (-[WebView _dispatchDidReceiveIconFromWebFrame:]):
---
 11 files changed, 97 insertions(+), 2 deletions(-)

[David Kilzer (:ddkilzer)]

Comment on attachment 32404 [details]
Patch v3

bugzilla-tool FAIL again, probably one of my local modifications.

[David Kilzer (:ddkilzer)]

Created attachment 32405 [details]
Patch v4


        <https://bugs.webkit.org/show_bug.cgi?id=27007>

        Reviewed by NOBODY (OOPS!).

WebCore:

        * WebCore.xcodeproj/project.pbxproj: Added IconDatabaseNone.cpp
        to the project.
        * loader/icon/IconDatabase.cpp: Added
        #if ENABLE(ICONDATABASE)/#endif macro guard.
        * loader/icon/IconDatabase.h: Removed three public methods from
        #if ENABLE(ICONDATABASE)/#endif macro so that they may be
        stubbed out in IconDatabaseNone.cpp.
        * loader/icon/IconDatabaseNone.cpp: Added
        #if !ENABLE(ICONDATABASE)/#endif macro guard.
        (WebCore::IconDatabase::importIconURLForPageURL): Added.
        (WebCore::IconDatabase::importIconDataForIconURL): Added.
        (WebCore::IconDatabase::shouldStopThreadActivity): Added.

WebKit/mac:

        * Misc/WebIconDatabase.mm:
        (defaultClient): Return 0 if ICONDATABASE is disabled.
        * WebCoreSupport/WebFrameLoaderClient.mm:
        (WebFrameLoaderClient::dispatchDidReceiveIcon): Commented out
        the body of the method if ICONDATABASE is disabled.
        (WebFrameLoaderClient::registerForIconNotification): Ditto.
        * WebCoreSupport/WebIconDatabaseClient.mm: Added #if
        ENABLE(ICONDATABASE)/#endif guard to source.
        * WebView/WebView.mm:
        (-[WebView setFrameLoadDelegate:]): Wrapped code that starts the
        icon database machinery in #if ENABLE(ICONDATABASE)/#endif.
        (-[WebView _registerForIconNotification:]): Wrapped method in
        #if ENABLE(ICONDATABASE)/#endif guard.
        (-[WebView _dispatchDidReceiveIconFromWebFrame:]): Ditto.
        * WebView/WebViewInternal.h: Wrapped methods in #if
        ENABLE(ICONDATABASE)/#endif guard.
        (-[WebView _registerForIconNotification:]):
        (-[WebView _dispatchDidReceiveIconFromWebFrame:]):
---
 11 files changed, 97 insertions(+), 2 deletions(-)

[Mark Rowe (bdash)]

What's with posting the changelogs in the comment of the bug?  Is that intentional?

[David Kilzer (:ddkilzer)]

(In reply to comment #8)
> What's with posting the changelogs in the comment of the bug?  Is that
> intentional?

That's the current behavior of "bugzilla-tool post-commits".  It is kind of annoying for large ChangeLogs, though.

[David Kilzer (:ddkilzer)]

$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/WebCore.xcodeproj/project.pbxproj
	M	WebCore/loader/icon/IconDatabase.cpp
	M	WebCore/loader/icon/IconDatabase.h
	M	WebCore/loader/icon/IconDatabaseNone.cpp
	M	WebKit/mac/ChangeLog
	M	WebKit/mac/Misc/WebIconDatabase.mm
	M	WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm
	M	WebKit/mac/WebCoreSupport/WebIconDatabaseClient.mm
	M	WebKit/mac/WebView/WebView.mm
	M	WebKit/mac/WebView/WebViewInternal.h
Committed r45750

http://trac.webkit.org/changeset/45750