[webkit-reviews] review granted: [Bug 16924] Shared PluginDatabase, PluginPackage, and PlugInInfoStore Implementations : [Attachment 19348] Updated patch to resolve more Windows build issues
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Feb 25 11:34:13 PST 2008
Jon Honeycutt <jhoneycutt at apple.com> has granted Rodney Dawes
<dobey at wayofthemonkey.com>'s request for review:
Bug 16924: Shared PluginDatabase, PluginPackage, and PlugInInfoStore
Implementations
http://bugs.webkit.org/show_bug.cgi?id=16924
Attachment 19348: Updated patch to resolve more Windows build issues
http://bugs.webkit.org/attachment.cgi?id=19348&action=edit
------- Additional Comments from Jon Honeycutt <jhoneycutt at apple.com>
Looks good! r=me, few comments below.
><html><body><pre style="word-wrap: break-word; white-space: pre-wrap;">Index:
WebCore/ChangeLog
>===================================================================
>--- WebCore/ChangeLog (revision 30564)
>+++ WebCore/ChangeLog (working copy)
>@@ -1,3 +1,43 @@
>+2008-02-25 Rodney Dawes <dobey at wayofthemonkey.com>
>+
>+ Reviewed by NOBODY (OOPS!!)
>+
>+ Add PluginInfoStore.cpp and new PluginDatabase.cpp to GTK+ and Qt
ports
>+ Remove old PlugInInfoStoreQt.cpp as it is obsoleted by shared code
>+ Add PluginInfoStore, PluginDatabase, and PluginStream files to Wx
build
>+ Add new PluginDatabase.cpp to Windows build
>+ Add temporary stubs for new PluginDatabase and PluginPackage
>+ shared classes to GTK+, Qt, and Wx ports
>+ Copy PluginDatabaseWin.cpp to PluginDatabase.cpp to preserve history
>+ Remove shared code from PluginDatabaseWin.cpp
>+ Remove Windows-specific code from PluginDatabase.cpp
>+ Use PlatformModule and PlatformFileTime instead of HMODULE and
FILETIME
>+ Remove extraneous PluginPackage:: from hash() class method prototype
>+ Subsume storeFileVersion into PluginPackage::fetchInfo
>+ Add cross-platform PlatformModuleVersion type definition
>+ Use PlatformModuleVersio to store the module version
>+ Rename m_fileVersion[ML]S to m_moduleVersion
>+ Change compareFileVersion to use PlatformModuleVersion as the
argument
>+ Move PluginView::determineQuirks and m_quirks to PluginPackage
>+ Updated determineQuirks for the PlatformModuleVersion
Typo: PlatformModuleVersio.
Please add punctuation to each sentence.
>Index: WebCore/plugins/PluginPackage.h
>
>+#if PLATFORM(WIN)
>+struct PlatformModuleVersion {
I think this would fit better in FileSystem.h with the other platform
definitions.
>+ int compareFileVersion(const PlatformModuleVersion&) const;
>+
>+ PluginQuirkSet m_quirks;
Please make m_quirks private and add a getter.
>Index: WebCore/plugins/win/PluginPackageWin.cpp
>@@ -76,49 +77,37 @@ void PluginPackage::freeLibraryTimerFire
> PluginPackage::PluginPackage(const String& path, const FILETIME&
lastModified)
> : RefCounted<PluginPackage>(0)
> , m_path(path)
>+ , m_moduleVersion(0)
Should remove this.
Needs MIMETypeRegistry.h as bfulgham mentioned.
>+ static const PlatformModuleVersion slPluginMinRequired(0x51BE0000,
0x00010000 );
Extra space here.
>+ // Determine the quirks for the MIME types this plug-in supports
>+ determineQuirks(type);
>+
I think there's a tab here. Please remove this and grep for any other tabs.
>Index: WebCore/plugins/win/PluginViewWin.cpp
Should use the quirks getter throughout this file.
More information about the webkit-reviews
mailing list