mozilla :: #screenshots

17 May 2017
00:27GitHub[screenshots] 6a68 pushed 5 new commits to bootstrap-button-wip:
00:27GitHubscreenshots/bootstrap-button-wip 4f2cfac Jared Hirsch: Create very simple button via CUI
00:27GitHubscreenshots/bootstrap-button-wip 4496c78 Jared Hirsch: Remove webextension button code from manifest.json
00:27GitHubscreenshots/bootstrap-button-wip a852792 Jared Hirsch: Call initButton on start; call handleStartup on XUL button click
00:34_6a68EOD update: I *think* (not sure) the button is correctly initializing the webextension only after it is first pressed.
00:35_6a68I'm not yet sure I'm correctly loading the code into all windows (in the multiple windows case).
00:35_6a68I'm also not sure if Talos ever opens more than one window...
00:35_6a68Going to kick off a Talos run tonight with these changes on top of 6.6.2 on Beta, plus 6.6.2 on Beta, plus Beta
00:35_6a68Disclaimer: the numbers may look good, but ultimately be meaningless
00:39_6a68Try run with WIP super lazy loading (bootstrap managed button):
00:40_6a68Try run with 6.6.2, for comparison:
00:40_6a68hopefully those will complete successfully overnight
01:05_6a68Sorry, I accidentally passed the --artifact argument, which is broken on beta
01:05_6a68Trying again:
01:09_6a68Here's a Try run with bootstrap-managed button on Beta:
01:10_6a68Here's a Try run with bootstrap-managed button on central:
01:11_6a68And here is a Try run with 6.6.2 and perf fixes on Beta, which can be compared to the bootstrap-managed button on Beta:
02:00clouserwthanks! ^
04:29GitHub[screenshots] akhilman commented on issue #2844: Do not know what was wrong but after refreshing my profile all works as expected.
08:38GitHub[screenshots] Standard8 created eslint-global-cleanup (+1 new commit):
08:38GitHubscreenshots/eslint-global-cleanup 8d0f843 Mark Banner: chore: Cleanup ESLint globaldefinitions - remove unnecessary ones that are already defined in environments.
08:38GitHub[screenshots] Standard8 opened pull request #2848: chore: Cleanup ESLint globaldefinitions - remove unnecessary ones that are already defined in environments. (master...eslint-global-cleanup)
12:01GitHub[screenshots] akhilman commented on issue #2844: This problem caused by custom css that fixes white on white input fields in white on dark gtk themes....
12:24GitHub[screenshots] SoftVision-CiprianMuresan opened issue #2849: The Delete button does not respond to clicks for a few seconds after hovering a saved shot
14:12GitHub[screenshots] johngruen commented on issue #2846: no this looks great! Thanks so much @garethcull, @clouserw @ianb how does this strike you guys?
15:29_6a68Welp, at least the patch built cleanly on top of central
15:35_6a68Looks like there are still major perf regressions with the lazy-loading bootstrap code
15:38_6a68our lazy button code vs central: sessionrestore is ~20% slower, oddly tabpaint is 7% _faster_, and tp5n nonmain file IO is 1000+% worse, but I think that's expected with moving webext code off the main thread
15:39_6a68I'll try to get beta to build cleanly and see how the lazy button code compares there
15:39_6a68I'll also try putting our non-lazy button code on central and see how that does
15:46clouserwwe could get kris to look at those results and see if they measure up
15:46clouserwI just shared a screenshot with someone and they say it's not working and took this screenshot: does that look like anything we know about?
15:46clouserwI can't reproduce it
15:50_6a68clouserw: that's a funky looking CSP error
15:50_6a68OK, pushed some more screenshots runs to Try:
15:51_6a68Talos run of just mozilla-central, with enough repeats to act as a statistically solid base
15:51_6a68Talos run of Screenshots current master
15:52_6a68Talos run of Screenshots lazybutton
15:53_6a68clouserw: if you send me that screenshots link, I can try to repro
15:54_6a68John-Galt: hey, we tried moving the button into the bootstrap, and lazy-loading the webextension on first button click. Do these numbers look any better than current blank webextension on central?
15:55_6a68only major regressions are sessionrestore ~20% and non-main tp5o, which IIUC was expected
15:59_6a68clouserw: works fine for me
15:59_6a68that screenshot
15:59clouserwyeah. he says he tried it in 3 different browsers with the same results (including chrome)
15:59clouserwI dunno
15:59_6a68I get a lot of the same CSP errors in the console, fwiw
16:00John-Galt_6a68: Yes. The sessionrestore regression is not real. Well, the Windows one is not real, anyway.
16:00_6a68oh really!
16:00clouserwdo you see the ab tests error? that seems like it could cause it too
16:00_6a68clouserw: actually, I don't see the window.abtests error
16:01clouserwme either (ever). I wonder if he has an old cookie set that it putting him into an ab test or something
16:01_6a68let me try in a profile without test pilot installed
16:01GitHub[screenshots] wresuolc commented on issue #2846: strikes well!
16:01_6a68works for me in opera and safari
16:04_6a68John-Galt: is that tp5o nonmain regression expected, and not a big deal?
16:05John-Galt_6a68: No, that's not expected. I have no idea what's causing it.
16:15ianbickingclouserw: re: that screenshot viewing error someone saw, thats the ublock thing, should be double-fixed in stage
16:15ianbickingrelud: can we do a deploy of stage to prod today?
16:16clouserwcool, thanks
16:16reludianbicking: yes
16:18GitHub[screenshots] ianb pushed 1 new commit to master:
16:18GitHubscreenshots/master 37f7561 Ian Bicking: Merge pull request #2848 from mozilla-services/eslint-global-cleanup...
16:18GitHub[screenshots] ianb deleted eslint-global-cleanup at 8d0f843:
16:29cloudops-ansiblescreenshots-dev build #20: building mozilla/screenshots:latest
16:32cloudops-ansiblescreenshots-dev build #20: mozilla/screenshots:latest deployed to Dev
18:22GitHub[screenshots] wresuolc commented on issue #2846: @johngruen wants to get input from Elvin on this before we move forward. Mahe is going to organize a meeteing
18:26_6a68ianbicking: aight, so the export thing. first thing is probably to update your copy of mozilla-central, since pulling down changes can take a while
18:26_6a68you mentioned you're using hg, not git, so I'm not sure if you'll hit fresh bugs, but here's the instructions:
18:27_6a68I've never run it using hg, my incantation is `EXPORT_MC_LOCATION=/path/to/gecko ./bin/ -m "commit msg" --no-switch-branch`
18:27_6a68then, over on the gecko side, I do `./mach try -b o -p win32,win64,linux64,macosx64 -u none -t all --rebuild-talos 5`
18:28ianbicking_6a68: sure, I got it crunching on a fetch; the export works the same, its just writing files into the tree
18:28_6a68hopefully this will all work smoothly
18:29_6a68The parent hg commit for all my pushes this morning was 41958333867b, so maybe roll back to that commit before exporting to m-c
18:30ianbickingoh, export_mc got fancier since I last ran it
18:30_6a68there are some undocumented arguments :-)
18:30_6a68I have to use --no-switch-branch because I don't have a branch named "default", and it barfs in that case
18:32ianbicking_6a68: ok, Ive exported lazybackground to my firefox checkout, and export_mc seems to have committed that change
18:33_6a68 I dunno how to roll back to an earlier commit in hg, but I believe they added it at some point
18:33ianbicking_6a68: I did "hg checkout 41958333867b first, that worked
18:33_6a68mach try should do it, then
18:34_6a68I put my incantation a bit up in scrollback
18:35mahe_6a68 jgruen does 2:30 PM PST work to meet with Elvin?
18:36_6a68I'll probably skip that meeting, unless i"m needed for some particular reason
18:36jgruenyeah i think just clouserw and I is good
18:37clouserwmahe: wfm
18:37maheElvin recommends reaching Brian.. any reason you want Elvin jgruen clouserw ?
18:38clouserwbrian works
18:39mahecool.. Brian's calendar says available for the same time.. I'll reach him and send out an invite
18:41reludianbicking: okay, i'm ready for a prod deploy. what do you want deployed, and when are you available to be on hand when i do it?
18:42reludpresumable we're deploying 7.0.0?
18:42ianbickingrelud: Im on hand now, we want mozilla/pageshot/7.0.0 deployed
18:42reludalright, deploying
18:43ianbickingnext deployment will be from mozilla/screenshots instead of mozilla/pageshot, but the scripts hadnt been updated yet
18:46ianbicking_6a68: Ill need to get commit access. Are you able to vouch (level 2)?
18:46_6a68nope, I'm only level 1 these days. Standard8 should be able to do it
18:47_6a68for now, I'll go ahead and push the code to Try
18:50mahe_6a68 jgruen haven't heard from Brian if he can join, but sent out an invite to block time on calendars..
18:51cloudops-ansiblescreenshots build #36: mozilla/pageshot:7.0.0 deployed to Prod
18:52reludcc ianbicking ^ done
18:52ianbickingrelud: thanks! clouserw ^^ that should fix the bug you saw earlier
18:54_6a68Try run for lazy background:
19:05jgruenrelud: thanks!
19:32GitHub[screenshots] ianb created fix-creating-is-closed (+1 new commit):
19:32GitHubscreenshots/fix-creating-is-closed e72a7e0 Ian Bicking: Fix #2842, if the /creating tab is closed open a new tab instead of updating the nonexistent tab
19:33GitHub[screenshots] ianb opened pull request #2850: Fix #2842, if the /creating tab is closed open a new tab instead of updating the nonexistent tab (master...fix-creating-is-closed)
20:02mahejgruen : MOTM meeting time.. today is the right day John
20:17GitHub[screenshots] johngruen commented on issue #2300: @ianb bumping this issue. can we close?
20:30GitHub[screenshots] ianb commented on issue #2300: Right now this is working because @relud setup an nginx response filter that rewrites image links to the new host. That's a pretty fragile solution, so we still need to do this properly. Probably all that means is using `config.contentOrigin` in the right place.
21:22GitHub[screenshots] johngruen commented on issue #2300: @ianb should be add another row in the x-functional doc for this?
21:23GitHub[screenshots] ianb commented on issue #2300: It's just normal server engineering, there's nothing exceptional about this. We've mitigated the security concern, but need to do some followup work.
21:45cloudops-ansiblescreenshots build #37: building mozilla/pageshot:7.0.0
21:45relud^ that's me testing an operational update
21:55GitHub[screenshots] relud commented on issue #2828: mozilla-services/cloudops-deployment#735
21:59maheis there an etherpad where notes from this meeting can go in jgruen?
22:00maheI didn't want to create a google doc or something new. .
22:00jgruenerr, yeah probably in the screenshots coordination meeting etherpad
22:00cloudops-ansiblescreenshots build #37: ami in stage failed /cc relud
22:06_6a68ianbicking: ok, it looks like the lazy background page did much worse than lazy button. here is the comparison with lazybutton as the base, and lazy background as the new:
22:08_6a68here's lazy background compared against current central
22:09_6a68I guess it's clear that the lazy button is the way to go
22:11_6a68Looking at ritu's email, we'll need to land and stick by June 2, otherwise we'll be trying to land during the soft freeze
22:11_6a68That gives us 2 weeks and 2 days
22:15GitHub[screenshots] relud commented on issue #2839: fixed
22:21cloudops-ansiblescreenshots build #38: building mozilla/pageshot:7.0.0
22:21relud^ me again
22:35GitHub[screenshots] 6a68 commented on issue #2850: I can review this
22:35ianbicking_6a68: that all makes sense, and the performance is unsurprising
22:36_6a68as far as context menu, it turns out we do have to modify the XUL DOM, but at least we can do that from inside our code, and don't have to directly hack on any .xul files in the tree
22:36ianbicking_6a68: I think the value of lazybackground is to support John-Galts performance work loading lots of files in the background page is something we can fix, and is a reasonable requirement for a high-performing built-in webextension
22:36_6a68yeah, I agree
22:37ianbickingbut I think we (both our team and the addons team) have a goal of being able to ship a webextension without any of these workarounds. We just might not meet that goal in 55, so lazybutton is also a good plan.
22:37* _6a68 nods
22:39cloudops-ansiblescreenshots build #38: ami in stage failed /cc relud
22:39_6a68I'll wait to update the screenshots-staff list until the lazybutton code is a little further along, in case the context menu DOM twiddling hurts perf
22:40ianbicking_6a68: feels fairly safe, not much different than the toolbar twiddling
22:41_6a68yeah, r-helmer was saying the same thing, shouldn't have much startup cost
22:41ianbickingheh, the unping
23:47GitHub[screenshots] 6a68 closed pull request #2850: Fix #2842, if the /creating tab is closed open a new tab instead of updating the nonexistent tab (master...fix-creating-is-closed)
23:47GitHub[screenshots] 6a68 closed issue #2842: Invalid tab ID error is constantly thrown if switching back to the tab from which the screenshot was performed
23:47GitHub[screenshots] 6a68 deleted fix-creating-is-closed at e72a7e0:
18 May 2017
No messages
Last message: 7 days and 22 hours ago