Query Document from Parent in case of nested workers

RESOLVED FIXED in Firefox 40

Status

()

Core
DOM: Security
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: ckerschb, Assigned: ckerschb)

Tracking

(Blocks: 1 bug)

unspecified
mozilla40
Points:
---

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment)

2.12 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

3 years ago
When creating a channel for a worker we should either have a document [1] or we should put an assertion that the principal used to create the channel is the systemPrincipal.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/workers/ScriptLoader.cpp#111
(Assignee)

Comment 1

3 years ago
Initially we thought we just have to query the document from the parent, in case we don't have one available. Altering the code for GetDocument() to something like the following:

> template <class Derived>
> nsIDocument*
> WorkerPrivateParent<Derived>::GetDocument() const
> {
>  AssertIsOnMainThread();
>  if (mLoadInfo.mWindow) {
>    return mLoadInfo.mWindow->GetExtantDoc();
>  }
>  // if we don't have a document, we should get the document from the parent
>  WorkerPrivate* parent = mParent;
>  while (parent) {
>    if (parent->mLoadInfo.mWindow) {
>      return parent->mLoadInfo.mWindow->GetExtantDoc();
>    }
>    parent = parent->GetParent();
>  }
>  // couldn't query a document, give up and return nullptr
>  return nullptr;
}


Apparently the parent is also null in this cases and we can not query a document from the parent either.
One of the problems is here, where we set the parent to 'null':
  http://mxr.mozilla.org/mozilla-central/source/dom/workers/WorkerPrivate.cpp#4280

Jonas, any ideas how we could proceed and fix?
Flags: needinfo?(jonas)
(Assignee)

Updated

3 years ago
Blocks: 1006868

Comment 2

3 years ago
The description here says "private worker", but comment 0 just says "worker".  Can you elaborate on what this means?

Do you just want to check for the existence of a document or do you actually need to actively call methods on the document?

This would also cause you troubles as it clears mWindow of any worker that has finished loading:

  http://mxr.mozilla.org/mozilla-central/source/dom/workers/WorkerPrivate.cpp#3711
(Assignee)

Comment 3

3 years ago
Just chatted with Jonas on IRC; we think that adding an assertion that the principal is the systemPrincipal when creating the channel *could* be added for:
a) dedicated workers [yes]
b) shared workers [mabye]
c) service workers [probably not]
Flags: needinfo?(jonas)
(Assignee)

Comment 4

3 years ago
(In reply to Ben Kelly [:bkelly] from comment #2)
> The description here says "private worker", but comment 0 just says
> "worker".  Can you elaborate on what this means?

Sorry for the confusion, I think comment 3 should answer your question.

> Do you just want to check for the existence of a document or do you actually
> need to actively call methods on the document?

We would pass the document along to be set on the 'loadInfo of the channel', which would happen here:
http://mxr.mozilla.org/mozilla-central/source/dom/workers/ScriptLoader.cpp#112

> This would also cause you troubles as it clears mWindow of any worker that
> has finished loading:

Agreed, but our usecase is slightly different. And the loadInfo would keep the reference to the window.
(Assignee)

Updated

3 years ago
Summary: Create Channel for private worker only if a document is passed or the principal is system → Query Document from Parent in case of nested workers
(Assignee)

Comment 5

3 years ago
Created attachment 8578289 [details] [diff] [review]
bug_1143848_workers.patch
Attachment #8578289 - Flags: review?(bent.mozilla)
(Assignee)

Comment 6

3 years ago
We decided it would be great to have the document in case of a nested worker in which case updating ::GetDocument() is what we want in the end.
(Assignee)

Updated

3 years ago
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Attachment #8578289 - Flags: review?(bent.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/73655764d8b4
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.