Closed Bug 1694229 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: XPConnect, enhancement, P1)

Product:
Core
Component:
XPConnect
Type:
enhancement

Tracking

()

Status:
VERIFIED FIXED
Milestone:
88 Branch
Tracking Flags:
Tracking Status
firefox88 --- verified

People

(Reporter: alexical, Assigned: alexical)

References

(Depends on 1 open bug, Regressed 1 open bug)

Details

(Whiteboard: [proton-infobars])

Attachments

(5 files)

Bug 1694229 - Update slow script warning visuals r?florian
48 bytes, text/x-phabricator-request
Details | Review
Bug 1694229 - Separate Enter/Exit Widget Events from Mouse Button events r?smaug
48 bytes, text/x-phabricator-request
Details | Review
Bug 1694229 - Show slow script warning only when critical input is pending r?smaug
48 bytes, text/x-phabricator-request
Details | Review
Bug 1694229 - Add mouse wheel scrolling to critical input list r?smaug
48 bytes, text/x-phabricator-request
Details | Review
Bug 1694229 - Show a different notification is selected tab is hanging r?florian
48 bytes, text/x-phabricator-request
Details | Review

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

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.

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

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

Depends on D106016

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

Depends on D106017

Severity: -- → S3
Priority: -- → P3
See Also: → https://jira.mozilla.com/browse/FIDEFE-197
Whiteboard: [proton-infobars]

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

Status: NEW → ASSIGNED
Priority: P3 → P1

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

Pushed by dothayer@mozilla.com: 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
Attachment #9205173 - Attachment description: Bug 1694229 - Show a different notification is selected tab is hanging r?florian → Bug 1694229 - Show a different notification if selected tab is hanging r?florian
Pushed by dothayer@mozilla.com: 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

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
Attachment #9205173 - Attachment description: Bug 1694229 - Show a different notification if selected tab is hanging r?florian → Bug 1694229 - Show a different notification is selected tab is hanging r?florian

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

Pushed by dothayer@mozilla.com: 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
Depends on: 1695726
Depends on: 1695767
Depends on: 1696482
Depends on: 1696978

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

Status: RESOLVED → VERIFIED
status-firefox88: fixed → verified
Depends on: 1702516
Flags: needinfo?(dothayer)
Regressions: 1755864
See Also: → 1432580
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: