Summary: | [filesystem/Chromium] filesystem URLs are not fully supported | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric U. <ericu> | ||||||||||
Component: | DOM | Assignee: | Eric U. <ericu> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | abarth, brettw, dglazkov, fishd, jamesr, kinuko, michaeln, serya, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Eric U.
2011-12-21 15:33:44 PST
Created attachment 120229 [details]
Small plumbing changes; the real work's done in googleURL and the Chromium codebase.
This won't build until the googleurl changes land.
Attachment 120229 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Updating OpenSource A LayoutTests/fast/html/font-weight-bold-for-b-and-strong-expected.png A LayoutTests/fast/html/font-weight-bold-for-b-and-strong-expected.txt A LayoutTests/fast/html/font-weight-bold-for-b-and-strong.html M LayoutTests/ChangeLog M Source/WebCore/ChangeLog M Source/WebCore/css/html.css r103468 = 97efa88a66ff7f597257cf86d0f8315cd016e1e3 (refs/remotes/origin/master) 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: 167254 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. Comment on attachment 120229 [details] Small plumbing changes; the real work's done in googleURL and the Chromium codebase. Attachment 120229 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10997176 Comment on attachment 120229 [details] Small plumbing changes; the real work's done in googleURL and the Chromium codebase. View in context: https://bugs.webkit.org/attachment.cgi?id=120229&action=review > Source/WebCore/page/SecurityOrigin.cpp:85 > +#if ENABLE(FILE_SYSTEM) This shouldn't be guarded with ENABLE(FILE_SYSTEM). It has nothing to do with FILE_SYSTEM. > Source/WebCore/platform/KURL.h:226 > +#if ENABLE(FILE_SYSTEM) This shouldn't be guarded with ENABLE(FILE_SYSTEM). It has nothing to do with FILE_SYSTEM. > Source/WebCore/platform/KURLGoogle.cpp:292 > +void KURLGooglePrivate::initInnerURL() { { should be on the next line. > Source/WebCore/platform/KURLGoogle.cpp:337 > + if (m_innerURL) { > + dest->m_innerURL = adoptPtr(new KURL(m_innerURL->copy())); > + } > + else > + dest->m_innerURL.clear(); Bad indent and we can drop the { } Created attachment 123813 [details]
Patch
Comment on attachment 123813 [details]
Patch
Rolled in code review feedback. Also, the googleurl changes are now in, so the bots should be happy with the change now.
Comment on attachment 123813 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123813&action=review > Source/WebCore/fileapi/EntryBase.cpp:78 > + result.append(encodeWithURLEscapeSequences(m_fullPath)); Based on what I experienced on my reverted change this will break the chromeos tests. (The tests expect toURL() return URL with unescaped directory separators) This will escape all the directory separators as well-- I want to make sure this is what we want. (In reply to comment #7) > (From update of attachment 123813 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=123813&action=review > > > Source/WebCore/fileapi/EntryBase.cpp:78 > > + result.append(encodeWithURLEscapeSequences(m_fullPath)); > > Based on what I experienced on my reverted change this will break the chromeos tests. (The tests expect toURL() return URL with unescaped directory separators) > > This will escape all the directory separators as well-- I want to make sure this is what we want. That's taken care of in WebCore::encodeWithURLEscapeSequences. After everything's escaped, I unescape just the directory separators. Comment on attachment 123813 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123813&action=review >>> Source/WebCore/fileapi/EntryBase.cpp:78 >>> + result.append(encodeWithURLEscapeSequences(m_fullPath)); >> >> Based on what I experienced on my reverted change this will break the chromeos tests. (The tests expect toURL() return URL with unescaped directory separators) >> >> This will escape all the directory separators as well-- I want to make sure this is what we want. > > That's taken care of in WebCore::encodeWithURLEscapeSequences. After everything's escaped, I unescape just the directory separators. Oh I see, now I found the corresponding change in the function. Cool so I no longer need to work around the escaping issue when I try relanding my change. Comment on attachment 123813 [details] Patch Attachment 123813 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11342115 Comment on attachment 123813 [details] Patch Attachment 123813 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11344106 Created attachment 123853 [details]
Patch
Comment on attachment 123853 [details]
Patch
Added missing header include.
Comment on attachment 123853 [details] Patch Clearing flags on attachment: 123853 Committed r105843: <http://trac.webkit.org/changeset/105843> All reviewed patches have been landed. Closing bug. This patch appears to have broken KURLTest.Encode: http://build.webkit.org/builders/Chromium%20Mac%20Release%20%28Tests%29/builds/12331/steps/webkit-unit-tests/logs/stdio [ RUN ] KURLTest.Encode /Users/chrome-bot/Desktop/WebKit-BuildSlave/chromium-mac-release-tests/build/Source/WebKit/chromium/tests/KURLTest.cpp:336: Failure Value of: output Actual: 4-byte object <60-8A 36-00> Expected: expectedOutput Which is: 4-byte object <50-64 32-00> Was this change in behavior intentional? If so, please update the test. Created attachment 123998 [details]
Patch
Comment on attachment 123998 [details]
Patch
Fixed a test expectation I missed last time.
Comment on attachment 123998 [details] Patch Clearing flags on attachment: 123998 Committed r105930: <http://trac.webkit.org/changeset/105930> All reviewed patches have been landed. Closing bug. As with the first time you landed this patch, r105930 broke the Mac build. The build has been broken red for over two hours at this point because of this. Please fix it, and pay attention to the build bots in the future. Since people really love to be able to compile WebKit, I fixed this in r105950 by reapplying the exact same build fix that was made the first time this was landed. This patch was removed due to a Chromium test failure. Scratch that--apparently I misunderstood. The patch is still in there. Sorry for the noise. *** Bug 75701 has been marked as a duplicate of this bug. *** |