This is a followup issue from bug 1354966 comment 25. Right now LookAndFeel::GetColor is whitelisted in the heap-write analysis [1]. We have a lock, but that lock lives in Gecko_GetLookAndFeelSystemColor, which means that it wouldn't protect other call paths to LookAndFeel::GetColor, if they exist. So the first-order fix here would be to whitelist Gecko_GetLookAndFeelSystemColor instead, alongside the other locked stuff. However, things are extra tricky because bug 1401063 removed the platform API calls from LookAndFeel::GetColor in the gtk case, which means that the analysis (which runs on linux) would actually be if we didn't whitelist Gecko_GetLookAndFeelSystemColor at all. This is kind of a dangerous place to be, so we should probably strip the platform API calls from all the NativeGetColor implementations, which in turn would allow us to eliminate sServoWidgetLock entirely. [1] http://searchfox.org/mozilla-central/rev/3dbb47302e114219c53e99ebaf50c5cb727358ab/js/src/devtools/rootAnalysis/analyzeHeapWrites.js#472

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #0) > which means that the > analysis (which runs on linux) would actually be if we didn't whitelist > Gecko_GetLookAndFeelSystemColor at all. s/actually be/actually be green/.

The current patches make progress on this, but are visually problematic at least on macOS (where I can easily test). They display incorrect colors in Chrome elements on macOS. The patches attempt to cache colors in nsLookAndFeel::NativeInit(), which seems to be intended for this purpose. But I see that ServoStyleSet::PreTraverseSync calls NativeInit on every traversal: http://searchfox.org/mozilla-central/rev/a4702203522745baff21e519035b6c946b7d710d/layout/style/ServoStyleSet.cpp#420. Bobby, is NativeInit intended to be a one-time only init? Or is it appropriate to be called per traversal (perhaps to handle changes of OS theming while the app is open)?

