| Summary: | Remove BLOB guards | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tibor Mészáros <mtiborinf> | ||||||||||||||||||||||
| Component: | WebKit Misc. | Assignee: | Csaba Osztrogonác <ossy> | ||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||
| Severity: | Normal | CC: | ap, bfulgham, buildbot, commit-queue, darin, gyuyoung.kim, ossy, rniwa, roger_fong | ||||||||||||||||||||||
| Priority: | P2 | ||||||||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||
|
Description
Tibor Mészáros
2014-05-13 05:22:05 PDT
Created attachment 231371 [details]
Patch
Removed the BLOB guards
Comment on attachment 231371 [details] Patch Attachment 231371 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4880475977940992 New failing tests: js/dom/global-constructors-attributes-shared-worker.html js/dom/global-constructors-attributes-dedicated-worker.html Created attachment 231380 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 231371 [details] Patch Attachment 231371 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4991889040211968 New failing tests: js/dom/global-constructors-attributes-shared-worker.html js/dom/global-constructors-attributes-dedicated-worker.html Created attachment 231381 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 231371 [details] Patch Attachment 231371 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6587396376231936 New failing tests: js/dom/global-constructors-attributes-shared-worker.html js/dom/global-constructors-attributes-dedicated-worker.html Created attachment 231385 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 231371 [details] Patch Attachment 231371 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5267504070918144 New failing tests: js/dom/global-constructors-attributes-shared-worker.html js/dom/global-constructors-attributes-dedicated-worker.html Created attachment 231390 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 231371 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231371&action=review Windows port doesn't enable blobs. I don't know why, perhaps it should. I'd certainly be super happy to have Blobs enabled unconditionally, it is very annoying and cofusing that some blob related code is enabled, and some is not. > Source/WebCore/WebCore.exp.in:-2865 > -#if ENABLE(BLOB) > __ZN7WebCore12BlobRegistryD2Ev > __ZN7WebCore12blobRegistryEv > __ZN7WebCore14LoaderStrategy18createBlobRegistryEv > __ZTVN7WebCore12BlobRegistryE > __ZTVN7WebCore16BlobRegistryImplE > -#endif This file is alphabetically sorted, except for conditional exports. Could you please move these lines up to maintain the rule? Created attachment 231391 [details]
Patch v2
Fixed patch
Comment on attachment 231391 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=231391&action=review > ChangeLog:3 > + Remove BLOB guards My comments about the previous version of the patch still apply. (In reply to comment #11) > Created an attachment (id=231391) [details] > Patch v2 > Fixed patch This patch managed to solve the failing test problem. Please fix the order problem mentioned by Alexey in Comment #10 , and fix the Windows build. You only need to export the necessary symbols with adding them to WebKit/WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in . I'm not sure why Windows port didn't enable BLOB before, but in this case I suggest starting a conversation about it on webkit-dev. Or maybe we should try to enable BLOB on Windows first and later remove the guards if everything works fine. Created attachment 231575 [details]
Patch v4
Change to alphabetical order in Source/WebCore/WebCore.exp.in, and added symboles to fix win build
I'm confused - doesn't this patch already enable BLOB on Windows by removing the guards? If so, I'll need to find out how to approve it from Apple side. Created attachment 231660 [details]
Patch
Try to make EWS happy. Mac: Add __ZN7WebCore12blobRegistryEv to Source/WebCore/WebCore.exp.in; Windows: Add missing files to Source/WebCore/WebCore.vcxproj/WebCore.vcxproj; revert WebKitExports.def.in changes - I think it isn't necessary, adding missing files to vcxproj file should be enough
(In reply to comment #15) > I'm confused - doesn't this patch already enable BLOB on Windows by removing the guards? If so, I'll need to find out how to approve it from Apple side. You're right, this patch enables BLOB on Windows with removing the guards, that's why I mentioned in Comment #13: "maybe we should try to enable BLOB on Windows first and later remove the guards if everything works fine." But before doing anything (enable BLOB on Windows / remove guards )": Tibor, please write a mail about it to webkit-dev to ask the opinion of the wider WebKit community. Maybe there is a good reason why isn't BLOB enabled on Windows. Created attachment 231684 [details]
Patch v5
Created attachment 231689 [details]
Patch v6
Patch does not applied, so I updated it.
Comment on attachment 231689 [details] Patch v6 LGTM, r=me. There weren't any objection on webkit-dev, so let's go ahead and land it. ( https://lists.webkit.org/pipermail/webkit-dev/2014-May/026576.html ) Comment on attachment 231689 [details] Patch v6 Clearing flags on attachment: 231689 Committed r169380: <http://trac.webkit.org/changeset/169380> All reviewed patches have been landed. Closing bug. |