New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 486505 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

UI freezes upon creating many context menu items in an event page

Project Member Reported by rob@robwu.nl, May 10 2015

Issue description

Chrome version: 42.0.2311.90

When an extension is running as an event page, every chrome.contextMenus API call is always followed by a contextmenu serialization action and an expensive file writing operation. This is wasteful and leads to a complete browser freeze when many contextMenu modifications are requested in quick succession.

Steps to reproduce:
1. Download manifest.json & background.js and load it as an extension.
2. Click on the extension button. The extension will now register 2000 plain context menu entries.

What happens:
Chrome freezes for several seconds while spinning two cores of my CPU at 100% and does busy IO. The Chrome profile used for this test was on a ramdisk, so the performance may be significantly worse when the user profile is backed by a slow storage device.
Load the attached tracing data at chrome://tracing to see what happens. According to the data, most time is spent here: https://cs.chromium.org/%22void%20ValueStoreFrontend::Set%22


If I change the event page to a background page (change "persistent":false to "persistent":true in manifest.json), Chrome will not freeze (it still takes a second before all contextmenu registration requests have completed though).
 
chrome-tracing-data.zip
865 KB Download
background.js
525 bytes View Download
manifest.json
335 bytes Download
That's just baaad. Is this a regression or you've always notice this?

Comment 2 by wolfw...@gmail.com, May 10 2015

@lazyboy
it's always been here. 
I'm experiencing it for at least a year since I'm stumble on this problem
http://stackoverflow.com/questions/27103694/chrome-contextmenus-create-freezes-chrome

Comment 3 by rob@robwu.nl, May 10 2015

#1
It is not a regression, it has always been there. I created this report after seeing and verifying an issue that Wolfwar reported on the chromium-extensions mailing list and Stack Overflow.

To fix the bug, you could either extend the contextMenus API to allow registration of multiple context menu entries at once (this can be fully backwards compatible, e.g. by allowing the chrome.contextMenus.create/update/remove to take an array of values.

That solves the performance impact of registering many context menu items.
It doesn't work if one really wants to update context menu items in quick succession, so it'd also be nice if the number of disk operations can be reduced.

Do you want to submit a CL to fix this bug?

Comment 4 by kalman@chromium.org, May 11 2015

I wouldn't put too much engineering into solving this bug. If it is some sort of thing that could be optimised in the ValueStore then that'd be worth it - it's used by a lot of things. Set() is *supposed* to be asynchronous.

Or is there something inherently wasteful about the way that we serialise context menu entries? I just can't imagine it being that much work, but I don't know how it's done.

What is the use case for an extension wanting to create 2,000 context menus? I think you could find in every API if you do N actions in quick succession, for sufficiently large N, it will slow things down.

Comment 5 by wolfw...@gmail.com, May 11 2015

#4
it doesn't have to be 2k items
I'm experiencing this issue with the tree system like this
http://prntscr.com/742thh

Comment 6 by kalman@chromium.org, May 11 2015

Yeah sounds like we should have a "children" properly at least, to let you set up a structure all at once. That would be a nice API.

A way to fix this even within the current API would be to implement something similar to "children" on top of the current API, which saves up the "create" calls and only executes them after the event loop completes.

Suggestions for easier to implement optimisations, which don't make the process even more fragile, welcome.

Comment 7 by wolfw...@gmail.com, Jul 27 2015

this should create 2k context items, but it stops on 998
Is there some max number of context items in chromium code
or some safety switch that recognized resource hog and killed the rest of the code execution?

Same thing happens in Opera with deep tree in context menu
at first I thought it was something wrong with my recursive function (it stops at some point, different point for different data)
until I tested Rob's code.... max items is 998

Comment 8 by kalman@chromium.org, Jul 27 2015

#7 what platform? I don't see any restriction in our code, so it could be a native restriction (like within cocoa).

Comment 9 by wolfw...@gmail.com, Jul 27 2015

@kalman
win 7 x64

no extra libraries or so
I simply placed Rob's manifest and background in one folder and install it in dev mode

http://prntscr.com/7xne9i

it should be 2000, but the max is 998
(tested in canary, chrome and Opera dev, beta and stable...same thing in all of them)


Comment 10 by rob@robwu.nl, Jul 28 2015

@kalman & @Wolfwar2

The number of extension-generated context menu items is limited to 1000.
https://cs.chromium.org/file%3Achrome_command_ids.h%20IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST

This is enforced by ContextMenuMatcher::AppendExtensionItems:
https://chromium.googlesource.com/chromium/src/+/413226da7a9a50853005b83a440c8026f823163a/chrome/browser/extensions/context_menu_matcher.cc#59

It's not just a restriction on the number of visible items, the number of actions is also constrained to the same limit: https://cs.chromium.org/file%3Aextension_context_menu_model.cc%20ExtensionContextMenuModel%3A%3AExecuteCommand


My bug report shows that calling the function many times can cause the browser to lock down, regardless of whether actual creation of these context menu items succeeded.
Hah, fun. I'm not sure what the effect of increasing this limit would be (which would require moving up all of those context menu reserved IDs, which is if they're persisted somewhere or if there are other assumptions, would obviously break).

To "fix" this I would just pull this 1000 limit into a constant in the API, compile-assert that the constant == that difference between the start and end, then properly set runtime.lastError.

1000+ context menu items is not something I want to potentially break things to support.

Sign in to add a comment