flypig.co.uk

I'm still feeling unwell. Plus I had a long couple of days at work yesterday and the day before. All in all this has left me in a bit of a mess and I spent last night trying to figure out how the parent browsing context and the new browsing context relate to one another. Although I did get to a piece of code that looked relevant, I wasn't able to piece together the processes either side that led up to it and that led away from it, all of which should have re-converged inside nsWindowWatcher::OpenWindowInternal().

What I did do was add in some code — mimicking the code further up the stack which usually performs the task elsewhere — to create the new browser context from the parent browser context into this method, in an attempt to short circuit the process that would usually require a new window to be created.

It built overnight and now I'm testing it out.

Unfortunately it leaves us with a segfault occurring inside nsGlobalWindowOuter::Print(). Let's take a record of the backtrace.

Thread 8 "GeckoWorkerThre" received signal SIGSEGV, Segmentation fault.
[Switching to LWP 6812]
nsGlobalWindowOuter::Print(nsIPrintSettings*, nsIWebProgressListener*,
    nsIDocShell*, nsGlobalWindowOuter::IsPreview,
    nsGlobalWindowOuter::IsForWindowDotPrint, std::function<void
    (mozilla::dom::PrintPreviewResultInfo const&)>&&, mozilla::ErrorResult&)
    (this=this@entry=0x7f8843e530, 
    aPrintSettings=aPrintSettings@entry=0x7e33d83c80,
    aListener=aListener@entry=0x7f88f44c00,
    aDocShellToCloneInto=aDocShellToCloneInto@entry=0x0, 
    aIsPreview=aIsPreview@entry=nsGlobalWindowOuter::IsPreview::No, 
    aForWindowDotPrint=aForWindowDotPrint@entry=nsGlobalWindowOuter::
    IsForWindowDotPrint::No, aPrintPreviewCallback=..., aError=...)
    at dom/base/nsGlobalWindowOuter.cpp:5351
5351        cloneDocShell->GetContentViewer(getter_AddRefs(cv));
(gdb) bt
#0  nsGlobalWindowOuter::Print(nsIPrintSettings*, nsIWebProgressListener*,
    nsIDocShell*, nsGlobalWindowOuter::IsPreview,
    nsGlobalWindowOuter::IsForWindowDotPrint, std::function<void
    (mozilla::dom::PrintPreviewResultInfo const&)>&&, mozilla::ErrorResult&)
    (this=this@entry=0x7f8843e530, 
    aPrintSettings=aPrintSettings@entry=0x7e33d83c80,
    aListener=aListener@entry=0x7f88f44c00,
    aDocShellToCloneInto=aDocShellToCloneInto@entry=0x0, 
    aIsPreview=aIsPreview@entry=nsGlobalWindowOuter::IsPreview::No, 
    aForWindowDotPrint=aForWindowDotPrint@entry=nsGlobalWindowOuter::
    IsForWindowDotPrint::No, aPrintPreviewCallback=..., aError=...)
    at dom/base/nsGlobalWindowOuter.cpp:5351
#1  0x0000007fbc7eb714 in mozilla::dom::CanonicalBrowsingContext::Print
    (this=this@entry=0x7f88c83400, aPrintSettings=0x7e33d83c80, aRv=...)
    at include/c++/8.3.0/bits/std_function.h:402
#2  0x0000007fbab82f08 in mozilla::dom::CanonicalBrowsingContext_Binding::print
    (args=..., void_self=0x7f88c83400, obj=..., cx_=0x7f881df400)
    at BrowsingContextBinding.cpp:4674
#3  mozilla::dom::CanonicalBrowsingContext_Binding::print_promiseWrapper
    (cx=0x7f881df400, obj=..., void_self=0x7f88c83400, args=...)
    at BrowsingContextBinding.cpp:4688
#4  0x0000007fbb2e2960 in mozilla::dom::binding_detail::GenericMethod
    <mozilla::dom::binding_detail::NormalThisPolicy,
    mozilla::dom::binding_detail::ConvertExceptionsToPromises>
    (cx=0x7f881df400, argc=<optimized out>, vp=<optimized out>)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/js/CallArgs.h:207
[...]
#29 0x0000007fbd1662cc in js::jit::MaybeEnterJit (cx=0x7f881df400, state=...)
    at js/src/jit/Jit.cpp:207
#30 0x0000007f8824be41 in ?? ()
Backtrace stopped: Cannot access memory at address 0x4d673b58921e
(gdb) 

Here's the code (it's the last line that's causing the segfault):

    nsCOMPtr<nsIDocShell> cloneDocShell = bc->GetDocShell();
    MOZ_DIAGNOSTIC_ASSERT(cloneDocShell);
    cloneDocShell->GetContentViewer(getter_AddRefs(cv));

And here's the reason for the segfault:

(gdb) p cloneDocShell
$1 = {<nsCOMPtr_base> = {mRawPtr = 0x0}, <No data fields>}
(gdb) 

So, from this, it looks very much like the browser context simply doesn't have a docShell. Maybe there's a whole bunch of other stuff it doesn't — but should — have as well?

