Bug 27992 - [V8] DOM Storage + Isolated worlds don't play well together (at least in Chromium layout tests)
Summary: [V8] DOM Storage + Isolated worlds don't play well together (at least in Chro...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-04 12:08 PDT by Jeremy Orlow
Modified: 2009-08-26 23:35 PDT (History)
4 users (show)

See Also:


Attachments
New tests and tests re-baselining (6.37 KB, patch)
2009-08-26 14:48 PDT, Yaar Schnitman
abarth: review+
eric: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Orlow 2009-08-04 12:08:58 PDT
The following has only been observed in the Chromium test shell.

LayoutTests/http/tests/security/isolatedworld/all-window-properties.html and LayoutTests/http/tests/security/isolatedworld/all-window-prototypes.html are failing 
when I enable local storage and session storage in the test shell.  It's not clear whether this is a real issue or not.

The expected result is simply printing "Done.".  Anything else that's printed are properties of the window object which shouldn't be there.

Here's a diff from one of the two results:
--- src/webkit/Debug/layout-test-results/LayoutTests/http/tests/security/isolatedworld/all-window-properties-expected.txt 
+++ src/webkit/Debug/layout-test-results/LayoutTests/http/tests/security/isolatedworld/all-window-properties-actual.txt 
@@ -1,1 +1,3 @@
+localStorage: FAIL: Visible in isolated world.
+sessionStorage: FAIL: Visible in isolated world.
 Done.


I'm going to exclude these two files in the test exceptions for now.

Chromium bug: http://code.google.com/p/chromium/issues/detail?id=18412
Comment 1 Jeremy Orlow 2009-08-18 14:04:03 PDT
Ping (because I'm concerned this could be a somewhat serious bug...but I just don't understand enough about isolated worlds to know for sure).
Comment 2 Adam Barth 2009-08-18 14:10:04 PDT
Yeah, sorry.  I've been working on other stuff.  I need to do another round of isolated worlds work...  It might be a couple weeks though.
Comment 3 Yaar Schnitman 2009-08-26 14:48:19 PDT
Created attachment 38639 [details]
New tests and tests re-baselining

It was decided that isolated scripts should be able to read properties of local and session storage set by normal scripts.
 
We had to fix the layout tests and add new ones:
1. isolatedWorld/all-window-properties.html: skips localStorage and sessionStorage
2. isolatedWorld/storage-properties.htmL: tests that properties can indeed be read.
3. isolatedWorld/storage-prototype.htmL: tests that properties on the prototype CAN NOT be read.

Also added new files to the platform-specific Skipped files.
Comment 4 Adam Barth 2009-08-26 20:36:28 PDT
Comment on attachment 38639 [details]
New tests and tests re-baselining

Great!  Thanks.

I'm marking this for the commit queue, but it's not likely to work because the patch isn't based in the right directory.  In the future, consider using "WebKitTools/Scripts/svn-create-patch" or "WebKitTools/Script/bugzilla-tool post-diff" to create the patch.
Comment 5 Eric Seidel (no email) 2009-08-26 20:38:52 PDT
Comment on attachment 38639 [details]
New tests and tests re-baselining

Rejecting patch 38639 from commit-queue.  This patch will require manual commit.

Patch https://bugs.webkit.org/attachment.cgi?id=38639 from bug 27992 failed to download and apply.
Comment 6 Eric Seidel (no email) 2009-08-26 23:24:29 PDT
patch -p0 "http/tests/security/isolatedWorld/all-window-properties.html" returned 1.  Pass --force to ignore patch failures.
Comment 7 Adam Barth 2009-08-26 23:26:57 PDT
Yeah, ok.  I'll land this manually.
Comment 8 Adam Barth 2009-08-26 23:35:03 PDT
Ack!  Hans!  The ChangeLog contains tabs!

http://trac.webkit.org/changeset/47809