I talked about this with Bill, I have a patch for it in my queue, I will file it together with the shim optimization patches. I think this should be lifted up to aurora. And this should be m7.

We also have a couple shims that operate on CPOWs (ContentDocShellTreeItem and ContentDocument). This regressed those, as well as prefetching. Unfortunately we don't have any tests for those shims. This is probably a good time to add them.

Here are some tests. They currently fail.

Comment on attachment 8604859 [details] [diff] [review] patch Review of attachment 8604859 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, thanks billm.

Thanks for tests Bill! Saved me tons of time with them...

Comment on attachment 8605112 [details] [diff] [review] interposition for CPOWS Review of attachment 8605112 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, sorry for the delay. Can you check in both patches when you're ready?

(In reply to Bill McCloskey (:billm) from comment #5) > Can you check in both patches when you're ready? It turns out the test does not work without the --e10s flag :( 52 INFO TEST-UNEXPECTED-FAIL | toolkit/components/addoncompat/tests/browser/browser_addonShims.js | got expected import result - Got [object XULElement], expected [object XULElement] https://treeherder.mozilla.org/#/jobs?repo=try&revision=89cd574587fc

Is there a way to run that part of the test in e10s mode only? I'm not quite sure how should I fix that test, I might land the patch without it, and land the test later... the fix should be uplifted to aurora ASAP I think.

Here's a new version of the test that works without --e10s.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f8993cea9f8e https://hg.mozilla.org/integration/mozilla-inbound/rev/63901344b6b0

https://hg.mozilla.org/mozilla-central/rev/f8993cea9f8e https://hg.mozilla.org/mozilla-central/rev/63901344b6b0

Comment on attachment 8605112 [details] [diff] [review] interposition for CPOWS I think this should be uplifted. Approval Request Comment [Feature/regressing bug #]: Bug 1101182 [User impact if declined]: Various add-on breakage in e10s mode (some part of the shim layer is broken) [Describe test coverage new/current, TreeHerder]: There is a test attached to the bug, it's doing fine on mc currently. [Risks and why]: I see no risk, simple and straightforward patch that gains a lot in terms of user experience for e10s users. [String/UUID change made/needed]: none

https://hg.mozilla.org/integration/mozilla-inbound/rev/da3d272ad1c2 https://hg.mozilla.org/integration/mozilla-inbound/rev/63671ebfa2dd

Backed out for frequent xpc::InterposeProperty crashes. And did this landed under the correct bug number? https://treeherder.mozilla.org/logviewer.html#?job_id=10268935&repo=mozilla-inbound

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

Also extremely frequent WinXP debug crashes: https://treeherder.mozilla.org/logviewer.html#?job_id=10275797&repo=mozilla-inbound And I'm pretty sure these were yours as well: https://treeherder.mozilla.org/logviewer.html#?job_id=10275970&repo=mozilla-inbound

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #13) > Backed out for frequent xpc::InterposeProperty crashes. And did this landed > under the correct bug number? It did not, these were for bug 1164014.

Setting this bug as Resolved Fixed per #c16. The discussion should move back to bug 1164014. (Also clearing the needinfo here and instead moving it to bug 1164014.)

Please post hg links rather than treeherder links. Also, set the status flags when you land :)

Any reason the tests weren't also landed?

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #19) > Please post hg links rather than treeherder links. Also, set the status > flags when you land :) alright: https://hg.mozilla.org/releases/mozilla-aurora/rev/12b3775fe29f (In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #20) > Any reason the tests weren't also landed? Hmm... I think the test should work as well on aurora. I was not entirely sure if it uses anything that we fixed on mc but was not uplifted, but now that I'm thinking about it, it should not be the case. Do you think we need to file another aurora request for that or is it OK to just land the test as well?

Tests can land without additional approval.