ltilve/chromium

Problems running some extensions inside browserAction sidebar

Closed this issue · 11 comments

It seems there are some problems on the behaviour of some extensions when their popup is shown as a sidebar. For instance 'Process sample' https://developer.chrome.com/extensions/samples#search:process%20monitor is working as expected on both cases, others as 'My Bookmarks' https://developer.chrome.com/extensions/samples#search:my%20bookmarks are apparently failing to populate the sidebar page. This issue seems to be happening also with "bookmarks-sidebar" example extension when the sidebar is not displayed by using the chrome.sidebar.show API call.

It looks like chrome.bookmark.getTree does not work in the sidebar mode.

If I execute below statement in extension's inspector
chrome.bookmarks.getTree(function(s){ alert(s); });
in popup mode it shows alert with [Object object], but in sidebar mode, this callback doesn't executed.

Maybe the permission handling for extension is not working properly in the sidebar mode.

Another interesting point is there are sidebar.js and popup.js in bookmark-sidebar extension, but it only uses popup.js in popup and sidebar mode. Maybe first intension of this extension is providing different UI and functionality between popup and sidebar.

We need to make a decision whether should we support this functionality or remove sidebar.js and sidebar.html in bookmark-sidebar extension.

In popup mode, chrome.bookmarks.getTree is reaching to BookmarksGetTreeFunction::RunOnReady() via below callstack. But in sidebar mode, it even does not reach to void ExtensionFunctionDispatcher::DispatchWithCallbackInternal. (it is #4 stack)

#0 0x000000010b304f62 in extensions::BookmarksGetTreeFunction::RunOnReady() at /Users/ryumiel/workspace/chromium/src/out/Debug/../../chrome/browser/extensions/api/bookmarks/bookmarks_api.cc:487
#1 0x000000010b302516 in extensions::BookmarksFunction::RunAndSendResponse() at /Users/ryumiel/workspace/chromium/src/out/Debug/../../chrome/browser/extensions/api/bookmarks/bookmarks_api.cc:233
#2 0x000000010b3024af in extensions::BookmarksFunction::RunAsync() at /Users/ryumiel/workspace/chromium/src/out/Debug/../../chrome/browser/extensions/api/bookmarks/bookmarks_api.cc:104
#3 0x000000010b6e60f9 in ChromeAsyncExtensionFunction::Run() at /Users/ryumiel/workspace/chromium/src/out/Debug/../../chrome/browser/extensions/chrome_extension_function.cc:121
#4 0x000000010becd156 in extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal(ExtensionHostMsg_Request_Params const&, content::RenderViewHost_, content::RenderFrameHost_, base::Callback<void (ExtensionFunction::ResponseType, base::ListValue const&, std::string const&, extensions::functions::HistogramValue)> const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../extensions/browser/extension_function_dispatcher.cc:413
#5 0x000000010becc945 in extensions::ExtensionFunctionDispatcher::Dispatch(ExtensionHostMsg_Request_Params const&, content::RenderViewHost_) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../extensions/browser/extension_function_dispatcher.cc:324
#6 0x000000010bed9c40 in extensions::ExtensionHost::OnRequest(ExtensionHostMsg_Request_Params const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../extensions/browser/extension_host.cc:339
#7 0x000000010bee2477 in void DispatchToMethodImpl<extensions::ExtensionHost, void (extensions::ExtensionHost::)(ExtensionHostMsg_Request_Params const&), ExtensionHostMsg_Request_Params, 0ul>(extensions::ExtensionHost, void (extensions::ExtensionHost::)(ExtensionHostMsg_Request_Params const&), Tuple<ExtensionHostMsg_Request_Params> const&, IndexSequence<0ul>) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/tuple.h:252
#8 0x000000010bee23a3 in void DispatchToMethod<extensions::ExtensionHost, void (extensions::ExtensionHost::*)(ExtensionHostMsg_Request_Params const&), ExtensionHostMsg_Request_Params>(extensions::ExtensionHost
, void (extensions::ExtensionHost::)(ExtensionHostMsg_Request_Params const&), Tuple<ExtensionHostMsg_Request_Params> const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/tuple.h:259
#9 0x000000010bedbef4 in bool ExtensionHostMsg_Request::Dispatch<extensions::ExtensionHost, extensions::ExtensionHost, void, void (extensions::ExtensionHost::*)(ExtensionHostMsg_Request_Params const&)>(IPC::Message const
, extensions::ExtensionHost_, extensions::ExtensionHost_, void_, void (extensions::ExtensionHost::)(ExtensionHostMsg_Request_Params const&)) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../extensions/common/extension_messages.h:577
#10 0x000000010bed9936 in extensions::ExtensionHost::OnMessageReceived(IPC::Message const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../extensions/browser/extension_host.cc:327
#11 0x000000010beda162 in non-virtual thunk to extensions::ExtensionHost::OnMessageReceived(IPC::Message const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../extensions/browser/extension_host.cc:324
#12 0x000000010a44eeb3 in content::WebContentsImpl::OnMessageReceived(content::RenderViewHost
, content::RenderFrameHost_, IPC::Message const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../content/browser/web_contents/web_contents_impl.cc:492
#13 0x000000010a44ec42 in content::WebContentsImpl::OnMessageReceived(content::RenderViewHost_, IPC::Message const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../content/browser/web_contents/web_contents_impl.cc:472
#14 0x000000010a450b8c in non-virtual thunk to content::WebContentsImpl::OnMessageReceived(content::RenderViewHost_, IPC::Message const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../content/browser/web_contents/web_contents_impl.cc:470
#15 0x000000010a12a99b in content::RenderViewHostImpl::OnMessageReceived(IPC::Message const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../content/browser/renderer_host/render_view_host_impl.cc:882
#16 0x000000010a12d512 in non-virtual thunk to content::RenderViewHostImpl::OnMessageReceived(IPC::Message const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../content/browser/renderer_host/render_view_host_impl.cc:861
#17 0x000000010a0ecb19 in content::RenderProcessHostImpl::OnMessageReceived(IPC::Message const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../content/browser/renderer_host/render_process_host_impl.cc:1543
#18 0x000000010a0ed072 in non-virtual thunk to content::RenderProcessHostImpl::OnMessageReceived(IPC::Message const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../content/browser/renderer_host/render_process_host_impl.cc:1498
#19 0x0000000102fdfea9 in IPC::ChannelProxy::Context::OnDispatchMessage(IPC::Message const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../ipc/ipc_channel_proxy.cc:294
#20 0x0000000102fe690f in base::internal::RunnableAdapter<void (IPC::ChannelProxy::Context::)(IPC::Message const&)>::Run(IPC::ChannelProxy::Context, IPC::Message const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/bind_internal.h:176
#21 0x0000000102fe673f in base::internal::InvokeHelper<false, void, base::internal::RunnableAdapter<void (IPC::ChannelProxy::Context::)(IPC::Message const&)>, base::internal::TypeList<IPC::ChannelProxy::Context* const&, IPC::Message const&> >::MakeItSo(base::internal::RunnableAdapter<void (IPC::ChannelProxy::Context::)(IPC::Message const&)>, IPC::ChannelProxy::Context* const&, IPC::Message const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/bind_internal.h:293
#22 0x0000000102fe66cf in base::internal::Invoker<IndexSequence<0ul, 1ul>, base::internal::BindState<base::internal::RunnableAdapter<void (IPC::ChannelProxy::Context::
)(IPC::Message const&)>, void (IPC::ChannelProxy::Context
, IPC::Message const&), base::internal::TypeList<IPC::ChannelProxy::Context_, IPC::Message> >, base::internal::TypeListbase::internal::UnwrapTraits<IPC::ChannelProxy::Context*, base::internal::UnwrapTraitsIPC::Message >, base::internal::InvokeHelper<false, void, base::internal::RunnableAdapter<void (IPC::ChannelProxy::Context::)(IPC::Message const&)>, base::internal::TypeList<IPC::ChannelProxy::Context const&, IPC::Message const&> >, void ()>::Run(base::internal::BindStateBase_) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/bind_internal.h:343
#23 0x0000000103a85aff in base::Callback<void ()>::Run() const at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/callback.h:396
#24 0x00000001018353a1 in base::debug::TaskAnnotator::RunTask(char const_, char const_, base::PendingTask const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/debug/task_annotator.cc:62
#25 0x00000001018b21f2 in base::MessageLoop::RunTask(base::PendingTask const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/message_loop/message_loop.cc:444
#26 0x00000001018b2376 in base::MessageLoop::DeferOrRunPendingTask(base::PendingTask const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/message_loop/message_loop.cc:454
#27 0x00000001018b25bd in base::MessageLoop::DoWork() at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/message_loop/message_loop.cc:566
#28 0x000000010180df78 in base::MessagePumpCFRunLoopBase::RunWork() at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/message_loop/message_pump_mac.mm:325
#29 0x000000010180d43b in base::MessagePumpCFRunLoopBase::RunWorkSource(void_) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/message_loop/message_pump_mac.mm:303
#30 0x00007fff8b04fa01 in CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION ()
#31 0x00007fff8b041b8d in __CFRunLoopDoSources0 ()
#32 0x00007fff8b0411bf in CFRunLoopRun ()
#33 0x00007fff8b040bd8 in CFRunLoopRunSpecific ()
#34 0x00007fff8802356f in RunCurrentEventLoopInMode ()
#35 0x00007fff880232ea in ReceiveNextEventCommon ()
#36 0x00007fff8802312b in BlockUntilNextEventMatchingListInModeWithFilter ()
#37 0x00007fff8979e9bb in DPSNextEvent ()
#38 0x00007fff8979df68 in -NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:
#39 0x00007fff89793bf3 in -NSApplication run
#40 0x000000010180ef1f in base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate
) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/message_loop/message_pump_mac.mm:649
chromium#41 0x000000010180dbcd in base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate
) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/message_loop/message_pump_mac.mm:235
chromium#42 0x00000001018b1d63 in base::MessageLoop::RunHandler() at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/message_loop/message_loop.cc:410
chromium#43 0x000000010190ccd5 in base::RunLoop::Run() at /Users/ryumiel/workspace/chromium/src/out/Debug/../../base/run_loop.cc:55
chromium#44 0x00000001002804b7 in ChromeBrowserMainParts::MainMessageLoopRun(int
) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../chrome/browser/chrome_browser_main.cc:1721
chromium#45 0x0000000109890c82 in content::BrowserMainLoop::RunMainMessageLoopParts() at /Users/ryumiel/workspace/chromium/src/out/Debug/../../content/browser/browser_main_loop.cc:826
chromium#46 0x000000010989ea6f in content::BrowserMainRunnerImpl::Run() at /Users/ryumiel/workspace/chromium/src/out/Debug/../../content/browser/browser_main_runner.cc:209
chromium#47 0x000000010988b6f6 in content::BrowserMain(content::MainFunctionParams const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../content/browser/browser_main.cc:26
chromium#48 0x00000001017617b7 in content::RunNamedProcessTypeMain(std::string const&, content::MainFunctionParams const&, content::ContentMainDelegate
) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../content/app/content_main_runner.cc:384
#49 0x000000010176291c in content::ContentMainRunnerImpl::Run() at /Users/ryumiel/workspace/chromium/src/out/Debug/../../content/app/content_main_runner.cc:783
chromium#50 0x0000000101760e20 in content::ContentMain(content::ContentMainParams const&) at /Users/ryumiel/workspace/chromium/src/out/Debug/../../content/app/content_main.cc:19
chromium#51 0x0000000100025fc3 in ChromeMain at /Users/ryumiel/workspace/chromium/src/out/Debug/../../chrome/app/chrome_main.cc:66
chromium#52 0x00000001000013b2 in main at /Users/ryumiel/workspace/chromium/src/out/Debug/../../chrome/app/chrome_exe_main_mac.cc:16
chromium#53 0x0000000100001384 in start ()

