"build-webkit --qt --release --symbian --minimal" has 2-3 small issues for QtWebkit.dll which will be good to fix quickly. Upside is that ARM UDEB builds are then much smaller and rebuilding is quite fast (even on a measly 4 core machine)! - linker error with unresolved PluginPackage methods (those in PluginPackageSymbian.cpp) - linker error with unresolved PluginPackage::unload() - USE_SYSTEM_MALLOC is set to 0 via qmake command-line args (due to --minimal it seems) , but is set to 1 explictly for symbian in all our .pro/.pri . This causes the autogenerated .mmp files to contain _both_ flags set. Leads to warnings at a minimum from compiler.
Created attachment 79856 [details] Patch Build fix for the plugins issue.
Siddharth, USE_SYSTEM_MALLOC is turned off by default. You have to turn it on by passing --system-malloc to build-webkit. Based on your comment it seems we always want this to be on for Symbian builds, so I will upload a patch to do that.
Comment on attachment 79856 [details] Patch Clearing flags on attachment: 79856 Committed r76466: <http://trac.webkit.org/changeset/76466>
All reviewed patches have been landed. Closing bug.
Only the plugins part was fixed. We still have the USE_SYSTEM_MALLOC issue. Reopening the bug.
/me Yael is awesome!
Created attachment 79927 [details] Patch USE_SYSTEM_MALLOC is controlled by webkit.pri for Symbian and should never be turned off. When building the qmake paramaters list for Symbian, do not add it to the list.
It would be good to avoid the special treatment for Symbian in build-webkit (and in webkitdirs.pm) especially that that the problem we're trying to solve is not specific to Symbian (e.g. I suspect that Windows minimal build would have the same problem). I suggest to move setting USE_SYSTEM_MALLOC out from the Qt-specific make-system (WebKit.pri) and set it instead in common source-code (Platform.h like e.g. Android). By doing so build-webkit --minimal will no longer set USE_SYSTEM_MALLOC to 0. This would require adding a guard to TCSystemAlloc.cpp and including it for every Qt build.
(In reply to comment #8) > It would be good to avoid the special treatment for Symbian in build-webkit (and in webkitdirs.pm) especially that that the problem we're trying to solve is not specific to Symbian (e.g. I suspect that Windows minimal build would have the same problem). > > I suggest to move setting USE_SYSTEM_MALLOC out from the Qt-specific make-system (WebKit.pri) and set it instead in common source-code (Platform.h like e.g. Android). By doing so build-webkit --minimal will no longer set USE_SYSTEM_MALLOC to 0. This would require adding a guard to TCSystemAlloc.cpp and including it for every Qt build. Sorry, I did not realize that we check webkit.pri to figure out the defaults :( I'll update the patch as you suggested.
Comment on attachment 79927 [details] Patch ? You're grepping the passed build flags? This doesn't make sense. If you pass FOO= before a command, it gets set in the environment. If you pass FOO= after build-webkit, it's supposed to treat all arguments it doesnt' understand as arguments to be passed to the underlying build system. I assume that's what you're tryign to do here? But if so, why special case USE_SYSTEM_MALLOC. Are we sure thsi is the right fix?
I seem to have a problem with the previously landed patch as linking still fails: Error: L6218E: Undefined symbol RFs::DriveToChar(int, TChar&) (referred from PluginDatabaseSymbian.o). Error: L6218E: Undefined symbol RFs::Close() (referred from PluginDatabaseSym bian.o). Error: L6218E: Undefined symbol RFs::Connect(int) (referred from PluginDataba seSymbian.o). Error: L6218E: Undefined symbol RFs::DriveList(TBuf8<(int)26>&) const (referr ed from PluginDatabaseSymbian.o). It seem that "LIBS += -lefsrv" should be outside of the ENABLE_NETSCAPE_PLUGIN_API guard for Symbian.
(In reply to comment #11) > I seem to have a problem with the previously landed patch as linking still fails: > > Error: L6218E: Undefined symbol RFs::DriveToChar(int, TChar&) (referred from > PluginDatabaseSymbian.o). > Error: L6218E: Undefined symbol RFs::Close() (referred from PluginDatabaseSym > bian.o). > Error: L6218E: Undefined symbol RFs::Connect(int) (referred from PluginDataba > seSymbian.o). > Error: L6218E: Undefined symbol RFs::DriveList(TBuf8<(int)26>&) const (referr > ed from PluginDatabaseSymbian.o). > > It seem that "LIBS += -lefsrv" should be outside of the ENABLE_NETSCAPE_PLUGIN_API guard for Symbian. You are probably right. Should we fix all the issues in one bug or create one per patch?
> > It seem that "LIBS += -lefsrv" should be outside of the ENABLE_NETSCAPE_PLUGIN_API guard for Symbian. > > You are probably right. Should we fix all the issues in one bug or create one per patch? Separate patch - this might qualify as a buildfix.
(In reply to comment #8) > > Separate patch - this might qualify as a buildfix. Fixed in http://trac.webkit.org/changeset/76606.
Created attachment 80081 [details] Patch Move definition of USE_SYSTEM_MALLOC out of pri file and put it in platform.h instead.
Comment on attachment 80081 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=80081&action=review r- to fix the Windows build. > Source/JavaScriptCore/wtf/Platform.h:786 > + This would only set USE_SYSTEM_MALLOC for the Symbian port and would break for example the Windows port of QtWebKit. I suggest we repeat the original rule from WebKit.pri. We already have a section for setting memory management related defaults for the Qt port, so perhaps we can add this rule there. #if PLATFORM(QT) /* We must not customize the global operator new and delete for the Qt port. */ #define ENABLE_GLOBAL_FASTMALLOC_NEW 0 +#if !PLATFORM(UNIX) || OS(SYMBIAN) +#define USE_SYSTEM_MALLOC 1 +#endif #endif > Source/JavaScriptCore/wtf/TCSystemAlloc.cpp:34 > +#if !(defined(USE_SYSTEM_MALLOC) && USE_SYSTEM_MALLOC) Can we just use !USE(SYSTEM_MALLOC) instead ? > Source/JavaScriptCore/wtf/wtf.pri:45 > +SOURCES += wtf/TCSystemAlloc.cpp I think we should move this entry together with the rest of the SOURCES list - alphabetically ordered.
(In reply to comment #16) > (From update of attachment 80081 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=80081&action=review > > r- to fix the Windows build. > Thank you for your review, and the offline explanation of how to fix the windows build. As you pointed out, we cannot use !USE(SYSTEM_MALLOC) . I will upload a new patch soon, addressing all the other issues.
Created attachment 80123 [details] Patch Address comment #16.
Comment on attachment 80123 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=80123&action=review r- to fix the guard, otherwise looks good. > Source/JavaScriptCore/wtf/Platform.h:834 > +#if !(PLATFORM(UNIX) || OS(SYMBIAN)) I think this should be "!PLATFORM(UNIX) || OS(SYMBIAN)" - the test above is false for Symbian.
(In reply to comment #19) > (From update of attachment 80123 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=80123&action=review > > r- to fix the guard, otherwise looks good. > > > Source/JavaScriptCore/wtf/Platform.h:834 > > +#if !(PLATFORM(UNIX) || OS(SYMBIAN)) > > I think this should be "!PLATFORM(UNIX) || OS(SYMBIAN)" - the test above is false for Symbian. Good catch :) A bad last minute change I did.
Created attachment 80190 [details] Patch Address comment #19
Comment on attachment 80190 [details] Patch LGTM, r+. Thanks.
Comment on attachment 80190 [details] Patch Clearing flags on attachment: 80190 Committed r76716: <http://trac.webkit.org/changeset/76716>
http://trac.webkit.org/changeset/76716 might have broken Leopard Intel Release (Tests)