1694229 - Restrict the slow script warning for content to cases where we are interacting with an unresponsive tab

Status: VERIFIED FIXED

(Core :: XPConnect, enhancement, P1)

Description

[Doug Thayer [:dthayer] (he/him)]

The actual goal here, per out-of-bug discussions, is a little broader, but it makes sense to track it under one bug.

• Restrict the warning to cases where we are trying to interact with a tab that is hung, either due to its own problems or a hang in a background tab in the same process. Types of interaction we care about:
  • Clicks
  • Touch events
  • Scrolling
  • Key presses
• Set the time threshold for showing the notification to a hard 10 seconds
• Blame the hang on the tab which owns the JS that's running
• Remove the wait button on the notification

Comment 1

[Doug Thayer [:dthayer] (he/him)]

Attachment: Bug 1694229 - Update slow script warning visuals r?florian

This patch removes the wait button on the slow script warning, on the suspicion
that it is confusing to the user since it's redundant with the close button. It
also changes the text of the notification to blame the hanging tab.

Comment 2

[Doug Thayer [:dthayer] (he/him)]

Attachment: Bug 1694229 - Separate Enter/Exit Widget Events from Mouse Button events r?smaug

If we don't do this, then just moving the mouse over a window experiencing a
slow script will cause it to show the notification. We could try to
deserialize the message inside nsContentUtils::IsMessageCriticalInputEvent, but
that seems overcomplicated compared to just adding a new message which proxies
to the original message handlers.

Depends on D106015

Comment 3

[Doug Thayer [:dthayer] (he/him)]

Attachment: Bug 1694229 - Show slow script warning only when critical input is pending r?smaug

We want to restrict the slow script warning to cases where the user is actually
trying to interact with the browser.

Depends on D106016

Comment 4

[Doug Thayer [:dthayer] (he/him)]

Attachment: Bug 1694229 - Add mouse wheel scrolling to critical input list r?smaug

Fairly trivial. At some point we may want to instead look for checkerboarding,
but for now this should be sufficient.

Depends on D106017

Comment 6

[Doug Thayer [:dthayer] (he/him)]

Attachment: Bug 1694229 - Show a different notification is selected tab is hanging r?florian

As part of this, we need to detect that the currently showing notification does
not match the one what we want to display. This also fixes the case where we
show a notification for tab A, then switch to tab B which is also hanging, and
end up listing the title for tab A as hanging in the notification.

Depends on D106018

Comment 7

[Blake Winton (:bwinton) (:☕️)]

As per guidance from Vicky, for tracking, we're marking all the bugs that people are working on as P1.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d32085239f92
Update slow script warning visuals r=florian
https://hg.mozilla.org/integration/autoland/rev/6a5d472e1b05
Separate Enter/Exit Widget Events from Mouse Button events r=smaug
https://hg.mozilla.org/integration/autoland/rev/a70e27ec7747
Show slow script warning only when critical input is pending r=smaug
https://hg.mozilla.org/integration/autoland/rev/7fef19f47442
Add mouse wheel scrolling to critical input list r=smaug
https://hg.mozilla.org/integration/autoland/rev/5ce24c91b0c1
Show a different notification is selected tab is hanging r=florian

Comment 9

[Cristian Brindusan [:cbrindusan]]

Backed out 5 changesets (Bug 1694229) for geckoview failures.
https://hg.mozilla.org/integration/autoland/rev/8708c121e21c0c6a179ac782960b2f5a92de7ba8

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&revision=5ce24c91b0c11da1ed2f55f7e5ed5d96184164bb&selectedTaskRun=Cjezj8IdTa67EmenKuihsw.0

Failure log:
https://treeherder.mozilla.org/logviewer?job_id=331153685&repo=autoland&lineNumber=2909

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d082f53d882e
Update slow script warning visuals r=florian
https://hg.mozilla.org/integration/autoland/rev/42cb688eae03
Separate Enter/Exit Widget Events from Mouse Button events r=smaug
https://hg.mozilla.org/integration/autoland/rev/286b311d32b2
Show slow script warning only when critical input is pending r=smaug
https://hg.mozilla.org/integration/autoland/rev/877471a44509
Add mouse wheel scrolling to critical input list r=smaug
https://hg.mozilla.org/integration/autoland/rev/cb3d9e8d32e6
Show a different notification if selected tab is hanging r=florian

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out 5 changesets (bug 1694229) by flod's request, lint failures.

