Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Description • 9 years ago

Right now, if we send a CPOW to the child and it runs a nested event loop, we cancel the CPOW. However, a related issue can happen in the parent as well if you send a sync message from child to parent and the parent runs a nested event loop. This can happen if an event is triggered on a page and the event handler in the chrome process opens an alert or something.

This patch cancels the sync message in the same way, although only if it's high priority. Normal priority sync messages can't be canceled. Luckily, we're much more careful with normal-priority sync messages. High priority messages are harder because typically trigger add-on code, which can do almost anything.

	Pulsebot
	Comment 1 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/52e0192973b3

	Carsten Book [:Tomcat]
	Comment 2 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/52e0192973b3

	Pulsebot
	Comment 3 • 9 years ago

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b268160e8919

	Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Comment 4 • 9 years ago

Backed out to see if it's related to a topcrash.

	William Lachance (:wlach)
	Comment 5 • 9 years ago

FYI backing this out seems to have regressed tps:

http://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=mozilla-inbound&newProject=mozilla-inbound&originalRevision=2499a66d5b37&newRevision=b268160e8919&originalSignature=2134698ffee49a2f33235c3640553083cf715b8a&newSignature=2134698ffee49a2f33235c3640553083cf715b8a

Running a few retriggers to be sure. I'll let jmaher weigh in when he returns, but it doesn't seem sensible to demand that this be fixed given the magnitude of the regression -- of course making sure things work properly is the correct trade off. Just giving a heads up.

	Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Comment 6 • 9 years ago

There was an "improvement" in Talos numbers when bug 967873 landed and a "regression" when it was backed out. I suspect both of these are spurious changes anyway, but in any case, this bug is unlikely to be involved. It was just backed out at the same time as bug 967873.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 7 • 9 years ago

I don't think there is anything specific to do here, we landed something, got an improvement and backed it out.  I did some retriggers and the link in comment 5 shows a lot of details now.

It will be nice to see this land again if possible- perf improvements are good.

	:Gijs (he/him)
	Comment 8 • 9 years ago

(In reply to Bill McCloskey (:billm) from comment #4)
> Backed out to see if it's related to a topcrash.

Should we reland this?