RESOLVED FIXED Bug 114950
DOMFileSystemBase: fix multiple definitions in the BlackBerry port 
https://bugs.webkit.org/show_bug.cgi?id=114950
Summary DOMFileSystemBase: fix multiple definitions in the BlackBerry port
Alberto Garcia
Reported 2013-04-22 05:17:51 PDT
The following methods are already defined in DOMFileSystemBlackBerry.cpp: crackFileSystemURL() createFileSystemURL() isValidType() supportsToURL()
Attachments
Patch (1.49 KB, patch)
2013-04-22 05:21 PDT, Alberto Garcia 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Alberto Garcia
Comment 1 2013-04-22 05:21:12 PDT
Created attachment 199017 [details] Patch
Xan Lopez
Comment 2 2013-04-24 06:58:27 PDT
Comment on attachment 199017 [details] Patch OK.
WebKit Commit Bot
Comment 3 2013-04-24 07:26:05 PDT
Comment on attachment 199017 [details] Patch Clearing flags on attachment: 199017 Committed r149034: <http://trac.webkit.org/changeset/149034>
WebKit Commit Bot
Comment 4 2013-04-24 07:26:07 PDT
All reviewed patches have been landed. Closing bug.
Benjamin Poulain
Comment 5 2013-04-28 21:04:15 PDT
What The Fuck? This should have: -Justification on why you did not make an abstraction instead. -Justification for using different implementation. Adding platform #ifdef anywhere at random places in WebCore is not okay, we are trying to remove them. Please explain.
Xan Lopez
Comment 6 2013-04-29 00:29:01 PDT
(In reply to comment #5) > What The Fuck? > > This should have: > -Justification on why you did not make an abstraction instead. > -Justification for using different implementation. > > Adding platform #ifdef anywhere at random places in WebCore is not okay, we are trying to remove them. Please explain. The file already had #ifdefs for Chromium, which were removed recently with the removal of the Chromium code. It just so happens that the BB port was also using this feature, but the #ifdefs were not upstreamed. So the justification is that in general adding an extra #ifdef when there's already one is not a big deal, but we can open another bug to improve the code.
Alberto Garcia
Comment 7 2013-04-29 03:15:14 PDT
(In reply to comment #5) > -Justification for using different implementation. Apart from what Xan said, I'll try to see if we can merge both implementations now that there's no Chromium code around.
Benjamin Poulain
Comment 8 2013-04-29 13:03:56 PDT
This has landed last week, there is no chromium code anymore. Even when chromium was there, they were doing a ton more maintenance than (In reply to comment #7) > (In reply to comment #5) > > -Justification for using different implementation. > > Apart from what Xan said, I'll try to see if we can merge both > implementations now that there's no Chromium code around. Thanks. That is the right thing to do.
Note You need to log in before you can comment on or make changes to this bug.