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

Issue 746812 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 3
Type: Bug



Sign in to add a comment

[MD Bookmarks] API listeners do not trigger after closing another bookmark manager window

Project Member Reported by tsergeant@chromium.org, Jul 20 2017

Issue description

Steps 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.
 
Labels: -Pri-1 Pri-2
* This doesn't happen in the old bookmark manager.

* This only happens if the original bookmark manager page load has an extra param (eg, chrome://bookmarks/?id=31)

Blocking: -658980
Cc: rdevlin....@chromium.org
Components: Platform>Extensions
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
Project Member

Comment 3 by sheriffbot@chromium.org, Jul 24

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
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
Cc: dbertoni@chromium.org lazyboy@chromium.org
Status: Available (was: Untriaged)
Definitely still nice to fix, but definitely also quite edge-casey, and lower priority.  Marking as available.
Cc: mmanchala@chromium.org
Labels: Hotlist-DesktopUIValid Hotlist-DesktopUIChecked OS-Linux OS-Mac OS-Windows
**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..!!
Labels: -Pri-2 Pri-3
#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