[MD Bookmarks] API listeners do not trigger after closing another bookmark manager window |
||||||
Issue descriptionSteps to reproduce 1. Open two bookmark managers in side-by-side windows 2. Close one of them 3. Delete a bookmark in the other one. Actual result: The bookmark deletion is not sent to the front end. This also happens with edits and other operations -- the page is broken until it is refreshed.
,
Jul 24 2017
Updated steps to reproduce: 1. Right click a folder in the bookmark bar, select "Bookmark manager" (to open a URL with /?id=something) 2. Open a new tab, navigate to chrome://bookmarks (you'll need to type it in manually) 3. Close that new tab 4. Try to edit something in the original bookmark manager tab. Nothing happens. When the page breaks, all chrome.bookmarks event listeners stop working. It's still possible to make changes through the bookmarks API, but none of them get sent back to the front-end until the page is reloaded. I traced through the code and found that in EventRouter::DispatchEventImpl [1], the set of listeners for the bookmarks event is empty, so no message is ever sent to the renderer process. Poking around some more, I found the comment in EventListener::ForURL pointing to issue 536858 [2]. In that issue, EventRouter was updated so that the structure which maps events to renderer processes is keyed by Origin, rather than by the full URL of the page. This prevents events listeners from firing. However, the corresponding code in the renderer process still uses the full URL. In extensions/renderer/event_bindings.cc [3], there's another map which keeps track of how many listeners are registered for each event, keyed by page load URL. When the tab is closed in step (3) above, the counts for "chrome://bookmarks/?id=1" are decremented from 1 to 0, causing a message to be sent to the browser to deregister the event listeners for that URL [4]. In the browser process, the listener is deduplicated by origin, causing all listeners for chrome://bookmarks to be removed [5]. Given that triggering this issue requires a few unusual manual steps (have to load a non-default URL, have to manually open a second bookmark manager), I don't think this blocks release. However, it's still definitely something we'd like to address, since the page completely breaks when the bug triggers. +devlin, since this issue is deep within the extensions system, is this something you might have a chance to fix? [1] https://cs.chromium.org/chromium/src/extensions/browser/event_router.cc?dr&l=477 [2] https://cs.chromium.org/chromium/src/extensions/browser/event_listener_map.cc?dr&l=43 [3] https://cs.chromium.org/chromium/src/extensions/renderer/event_bindings.cc?l=86 [4] https://cs.chromium.org/chromium/src/extensions/renderer/event_bindings.cc?l=293 [5] https://cs.chromium.org/chromium/src/extensions/browser/event_router.cc?dr&l=223
,
Jul 24
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 27
Definitely still nice to fix, but definitely also quite edge-casey, and lower priority. Marking as available.
,
Nov 23
**UI Mass Triage** As per C#0 and C#2 observing same issue from M-63(63.0.3209.0-New UI started) to latest Canary 72.0.3618.0 on all OS. rdevlin@ : As per C#4 please look into this issue Note : Bookmark is getting deleted from M-60 60.0.3702.0 to 63.0.3208.0 i.e. in Old UI Thanks..!!
,
Nov 26
#4 is still correct. > Definitely still nice to fix, but definitely also quite edge-casey, and lower priority. Marking as available. This isn't something the extensions team will have the bandwidth to tackle in the near future, so if anyone else wanted to jump on it, please do. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by tsergeant@chromium.org
, Jul 21 2017