(In reply to Brad Werth [:bradwerth] from comment #5) > Bobby, is NativeInit intended to be a one-time only init? Or is it > appropriate to be called per traversal (perhaps to handle changes of OS > theming while the app is open)? Bug 1386915 added NativeInit as a per-traversal call. At the time, only gtk/Linux did anything with NativeInit. This bug may change that and require change to the approach of calling it once per Stylo traversal.

(In reply to Brad Werth [:bradwerth] from comment #6) > Bug 1386915 added NativeInit as a per-traversal call. At the time, only > gtk/Linux did anything with NativeInit. This bug may change that and require > change to the approach of calling it once per Stylo traversal. I'm still not entirely sure why this matters. It's called every traversal, but it checks mInitialized and early-returns, so it's just a lazy init routine. I don't know why that's relevant to this bug.

And summarizing from the meeting: * We should figure out if it's correct to cache the system colors on all the platforms. If so, we should do it on mac/windows. If not, we should back out the caching on linux, so that our linux-only static analysis properly recognizes that those codepaths trigger widget calls. * We should probably go ahead and do bug 1403699 immediately, and just lock in Gecko_GetLookAndFeelSystemColor for now.

Okay, got this working visually for macOS by doing the color caching as demanded by the first call to NativeGetColor, which is the approach taken by the existing, presumably correct gtk implementation. I'm now asking for review by DOM peers for macOS and Windows to see if this approach is acceptable on those platforms while I move ahead on implementation on Android and iOS.

For part 2, where is the code that invalidates the cached values when we receive a WM_SETTINGCHANGE?

(In reply to Jim Mathies [:jimm] from comment #13) > For part 2, where is the code that invalidates the cached values when we > receive a WM_SETTINGCHANGE? I've updated all parts of the patch (including part 2 for Windows) to reset the mInitialized variable within the nsLookAndFeel::RefreshImpl() function. That function is called in platform-specific circumstances. On Windows, it is called in response to the WM_SETTINGCHANGE event.

Comment on attachment 8914567 [details] Bug 1403690 Part 3: macOS change nsLookAndFeel::NativeGetColor to use cached colors. https://reviewboard.mozilla.org/r/185902/#review193342 Caching these colors is fine as long as they're invalidated by a call to NotifyThemeChanged, which does seem to be the case. There are a few instances of end-of-line whitespace in the patch.

Comment on attachment 8917155 [details] Bug 1403690 Part 4: gtk rearrange header and implementation to keep init and refresh functions together. https://reviewboard.mozilla.org/r/188168/#review193390

Comment on attachment 8914565 [details] Bug 1403690 Part 1: Adjust heap write analysis whitelist to include Gecko_GetLookAndFeelSystemColor. https://reviewboard.mozilla.org/r/185898/#review193420 ::: js/src/devtools/rootAnalysis/analyzeHeapWrites.js:483 (Diff revision 3) > // Unable to trace through dataflow, but straightforward if inspected. > "Gecko_NewNoneTransform", > > // Need main thread assertions or other fixes. > /EffectCompositor::GetServoAnimationRule/, > - /LookAndFeel::GetColor/, > + /Gecko_GetLookAndFeelSystemColorr/, two rs?

Comment on attachment 8914566 [details] Bug 1403690 Part 2: Windows change nsLookAndFeel::NativeGetColor to use cached colors. https://reviewboard.mozilla.org/r/185900/#review193694

Comment on attachment 8917132 [details] Bug 1403690 Part 5: Android change pref and color caching to match approach used in gtk, macOS, and Windows. https://reviewboard.mozilla.org/r/188136/#review194870

Patches 1-5 have r+. We're just waiting for ted to review patch 6. I don't think we need to uplift this to Beta 57.

Comment on attachment 8917156 [details] Bug 1403690 Part 6: UIKit change nsLookAndFeel::NativeGetColor to use cached colors. https://reviewboard.mozilla.org/r/188170/#review197156

Pushed by bwerth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5e820fb50295 Part 1: Adjust heap write analysis whitelist to include Gecko_GetLookAndFeelSystemColor. r=manishearth https://hg.mozilla.org/integration/autoland/rev/69668003e827 Part 2: Windows change nsLookAndFeel::NativeGetColor to use cached colors. r=jimm https://hg.mozilla.org/integration/autoland/rev/c8262b239d17 Part 3: macOS change nsLookAndFeel::NativeGetColor to use cached colors. r=mstange https://hg.mozilla.org/integration/autoland/rev/349f51081127 Part 4: gtk rearrange header and implementation to keep init and refresh functions together. r=karlt https://hg.mozilla.org/integration/autoland/rev/6a5e056b7fb9 Part 5: Android change pref and color caching to match approach used in gtk, macOS, and Windows. r=snorp https://hg.mozilla.org/integration/autoland/rev/29e6612a4083 Part 6: UIKit change nsLookAndFeel::NativeGetColor to use cached colors. r=ted

Backed out for build bustage in widget/windows/nsLookAndFeel.cpp on Windows and Android: https://hg.mozilla.org/integration/autoland/rev/e242af21e3d2ec780a7fa9f56fbd2ce2e0c14d89 https://hg.mozilla.org/integration/autoland/rev/3003177f00f3aa9b359304fcc67d8acec08281fd https://hg.mozilla.org/integration/autoland/rev/087a3875d2855c6f8d29a402f5f8a8aef431460f https://hg.mozilla.org/integration/autoland/rev/9c19645d8078c70eb7d054ecf30fdfa00adfd186 https://hg.mozilla.org/integration/autoland/rev/888af9141ff82c22805a59e1e85425528ab20565 https://hg.mozilla.org/integration/autoland/rev/6ca4d18dede7c3536417e7c01e5fd6acfe4e2a3e Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=29e6612a4083dce3e7838e95f887bf77c61d2c2a&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Build log Windows: https://treeherder.mozilla.org/logviewer.html#?job_id=138924182&repo=autoland z:/build/build/src/widget/windows/nsLookAndFeel.cpp(87): error C2723: 'nsLookAndFeel::NativeInit': 'final' specifier illegal on function definition z:/build/build/src/widget/windows/nsLookAndFeel.cpp(294): error C2065: 'mHasAccentColor': undeclared identifier z:/build/build/src/widget/windows/nsLookAndFeel.cpp(295): error C2065: 'mAccentColor': undeclared identifier z:/build/build/src/widget/windows/nsLookAndFeel.cpp(302): error C2065: 'mHasAccentColorText': undeclared identifier z:/build/build/src/widget/windows/nsLookAndFeel.cpp(303): error C2065: 'mAccentColorText': undeclared identifier z:/build/build/src/widget/windows/nsLookAndFeel.cpp(886): error C2065: 'mHasAccentColor': undeclared identifier z:/build/build/src/widget/windows/nsLookAndFeel.cpp(889): error C2065: 'mHasAccentColorText': undeclared identifier Build log Android: https://treeherder.mozilla.org/logviewer.html#?job_id=138924174&repo=autoland [task 2017-10-23T15:55:07.909Z] 15:55:07 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/widget/android/Unified_cpp_widget_android1.cpp:20:0: [task 2017-10-23T15:55:07.910Z] 15:55:07 INFO - /builds/worker/workspace/build/src/widget/android/nsLookAndFeel.cpp: In member function 'void nsLookAndFeel::EnsureInit()': [task 2017-10-23T15:55:07.910Z] 15:55:07 INFO - /builds/worker/workspace/build/src/widget/android/nsLookAndFeel.cpp:508:49: error: 'NS_SUCCESS' was not declared in this scope [task 2017-10-23T15:55:07.910Z] 15:55:07 INFO - mInitializedSystemColors = NS_SUCCESS(rv); [task 2017-10-23T15:55:07.911Z] 15:55:07 INFO - ^ [task 2017-10-23T15:55:07.911Z] 15:55:07 INFO - /builds/worker/workspace/build/src/config/rules.mk:1072: recipe for target 'Unified_cpp_widget_android1.o' failed [task 2017-10-23T15:55:07.911Z] 15:55:07 INFO - make[5]: *** [Unified_cpp_widget_android1.o] Error 1

Pushed by bwerth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/34accef71609 Part 1: Adjust heap write analysis whitelist to include Gecko_GetLookAndFeelSystemColor. r=manishearth https://hg.mozilla.org/integration/autoland/rev/251b52554fee Part 2: Windows change nsLookAndFeel::NativeGetColor to use cached colors. r=jimm https://hg.mozilla.org/integration/autoland/rev/9861b50e325b Part 3: macOS change nsLookAndFeel::NativeGetColor to use cached colors. r=mstange https://hg.mozilla.org/integration/autoland/rev/7636f668f84e Part 4: gtk rearrange header and implementation to keep init and refresh functions together. r=karlt https://hg.mozilla.org/integration/autoland/rev/cc5bcbafc6cc Part 5: Android change pref and color caching to match approach used in gtk, macOS, and Windows. r=snorp https://hg.mozilla.org/integration/autoland/rev/04439a161b82 Part 6: UIKit change nsLookAndFeel::NativeGetColor to use cached colors. r=ted

Backed out for crashing many Android XPCshell runs: https://hg.mozilla.org/integration/autoland/rev/0617fbf40ce4751e4f9a5565ea7e808c60eb2aba https://hg.mozilla.org/integration/autoland/rev/3e8355db9d8867168ae99b8b9334e0dcf416cc39 https://hg.mozilla.org/integration/autoland/rev/925684bd136dfa101a3a9ef28f07a81b94d0c021 https://hg.mozilla.org/integration/autoland/rev/d341020e5fb5c94add8b641ef9007397877427db https://hg.mozilla.org/integration/autoland/rev/3d856c36245d71a086e43496c88eacc1b3c98bb6 https://hg.mozilla.org/integration/autoland/rev/e945c00c958af4ce52235f886d1435b6f213bb7a Range with push and failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&fromchange=43e45144dd9fffa826b7b4d8289f77ddeea172c6&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=success&filter-searchStr=android%20xpcshell&tochange=0617fbf40ce4751e4f9a5565ea7e808c60eb2aba&selectedJob=138955242 Unfortunately xpcshell tests are missing symbols (bug 1389805). Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=138955867&repo=autoland [task 2017-10-23T18:24:58.072Z] 18:24:58 INFO - TEST-START | xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js [task 2017-10-23T18:25:20.130Z] 18:25:20 WARNING - TEST-UNEXPECTED-FAIL | xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | xpcshell return code: 139 [task 2017-10-23T18:25:20.132Z] 18:25:20 INFO - TEST-INFO took 22058ms [task 2017-10-23T18:25:20.134Z] 18:25:20 INFO - >>>>>>> [task 2017-10-23T18:25:20.134Z] 18:25:20 INFO - xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | xpcw: cd /storage/sdcard/tests/xpc/toolkit/components/extensions/test/xpcshell [task 2017-10-23T18:25:20.135Z] 18:25:20 INFO - xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | xpcw: xpcshell -r /storage/sdcard/tests/xpc/c/httpd.manifest --greomni /data/local/xpcb/target.apk -m -s -e const _HEAD_JS_PATH = "/storage/sdcard/tests/xpc/head.js"; -e const _MOZINFO_JS_PATH = "/storage/sdcard/tests/xpc/p/mozinfo.json"; -e const _TESTING_MODULES_DIR = "/storage/sdcard/tests/xpc/m"; -f /storage/sdcard/tests/xpc/head.js -e const _SERVER_ADDR = "localhost" -e const _HEAD_FILES = ["/storage/sdcard/tests/xpc/toolkit/components/extensions/test/xpcshell/head.js", "/storage/sdcard/tests/xpc/toolkit/components/extensions/test/xpcshell/head_telemetry.js"]; -e const _JSDEBUGGER_PORT = 0; -e const _TEST_FILE = ["test_ext_runtime_connect_no_receiver.js"]; -e const _TEST_NAME = "xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js" -e _execute_test(); quit(0); [task 2017-10-23T18:25:20.135Z] 18:25:20 INFO - xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | [861, Unnamed thread 45c16080] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/workspace/build/src/xpcom/base/nsTraceRefcnt.cpp, line 171 [task 2017-10-23T18:25:20.136Z] 18:25:20 INFO - xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | [861, Main Thread] WARNING: No CID found when attempting to map contract ID: file /builds/worker/workspace/build/src/xpcom/components/nsComponentManager.cpp, line 508 [task 2017-10-23T18:25:20.136Z] 18:25:20 INFO - xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | [861, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 2872 [task 2017-10-23T18:25:20.136Z] 18:25:20 INFO - (xpcshell/head.js) | test MAIN run_test pending (1) [task 2017-10-23T18:25:20.136Z] 18:25:20 INFO - (xpcshell/head.js) | test run_next_test 0 pending (2) [task 2017-10-23T18:25:20.137Z] 18:25:20 INFO - (xpcshell/head.js) | test MAIN run_test finished (2) [task 2017-10-23T18:25:20.137Z] 18:25:20 INFO - running event loop [task 2017-10-23T18:25:20.138Z] 18:25:20 INFO - xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | Starting check_remote [task 2017-10-23T18:25:20.138Z] 18:25:20 INFO - (xpcshell/head.js) | test check_remote pending (2) [task 2017-10-23T18:25:20.138Z] 18:25:20 INFO - TEST-PASS | xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | check_remote - [check_remote : 241] useRemoteWebExtensions matches - false == false [task 2017-10-23T18:25:20.139Z] 18:25:20 INFO - TEST-PASS | xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | check_remote - [check_remote : 241] testing from extension process - true == true [task 2017-10-23T18:25:20.139Z] 18:25:20 INFO - (xpcshell/head.js) | test run_next_test 0 finished (2) [task 2017-10-23T18:25:20.139Z] 18:25:20 INFO - (xpcshell/head.js) | test run_next_test 1 pending (2) [task 2017-10-23T18:25:20.139Z] 18:25:20 INFO - (xpcshell/head.js) | test check_remote finished (2) [task 2017-10-23T18:25:20.140Z] 18:25:20 INFO - xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | Starting test_connect_without_listener [task 2017-10-23T18:25:20.140Z] 18:25:20 INFO - (xpcshell/head.js) | test test_connect_without_listener pending (2) [task 2017-10-23T18:25:20.140Z] 18:25:20 INFO - "Extension attached" [task 2017-10-23T18:25:20.140Z] 18:25:20 INFO - xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | [861, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file /builds/worker/workspace/build/src/netwerk/protocol/res/SubstitutingProtocolHandler.cpp, line 333 [task 2017-10-23T18:25:20.140Z] 18:25:20 INFO - (xpcshell/head.js) | test run_next_test 1 finished (2) [task 2017-10-23T18:25:20.141Z] 18:25:20 INFO - xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | [861, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004002: file /builds/worker/workspace/build/src/toolkit/components/resistfingerprinting/nsRFPService.cpp, line 182 [task 2017-10-23T18:25:20.141Z] 18:25:20 INFO - xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | [861, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file /builds/worker/workspace/build/src/extensions/cookie/nsPermissionManager.cpp, line 2874 [task 2017-10-23T18:25:20.142Z] 18:25:20 INFO - xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | [861, Main Thread] WARNING: Vsync not supported. Falling back to software vsync: file /builds/worker/workspace/build/src/gfx/thebes/gfxAndroidPlatform.cpp, line 401 [task 2017-10-23T18:25:20.142Z] 18:25:20 INFO - xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | Segmentation fault [task 2017-10-23T18:25:20.143Z] 18:25:20 INFO - xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | 13 [task 2017-10-23T18:25:20.143Z] 18:25:20 INFO - <<<<<<< [task 2017-10-23T18:25:20.410Z] 18:25:20 INFO - mozcrash Copy/paste: /builds/worker/workspace/build/linux64-minidump_stackwalk /tmp/tmpqwMkKx/725db12b-7b39-2e8f-ec32-195c5d33c83d.dmp /builds/worker/workspace/build/symbols [task 2017-10-23T18:25:20.779Z] 18:25:20 INFO - mozcrash Saved minidump as /builds/worker/workspace/build/blobber_upload_dir/725db12b-7b39-2e8f-ec32-195c5d33c83d.dmp [task 2017-10-23T18:25:20.780Z] 18:25:20 INFO - mozcrash Saved app info as /builds/worker/workspace/build/blobber_upload_dir/725db12b-7b39-2e8f-ec32-195c5d33c83d.extra [task 2017-10-23T18:25:20.782Z] 18:25:20 WARNING - PROCESS-CRASH | xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js | application crashed [@ libxul.so + 0x1398ea4] [task 2017-10-23T18:25:20.782Z] 18:25:20 INFO - Crash dump filename: /tmp/tmpqwMkKx/725db12b-7b39-2e8f-ec32-195c5d33c83d.dmp [task 2017-10-23T18:25:20.782Z] 18:25:20 INFO - Operating system: Android [task 2017-10-23T18:25:20.782Z] 18:25:20 INFO - 0.0.0 Linux 2.6.29-gea477bb #1 Wed Sep 26 11:04:45 PDT 2012 armv7l [task 2017-10-23T18:25:20.782Z] 18:25:20 INFO - CPU: arm [task 2017-10-23T18:25:20.782Z] 18:25:20 INFO - ARMv7 ARM Cortex-A8 features: swp,half,thumb,fastmult,vfpv2,edsp,neon,vfpv3 [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - 1 CPU [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - GPU: UNKNOWN [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - Crash reason: SIGSEGV [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - Crash address: 0x0 [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - Process uptime: not available [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - Thread 0 (crashed) [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - 0 libxul.so + 0x1398ea4 [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - r0 = 0x00000078 r1 = 0x89e5edd7 r2 = 0x89e5edd7 r3 = 0x89e5edd7 [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - r4 = 0x00000000 r5 = 0x00000000 r6 = 0x00000000 r7 = 0x45c06854 [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - r8 = 0x00000000 r9 = 0xbebb4168 r10 = 0xbebb4248 r12 = 0x00000003 [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - fp = 0x00000000 sp = 0xbebb4010 lr = 0x41a3be13 pc = 0x41a3bea4 [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - Found by: given as instruction pointer in context [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - 1 libxul.so + 0x137a9ed [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - sp = 0xbebb4028 pc = 0x41a1d9ef [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.786Z] 18:25:20 INFO - 2 libxul.so + 0x137ca05 [task 2017-10-23T18:25:20.787Z] 18:25:20 INFO - sp = 0xbebb4030 pc = 0x41a1fa07 [task 2017-10-23T18:25:20.787Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.787Z] 18:25:20 INFO - 3 libxul.so + 0x1390a0f [task 2017-10-23T18:25:20.788Z] 18:25:20 INFO - sp = 0xbebb4058 pc = 0x41a33a11 [task 2017-10-23T18:25:20.788Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.788Z] 18:25:20 INFO - 4 libxul.so + 0x1390a59 [task 2017-10-23T18:25:20.789Z] 18:25:20 INFO - sp = 0xbebb4060 pc = 0x41a33a5b [task 2017-10-23T18:25:20.789Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.789Z] 18:25:20 INFO - 5 libxul.so + 0x1390a4f [task 2017-10-23T18:25:20.790Z] 18:25:20 INFO - sp = 0xbebb4064 pc = 0x41a33a51 [task 2017-10-23T18:25:20.790Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.790Z] 18:25:20 INFO - 6 libxul.so + 0x136e36d [task 2017-10-23T18:25:20.791Z] 18:25:20 INFO - sp = 0xbebb4070 pc = 0x41a1136f [task 2017-10-23T18:25:20.791Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.791Z] 18:25:20 INFO - 7 libxul.so + 0x1cc8057 [task 2017-10-23T18:25:20.792Z] 18:25:20 INFO - sp = 0xbebb4090 pc = 0x4236b059 [task 2017-10-23T18:25:20.792Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.793Z] 18:25:20 INFO - 8 libnss3.so + 0x94ad3 [task 2017-10-23T18:25:20.793Z] 18:25:20 INFO - sp = 0xbebb4098 pc = 0x400c1ad5 [task 2017-10-23T18:25:20.793Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.794Z] 18:25:20 INFO - 9 libxul.so + 0x52a17 [task 2017-10-23T18:25:20.794Z] 18:25:20 INFO - sp = 0xbebb40a0 pc = 0x406f5a19 [task 2017-10-23T18:25:20.794Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.795Z] 18:25:20 INFO - 10 libxul.so + 0x5574d [task 2017-10-23T18:25:20.795Z] 18:25:20 INFO - sp = 0xbebb40b0 pc = 0x406f874f [task 2017-10-23T18:25:20.795Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.796Z] 18:25:20 INFO - 11 libxul.so + 0x1cc4f2f [task 2017-10-23T18:25:20.796Z] 18:25:20 INFO - sp = 0xbebb40d0 pc = 0x42367f31 [task 2017-10-23T18:25:20.796Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.797Z] 18:25:20 INFO - 12 libxul.so + 0x2bdc37e [task 2017-10-23T18:25:20.797Z] 18:25:20 INFO - sp = 0xbebb40f8 pc = 0x4327f380 [task 2017-10-23T18:25:20.797Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.798Z] 18:25:20 INFO - 13 libxul.so + 0x1c0c131 [task 2017-10-23T18:25:20.798Z] 18:25:20 INFO - sp = 0xbebb4138 pc = 0x422af133 [task 2017-10-23T18:25:20.798Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.798Z] 18:25:20 INFO - 14 libxul.so + 0x561169 [task 2017-10-23T18:25:20.799Z] 18:25:20 INFO - sp = 0xbebb4150 pc = 0x40c0416b [task 2017-10-23T18:25:20.799Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.806Z] 18:25:20 INFO - 15 libxul.so + 0x1c0bf07 [task 2017-10-23T18:25:20.806Z] 18:25:20 INFO - sp = 0xbebb418c pc = 0x422aef09 [task 2017-10-23T18:25:20.806Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.806Z] 18:25:20 INFO - 16 libxul.so + 0x9e095 [task 2017-10-23T18:25:20.806Z] 18:25:20 INFO - sp = 0xbebb41a8 pc = 0x40741097 [task 2017-10-23T18:25:20.806Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.806Z] 18:25:20 INFO - 17 libxul.so + 0x56d6c3 [task 2017-10-23T18:25:20.807Z] 18:25:20 INFO - sp = 0xbebb41d8 pc = 0x40c106c5 [task 2017-10-23T18:25:20.807Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.807Z] 18:25:20 INFO - 18 libxul.so + 0x56d7b1 [task 2017-10-23T18:25:20.807Z] 18:25:20 INFO - sp = 0xbebb4208 pc = 0x40c107b3 [task 2017-10-23T18:25:20.807Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.808Z] 18:25:20 INFO - 19 libxul.so + 0x5777cf [task 2017-10-23T18:25:20.808Z] 18:25:20 INFO - sp = 0xbebb4228 pc = 0x40c1a7d1 [task 2017-10-23T18:25:20.808Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.808Z] 18:25:20 INFO - 20 libnss3.so + 0x94ad3 [task 2017-10-23T18:25:20.808Z] 18:25:20 INFO - sp = 0xbebb4240 pc = 0x400c1ad5 [task 2017-10-23T18:25:20.808Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.808Z] 18:25:20 INFO - 21 libnss3.so + 0x94ad3 [task 2017-10-23T18:25:20.809Z] 18:25:20 INFO - sp = 0xbebb4250 pc = 0x400c1ad5 [task 2017-10-23T18:25:20.809Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.809Z] 18:25:20 INFO - 22 libnss3.so + 0x94ad3 [task 2017-10-23T18:25:20.809Z] 18:25:20 INFO - sp = 0xbebb4260 pc = 0x400c1ad5 [task 2017-10-23T18:25:20.809Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.809Z] 18:25:20 INFO - 23 libxul.so + 0x52a17 [task 2017-10-23T18:25:20.809Z] 18:25:20 INFO - sp = 0xbebb4268 pc = 0x406f5a19 [task 2017-10-23T18:25:20.810Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.810Z] 18:25:20 INFO - 24 libxul.so + 0x2b32ca4 [task 2017-10-23T18:25:20.810Z] 18:25:20 INFO - sp = 0xbebb4274 pc = 0x431d5ca6 [task 2017-10-23T18:25:20.810Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.810Z] 18:25:20 INFO - 25 libxul.so + 0x5574d [task 2017-10-23T18:25:20.810Z] 18:25:20 INFO - sp = 0xbebb4278 pc = 0x406f874f [task 2017-10-23T18:25:20.811Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.811Z] 18:25:20 INFO - 26 libxul.so + 0x217ab29 [task 2017-10-23T18:25:20.811Z] 18:25:20 INFO - sp = 0xbebb42a0 pc = 0x4281db2b [task 2017-10-23T18:25:20.811Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.811Z] 18:25:20 INFO - 27 libxul.so + 0x3c215 [task 2017-10-23T18:25:20.811Z] 18:25:20 INFO - sp = 0xbebb42b0 pc = 0x406df217 [task 2017-10-23T18:25:20.812Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.812Z] 18:25:20 INFO - 28 libxul.so + 0x54346d [task 2017-10-23T18:25:20.812Z] 18:25:20 INFO - sp = 0xbebb42c8 pc = 0x40be646f [task 2017-10-23T18:25:20.812Z] 18:25:20 INFO - Found by: stack scanning [task 2017-10-23T18:25:20.812Z] 18:25:20 INFO - 29 libxul.so + 0x572c81 [task 2017-10-23T18:25:20.812Z] 18:25:20 INFO - sp = 0xbebb42e0 pc = 0x40c15c83 [task 2017-10-23T18:25:20.813Z] 18:25:20 INFO - Found by: stack scanning

I figured it out; the Android implementation was fetching both SystemColors and ShowPassword during the same call, which was problematic. I split the initialization method into two parts, and only init each part when needed, and the tests work again.

Pushed by bwerth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8b8017ae9cc1 Part 1: Adjust heap write analysis whitelist to include Gecko_GetLookAndFeelSystemColor. r=manishearth https://hg.mozilla.org/integration/autoland/rev/cf0f2ec461a7 Part 2: Windows change nsLookAndFeel::NativeGetColor to use cached colors. r=jimm https://hg.mozilla.org/integration/autoland/rev/1570e72037e2 Part 3: macOS change nsLookAndFeel::NativeGetColor to use cached colors. r=mstange https://hg.mozilla.org/integration/autoland/rev/4df8244690cf Part 4: gtk rearrange header and implementation to keep init and refresh functions together. r=karlt https://hg.mozilla.org/integration/autoland/rev/436c11e3c389 Part 5: Android change pref and color caching to match approach used in gtk, macOS, and Windows. r=snorp https://hg.mozilla.org/integration/autoland/rev/c9589d98c0b8 Part 6: UIKit change nsLookAndFeel::NativeGetColor to use cached colors. r=ted

https://hg.mozilla.org/mozilla-central/rev/8b8017ae9cc1 https://hg.mozilla.org/mozilla-central/rev/cf0f2ec461a7 https://hg.mozilla.org/mozilla-central/rev/1570e72037e2 https://hg.mozilla.org/mozilla-central/rev/4df8244690cf https://hg.mozilla.org/mozilla-central/rev/436c11e3c389 https://hg.mozilla.org/mozilla-central/rev/c9589d98c0b8

> Part 1: Adjust heap write analysis whitelist to include Gecko_GetLookAndFeelSystemColor Why are we doing this? Isn't the whole point of this patch set to make Gecko_GetLookAndFeelSystemColor not call into any external code? It would seem that whitelisting in this way prevents the static analysis from verifying that this was all done correctly.

(In reply to Bobby Holley (parental leave - send mail for anything urgent) from comment #66) > > Part 1: Adjust heap write analysis whitelist to include Gecko_GetLookAndFeelSystemColor > > Why are we doing this? Isn't the whole point of this patch set to make > Gecko_GetLookAndFeelSystemColor not call into any external code? It would > seem that whitelisting in this way prevents the static analysis from > verifying that this was all done correctly. You're right; I wasn't thinking clearly. I filed Bug 1412868 to sort this out and I'll have a patch shortly.