Closed Bug 1410482 Opened 3 years ago Closed 2 years ago
enable AOM for privileged content
No description provided.
Olli, who's a good candidate to review? You're not accepting those.
I could review next week, if that is ok. Could you use AddBoolVarCache and not GetBool. The cache is quite a bit faster.
3 years ago
Duplicate of this bug: 1410481
(In reply to Olli Pettay [:smaug] (work week Oct 16-20) from comment #3) > I could review next week, if that is ok. sure, thank you. > Could you use AddBoolVarCache and not GetBool. The cache is quite a bit > faster. I will, np, I didn't know it may be a hot path. I guess it's invoked not only once when the browser starts up?
address Olli's comments
apparently attached a wrong patch, fixed
Comment on attachment 8921045 [details] [diff] [review] aomenable [Func="mozilla::dom::AccessibleNode::IsAOMEnabled"] isn't probably needed. I'd expect [Func="AccessibleNode::IsAOMEnabled"] to work too, assuming AccessibleNode uses namespaces and such in the normal way.
Attachment #8921045 - Flags: feedback?(bugs) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/2602e10160a18c0b998c749d8aa2005fc8ef6614 Bug 1410482 - enable AOM for privileged content, r=smaug
Backed out for leaks in devtools tests: https://hg.mozilla.org/integration/mozilla-inbound/rev/9fddd9d18e1d1d879598b1a1e42e15cc17783cee Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=2602e10160a18c0b998c749d8aa2005fc8ef6614&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry Different push which also ran asan failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=d84ea38d6fd086a627f0321be28103be88fad175&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry Failure log debug: https://treeherder.mozilla.org/logviewer.html#?job_id=138962123&repo=mozilla-inbound > TEST-UNEXPECTED-FAIL | leakcheck | tab process: 407504 bytes leaked (APZEventState, ActiveElementManager, AsyncLatencyLogger, BackstagePass, CSPService, ...) Failure log asan: https://treeherder.mozilla.org/logviewer.html#?job_id=138978815&repo=mozilla-inbound
AccessibilityService is connected/caused with the leaks. If no GetOrCreateService() is called by AOM object, then no leaks. Good build: https://hg.mozilla.org/try/rev/1c81188d5618066332ba623fd3c26ebbc859790d https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c2e746ebbe929bfbdd836e8a7a9f3b97ecd39a4 Bad build: https://hg.mozilla.org/try/rev/34317d2fdf9c9954aca9af303fb33d0e653ec457 https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f2347d35a57c2af114f1f4382cdc997b874dfdf&selectedJob=158252012  points out to nsComponentManagerImpl. The interesting thing is the logs has a line of: Marionette DEBUG Received observer notification "xpcom-shutdown" after leak reports. Since we use "xpcom-shutdown" to shutdown accessibility service, then it makes me wonder if this could be a timing issue. Olli, do you have any suggestions/advices how to investigate the issue further?  https://treeherder.mozilla.org/logviewer.html#?job_id=158252265&repo=try&lineNumber=166708
Flags: needinfo?(surkov.alexander) → needinfo?(bugs)
Does someone perhaps create the service after xpcom-shutdown?
(In reply to Olli Pettay [:smaug] from comment #31) > Does someone perhaps create the service after xpcom-shutdown? unlikely, I tried to crash the browser  if somebody makes attempt to initialize AccessibilityService after XPCOM shutdown. No crash .  https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9d29ff0f57ce5ae1959ecdc4fbd440179a39b8b&selectedJob=158534669  https://hg.mozilla.org/try/rev/ac7671a7d94f0f34100cd598cc5f1a0ed360693e
Olli, any other advice how to approach it, or maybe names of who was working in this area and might have ideas?
hmm, so do we leak AccessibleNodes? What is the most recent link a leak log? I'm trying to recall the ownership model of AccessibleNodes. Why we aren't cycle collecting its member variables?
seems passing now, not sure what was a fix, will land it
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8589b95008dd enable AOM for privileged content, r=smaug
You need to log in before you can comment on or make changes to this bug.