Backed out push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=cb3d9e8d32e637010bdb1ab6c652449e3f8d42ac

Backout link: https://hg.mozilla.org/integration/autoland/rev/1b5bec432c05cc0691b1453cc06a33aadbe972e9

Push with failure: https://treeherder.mozilla.org/jobs?repo=try&revision=3ef17f3b917d05bddb8c302373dbe4e5d4f665ce&selectedTaskRun=CCp04-A8T5GLLRVZHCv8Ig.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=331228396&repo=try&lineNumber=69

[task 2021-02-25T19:53:29.445Z] executing ['bash', '-cx', './mach lint -W -l l10n -f treeherder -f json:/builds/worker/mozlint.json *']in /builds/worker/checkouts/gecko
[task 2021-02-25T19:53:29.448Z] + ./mach lint -W -l l10n -f treeherder -f json:/builds/worker/mozlint.json accessible aclocal.m4 AUTHORS browser build build.gradle caps Cargo.lock Cargo.toml chrome client.mk client.py CLOBBER config configure.in configure.py devtools docs docshell dom editor extensions gfx GNUmakefile gradle gradle.properties gradlew gradlew.bat hal image intl ipc js layout LICENSE mach mach.ps1 Makefile.in media memory mfbt mobile modules moz.build moz.configure mozglue mozilla-config.h.in netwerk nsprpub old-configure.in other-licenses package.json package-lock.json parser python README.txt remote security services servo settings.gradle startupcache storage substitute-local-geckoview.gradle taskcluster testing test.mozbuild third_party toolkit tools try_task_config.json uriloader view widget xpcom xpfe
[task 2021-02-25T19:53:30.602Z] created virtual environment CPython3.6.9.final.0-64 in 274ms
[task 2021-02-25T19:53:30.602Z]   creator CPython3Posix(dest=/builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/init_py3, clear=False, no_vcs_ignore=False, global=False)
[task 2021-02-25T19:53:30.602Z]   seeder FromAppData(download=False, pip=bundle, setuptools=bundle, wheel=bundle, via=copy, app_data_dir=/builds/worker/.local/share/virtualenv)
[task 2021-02-25T19:53:30.602Z]     added seed packages: pip==20.3.1, setuptools==51.0.0, wheel==0.36.1
[task 2021-02-25T19:53:30.602Z]   activators BashActivator,CShellActivator,FishActivator,PowerShellActivator,PythonActivator,XonshActivator
[task 2021-02-25T19:53:30.688Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2021-02-25T19:53:33.680Z] 
[task 2021-02-25T19:53:33.953Z] manifests [==================================>                ]  9816/14015 01s
[task 2021-02-25T19:53:36.074Z]                                                                                 
[task 2021-02-25T19:53:36.074Z] requesting all changes
[task 2021-02-25T19:53:36.074Z] adding changesets
[task 2021-02-25T19:53:36.074Z] adding manifests
[task 2021-02-25T19:53:36.074Z] adding file changes
[task 2021-02-25T19:53:36.074Z] added 14015 changesets with 18646 changes to 1494 files
[task 2021-02-25T19:53:36.502Z] new changesets 90d04a36eef7:327ddbc9f129
[task 2021-02-25T19:53:36.502Z] updating to branch default
[task 2021-02-25T19:53:36.564Z] 
[task 2021-02-25T19:53:36.651Z] updating [====>                                                     ]  100/1087
[task 2021-02-25T19:53:36.672Z]                                                                                 
[task 2021-02-25T19:53:36.672Z] 1087 files updated, 0 files merged, 0 files removed, 0 files unresolved
[task 2021-02-25T19:53:36.777Z] 0 files updated, 0 files merged, 0 files removed, 0 files unresolved
[task 2021-02-25T19:53:40.095Z] TEST-UNEXPECTED-WARNING | /builds/worker/checkouts/gecko/browser/locales/en-US/browser/browser.ftl:544:1 | Changes to string require a new ID: crashed-subframe-submit (l10n)
[task 2021-02-25T19:53:40.095Z] TEST-UNEXPECTED-WARNING | /builds/worker/checkouts/gecko/browser/locales/en-US/browser/browserContext.ftl:108:1 | Changes to string require a new ID: main-context-menu-open-link-new-window (l10n)
[task 2021-02-25T19:53:40.095Z] TEST-UNEXPECTED-WARNING | /builds/worker/checkouts/gecko/browser/locales/en-US/browser/browserContext.ftl:132:1 | Changes to string require a new ID: main-context-menu-copy-email (l10n)
[task 2021-02-25T19:53:40.095Z] TEST-UNEXPECTED-WARNING | /builds/worker/checkouts/gecko/browser/locales/en-US/browser/defaultBrowserNotification.ftl:7:1 | Changes to string require a new ID: default-browser-notification-button (l10n)
[task 2021-02-25T19:53:40.096Z] TEST-UNEXPECTED-WARNING | /builds/worker/checkouts/gecko/browser/locales/en-US/chrome/browser/browser.properties:707:1 | Changes to string require a new ID: processHang.button_stop.label (l10n)
[task 2021-02-25T19:53:40.096Z] TEST-UNEXPECTED-WARNING | /builds/worker/checkouts/gecko/browser/locales/en-US/chrome/browser/browser.properties:955:1 | Changes to string require a new ID: pendingCrashReports.alwaysSend (l10n)
[task 2021-02-25T19:53:40.096Z] TEST-UNEXPECTED-WARNING | /builds/worker/checkouts/gecko/browser/locales/en-US/chrome/browser/browser.properties:965:1 | Changes to string require a new ID: decoder.decodeError.button (l10n)
[task 2021-02-25T19:53:40.096Z] TEST-UNEXPECTED-WARNING | /builds/worker/checkouts/gecko/browser/locales/en-US/chrome/browser/browser.properties:976:1 | Changes to string require a new ID: captivePortal.showLoginPage2 (l10n)
[task 2021-02-25T19:53:40.096Z] TEST-UNEXPECTED-WARNING | /builds/worker/checkouts/gecko/browser/locales/en-US/chrome/browser/feeds/subscribe.properties:9:1 | Changes to string require a new ID: addProtocolHandlerAddButton (l10n)
[taskcluster 2021-02-25 19:53:40.426Z] === Task Finished ===
[taskcluster 2021-02-25 19:53:40.634Z] Unsuccessful task run with exit code: 1 completed in 37.63 seconds

