Page MenuHomePhabricator

Performance problem on Chromium
Closed, ResolvedPublicBUG REPORT

Description

What is the problem?

Trying to recover an edit on a large article can hang the browser tab.

This happens on Chromium but not Firefox.

Steps to reproduce problem
  1. Login to https://en.wikipedia.beta.wmflabs.org
  2. Go to https://en.wikipedia.beta.wmflabs.org/w/index.php?title=Barack_Obama_Editable&action=edit
  3. Make an edit to the page (but don't save)
  4. Wait at least 5 seconds (for the edit data to be stored in indexeddb)
  5. Reload the page

Expected behavior: Page reloads fine
Observed behavior: Browser tab is unresponsive

Environment

Operating system: Debian 11
Browser: Chromium 115.0.5790.170
Wiki(s): https://en.wikipedia.beta.wmflabs.org MediaWiki 1.41.0-alpha (a8d3383) 11:01, 8 August 2023.

QA Results - Beta

Event Timeline

It looks like this is due to textSelection API being very slow when inserting large numbers of lines (the Barack Obama article has about 2,300).

In jquery.textSelection, setContents, it calls document.execCommand( 'insertText' ) to do the actual insertion. The following snippet demonstrates the issue I think:

let textbox = document.getElementById( 'wpTextbox1' );
textbox.value = '';
textbox.focus();
console.time();
document.execCommand( 'insertText', false, "testing\n".repeat( 100000 ) );
console.timeEnd();
Test stringFirefoxChromium
"testing ".repeat( 100000 )~20 ms~200 ms
"testing\n".repeat( 100000 )~20 ms~x ms

document.execCommand( 'insertText' ) is used to preserve the undo stack. This API is mostly used by tools to insert small snippets of wikitext, and you want to be able to undo these insertions with ctrl+z.

The use case here of replacing the entire contents of the input does not need to preserve the undo stack (in fact it probably shouldn't, as we don't want the user accidentally undoing back to the pre-recovery state of the document with one key press).

I would suggest adding another argument to textSelection#setContents to force overwriting the contents of the field without execCommand.

I would suggest the following:

  1. File an upstream bug with Chromium. Firefox can paste 1 million "x\n" in less than 1s, whereas Chrome can't do 1,000 in that time.
  2. Test this in Safari too, to see if there is a shared bug of webkit origin
  3. In the library check for >100 linebreaks and if $.client.profile().name === 'chrome' (or .layout === 'webkit' if present in Safari too), and always use the fallback ($.val) in that case. The library should always avoid attempting to do something we suspect will hang the browser.
  4. In a separate commit, add a "clearUndoStack" option to textSelection#setContents and use here

Chromium bug https://bugs.chromium.org/p/chromium/issues/detail?id=1451039 may or may not be related. It talks about some browser automation APIs being slow in a similar way to what we're seeing with execCommand(insertText), and it's recent, so this may be a new issue.

I can reproduce (using the snippet from T343795#9083114) the slowness (100% CPU) on chromium 90.0.4430.212-1~deb10u1 from Debian buster (15 May 2021), so it doesn't seem to be a bug recently introduced in Chrome.

Change 951150 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/core@master] jquery.textSelection: Avoid insertText for >100 lines in Chrome/Safari

https://gerrit.wikimedia.org/r/951150

Change 951150 merged by jenkins-bot:

[mediawiki/core@master] jquery.textSelection: Avoid insertText for >100 lines in Chrome/Safari

https://gerrit.wikimedia.org/r/951150

@Esanders @Samwilson The page reloads fine now but after the page reloads, it brings you to the very bottom of the article instead of the top. Did you want me to close this task and create a new task?

Status: ✅ PASS
Environment: Beta
OS: macOS Ventura 13.5
Browser: Chrome 116
Device: MBP
Emulated Device:: N/A
Test Link:
https://en.wikipedia.beta.wmflabs.org/w/index.php?title=Barack_Obama_Editable&action=edit

✅AC1: Page reloads fine after non-saved edit

Part 1 of 2

Part 2 of 2- works but see the potential issue below

Possible Issue:

In the Part 2 of 2 webm, when you reload after it is stored in the IndexDB, it goes to the bottom of the article. Should it go to the top of the article like all articles when they are reloaded?

In the Part 2 of 2 webm, when you reload after it is stored in the IndexDB, it goes to the bottom of the article. Should it go to the top of the article like all articles when they are reloaded?

Interesting. I wonder if that is a bug in Realtime Preview or Edit Recovery?

@Samwilson Since the actual task did pass, as seen from above. I will move this to Done. I did create T346370: RTP- reloads at the bottom of the page for the issue which seems to be RTP. Thanks for all your work!