Possible suspect.

bool WebContentsImpl::OnMessageReceived(RenderViewHost* render_view_host,
RenderFrameHost* render_frame_host,
const IPC::Message& message) {
DCHECK(render_view_host || render_frame_host);
if (GetWebUI() &&
static_cast<WebUIImpl*>(GetWebUI())->OnMessageReceived(message)) {
return true;
}

ObserverListBase::Iterator it(&observers_);
WebContentsObserver* observer;
if (render_frame_host) {
while ((observer = it.GetNext()) != NULL)
if (observer->OnMessageReceived(message, render_frame_host))
return true;
} else {
while ((observer = it.GetNext()) != NULL)
if (observer->OnMessageReceived(message)) // the extension call should be handled in this statement.
return true;
}

Maybe in sidebar mode, we do not add WebContentsObserver

There are problems running some extensions inside a sidebar, instead of with a popup, due to apparent certain calls to chrome API not working (eg., getting the bookmarks). It is not happening for all the cases, so still investigation in progress.

According to the backtrace. When calling the bookmarks API, this go through IPC, and it is intercepted by ExtensionHost, which unmarshall it and do the proper call.

This happens when having a popup. But when using a sidebar, basically we are not creating an ExtensionHost, so we don't handle that call.

Following the code from
ExtensionActionViewController::TriggerPopupWithURL() an
ExtensionViewHost is created through
ExtensionViewHostFactory::CreatePopupHost() (line 346). Checking
this factory, there is a similar call for creating a dialogue. Probably
we should add here a new method, CreateSidebarHost().

Tracking ExtensionViewHostFactory::CreatePopupHost() it ends up creating a ExtensionViewHost, which inherits from ExtensionHost.

Taking a look at ExtensionHost constructor, surprisingly it is very similar to the code in SidebarContainer constructor.

SidebarContainer maybe should have been implemented as an ExtensionHost. We could consider moving the code from SidebarContainer to ExtensionHost, but I it is probably safer if we just call the proper methods in SidebarManager/SidebarContainer from ExtensionHost.

So summing up, when creating an ExtensionHost with VIEW_TYPE_EXTENSION_SIDEBAR type, it would act as a wrapper of SidebarManager.

Once we have this ExtensionViewManager, we would need to create an ExtensionSidebar following the same approach used in ExtensionPopup. Difference is that ExtensionPopup really creates a native widget (Aura in Linux, Cocoa in Mac), while here we don't need such widget. Maybe after all we don't need such ExtensionSidebar if we don't need to create a proper widget.

The problems calling some extension APIs (in particular, asynchronous calls to extension methods, as the event listeners were properly being managed), are apparently solved by wrapping the Sidebar container as an extension host as pushed at 3da6ddc

WIP Patch is pushed at 3aa2d0215f4783f377effd5eb6c0b4d1777ec5d6

c265273f046d537bb01cf6a826e551aa3c6e4fdc fixed sidebar loading.

Pending issue is re-enable loadURL feature

20dfa70a53343bfa6d8430c82d37a734b1d6395b should support window.close inside of sidebar.
but it is not working as expected. I think we should follow pattens which is used in extension_popup.cc for this case.

Still working in progress

Fix committed at fcd532bd8278e75707c54faeaf46c0008b7934f2 in igalia-sidebar-CL-raw and igalia-sidebar-CL.
It runs as expected in Mac, but the test for Linux is needed.

It runs as expected in Linux, too. But the unit_test was broken.
It is going to be handled in #16