Looking back at the original code that creates the new browser context inside EmbedLiteViewChild::InitGeckoWindow(), there's this snippet of code following the creation that looks particularly relevant:

  // nsWebBrowser::Create creates nsDocShell, calls InitWindow for nsIBaseWindow,
  // and finally creates nsIBaseWindow. When browsingContext is passed to
  // nsWebBrowser::Create, typeContentWrapper type is passed to the nsWebBrowser
  // upon instantiation.
  mWebBrowser = nsWebBrowser::Create(mChrome, mWidget, browsingContext,
                                     nullptr);

With the short-circuit we introduced yesterday this is no longer being executed. So no browser context. So segfaults. So no good.

Back to the drawing board and, in particular, to figuring out the path to how this EmbedLiteViewChild::InitGeckoWindow() method gets called.

It would be so nice to cheat on this. I could execute the code in the debugger, stick a breakpoint on the method and observe the path in the backtrace, were it not for the fact I introduced that short circuit. Since I did, this method no longer gets called, so the breakpoint will no longer fire.

The short circuit is broken anyway so I've removed it and set the build running. But it will be hours before it completes so I may as well peruse the code in the meantime in case I can figure it out manually.

I've also just noticed that just after creation of the mWebBrowser that we saw above, there's this call here:

  rv = mWebBrowser->SetVisibility(true);

I'm also interested to know whether this could be of use to us, so that's also something to follow in the code (although I don't think it'll make any difference on the tab display side).

So from manual inspection, this InitGeckoWindow() gets triggered by the EmbedLiteViewChild constructor. The constructor isn't called directly, but is inherited by EmbedLiteViewThreadChild and EmbedLiteViewProcessChild. I think the browser uses the thread version so I'm going to follow that route.

An instance of EmbedLiteViewThreadChild is created in the EmbedLiteAppThreadChild::AllocPEmbedLiteViewChild() method, which isn't called from anywhere in the main codebase. However, just to highlight how frustratingly convoluted this is, there is some generated code which is presumably the place responsible for calling it:

auto PEmbedLiteAppChild::OnMessageReceived(const Message& msg__) ->
    PEmbedLiteAppChild::Result
{
[...]
    switch (msg__.type()) {
    case PEmbedLiteApp::Msg_PEmbedLiteViewConstructor__ID:
        {
[...]
            msg__.EndRead(iter__, msg__.type());
            PEmbedLiteViewChild* actor =
                (static_cast<EmbedLiteAppChild*>(this))->
                AllocPEmbedLiteViewChild(windowId, id, parentId,
                parentBrowsingContext, isPrivateWindow, isDesktopMode);
[...]

The sending of this message (through a bit of generated indirection) happens as a result of a call to PEmbedLiteAppParent::SendPEmbedLiteViewConstructor(), which (finally we got there) is called here:

EmbedLiteView*
EmbedLiteApp::CreateView(EmbedLiteWindow* aWindow, uint32_t aParent,
  uintptr_t aParentBrowsingContext, bool aIsPrivateWindow, bool isDesktopMode)
{
  LOGT();
  NS_ASSERTION(mState == INITIALIZED, "The app must be up and runnning by now");
  static uint32_t sViewCreateID = 0;
  sViewCreateID++;

  PEmbedLiteViewParent* viewParent = static_cast<PEmbedLiteViewParent*>(
      mAppParent->SendPEmbedLiteViewConstructor(aWindow->GetUniqueID(),
          sViewCreateID, aParent, aParentBrowsingContext, aIsPrivateWindow,
          isDesktopMode));
  EmbedLiteView* view = new EmbedLiteView(this, aWindow, viewParent, sViewCreateID);
  mViews[sViewCreateID] = view;
  return view;
}

This is part of the main codebase, not auto-generated.

There's always a balance between minimising changes to the entire codebase and minimising changes to the gecko codebase. My preference is for the latter wherever possible since it will help minimise maintenance in the long run. In this case, working on this principle, it looks like the best thing to do is to allow the print status to be passed down through the EmbedLite code and into the front end. This will allow the front end to choose how to deal with the new window, which after some additional front end changes, could be to hide the window.

Before I do that, I'm going to double check the reason why all of this isn't necessary in ESR 78. I think I've reached the stage where I understand the process flow in the ESR 91 print, browser context and window creation code sufficiently to make it worthwhile attempting a comparison.

So what I want to do is to check whether nsGlobalWindowOuter::Print() is called as part of the ESR 78 process and, if so, figure out what happens at this point:

    // We're already a print preview window, just reuse our browsing context /
    // content viewer.
    bc = sourceBC;
    nsCOMPtr<nsIDocShell> docShell = bc->GetDocShell();
    if (!docShell) {
      aError.ThrowNotSupportedError("No docshell");
      return nullptr;
    }

If that code is being executed it might imply a route that could work on ESR 91 that also avoids having to open the new window.

That's it for today though. I'll have to pick this question up again tomorrow.

If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.