1138536 - winEmbed crash

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Embedding: GRE Core, defect)

Description

[Steve Williams]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:37.0) Gecko/20100101 Firefox/37.0
Build ID: 20150223185154

Steps to reproduce:

Compiled & ran winEmbed.


Actual results:

Started, then exited.


Expected results:

Should have loaded startup page

Comment 1

[Steve Williams]

crash winEmbed, truck.    

ClientLayerManager::ForwardTransaction, [mozilla]\gfx\layers\client\ClientLayerManager.cpp

mLatestTransactionId = mTransactionIdAllocator->GetTransactionId(); 

crashes due to null mTransactionIdAllocator

Comment 2

[Steve Williams]

So stepped through Firefox to see what it does differently & have a proposed fix.

File : [mozilla]\embedding\browser\nsWebBrowser.cpp

Add :

mDocShell->CreateAboutBlankContentViewer(nsContentUtils::SubjectPrincipal());


As the last call in nsWebBrowser::Create()

ie.

 
  mDocShell->CreateAboutBlankContentViewer(nsContentUtils::SubjectPrincipal());

  mInitInfo = nullptr;

  return NS_OK;
}

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

jst, smaug, you've both reviewed changes in this file recently. Can one of you help Steve get his proposed change looked at and in shape to land?

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

I didn't know winEmbed is still supported -
but anyhow, Steve, what does actually crash and where? What is the stack trace?

Is this a recent regression?


And I wouldn't create aboutBlank content viewer if not needed. Firefox E10s relies on
nsWebBrowser, and making nsWebBrowser creation slower should be avoided if possible.

Comment 5

[Steve Williams]

Hi Olli,

I have a requirement for winEmbed, hence need to keep it working.

Would also like gtkEmbed back - Firefox is not the only possible Gecko target.

comment 1 explains where the crash occurs. 

ClientLayerManager::mLatestTransactionId is null without this.

mozilla\xpfe\appshell\nsWebShellWindow.cpp

nsWebShellWindow::Initialize initializes this correctly by setting about::blank

agree factor between nsWebShellWindow::Initialize and sWebBrowser::Create() functions is a better approach but I don't have resources to do that right now so quick patch is the best contribution I can make right now. the rationale being if it's good enough for firefox, it's good enough for winEmbed.

request winEmbed receives minimal test coverage to keep it spinning. 

It likely broke when the OMTC, etc was activated - ClientLayerManager is part of that. so ff36 or thereabouts.

don't usually run winEmbed directly, but got another bug to look at so wanted to go back to basics & thus found this.

Comment 6

[Steve Williams]

ok, so I hear you on the performance overhead & gave it some more thought.

the crash occurs through nsWindow::OnPaint

given the async composite doesn't present through here anyhow, this is both a fix and a small optimization ... I *think*. minimal testing confirms it's an alternative solution, but that's minimal testing.

[mozilla]\widget\windows\nsWindowGfx.cpp

in : bool nsWindow::OnPaint(HDC aDC, uint32_t aNestingLevel)


      case LayersBackend::LAYERS_CLIENT:
        // This crashes winEmbed & present doesn't occur through here anyway, so commented out.
        //result = listener->PaintWindow(this, region);
        break;

Comment 7

[Steve Williams]

stack trace : http://advance-software.com/misc/winembed_crash.png

Comment 8

[Andreas Fagschlunger]

Based on winEmbed I did a custom KIOSK-Browser, we're using it successfully on hundreds of ATMs. But on some machines the Browser crashes with a STACK_OVERFLOW_EXCEPTION. Before I file a new bug I wanted to upgrade to the latest version - but I run in the same issue as mentioned here.

PRIMARY_PROBLEM_CLASS:  NULL_POINTER_READ

BUGCHECK_STR:  APPLICATION_FAULT_NULL_POINTER_READ

LAST_CONTROL_TRANSFER:  from 000000000256e8dd to 000000000256f921

STACK_TEXT:  
0019eadc 0256e8dd 00000001 06181390 01f6ef50 xul!mozilla::layers::ClientLayerManager::ForwardTransaction+0x2d [c:\builds\moz2_slave\rel-m-rel-xr_w32_bld-000000000\build\gfx\layers\client\clientlayermanager.cpp @ 557]
0019eb0c 030cbf2a 030c8d56 07c15280 00000000 xul!mozilla::layers::ClientLayerManager::EndTransaction+0x42 [c:\builds\moz2_slave\rel-m-rel-xr_w32_bld-000000000\build\gfx\layers\client\clientlayermanager.cpp @ 320]
0019eb48 02d9d857 07c29800 00000000 00000000 xul!nsWebBrowser::PaintWindow+0x9a [c:\builds\moz2_slave\rel-m-rel-xr_w32_bld-000000000\build\embedding\browser\nswebbrowser.cpp @ 1634]
0019ecbc 02d9f4f2 00000000 00000000 0000000f xul!nsWindow::OnPaint+0x301 [c:\builds\moz2_slave\rel-m-rel-xr_w32_bld-000000000\build\widget\windows\nswindowgfx.cpp @ 530]
0019edbc 760974bb 00520d22 0000000f 00000000 xul!nsWindow::ProcessMessage+0x418 [c:\builds\moz2_slave\rel-m-rel-xr_w32_bld-000000000\build\widget\windows\nswindow.cpp @ 4781]
07c29800 00000000 00000000 00000000 00000000 USER32!RealDefWindowProcWorker+0x73


FAULTING_SOURCE_CODE:  
   553:   }
   554: 
   555:   mPhase = PHASE_FORWARD;
   556: 
>  557:   mLatestTransactionId = mTransactionIdAllocator->GetTransactionId();
   558:   TimeStamp transactionStart;
   559:   if (!mTransactionIdAllocator->GetTransactionStart().IsNull()) {
   560:     transactionStart = mTransactionIdAllocator->GetTransactionStart();
   561:   } else {
   562:     transactionStart = mTransactionStart;


SYMBOL_STACK_INDEX:  0

SYMBOL_NAME:  xul!mozilla::layers::ClientLayerManager::ForwardTransaction+2d

FOLLOWUP_NAME:  MachineOwner

MODULE_NAME: xul

IMAGE_NAME:  xul.dll

DEBUG_FLR_IMAGE_TIMESTAMP:  5553fbe2

Comment 9

[Matt Woodrow (:mattwoodrow)]

(In reply to Steve Williams from comment #6)

> 
>       case LayersBackend::LAYERS_CLIENT:
>         // This crashes winEmbed & present doesn't occur through here
> anyway, so commented out.
>         //result = listener->PaintWindow(this, region);
>         break;

Commenting this out isn't correct, it leads through to [1], which is important.

I'm not sure how the embedded rendering is meant to work, but nsWebBrowser::PaintWindow seems very broken.

It looks like it's just attempting to draw a solid colour and nothing else, is that expected?

Drawing into a layer manager always happens from nsRefreshDriver these days, PaintWindow is just where we present the content (non-OMTC), or flush the compositor thread (OMTC). Attempting to create new layers from here is unlikely to ever work well.

What content are we supposed to draw here?


[1] http://mxr.mozilla.org/mozilla-central/source/view/nsViewManager.cpp#334

Comment 10

[Benjamin Smedberg]

Mass change of bugs in the "Embedding: GRE Core" component in preparation for archiving it. I believe that these are no longer relevant; but if they are, they should be reopened and moved into a component relevant to the code in question.