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

RESOLVED FIXED in Firefox 25

Status

()

Firefox
SocialAPI
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mak, Assigned: markh)

Tracking

unspecified
Firefox 25
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
Created attachment 772992 [details] [diff] [review]
remove data urls from tests, prevent data url usage

https://tbpl.mozilla.org/?tree=Try&rev=90867e4838a1
Attachment #772992 - Flags: review?(mhammond)
@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?
(Assignee)

Comment 7

5 years ago
Created attachment 773751 [details] [diff] [review]
Use a .sjs file to avoid data: urls

(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
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
You need to log in before you can comment on or make changes to this bug.