Instantiate an AutoNoJSAPI when dispatching IPC messages

RESOLVED FIXED in mozilla37

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

unspecified
mozilla37
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Assignee

Description

5 years ago
Split off from bug 1102521.
Assignee

Comment 1

5 years ago
Is this what you had in mind bill? This seems a bit weird, because we need to
do the main-thread check (since AutoNoJSAPI doesn't know what to do on the
background IPC thread). It seems more logical to only push the NoJSAPI when
we're re-entering with an existing sync message waiting on the callstack.
Attachment #8527144 - Flags: review?(wmccloskey)
(In reply to Bobby Holley (:bholley) from comment #1)
> Created attachment 8527144 [details] [diff] [review]
> Instantiate an AutoNoJSAPI when dispatching IPC messages. v1
> 
> Is this what you had in mind bill?

Yes.

> It seems more logical to only push the NoJSAPI when
> we're re-entering with an existing sync message waiting on the callstack.

I don't understand why, but maybe that's because I guess I don't understand the motivation for this change. I thought we just wanted to ensure that anyone using JS had to use AutoJSAPI or AutoEntryScript. So why would we only use NoJSAPI in the case you mention? Any message can cause us to run JS, and I think we want to enforce the use of AutoJSAPI for all of them.

Are you trying to pick a path that's mainly used by CPOWs? CPOWs often do take that path, but many of them don't. I guess I'd be okay doing it that way as an incremental step towards adding NoJSAPI to DispatchMessage. It does look like the try push is pretty orange.
Comment on attachment 8527144 [details] [diff] [review]
Instantiate an AutoNoJSAPI when dispatching IPC messages. v1

We talked about this and it's what we want to do. We still need to sort out the oranges though.
Attachment #8527144 - Flags: review?(wmccloskey) → review+
Assignee

Comment 5

5 years ago
I actually took another crack at this earlier this week, but try was closed.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b1527e14eb1b
Assignee

Comment 6

5 years ago
This is green modulo unified compilation bustage - I've done a local unified build to verify my fix.

https://hg.mozilla.org/integration/mozilla-inbound/rev/96dbd5483c05
https://hg.mozilla.org/mozilla-central/rev/96dbd5483c05
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.