Bug 65908 - Remove the temporary workaround added by http://trac.webkit.org/changeset/51338
Summary: Remove the temporary workaround added by http://trac.webkit.org/changeset/51338
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-09 04:40 PDT by Marja Hölttä
Modified: 2011-08-11 12:25 PDT (History)
4 users (show)

See Also:


Attachments
Removing the workaround. (1.53 KB, patch)
2011-08-09 04:42 PDT, Marja Hölttä
no flags Details | Formatted Diff | Diff
Patch for landing (3.12 KB, patch)
2011-08-11 11:24 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marja Hölttä 2011-08-09 04:40:20 PDT
SecurityOrigin for a file URL returns "file://", and SecurityOrigin::createFromString("file://") creates a unique (null) security origin (because "file://" is first canonicalized to "file:///" and that is a directory). This means the conversion SecurityOrigin -> string -> SecurityOrigin doens't give back the same SecurityOrigin in case of file URLs.

WebStorageNamespaceImpl::createStorageArea contains a workaround which changes the string "file://" into "file:///a" before giving it to SecurityOrigin::createFromString.

If SecurityOrigin::m_enforceFilePathSeparation is true, then SecurityOrigin::toString() returns "null" for file URLs, and this problem does not exist, and the workaround code is not ran.

This bug is for trying out what breaks if the workaround is removed. I'll submit a patch which just removes it, to see whether there are any tests that rely on the workaround code.
Comment 1 Marja Hölttä 2011-08-09 04:42:12 PDT
Created attachment 103347 [details]
Removing the workaround.
Comment 2 jochen 2011-08-09 05:48:24 PDT
(In reply to comment #0)
> This bug is for trying out what breaks if the workaround is removed. I'll submit a patch which just removes it, to see whether there are any tests that rely on the workaround code.

I'd expect the test failures to be mainly when running layout tests in Chromium, so you'll need to apply this patch to a checkout of chromium and run the tests there (webkit/tools/layout_tests/run_webkit_tests.py and probably browser_tests)
Comment 3 Marja Hölttä 2011-08-10 01:46:06 PDT
(In reply to comment #2)
> I'd expect the test failures to be mainly when running layout tests in Chromium, so you'll need to apply this patch to a checkout of chromium and run the tests there (webkit/tools/layout_tests/run_webkit_tests.py and probably browser_tests)

Yep. Removing the workaround didn't add any (relevant) test failures there either.
Comment 4 jochen 2011-08-10 01:58:37 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > I'd expect the test failures to be mainly when running layout tests in Chromium, so you'll need to apply this patch to a checkout of chromium and run the tests there (webkit/tools/layout_tests/run_webkit_tests.py and probably browser_tests)
> 
> Yep. Removing the workaround didn't add any (relevant) test failures there either.

ok, the patch looks good, you should mark it r? c?, maybe Adam can do the actual review then
Comment 5 Adam Barth 2011-08-11 10:10:20 PDT
Comment on attachment 103347 [details]
Removing the workaround.

This looks great, but we'll need a ChangeLog.
Comment 6 Adam Barth 2011-08-11 11:24:22 PDT
Created attachment 103648 [details]
Patch for landing
Comment 7 Adam Barth 2011-08-11 11:25:15 PDT
I added a ChangeLog for you.
Comment 8 WebKit Review Bot 2011-08-11 12:25:03 PDT
Comment on attachment 103648 [details]
Patch for landing

Clearing flags on attachment: 103648

Committed r92872: <http://trac.webkit.org/changeset/92872>
Comment 9 WebKit Review Bot 2011-08-11 12:25:07 PDT
All reviewed patches have been landed.  Closing bug.