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

Status: RESOLVED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Marco Bonardo [:mak]]

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.

Comment 1

[Shane Caraveo (:mixedpuppy)]

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.

Comment 2

[Shane Caraveo (:mixedpuppy)]

Attachment: remove data urls from tests, prevent data url usage

https://tbpl.mozilla.org/?tree=Try&rev=90867e4838a1

Comment 3

[Shane Caraveo (:mixedpuppy)]

@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.

Comment 4

[Marco Bonardo [:mak]]

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.

Comment 5

[Marco Bonardo [:mak]]

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.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

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?

Comment 7

[Mark Hammond [:markh] [:mhammond]]

Attachment: Use a .sjs file to avoid data: urls

(In reply to :Gavin Sharp (use [e-mail] 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.

Comment 8

[Shane Caraveo (: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.

Comment 9

[Mark Hammond [:markh] [:mhammond]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f7cafbd4ae09

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f7cafbd4ae09