ltilve/chromium

Remove SidebarManager from browser process

ltilve opened this issue · 2 comments

From Devlin review:
https://codereview.chromium.org/1152613003/diff/20001/chrome/browser/browser_process.h#newcode124

We almost certainly don't want to have a SidebarManager as part of the browser
process (sidebars cannot, for instance, cross profiles). This should be tied
more closely to a profile, and should probably either be a
BrowserContextKeyedService (or one of its subclasses) or live on the
ExtensionSystem. My first inclination is probably that the latter makes more
sense.

We are working on some changes to avoid having sidebarManager living at the process browser, but at the extensionSystem instead. To make this, it's also neccesary to avoid the way that its getInstance() was called, to get the extensionSystem from the browserContext instead, and solve some scoping problems. However this approach is still not working. The changes at the temporary branch https://github.com/ltilve/chromium/commits/lkgr-igalia-sidebar-CL

We managed to get the code to work after moving sidebarManager to extensions namespace, squashed all the changes ff93fc0 and updated codereview issue https://codereview.chromium.org/1152613003/#msg13