Comment 12

[Bogdan Tara[:bogdan_tara | bogdant]]

Also seeing the following starting to perma fail with the backed out changes: https://treeherder.mozilla.org/logviewer?job_id=331232688&repo=autoland&lineNumber=5455

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb543381ab2a
Update slow script warning visuals r=florian
https://hg.mozilla.org/integration/autoland/rev/f8fa05f30e80
Separate Enter/Exit Widget Events from Mouse Button events r=smaug
https://hg.mozilla.org/integration/autoland/rev/34a7a3d0ce74
Show slow script warning only when critical input is pending r=smaug
https://hg.mozilla.org/integration/autoland/rev/326c2dd56773
Add mouse wheel scrolling to critical input list r=smaug
https://hg.mozilla.org/integration/autoland/rev/8738ac929630
Show a different notification is selected tab is hanging r=florian

Comment 14

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/eb543381ab2a
https://hg.mozilla.org/mozilla-central/rev/f8fa05f30e80
https://hg.mozilla.org/mozilla-central/rev/34a7a3d0ce74
https://hg.mozilla.org/mozilla-central/rev/326c2dd56773
https://hg.mozilla.org/mozilla-central/rev/8738ac929630

Comment 15

[Peter]

Verified that the slow script warning only appears when the user interacts with the unresponsive tab (after 10 seconds) and it blames the hang on the tab that is causing it. The wait button is also removed from the notification.

I've verified it on MacOS 11, Ubuntu 20.04 and Windows 10 using the latest Nightly 88.0a1