mozilla :: #screenshots

10 Aug 2017
05:11GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master: https://git.io/v7MRj
05:11GitHubscreenshots/master eec6629 YFdyh000: Pontoon: Update Chinese (zh-CN) localization of Firefox Screenshots...
05:11GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master: https://git.io/v7M0U
05:11GitHubscreenshots/master 243d635 Lasse Liehu: Pontoon: Update Finnish (fi) localization of Firefox Screenshots...
05:17circleci-botFailed: mozilla-pontoon's build (#2994; push) in mozilla-services/screenshots (master) -- https://circleci.com/gh/mozilla-services/screenshots/2994?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
05:18circleci-botFailed: mozilla-pontoon's build (#2995; push) in mozilla-services/screenshots (master) -- https://circleci.com/gh/mozilla-services/screenshots/2995?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
06:10GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master: https://git.io/v7Mug
06:10GitHubscreenshots/master 8a8f141 stoyan: Pontoon: Update Bulgarian (bg) localization of Firefox Screenshots...
06:10GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master: https://git.io/v7Mu2
06:10GitHubscreenshots/master 1a7f530 Michal Stanke: Pontoon: Update Czech (cs) localization of Firefox Screenshots...
06:17circleci-botFailed: mozilla-pontoon's build (#2996; push) in mozilla-services/screenshots (master) -- https://circleci.com/gh/mozilla-services/screenshots/2996?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
06:17circleci-botFailed: mozilla-pontoon's build (#2997; push) in mozilla-services/screenshots (master) -- https://circleci.com/gh/mozilla-services/screenshots/2997?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
06:50GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master: https://git.io/v7Mgo
06:50GitHubscreenshots/master fcfe3d4 Dinesh: Pontoon: Update Telugu (te) localization of Firefox Screenshots...
06:57circleci-botFailed: mozilla-pontoon's build (#2998; push) in mozilla-services/screenshots (master) -- https://circleci.com/gh/mozilla-services/screenshots/2998?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
07:10GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master: https://git.io/v7M2H
07:10GitHubscreenshots/master 97daff6 Drashti: Pontoon: Update Gujarati (gu-IN) localization of Firefox Screenshots...
07:50GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master: https://git.io/v7Mwl
07:50GitHubscreenshots/master eab262e Amin Mahmudian: Pontoon: Update Persian (fa) localization of Firefox Screenshots...
11:40GitHub[screenshots] niharikak101 created binary-image (+1 new commit): https://git.io/v7MAq
11:40GitHubscreenshots/binary-image 848ea81 Niharika Khanna: send image to server in binary
11:47GitHub[screenshots] niharikak101 opened pull request #3308: Send image to server in binary - wip (master...binary-image) https://git.io/v7MAy
11:48circleci-botFailed: niharikak101's build (#3001; push) in mozilla-services/screenshots (binary-image) -- https://circleci.com/gh/mozilla-services/screenshots/3001?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
12:19GitHub[screenshots] johngruen created clean-up-home (+1 new commit): https://git.io/v7Mh5
12:19GitHubscreenshots/clean-up-home bd59043 John Gruen: clean up home
12:24GitHub[screenshots] johngruen opened pull request #3309: clean up home (master...clean-up-home) https://git.io/v7MjC
12:26GitHub[screenshots] johngruen opened issue #3310: Address sass-lint warnings in home.scss https://git.io/v7MjD
12:27GitHub[screenshots] johngruen commented on issue #3309: Note, filed #3310 as an eventual follow up https://git.io/v7MjA
12:28GitHub[screenshots] johngruen commented on issue #3309: Looks like this is doing that nightly fail thing :) https://git.io/v7DeU
12:29GitHub[screenshots] johngruen commented on issue #3310: Note, this bug only applies when #3309 lands https://git.io/v7DeL
12:31GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master: https://git.io/v7Del
12:31GitHubscreenshots/master 55273ac Jordi Serratosa: Pontoon: Update Catalan (ca) localization of Firefox Screenshots...
12:32GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master: https://git.io/v7De8
12:32GitHubscreenshots/master ed469c7 Slimane Amiri: Pontoon: Update Kabyle (kab) localization of Firefox Screenshots...
12:40circleci-botFailed: mozilla-pontoon's build (#3003; push) in mozilla-services/screenshots (master) -- https://circleci.com/gh/mozilla-services/screenshots/3003?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
14:11GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master: https://git.io/v7D3L
14:11GitHubscreenshots/master c69a650 Mikalai Kozel: Pontoon: Update Belarusian (be) localization of Firefox Screenshots...
14:36GitHub[screenshots] jvehent created zap-baseline (+1 new commit): https://git.io/v7DZE
14:36GitHubscreenshots/zap-baseline cc88694 Julien Vehent [:ulfr]: Add ZAP Baseline scan to test section of circleci
14:36GitHub[screenshots] jvehent opened pull request #3311: Add ZAP Baseline scan to test section of circleci (master...zap-baseline) https://git.io/v7DZz
15:11GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master: https://git.io/v7DlK
15:11GitHubscreenshots/master c052102 Pin-guang Chen: Pontoon: Update Chinese (zh-TW) localization of Firefox Screenshots...
15:18circleci-botFailed: mozilla-pontoon's build (#3007; push) in mozilla-services/screenshots (master) -- https://circleci.com/gh/mozilla-services/screenshots/3007?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
15:18GitHub[screenshots] ianb opened issue #3312: Remove favicon from My Shots https://git.io/v7D8y
15:27GitHub[screenshots] ianb opened issue #3313: Create script to build release-ready XPI https://git.io/v7DBJ
15:39GitHub[screenshots] pdehaan commented on pull request #3309 bd59043: Not sure if you want to use the `@include respond-to("small") {` style helpers for consistency.... https://git.io/v7DR7
15:40GitHub[screenshots] pdehaan commented on pull request #3309 bd59043: Not sure if I'm missing something, but wouldn't this just overrule what you did on lines 316-320 above? https://git.io/v7D0J
16:00GitHub[screenshots] johngruen commented on issue #3306: Discussed in stand up https://git.io/v7Dui
16:28GitHub[screenshots] 6a68 opened issue #3314: Screenshots doesn't work on dxr.mozilla.org https://git.io/v7Daa
16:29GitHub[screenshots] 6a68 commented on issue #3301: I asked about this in #teamaddons, and both aswan and zombie thought that JS code shouldn't be able to cause this sort of crash. Digging further. https://git.io/v7DaP
16:30GitHub[screenshots] 6a68 commented on issue #3302: Closing: the actual fix may be in the JS interpreter code, see discussion in #3301 https://git.io/v7DaH
16:36GitHub[screenshots] ianb opened issue #3315: Keep webextension/buildSettings.js.template out of XPI https://git.io/v7DVo
16:37557A8BLXW[screenshots] 6a68 closed issue #3301: Detach sessionstore observer on shutdown (bug 1387598) https://git.io/v71zI
16:37203A924D1[screenshots] 6a68 commented on issue #3301: Yeah, Gijs is of the same opinion - JS code shouldn't be able to cause this kind of crash. Closing, I'll investigate further in IRC and reassign the bugzilla bug to the correct JS API person. https://git.io/v7DVM
16:37firebothttps://bugzil.la/1387598 NEW, nobody@mozilla.org Intermittent sessionrestore_no_auto_restore | application crashed [@ GetNameOperation]
16:37_6a68ianbicking: FYI, I'm being told that JS code should never be able to cause process crashes, so now I'm looking for a JS interpreter engineer to take the bug. We don't need to land or release that fix
16:38ianbicking_6a68: are we getting crashes, not just failed tests?
16:39_6a68yeah https://bugzilla.mozilla.org/show_bug.cgi?id=1387598
16:39_6a68crashes in the JS interpreter on windows only, intermittently
16:40_6a68it seems obvious, now that I think about it, that JS should be totally sandboxed
16:58jgruenhey clouserw ianbicking _6a68 do we need to have an eng meeting today?
16:58jgruenseems like we already did
16:59ianbickingyeah, I feel like I just got shit to do, we talked it out earlier
16:59ianbickingif we hadnt had that extra 15 minutes...
16:59ianbickingshould we turn the standups into 30 minutes again?
17:11clouserwI whacked the engineering meeting
17:12clouserwdo we still need to have this cross-functional meeting?
17:12ianbickingclouserw: I dont think so, we have lots of ad hoc cross functional needs, but those arent served by that meeting
17:13jgruenyeah i agree with that too
17:14jgruenheads down afternoon would be dope as hell
17:15clouserwjgruen: should I delete today's or delete it permanently? :)
17:15jgruentoday
17:15clouserwaww
17:15jgruenwe'll play it by ear next week
17:15clouserwone of the last cory meetings!
17:16GitHub[screenshots] ianb created prod-xpi-builder (+1 new commit): https://git.io/v7Din
17:16GitHubscreenshots/prod-xpi-builder c19ca30 Ian Bicking: Fix #3313, create script to create release-ready XPIs...
17:16GitHub[screenshots] ianb opened pull request #3316: Fix #3313, create script to create release-ready XPIs (master...prod-xpi-builder) https://git.io/v7DiW
17:26circleci-botFailed: ianb's build (#3008; push) in mozilla-services/screenshots (prod-xpi-builder) -- https://circleci.com/gh/mozilla-services/screenshots/3008?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
17:31_6a68will be a minute late
17:31ianbicking_6a68: all the meetings are cancelled!
17:33_6a68oh cool
17:33_6a68I gotta get a calendar app that actually updates properly
17:35_6a68oh, fun!
17:35GitHub[screenshots] ianb commented on issue #3312: This will implicitly fix #2648 https://git.io/v7DXx
17:35_6a68jgruen | clouserw | ianbicking: RyanVM just backed out https://bugzil.la/1386333 because of ts_paint breakage
17:35firebotBug 1386333 FIXED, jhirsch@mozilla.com Remove Screenshots rollout pref
17:35_6a68he mentioned in #fx-team that he thought it might be related to that other crash bug I mentioned this morning
17:36_6a68I guess he'll comment in the bug in a bit
17:36jgruenokay
17:37_6a68;_;
17:37_6a68backed out of Beta, specifically
17:37jgruen_6a68: which bug is causing that to be backed out?
17:37_6a68he said he'll comment in the bug after lunch with links
17:37jgruendoes this block shipping in 54?
17:38jgruenerrr 55
17:38_6a68jgruen: I have no idea, but it probably should be looked at
17:38jgruenis there a bug?
17:38_6a68the bug earlier today, I found out, was actually occurring in a very special optimized build on windows
17:39_6a68he's commenting in the 1386333 bug now
17:39_6a68if these errors are all occurring on the special pgo builds, we may just be seeing an optimizer artifact, not something that would affect users
17:40jgruenpgo?
17:40_6a68yeah one sec, they were explaining it to me in #teamaddons
17:41GitHub[screenshots] niharikak101 pushed 1 new commit to binary-image: https://git.io/v7D1r
17:41GitHubscreenshots/binary-image 876f48c Niharika Khanna: send image from content to background in binary
17:42_6a68jgruen: https://screenshots.firefoxusercontent.com/images/06b1c34f-4e25-43b7-8219-a64326e045e5.png
17:42_6a68TLDR being the first embedded webextension system addon makes us a canary of sorts
17:44jgruenclearly
17:45GitHub[screenshots] niharikak101 commented on issue #3308: The second commits adds a fix for #3218. https://git.io/v7DMv
17:46jgruen_6a68: i'm not totally down with the "i'm backing this out and going to lunch before explaining why" thing
17:46jgruenbut okay
17:48circleci-botFailed: niharikak101's build (#3009; push) in mozilla-services/screenshots (binary-image) -- https://circleci.com/gh/mozilla-services/screenshots/3009?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
17:48_6a68he's around and responsive, no worries
17:50GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master: https://git.io/v7DM2
17:50GitHubscreenshots/master 570a804 Tymur Faradzhev: Pontoon: Update Ukrainian (uk) localization of Firefox Screenshots...
17:54jgruenlooks like he commented
17:54_6a68 https://bugzilla.mozilla.org/show_bug.cgi?id=1386333#c8
17:54jgruenno idea why we would just now be seeing this
17:54firebotBug 1386333 FIXED, jhirsch@mozilla.com Remove Screenshots rollout pref
17:54_6a68yup, jinx
17:54jgruenaside from the pgo thing
17:54_6a68jgruen: because shield didn't pref us on when talos ran, probably?
17:55jgruenbut you ran plenty of tests with the pref flipped, yeah?
17:55jgruenit seems like those paint crashes would have shown up
17:55jgruennot to mention those empty webExt tests
17:56_6a68Yeah, true
17:56_6a68Talking to till in #jsapi about wtf is going on here
17:56_6a68the conversation is not reassuring
17:57_6a68"_6a68: I agree with RyanVM - those are scary stack traces"
17:58circleci-botFailed: mozilla-pontoon's build (#3010; push) in mozilla-services/screenshots (master) -- https://circleci.com/gh/mozilla-services/screenshots/3010?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
18:01jgruen_6a68 from jsapi: "I think it's most likely that changed timing uncovered preexisting issues"
18:01jgruenour issues or webext issues?
18:02_6a68yup
18:02jgruennot sure what he's referring to
18:02_6a68not our issues. addon code should never be able to crash the browser
18:02_6a68by chaining promises, each then() resolution requires letting the event loop spin
18:02_6a68it's like adding a few setTimeout(0) calls to the code
18:02_6a68apparently that's such brittle code that we're crashing things
18:02_6a68not pgo builds
18:03_6a68regular builds, according to RyanVM
18:05_6a68jgruen: I don't see how we can proceed with the current 55 release plan, given that we're now backed out of Beta on Thursday
18:05_6a68we probably need to get some more data from #jsapi and then draft a sadface email?
18:11_6a68jgruen: dunno if you're following along in #jsapi, but this is bad news bears
18:11jgrueni am
18:13* clouserw too
18:14jgruenwhelp, i had told my wife we'd be able to hang out tonight...
18:16clouserwthe mozilla monster beckons
18:16clouserw_6a68: thanks for digging on this
18:17clouserwdoesn't look like smaug is in many channels on here
18:17jgruen:)
18:18_6a68jgruen: go see your wife, this can wait
18:18clouserwbut he's also not on slack
18:18_6a68like, really
18:18_6a68looks like #developers will work
18:18ianbickingclouserw: per phonebook hes in Finland, GMT+2
18:18jgruenwell, i'd like to figure out if we're shipping in two weeks
18:18jgruenit looks like the answer is no we are not
18:19_6a68jgruen: yeah, but this slow motion crash and burn will probably require some data gathering
18:19clouserwoh, #developers is hidden channel
18:19_6a68jgruen: just a suggestion, happy to have more people around to help :-)
18:19_6a68ok, I created an etherpad to track this mess https://public.etherpad-mozilla.org/p/screenshots-55-questions
18:20jgruenwell i wasn't going anywhere for another hour and a half anway
18:20jgruenat least
18:20_6a68noice
18:20_6a68the subject line can just be, like, screenshots emoji, explosion emoji, hanky emoji
18:20_6a68and the body of the email can just be, 55 frowny face with tears emoji
18:24jgruenso, talos tests on 64 bit only?
18:25_6a68jgruen: "_6a68: based on the crashes we see, I'd expect these to affect users, too :("
18:25_6a68that's the latest in #jsapi
18:25jgruenyeah
18:25_6a68 /giphy river of tears
18:26_6a68you know, I'd rather find out now, than ship a crashy 56
18:26_6a68er, 55
18:26_6a68asking Mossop for help in #teamaddons, fwiw
18:26_6a68we probably need a superreviewer-type to help us find a person who can identify the actual bug
18:33circleci-botFailed: jvehent's build (#3011; ssh by jvehent) in mozilla-services/screenshots (zap-baseline) -- https://circleci.com/gh/mozilla-services/screenshots/3011?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
18:48jgruenboy if this comes down to a compilation issue, it really makes you wonder about what kind of fucked up karma we all have
18:49ianbickinghow is everything not broken always?
18:49jgruenianbicking: we use macs
18:50jgruenthis seems to maybe be some kind of diff btwn the way MS compilation schemes work
18:50jgruenor at least that's one theory
18:54_6a68Being so close to 57, I wonder how much deep code is ifdef'd to work in nightly only
18:54jgruen_6a68: yeah there's a greater than zero chance that that's the case
18:56jgruenhttps://screenshots.dev.mozaws.net/HE53N4RQ8i9JwnnA/irccloud.mozilla.com
18:56jgruenhow would we even go about resolving a bug caused by a diff in windows compilation?
18:57_6a68we wouldn't
18:57_6a68someone on the neckbeardiest of teams would have it triaged into their queue
18:57_6a68and we'd probably slip to 56?
18:57jgruenkick ass
18:57_6a68since you'd probably not want to ship a compilation-related error into production without letting it soak? I'm just guessing here
18:58jgrueni'll just leave a whole page of this here: https://giphy.com/search/scanners
19:08GitHub[screenshots] ianb pushed 1 new commit to master: https://git.io/v7DFw
19:08GitHubscreenshots/master 304ec18 Ian Bicking: Merge pull request #3311 from mozilla-services/zap-baseline...
19:08GitHub[screenshots] ianb deleted zap-baseline at cc88694: https://git.io/v7DFr
19:08jgruen_6a68: here's a dumb question...
19:09jgruensay i fire up my vm
19:09jgruendownload beta
19:09jgruenturn on screenshots and start mashing
19:09jgruenwhat am i gonna see
19:09jgruencrashes?
19:09jgruenwhat
19:09GitHub[screenshots] ianb pushed 1 new commit to master: https://git.io/v7DFi
19:09GitHubscreenshots/master 12b37f1 Ian Bicking: Merge pull request #3309 from mozilla-services/clean-up-home...
19:09GitHub[screenshots] ianb deleted clean-up-home at bd59043: https://git.io/v7DFP
19:09GitHub[screenshots] ianb closed pull request #3304: Correctly position share panel on non-Firefox browsers (master...2873-share-panel-offset) https://git.io/v7155
19:09GitHub[screenshots] ianb closed issue #2873: The "Share" pop-up is wrongly opened over the "Report", "Share" and "Download" buttons on other browsers https://git.io/vHLdI
19:09GitHub[screenshots] ianb deleted 2873-share-panel-offset at 28e6e6c: https://git.io/v7DFy
19:11_6a68jgruen: well, I'm not sure. just downloading beta with screenshots turned on by default seems to be enough to cause crashes, though I dunno how much of the time
19:11_6a68er, just starting a copy of beta with screenshots not preffed off at start time, may crash
19:12jgruenwhat does a ts_paint crash look like anyway?
19:12_6a68An addon shouldn't be able to crash the whole of firefox, so it's worrying that we don't understand the root cause
19:12jgruenyep
19:13_6a68oh interesting https://bugzilla.mozilla.org/show_bug.cgi?id=1386333#c13
19:13firebotBug 1386333 FIXED, jhirsch@mozilla.com Remove Screenshots rollout pref
19:14GitHub[screenshots] ianb pushed 1 new commit to master: https://git.io/v7Dbf
19:14GitHubscreenshots/master 458c203 Ian Bicking: Update version to 14.0.0 plus changelog
19:14GitHub[screenshots] ianb tagged 14.0.0 at master: https://git.io/v7DbJ
19:14GitHub[screenshots] ianb merged master into server-prod: https://git.io/v7Dbk
19:16circleci-botFailed: ianb's build (#3012; push) in mozilla-services/screenshots (master) -- https://circleci.com/gh/mozilla-services/screenshots/3012?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
19:17_6a68jgruen: based on kris's just-added comment https://bugzilla.mozilla.org/show_bug.cgi?id=1386333#c13, it looks like what's happening is Talos triggering crashes by shutting down before the addon is fully started up? I think?
19:18_6a68I don't know whether real-world users actually would be impacted by this
19:21jgruen_6a68: i'm on 55 beta on windows
19:21jgruenunable to take a screenshot
19:21_6a68wut
19:22jgruenhttps://irccloud.mozilla.com/file/fS5lI21f/Screen%20Shot%202017-08-10%20at%209.21.58%20PM.png
19:22circleci-botFailed: ianb's build (#3015; push) in mozilla-services/screenshots (master) -- https://circleci.com/gh/mozilla-services/screenshots/3015?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
19:22jgruen_6a68: pbm is not on
19:22_6a68jgruen: that's fun
19:22jgruenactually, this is release
19:22circleci-botFailed: ianb's build (#3016; push) in mozilla-services/screenshots (server-prod) -- https://circleci.com/gh/mozilla-services/screenshots/3016?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
19:24_6a68jgruen: could you file a bug with exact steps, and I'll try to repro?
19:25_6a68jgruen: are you using a modern.ie VM? I can try to get the same VM
19:26jgruenyeah i may be using a jacked up profile from the last time we found a crazy windows bug
19:26_6a68it's weird that the tab.incognito check would fail
19:28jgruen_6a68: works like a charm on a clean profile
19:29jgrueni have no idea what the old profile was, so i'm not sure there's an actionable bug there
19:41_6a68cool
19:41GitHub[screenshots] ianb created fix-tests (+1 new commit): https://git.io/v7DAg
19:41GitHubscreenshots/fix-tests 5b648db Ian Bicking: Fix #3306, make tests resilient to a browserAction or pageAction...
19:41GitHub[screenshots] ianb opened pull request #3317: Fix #3306, make tests resilient to a browserAction or pageAction (master...fix-tests) https://git.io/v7DA2
19:46_6a68jgruen: huh, kris seems to think this is not a big deal at all https://bugzilla.mozilla.org/show_bug.cgi?id=1386333#c15
19:46firebotBug 1386333 FIXED, jhirsch@mozilla.com Remove Screenshots rollout pref
19:58GitHub[screenshots] ianb commented on issue #3317: We got some failed test collision here, the danger of having failing tests and then not noticing failed-harder tests. Anyway, this DOES fix the browser test that was broken. https://git.io/v7Dpq
20:05GitHub[screenshots] ianb pushed 1 new commit to fix-tests: https://git.io/v7Dhs
20:05GitHubscreenshots/fix-tests a3c5e41 Ian Bicking: Disable failure checking for zap test
20:14circleci-botFixed: ianb's build (#3018; push) in mozilla-services/screenshots (fix-tests) -- https://circleci.com/gh/mozilla-services/screenshots/3018?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
20:47GitHub[screenshots] 6a68 closed pull request #3316: Fix #3313, create script to create release-ready XPIs (master...prod-xpi-builder) https://git.io/v7DiW
20:47GitHub[screenshots] 6a68 closed issue #3313: Create script to build release-ready XPI https://git.io/v7DBJ
20:47GitHub[screenshots] 6a68 deleted prod-xpi-builder at c19ca30: https://git.io/v7yJY
20:49jgruenboy, treeherder takes a long ass time
20:50_6a68lol
20:51_6a68jgruen: did you see kris's comment in the bug? https://bugzilla.mozilla.org/show_bug.cgi?id=1386333#c13
20:51firebotBug 1386333 FIXED, jhirsch@mozilla.com Remove Screenshots rollout pref
20:51jgruenyeah
20:51_6a68it sounds like he thinks just lowering the shutdown timeout (that fixed the other crash bug) will fix this one, too
20:52_6a68so, I guess everything is fine?
20:52jgruenhaha, i guess
20:52jgruenseems like something we should write up and sent to release drivers
20:52_6a68I guess we wait for him to land something, request beta uplift, re-request beta uplift for our patch that removes the pref from beta, and see if talos asplode?
20:52_6a68yeah, feel free to grab any useful details out of that etherpad
20:53_6a68https://public.etherpad-mozilla.org/p/screenshots-55-questions
20:53jgruenyeah, i'm still a little concerned about our ability to ship on time
20:53_6a68I still feel weird about the whole thing. Are we really just tightening the band-aid and not really fixing the underlying issue?
20:53jgruenme too
20:53jgruenclouserw: what do you think here?
20:53_6a68The other bug was one of the top crashers in 56 before the timeout was put in place
20:54jgruenkris' patch would also not touch release i assume
20:54jgruenso we might cause crashes there
20:56clouserwI don't know what that one magic timeout is, but seems like it fixes everything?
20:57_6a68jgruen: Well, the labels on this bug say that 55 is unaffected, but I don't know if that's accurate https://bugzilla.mozilla.org/show_bug.cgi?id=1380267
20:57firebotBug 1380267 FIXED, kmaglione+bmo@mozilla.com Crash in AsyncShutdownTimeout | profile-change-teardown | Extension Shutdown: jid0-GXjLLfbCoAx0LcltE
20:57clouserwbut John-Galt landing a patch to adjust the timeout again sounds like the right next step
20:58_6a68clouserw: I don't really think we're fixing anything, we're just force-killing the addon process before the main process exits by setting an arbitrary timeout https://bugzilla.mozilla.org/show_bug.cgi?id=1380267#c23
20:58_6a68That seems meaningfully different from "here's the synchronization bug that allowed the parent process to exit before its child process"
20:59clouserwdetails...
21:00_6a68lolol, yeah, was just thinking this might not be a meaningful difference at a high level
21:00clouserwJohn-Galt: are you able to land a patch to adjust that timeout today? I have a meeting with my manager in an hour and he'll ask if we need anything else. John-Galt: is there anything else you need?
21:00jgruen_6a68: i think 55 is unaffected b/c the code that threw the error was not there
21:00jgruenor at least that's how i remember it
21:01jgruen_6a68 clouserw one way to resolve this is by moving forward
21:01jgruenwe can ship to 1% of release and see what happens
21:03_6a68true, that is one way
21:03_6a68Of course, by "ship to 1% of release", you actually mean flip prefs, which seems to hit a different set of timing bugs than just having the thing enabled
21:03_6a68So we may not learn what we want to learn via shield
21:06jgruen_6a68: how would a that be materially different after the first time a user restarts their browser?
21:06ianbickingits possible we could ship an update that wont apply until restart
21:07_6a68jgruen: hmm, does shield permanently flip the firefox pref, or does it just flip the pref each time it is run?
21:07jgruenidk,
21:07GitHub[screenshots] 6a68 closed pull request #3317: Fix #3306, make tests resilient to a browserAction or pageAction (master...fix-tests) https://git.io/v7DA2
21:07557A8BO7N[screenshots] 6a68 pushed 1 new commit to master: https://git.io/v7yTI
21:07557A8BO7Nscreenshots/master 3c15014 Ian Bicking: Fix #3306, make tests resilient to a browserAction or pageAction (#3317)...
21:07203A9272Q[screenshots] 6a68 closed issue #3306: Tests broken due to Photon-related regression in Nightly https://git.io/v71hb
21:07GitHub[screenshots] 6a68 deleted fix-tests at a3c5e41: https://git.io/v7yTL
21:07jgruenthe one thing i've learned in all of this is that pretty much you need to know how everything works if you want anything to work
21:08jgrueni would assume that once the pref is flipped it's flipped, but i guess i have no proof of that
21:09_6a68yeah, there is really no trustworthy abstraction layer
21:09jgruenasking matt, he's relatively trustworthy
21:09_6a68lol
21:10GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master: https://git.io/v7yTK
21:10GitHubscreenshots/master 814d07d ajtzibsyan: Pontoon: Update Kaqchikel (cak) localization of Firefox Screenshots...
21:12_6a68jgruen: good point - the only way we've gotten this far is because other teams at mozilla are super helpful
21:13_6a68human abstraction layers ftw
21:16GitHub[screenshots] 6a68 closed pull request #3302: Detach app startup observer on shutdown (master...detach-startup-observer) https://git.io/v71aq
21:20jgruen_6a68 matt_g thinks the pref stays flipped during the experiment...i just asked mythmon on slack
21:20jgruenand now i realize he's in this channel too
21:20_6a68cool! and if the experiment ends, we'd have to push a system addon to delete the pref?
21:20_6a68er, not if, but when
21:21_6a68that might be the change that triggers a wave of surprises
21:21jgruen_6a68: we would ship a system add-on to change the pref, not delete it
21:21_6a68oh, right - so after the first restart after shield flips the pref, we'd see any possible timing issues
21:22_6a68idk if change vs delete matters here, I think it's more about the timing of turning on screenshots
21:22circleci-botFixed: 6a68's build (#3020; push) in mozilla-services/screenshots (master) -- https://circleci.com/gh/mozilla-services/screenshots/3020?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
21:22cloudops-ansiblescreenshots-dev build #828: building mozilla/screenshots:latest
21:23jgruenfirefox is basically rashomon
21:23_6a68lol
21:25cloudops-ansiblescreenshots-dev build #828: mozilla/screenshots:latest deployed to Dev
21:26_6a68all the trees are open right now, so if kris lands his fix today, we request uplift tomorrow, it hits beta friday or monday, seems like we would want to give it some time to soak on beta (a week?)
21:26cloudops-ansiblescreenshots-dev build #829: building mozilla/screenshots:latest
21:27jgruen_6a68: https://irccloud.mozilla.com/file/IzCoAqTb/Screen%20Shot%202017-08-10%20at%2011.27.25%20PM.png
21:28_6a68jgruen: oh, so there will be a delay between startup and the shield addon loading/running/flipping prefs
21:28mythmonjgruen: should i just be talking here?
21:29_6a68mythmon: lol, that works for me
21:29cloudops-ansiblescreenshots-dev build #829: mozilla/screenshots:latest deployed to Dev
21:29jgruenyeah
21:29jgrueni was about to suggest that
21:29_6a68or I could go talk over there...but public is better
21:29jgruenmythmon: i will quote you:
21:29jgruen"so there is a brief amount of time right after firefox starts where the pref is back to whatever the firefox-default was, before shield kicks in and sets it back to whatever the shield recipe says it should be"
21:30_6a68yeah, that's the key thing - shield takes a minute to be loaded and executed, so we aren't quite getting the same set of timing issues that we get when the pref is set at actual startup
21:31_6a68so we can't rely on shield data to tell us what'll happen when it goes to 100% of users
21:31_6a68is balrog capable of releasing to a subset of users? I think the answer was no, need to double check
21:31_6a68rhelmer: ^^
21:32jgruen_6a68: afaik it is, we talked about doing a per-lang release the other day
21:32_6a68jgruen: well, I mean 1% of a group of users, not a group of users with user-agent property X
21:33_6a68like, 1% of release gets a system addon that flips the pref for reals
21:33_6a68or removes it for reals
21:33jgruenah, probably not
21:34_6a68OK, then I guess the question is whether these crashes could affect real users
21:34_6a68Based on kris's comments, I think that is unlikely, and if it happens, would be at shutdown
21:34_6a68But then, if FF crashes on shutdown, does that corrupt your profile?
21:34jgrueni have no idea
21:34_6a68jgruen: maybe we can give QA copies of release with the profile removed, and see if they find anything
21:35jgruen_6a68: with the pref removed?
21:35_6a68yup
21:35_6a68removed or default flipped, doesn't really matter
21:35_6a68we just need to have the pref flipped at startup, I think, mirroring what would happen at 100% release
21:35jgruen_6a68: it seems like they could flip the pref and fuzz
21:36jgruenlike, no special build needed
21:36_6a68I guess that's true. if they set the stystem-disabled pref to false, then restart, it should persist
21:36jgruenbut we could also grab a build of beta from ryan's treeherder tests right?
21:36jgrueni forget which B or D you click to get a build
21:37_6a68one sec, I know I documented this
21:38_6a68ah well
21:38_6a68jgruen: D is decision task, B is build task
21:39_6a68the pink Bs are canceled builds
21:39jgruen_6a68: is there a build somewhere in here: https://treeherder.mozilla.org/logviewer.html#?job_id=122287926&repo=mozilla-beta&lineNumber=1807
21:40jgrueni guess not, huh, so the best we can do is ask them to flip the pref
21:40jgrueni can email cosmin
21:41_6a68I think there is a build, one sec
21:41_6a68so if you click this guy https://screenshots.firefoxusercontent.com/images/9bb5372a-2cdd-4eb9-b39d-07a8b072b166.png
21:41_6a68the thing pops up from the bottom
21:41_6a68and I think it's in there somewhere...
21:42_6a68might be target.zip? https://screenshots.firefoxusercontent.com/images/33779610-e26c-4d5c-b0d1-e21840fd18a6.png
21:42_6a68or if not, maybe (scrolling down the job details) the target.installer.exe
21:42_6a68starting a VM now to try
21:48_6a68win 10 refuses to run the installer.exe, so there's that
21:48jgruenkick ass
21:49jgruen_6a68: i put a note i was going to send cosmin into the etherpad
21:49jgruengimme an edit?
21:49_6a68k, looking
21:49rhelmer_6a68: yeah there's a small set of things sent in the update request that balrog can use to decide... shield is a lot more flexible there.
21:50_6a68rhelmer: Cool, thanks. Unfortunately, we are seeing crashes due to subtle timing issues at startup, so shield isn't a good predictor of how things will behave after we go to 100% via a system addon
21:50_6a68We're trying to figure out the risks
21:50rhelmerugh, yeah that's rough..
21:51_6a68yes, screenshots is one big edge case ^_^
21:51jgruen_6a68: i'm leaning toward declaring bankruptcy on 55
21:52jgruentoo many moving pieces, too little time
21:52_6a68jgruen: I'm with you, I'd rather ship late and not crash granny's machine
21:53jgrueni think we should sleep on it basically
21:53jgrueni'll email cosmin and ask him to test
21:53_6a68you should probably chat about it with clouserw though, he's been in favor of just shipping for a while
21:53_6a68cool, sounds good
21:53jgrueni know
21:53_6a68I mean, 55 is already in release
21:53jgruenclouserw: tell me to yolo
21:54jgrueni'll email pdol and jeff tomorrow
21:54_6a68jgruen: doc looks good btw, added one thing but seems fine
21:54jgrueni already told jeff about this
21:54_6a68we may want to get a definitive risk assessment from John-Galt before making a final call
21:54jgruensent
21:54_6a68sweet
21:56jgruenhttps://bugzilla.mozilla.org/show_bug.cgi?id=1386333#c15
21:56firebotBug 1386333 FIXED, jhirsch@mozilla.com Remove Screenshots rollout pref
21:56jgrueni'm not sure how much more definitive he's going to get than this
21:56_6a68heh
21:56_6a68probably true
21:57_6a68To me, it seems unacceptably risky to rely on shield to roll out the feature, since different timing issues occur when shield is used
21:57jgruenthat's how i feel as well
21:57_6a68We're back to the all-or-nothing pref flip, with the possibility of introducing crashes
21:57ianbickingwe can do our own rollout
21:58jgruenthat's another possibility
21:58_6a68I think the fact that shield can't do what we need is really important
21:58_6a68ianbicking: do you mean just doing a coin flip to decide whether to enable or not?
21:58ianbickingWe had a rollout for Hello that was not much code
21:58ianbicking_6a68: we did a coin flip, and compared it to a DNS record
21:58mythmonshield can do what you need
21:58_6a68if (Math.random < 0.1) enable()
21:58mythmonwe just don&#39;t recommend it, because it is more complex
21:59_6a68mythmon: oh really? there&#39;s a secret menu?
21:59mythmonnot even that secret. there&#39;s a checkbox for what mode to be in
22:00mythmonwe can choose to do things on the default branch, which is the normal safe route, or the user branch, which doesn&#39;t have this &quot;At start up&quot; behavior, but is riskier: it is more likely we&#39;ll stomp on user preference
22:01_6a68mythmon: we deliberately created a separate system-disabled pref for just this reason :-)
22:01mythmonyeah, i was about to say that because you have your own pref, you should be safe to use it
22:01mythmonbut that&#39;s a policy decision, i&#39;m just the developer
22:01_6a68so shield could set a pref in the user&#39;s prefs, would the pref still be removed when the shield study ends?
22:02_6a68or would the pref change be persisted indefinitely?
22:02_6a68if the latter, that could work for us, I think
22:02mythmonfor user branch studies, we store the original value when the experiment starts (including if it is missing), and restore that when the study ends
22:04jgruenmythmon, _6a68 pending QA from cosmin, i think we should do a very small deployment on user branch
22:05jgruenon release
22:05_6a68let&#39;s start on beta, just in case we break stuff?
22:06jgruenlet&#39;s see what cosmin says
22:06jgrueni hav