1894614 - Long hangs (75s) seen when opening the address bar due to waiting for the clipboard

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Mark Banner (:standard8)]

In Firefox 125 we enabled a new feature on release (bug 1882478) to present a suggestion on the address bar if the user has a url on their clipboard.

We have had a couple of reports on Reddit of users seeing long hangs when clicking on the address bar.

One user has been able to capture a profile, which shows a 75 second hang with this stack:

ZwUserGetClipboardData [win32u.dll]
GetClipboardData [user32.dll]
CClipDataObject::OleGetClipboardData(unsigned int, void**) [com\ole32\ole232\clipbrd\clipdata.cpp]
CClipDataObject::GetData(tagFORMATETC*, tagSTGMEDIUM*) [com\ole32\ole232\clipbrd\clipdata.cpp]
RepeatedlyTryGetData::<lambda_19>::operator()() const [widget/windows/nsClipboard.cpp]
RepeatedlyTry(RepeatedlyTryGetData::<lambda_19>, std::_Binder<std::_Unforced,void (&)(long, const nsTString<char> &),const std::_Ph<1> &,nsTLiteralString<char> >) [widget/windows/nsClipboard.cpp]
RepeatedlyTryGetData(IDataObject&, tagFORMATETC*, tagSTGMEDIUM*) [widget/windows/nsClipboard.cpp]
nsClipboard::FillSTGMedium(IDataObject*, unsigned int, tagFORMATETC*, tagSTGMEDIUM*, unsigned long) [widget/windows/nsClipboard.cpp]
nsClipboard::GetNativeDataOffClipboard(IDataObject*, unsigned int, unsigned int, char const*, void**, unsigned int*) [widget/windows/nsClipboard.cpp]
nsClipboard::GetDataFromDataObject(IDataObject*, unsigned int, nsIWidget*, nsITransferable*) [widget/windows/nsClipboard.cpp]
nsClipboard::GetNativeClipboardData(nsITransferable*, int) [widget/windows/nsClipboard.cpp]
nsBaseClipboard::GetData(nsITransferable*, int, mozilla::dom::WindowContext*) [widget/nsBaseClipboard.cpp]
...
readFromClipboard [chrome://browser/content/browser.js:3091:27]
isActive [resource:///modules/UrlbarProviderClipboard.sys.mjs:49:11]
js::RunScript
tryMethod [resource:///modules/UrlbarUtils.sys.mjs:2365:12]
start [resource:///modules/UrlbarProvidersManager.sys.mjs:405:14]
startQuery [resource:///modules/UrlbarProvidersManager.sys.mjs:215:19]

Comment 1

[Marco Bonardo [:mak]]

Let's disable the clipboard feature by default, users can still flip it on from Preferences if they wish.
It sounds like we really need bug 1874203.

Comment 2

[Karandeep Lubana [:klubana]]

Attachment: Bug 1894614 - Disable the clipboard suggestions in the address bar due to performance concerns. r=mak

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e91d8e436dcc
Disable the clipboard suggestions in the address bar due to performance concerns. r=mak

Comment 4

[Marco Bonardo [:mak]]

For Firefox 126, Donal is likely already adding this, just adding it here for tracking purposes.

Release Note Request (optional, but appreciated)
[Why is this notable]: Disabling a previously released and rel-noted feature
[Affects Firefox for Android]: no
[Suggested wording]: The Clipboard Suggestion feature was temporarily disabled while the team investigates a potential performance issue. The feature will be re-enabled in a future release once the performance issue is addressed.
[Links (documentation, blog post, etc)]:

Comment 5

[Karandeep Lubana [:klubana]]

Attachment: Bug 1894614 - Disable the clipboard suggestions in the address bar due to performance concerns. r=mak

Original Revision: https://phabricator.services.mozilla.com/D209257

Comment 6

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: performance issue
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: Check the feature is properly disabled (no checkbox in about:preferences, no result in the address bar)
• Risk associated with taking this patch: Low
• Explanation of risk level: We're flipping of the feature gate preference, so returning to the status before the feature was released in 125
• String changes made/needed: None
• Is Android affected?: no

Comment 7

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/e91d8e436dcc

Comment 8

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/caa1740c9ba1

Comment 9

[Emilio Cobos Álvarez (:emilio)]

We have async clipboard APIs, is there any reason why navigator.clipboard.readText() is not usable?

Comment 10

[Marco Bonardo [:mak]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #9)

We have async clipboard APIs, is there any reason why navigator.clipboard.readText() is not usable?

https://bugzilla.mozilla.org/show_bug.cgi?id=1894718
Even if ideally we'd like to be able to peek at the clipboard size even before actually fetching it, or only fetch if size is less than N. But that capability is not available (https://bugzilla.mozilla.org/show_bug.cgi?id=1874203).

Comment 11

[jscher2000]

On SUMO, a Windows user reported only experiencing the problem when they have a remote desktop session open on a Linux host. Not sure whether that is relevant. https://support.mozilla.org/questions/1446274

Comment 12

[Adrian Florinescu [:aflorinescu]]

Pref'd off of the feature update verified with 127.0a1 (2024-05-06) /126.0 (64-bit) RC on Windows 10/ Mac13.

Additionally, we have tried to reproduce the issue with 125.0 , 125.0.1, 125.0.2, 125.03 on Windows 11 Version 21h2 22H2 (OS Build 22000.1817) and Windows 11 Version 22H2 (OS Build 22621.2861) with no success. Seems that there are additional prerequisites to hit this.

Comment 13

[Adrian Florinescu [:aflorinescu]]

Updating flags since this issue is not actually fixed, disabled flags for the 126/127 would make more sense?

Comment 14

[Marco Bonardo [:mak]]

(In reply to jscher2000 from comment #11)

On SUMO, a Windows user reported only experiencing the problem when they have a remote desktop session open on a Linux host. Not sure whether that is relevant. https://support.mozilla.org/questions/1446274

It may be relevant, depending on how remote desktop interacts with OLE clipboard. I have a small exe that when executed misuses the clipboard causing Firefox (or any other app using OLE clipboard) to hang.

Comment 15

[jscher2000]

Another SUMO user who has remote desktop with clipboard enabled added that after disabling clipboard suggestions, pasting into the address bar triggers the same problem. https://support.mozilla.org/en-US/questions/1446876#answer-1652076

Presumably there is something special about parsing a paste into the address bar which doesn't affect other fields. Is there a bug open for that? I couldn't find one, but my bug search skills are not very good.

Comment 16

[Marco Bonardo [:mak]]

(In reply to jscher2000 from comment #15)

Another SUMO user who has remote desktop with clipboard enabled added that after disabling clipboard suggestions, pasting into the address bar triggers the same problem. https://support.mozilla.org/en-US/questions/1446876#answer-1652076

First thing to note here, is that on Windows any third party app can lock any other app using the OLE clipboard when it tries to access the clipboard. I demonstrated it locally with few lines of code. That's not really fixable on our side, it's a Windows problem.

The only thing the urlbar does differently from other input fields is parsing the string to check if it's a URL, so of course it may have performance problems with very large strings. The user said it was a short string so it was not that problem. Then I suspect a locked clipboard as I explained, but that same issue was there in any previous versions of Firefox then.

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1882478

Comment 18

[Daniel Holbert [:dholbert]]

Bad bot - this was already fixed (by preffing off the feature) on nightly in the 127 cycle.

(ni=suhaib to see if we can figure out what happened in comment 17 and prevent it in other bugs. Thanks!)

Comment 19

[Suhaib Mujahid [:suhaib]]

(In reply to Daniel Holbert [:dholbert] from comment #18)

Bad bot - this was already fixed (by preffing off the feature) on nightly in the 127 cycle.

(ni=suhaib to see if we can figure out what happened in comment 17 and prevent it in other bugs. Thanks!)

Since the regression affected 125, all newer versions will be marked as affected until a version is marked as fixed or verified. I think this is what happened in comment 17. What do you think the bot should do differently?

Comment 20

[Daniel Holbert [:dholbert]]

Thanks. I guess the upshot of the bot issue is that we should never set the trunk status as disabled, when we fix a bug by preffing off a feature on trunk indefinitely (i.e. not in a way where version numbers are relevant). Otherwise, the change from affected-to-disabled makes the bot implicitly think the bug will still affect future versions.

I filed https://github.com/mozilla/bugbot/issues/2403 on seeing if we can make the bot handle this more gracefully/helpfully.

Comment 21

[Adrian Florinescu [:aflorinescu]]

Removing qe+ for now, please add it back when we have a patch here.