Per http://dev.w3.org/2009/dap/file-system/file-dir-sys.html#the-flags-interface, Flags is marked as [NoInterfaceObject], yet in Chromium WebKitFlags is a constructor hanging off of DOMWindow. And there's custom V8 code that knows to transform a V8WebKitFlags into a WebKitFlags. According to ericu, WebKitFlags shouldn't actually be constructable, so this bug just calls for deletion. We shouldn't have to worry much about compat, since the normal use case is to just use an object literal in the API and the thing is vendor-prefixed anyway.
Created attachment 108911 [details] Patch
Comment on attachment 108911 [details] Patch Attachment 108911 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/9883386
Created attachment 108919 [details] Added missing build file changes.
Comment on attachment 108919 [details] Added missing build file changes. View in context: https://bugs.webkit.org/attachment.cgi?id=108919&action=review Thanks for fixing this! > Source/WebCore/bindings/js/JSDirectoryEntryCustom.cpp:69 > + return jsUndefined(); I don't think we should return here? (At least we should handle null and undefined cases)
Comment on attachment 108919 [details] Added missing build file changes. View in context: https://bugs.webkit.org/attachment.cgi?id=108919&action=review >> Source/WebCore/bindings/js/JSDirectoryEntryCustom.cpp:69 >> + return jsUndefined(); > > I don't think we should return here? (At least we should handle null and undefined cases) Ah, you're right--I changed my mind on that and reverted that code elsewhere, but missed this one. I'll fix it.
Created attachment 113393 [details] I changed it just to ignore any non-object arguments, just as if you'd passed in null or {}.
Comment on attachment 113393 [details] I changed it just to ignore any non-object arguments, just as if you'd passed in null or {}. Rejecting attachment 113393 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: bKitFlags.idl' patching file Source/WebCore/page/DOMWindow.idl Hunk #1 succeeded at 195 with fuzz 2. patching file Source/WebCore/workers/WorkerContext.idl patching file LayoutTests/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file LayoutTests/fast/filesystem/flags-passing-expected.txt patching file LayoutTests/fast/filesystem/script-tests/flags-passing.js Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Eric Seidel', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/10996182
Created attachment 120242 [details] Patch
Attachment 120242 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Updating OpenSource First, rewinding head to replay your work on top of it... Applying: Inform the scrolling coordinator when scrollbar layers come and go Using index info to reconstruct a base tree... <stdin>:474806: trailing whitespace. [Chromium] DatabaseTrackerChromium: iterating DatabaseSet races with Database disposal on worker thread <stdin>:474827: trailing whitespace. Nothing to test, just removing redundant code. Correct behavior tested by <stdin>:475346: trailing whitespace. warning: 3 lines add whitespace errors. Falling back to patching base and 3-way merge... warning: too many files (created: 167522 deleted: 3), skipping inexact rename detection Auto-merging LayoutTests/ChangeLog CONFLICT (content): Merge conflict in LayoutTests/ChangeLog Auto-merging LayoutTests/platform/chromium/test_expectations.txt CONFLICT (content): Merge conflict in LayoutTests/platform/chromium/test_expectations.txt Auto-merging Source/WebCore/ChangeLog CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog Auto-merging Source/WebCore/page/ScrollingCoordinator.h CONFLICT (content): Merge conflict in Source/WebCore/page/ScrollingCoordinator.h Auto-merging Source/WebCore/page/mac/ScrollingCoordinatorMac.mm CONFLICT (content): Merge conflict in Source/WebCore/page/mac/ScrollingCoordinatorMac.mm Auto-merging Source/WebCore/platform/ScrollView.cpp Auto-merging Source/WebCore/rendering/RenderLayerCompositor.cpp CONFLICT (content): Merge conflict in Source/WebCore/rendering/RenderLayerCompositor.cpp Auto-merging Source/WebKit2/ChangeLog CONFLICT (content): Merge conflict in Source/WebKit2/ChangeLog Auto-merging Tools/ChangeLog CONFLICT (content): Merge conflict in Tools/ChangeLog Auto-merging Tools/Scripts/build-webkit Auto-merging Tools/Scripts/webkitdirs.pm CONFLICT (content): Merge conflict in Tools/Scripts/webkitdirs.pm Failed to merge in the changes. Patch failed at 0001 Inform the scrolling coordinator when scrollbar layers come and go When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 158. If any of these errors are false positives, please file a bug against check-webkit-style.
I think I may have missed some discussion about this, but I thought the direction we were taking the platform as a whole was to make these sorts of dictionaries constructible, but not required, i.e. you can pass any object, but you can also construct it if you like. The advantage of doing it this way is that it allows for feature detection. That all said, whatever constructor/API we expose should have a less generic name than Flags since we're adding these sorts of dictionaries all over the platform.
(In reply to comment #10) > I think I may have missed some discussion about this, but I thought the direction we were taking the platform as a whole was to make these sorts of dictionaries constructible, but not required, i.e. you can pass any object, but you can also construct it if you like. The advantage of doing it this way is that it allows for feature detection. We could certainly do that here, but I don't think it's necessary. You can just check for [prefix]requestFileSystem to do feature detection. Anyway, that's a spec decision, not an implementation decision. This patch just brings us in line with the spec. > That all said, whatever constructor/API we expose should have a less generic name than Flags since we're adding these sorts of dictionaries all over the platform. Indeed; I wanted to get rid of this first of all. If we want to add back in a constructable object later, we can take the time to come up with a better name.
Comment on attachment 120242 [details] Patch Clearing flags on attachment: 120242 Committed r103624: <http://trac.webkit.org/changeset/103624>
All reviewed patches have been landed. Closing bug.
Reverted r103624 for reason: Broke Snow Leopard builders Committed r103625: <http://trac.webkit.org/changeset/103625>
Created attachment 121018 [details] Patch
Comment on attachment 121018 [details] Patch Build fix: removed the last few references to the file and global object.
Comment on attachment 121018 [details] Patch Clearing flags on attachment: 121018 Committed r104126: <http://trac.webkit.org/changeset/104126>