Closed Bug 891516 Opened 7 years ago Closed 7 years ago

Frameworker tries to use the DOM Storage of a resource or about uri for data uris

Categories

(Firefox Graveyard :: SocialAPI, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 25

People

(Reporter: mak, Assigned: markh)

References

Details

Attachments

(1 file, 1 obsolete file)

removing the possibility to use DOM Storage from about pages uncovered the fact Frameworker is trying to copy the DOM Storage object from either hiddenWindow.html (Windows/Linux) or about:blank (Mac) when trying to build one for data uris. This is not the expected behavior.

Tests using DOM Storage or indexedDB cannot rely on data uris cause these have an about:blank principal that doesn't support those features.
of note: we dont allow providers to use data urls here, we're only using them in tests.  We can change the tests and prevent use of data or about uris in frameworker.
@mak this is a larger change than I anticipated, might be worth while testing this on top of bug 889442 prior to mark spending time on review.
Flags: needinfo?(mak77)
it passes the test now, though if I add an assert in DOM storage, looks like something is still trying to access DOM Storage for about:blank
I'm trying to verify what's doing that.
Flags: needinfo?(mak77)
ah, it's this call http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#2577 in nsGlobalWindow::SetNewDocument, very likely unrelated and unavoidable.

Checking the (inverted) scope key for DOM Storage now properly shows "example.com", so looks fine.
To avoid bloating worker_social.js too much (and separating the worker logic from the test logic), can we have it accept a "run-js" message that just eval()s the passed-in data? That way tests can continue to define the worker code as a function in the test, but instead of doing e.g.:

let worker = getFrameWorkerHandle(makeWorkerUrl(run), undefined, "testSimple");

they'd do:

let worker = getFrameWorkerHandle(worker_social.js, undefined, "testSimple");
worker.postMessage({topic:"run-js", script: run.toSource()});

or somesuch?
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #6)
> To avoid bloating worker_social.js too much (and separating the worker logic
> from the test logic), can we have it accept a "run-js" message that just

That would solve many, but not all use-cases - but a .sjs file can solve them all.  The new .sjs file in this patch just echos the query-string back, so instead of a data URL we use a http URL pointing at this .sjs with the encoded javascript as the query string.  End result is much lighter-weight and (almost) no changes to the tests.
Assignee: mixedpuppy → mhammond
Attachment #772992 - Attachment is obsolete: true
Attachment #772992 - Flags: review?(mhammond)
Attachment #773751 - Flags: review?(mixedpuppy)
Comment on attachment 773751 [details] [diff] [review]
Use a .sjs file to avoid data: urls

very cool!

One thing, some of the tests have no checks in them, I had added those into the patch I did.  For example, I think testPrototypes should have 

ok(!!e.data.data.customfunction, "customfuction exists")

rather than using it as part of the if statement.  They would probably time out otherwise, but a message might be helpful.
Attachment #773751 - Flags: review?(mixedpuppy) → review+
https://hg.mozilla.org/mozilla-central/rev/f7cafbd4ae09
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.