1243760 - Replace nsPIDOMWindow with nsPIDOMWindowInner/Outer in C-C due to bug 1241764

Reporter

Description

•

8 years ago

Let's get prepared for when bug 1241764 lands.

There are about 40 files in C-C which reference nsIDOMWindow and nsPIDOMWindow.

Jorg K (CEST = GMT+2)

Reporter

Updated

•

8 years ago

Depends on: 1241764

Jorg K (CEST = GMT+2)

Reporter

Comment 1

•

8 years ago

References to <nsIDOMWindow> (24):
im/components/mintrayr/trayToolkit.cpp
im/components/mintrayr/trayToolkit.h
mail/components/migration/src/nsProfileMigrator.cpp
mailnews/addrbook/src/nsAbContentHandler.cpp
mailnews/addrbook/src/nsAbLDAPListenerBase.cpp
mailnews/addrbook/src/nsAbManager.cpp
mailnews/base/src/nsMessenger.h
mailnews/base/src/nsMessengerBootstrap.cpp
mailnews/base/src/nsMessengerContentHandler.cpp
mailnews/base/src/nsMessengerOSXIntegration.mm
mailnews/base/src/nsMessengerUnixIntegration.cpp
mailnews/base/src/nsMessengerWinIntegration.cpp
mailnews/base/src/nsMsgMailSession.cpp
mailnews/base/src/nsMsgPrintEngine.cpp
mailnews/base/src/nsMsgPrintEngine.h
mailnews/base/src/nsMsgProgress.cpp
mailnews/compose/src/nsMsgCompose.cpp
mailnews/compose/src/nsMsgComposeContentHandler.cpp
mailnews/compose/src/nsMsgComposeService.cpp
mailnews/compose/src/nsMsgComposeService.h
mailnews/local/src/nsPop3Sink.cpp
mailnews/mapi/mapihook/src/msgMapiHook.cpp
mailnews/news/src/nsNNTPNewsgroupList.cpp
mailnews/news/src/nsNntpService.cpp

References to <nsPIDOMWindow> (15):
im/components/mintrayr/trayToolkit.cpp
mailnews/addrbook/src/nsAbContentHandler.cpp
mailnews/base/src/nsMessenger.cpp
mailnews/base/src/nsMessengerOSXIntegration.mm
mailnews/base/src/nsMessengerUnixIntegration.cpp
mailnews/base/src/nsMessengerWinIntegration.cpp
mailnews/base/src/nsMsgMailSession.cpp
mailnews/base/src/nsMsgPrintEngine.h
mailnews/base/src/nsMsgProgress.cpp
mailnews/base/src/nsMsgWindow.cpp
mailnews/compose/src/nsMsgCompose.cpp
mailnews/compose/src/nsMsgComposeContentHandler.cpp
mailnews/compose/src/nsMsgComposeService.cpp
mailnews/compose/src/nsMsgSend.h
mailnews/news/src/nsNNTPNewsgroupList.cpp

Merged: 27 files:
im/components/mintrayr/trayToolkit.cpp
im/components/mintrayr/trayToolkit.h
mail/components/migration/src/nsProfileMigrator.cpp
mailnews/addrbook/src/nsAbContentHandler.cpp
mailnews/addrbook/src/nsAbLDAPListenerBase.cpp
mailnews/addrbook/src/nsAbManager.cpp
mailnews/base/src/nsMessenger.cpp
mailnews/base/src/nsMessenger.h
mailnews/base/src/nsMessengerBootstrap.cpp
mailnews/base/src/nsMessengerContentHandler.cpp
mailnews/base/src/nsMessengerOSXIntegration.mm
mailnews/base/src/nsMessengerUnixIntegration.cpp
mailnews/base/src/nsMessengerWinIntegration.cpp
mailnews/base/src/nsMsgMailSession.cpp
mailnews/base/src/nsMsgPrintEngine.cpp
mailnews/base/src/nsMsgPrintEngine.h
mailnews/base/src/nsMsgProgress.cpp
mailnews/base/src/nsMsgWindow.cpp
mailnews/compose/src/nsMsgCompose.cpp
mailnews/compose/src/nsMsgComposeContentHandler.cpp
mailnews/compose/src/nsMsgComposeService.cpp
mailnews/compose/src/nsMsgComposeService.h
mailnews/compose/src/nsMsgSend.h
mailnews/local/src/nsPop3Sink.cpp
mailnews/mapi/mapihook/src/msgMapiHook.cpp
mailnews/news/src/nsNNTPNewsgroupList.cpp
mailnews/news/src/nsNntpService.cpp

Any takers, perhaps for a part? I could perhaps do a third, from compose downwards.

Jorg K (CEST = GMT+2)

Reporter

Comment 2

•

8 years ago

Looks like it was too optimistic only looking for <nsIDOMWindow> and <nsPIDOMWindow> where pointers to this type are declared. There are also, for example:
static void activateWindow( nsIDOMWindow *win )
And there is JS:
let win = aSubject.QueryInterface(Components.interfaces.nsIDOMWindow);
and IDL files:
interface nsIDOMWindow;
and header files:
#include "nsIDOMWindow.h"

Not sure about the JS since on dev-platform there was this discussion:
Q:
Does this mean that window objects will no longer implement
nsIDOMWindow (at least as far as JS is concerned)? Querying for
nsIDOMWindow is something add-ons do a lot and I'd expect to see a lot
of add-ons break if we changed that.
A:
They will continue to implement nsIDOMWindow just like they do
nsIDOMWindowInternal.  We're simply not going to use it for anything.

Frank-Rainer Grahl (:frg)

Comment 3

•

8 years ago

Seamonkey is also affected: 21 occurences of nsIDOMWindow:

 suite\browser\navigator.js
 suite\browser\nsBrowserContentHandler.js
 suite\browser\nsTypeAheadFind.js
 suite\browser\test\mochitest\feed_discovery.html
 suite\browser\test\mochitest\test_contextmenu.html
 suite\common\aboutSessionRestore.js
 suite\common\bindings\notification.xml
 suite\common\nsContextMenu.js
 suite\common\public\nsISessionStore.idl
 suite\common\public\nsISuiteDownloadManagerUI.idl
 suite\common\public\nsISuiteGlue.idl
 suite\common\src\nsSessionStore.js
 suite\common\src\nsSuiteDownloadManagerUI.js
 suite\common\tests\browser\browser_490040.js
 suite\common\tests\browser\browser_514751.js
 suite\common\tests\browser\browser_586068-cascaded_restore.js
 suite\common\tests\browser\browser_615394-SSWindowState_events.js
 suite\common\tests\browser\head.js
 suite\modules\WindowsPreviewPerTab.jsm
 suite\smile\src\smileApplication.js
 suite\smile\test\browser_Application.js

Stefan Sitter [:ssitter]

Comment 4

•

8 years ago

Bug 1241764 landed on mozilla-central yesterday. I'll paste some of the reported build errors here to make this bug show up when searching for them:

[...]mailnews/local/src/nsPop3Sink.cpp:706:33: error: no matching function for call to 'nsIPromptService::ConfirmEx(nsCOMPtr<nsIDOMWindow>&, std::nullptr_t, const char_type*, nsIPromptService::<anonymous enum>, std::nullptr_t, std::nullptr_t, std::nullptr_t, std::nullptr_t, bool*, int32_t*)'

[...]mailnews/news/src/nsNNTPNewsgroupList.cpp:230:12: error: use of class template 'nsPIDOMWindow' requires template arguments

[...]mailnews/addrbook/src/nsAbContentHandler.cpp:71:22: error: use of class template 'nsPIDOMWindow' requires template arguments

[...]mailnews/local/src/nsPop3Sink.cpp:699:35: error: no viable conversion from 'nsCOMPtr<nsIDOMWindow>' to 'mozIDOMWindowProxy *'

[...]/mailnews/news/src/nsNNTPNewsgroupList.cpp(230) : error C3203: 'nsPIDOMWindow' : unspecialized class template can't be used as a template argument for template parameter 'T', expected a real type

[...]/mailnews/addrbook/src/nsAbContentHandler.cpp(71) : error C3203: 'nsPIDOMWindow' : unspecialized class template can't be used as a template argument for template parameter 'T', expected a real type

[...]/mailnews/local/src/nsPop3Sink.cpp(706) : error C2664: 'nsresult nsIPromptService::ConfirmEx(mozIDOMWindowProxy *,const char16_t *,const char16_t *,uint32_t,const char16_t *,const char16_t *,const char16_t *,const char16_t *,bool *,int32_t *)' : cannot convert argument 1 from 'nsCOMPtr<nsIDOMWindow>' to 'mozIDOMWindowProxy *'

Frank-Rainer Grahl (:frg)

Comment 5

•

8 years ago

Explanation for dummies like me:

https://developer.mozilla.org/en-US/docs/Inner_and_outer_windows

Tricky part just seems to be figure out if inner or outer should be used.

FRG

Jorg K (CEST = GMT+2)

Reporter

Comment 6

•

8 years ago

Do we have a plan to fix this yet? Someone with a fast Linux box could make better progress than someone with a slow Windows box (myself).

On IRC we discussed that we need to:
- change nsIDOMWindow to nsIDOMWindowInner/Outer or mozIDOMWindowProxy depending on the case.
- change nsPIDOMWindow to nsPIDOMWindowInner/Outer depending on the case.

Here's some info from http://logs.glob.uno/?c=developers:
18:05 jcranmer khuey: do you have an easy guideline for telling when a nsPIDOMWindow needs to be an inner or an outer window?
18:05 jcranmer for those of us who aren't well-versed in core DOM internals code
18:11 khuey jcranmer: well, in general if you are doing DOM object type things you want the inner, and if you want something that survives across navigations, etc you want the outer
18:11 khuey long term I think all of outer window should move into docshell
18:11 khuey so it's helpful to think of it like that
18:12 khuey if you get called from script today you're getting the inner window
18:12 khuey because XPConnect innerizes everything except the newly added mozIDOMWindowProxy
18:12 khuey if you just want to call GetDocShell on it (which is what a lot of mailnews/ looks like it wants) it doesn't really matter
18:13 jcranmer khuey: thanks
18:14 khuey jcranmer: if you have questions about specific bits of c-c code I can take a look at them

And from #maildev:
[2016-01-30 18:48:52] <jcranmer> I think most of them should be outer dom windows
[2016-01-30 18:49:10] <aleth> and if something is = ...->GetInnerWindow() it will be obvious too
[2016-01-30 18:49:30] <jcranmer> although compose may be an innerWindow

Philip Chee

Assignee

Comment 7

•

8 years ago

Attached patch WIP.1 patch in progress (obsolete) — Details — Splinter Review

Philip Chee

Assignee

Updated

•

8 years ago

User Story: (updated)

Jorg K (CEST = GMT+2)

Reporter

Comment 8

•

8 years ago

Philip/Ratty told me on IRC that he will continue to work on the bug - great, thanks!
His process is to start the build and then fix compile errors as they occur.
This doesn't lend itself well to dividing the work. So I suggest the following to avoid that two people work on it concurrently:
Whoever wants to continue leaves a comment and we assume that this person is working on it until they attach the next patch. Is this OK?

So right now, Philip/Ratty is working on it.
(I'm happy to boot my Linux box and do a bit after Philip/Ratty is done.)

Philip Chee

Assignee

Comment 9

•

8 years ago

Attached patch WIP.2 patch in progress mostly done (obsolete) — Details — Splinter Review

Attachment #8714123 - Attachment is obsolete: true

Jorg K (CEST = GMT+2)

Reporter

Comment 10

•

8 years ago

Great. Sorry, 23:15 here now, so no time tonight and tomorrow I'm mostly busy. So please let me know if you're finishing it off. You might as well claim the whole fame ;-)

Philip Chee

Assignee

Comment 11

•

8 years ago

Attached patch Patch v1 compiles out of the box. (obsolete) — Details — Splinter Review

Complete patch except for:
Skipped:
========
im/components/mintrayr/trayToolkit.cpp
im/components/mintrayr/trayToolkit.h
mail/components/migration/src/nsProfileMigrator.cpp

Not tested (no compile environment):
====================================
mailnews/base/src/nsMessengerOSXIntegration.mm
mailnews/base/src/nsMessengerUnixIntegration.cpp

TODO:
=====
Submit to try.

Attachment #8714491 - Attachment is obsolete: true

Flags: needinfo?(mozilla)

Attachment #8714518 - Flags: feedback?(mozilla)

aleth [:aleth]

Comment 12

•

8 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b7541fb70c6a

Jorg K (CEST = GMT+2)

Reporter

Comment 13

•

8 years ago

Attached patch More stuff. (obsolete) — Details — Splinter Review

I noticed the omission in nsIMsgSend.idl and while I was compiling the previous version of your patch, I've added changes to nsProfileMigrator.cpp.

Compilation hasn't finished yet.

Flags: needinfo?(mozilla)

Jorg K (CEST = GMT+2)

Reporter

Comment 14

•

8 years ago

Comment on attachment 8714518 [details] [diff] [review]
Patch v1 compiles out of the box.

Good. You might want to merge the two lines from nsProfileMigrator.cpp.
Still compiling. Did you try running it?

Attachment #8714518 - Flags: feedback?(mozilla) → feedback+

aleth [:aleth]

Comment 15

•

8 years ago

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=43e630db8a31

Jorg K (CEST = GMT+2)

Reporter

Comment 16

•

8 years ago

Attached patch Patch v1a - very minor tweaks to Philip's excellent work. (obsolete) — Details — Splinter Review

Added nsProfileMigrator.cpp and corrected nsMessengerOSXIntegration.mm (I hope). Will push to try if I can work out how to do it from Linux.

What about stuff like in JS?
let win = aSubject.QueryInterface(Components.interfaces.nsIDOMWindow);

Attachment #8714518 - Attachment is obsolete: true

Attachment #8714523 - Attachment is obsolete: true

Jorg K (CEST = GMT+2)

Reporter

Comment 17

•

8 years ago

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b5b6988e7039

Jorg K (CEST = GMT+2)

Reporter

Comment 18

•

8 years ago

Compiles but doesn't run :-( - I've tried Linux and Windows.

No idea why the Mac compile fails. Oh, I see that it's been busted for a while. Anyway, the
  mailnews/base/src/nsMessengerOSXIntegration.mm:127:30: error: no viable conversion from
  'nsGetterAddRefs<nsPIDOMWindowOuter>' to 'mozIDOMWindowProxy **'
is now gone.

Jorg K (CEST = GMT+2)

Reporter

Updated

•

8 years ago

Attachment #8714668 - Attachment description: Patch v1a - minor tweaks. → Patch v1a - very minor tweaks to Philip's excellent work.

Philip Chee

Assignee

Comment 19

•

8 years ago

> -  nsCOMPtr<nsIDOMWindow> migrateWizard;
> +  nsCOMPtr<mozIDOMWindowProxy> migrateWizard;
>    return ww->OpenWindow(nullptr,

> -  nsCOMPtr<nsIDOMWindow> migrateWizard;
> +  nsCOMPtr<mozIDOMWindowProxy> migrateWizard;
>    return ww->OpenWindow(nullptr,

According to the mozilla-central patch these should be nsPIDOMWindowOuter
(I may have made the same mistake in my patch)

Jorg K (CEST = GMT+2)

Reporter

Comment 20

•

8 years ago

I think the compiler was asking for mozIDOMWindowProxy (I don't remember now).
Anyway, back over to you. Perhaps you can get it to start ;-)

ISHIKAWA, Chiaki

Comment 21

•

8 years ago

Probably, someone has a good idea of why the patch upto comment 15 (including the change mentioned in comment 15) does not produce a working C-C TB.

Just for your information, this is how the TB binary crashed under gdb under linux64 bit.
This is a debug build, but I used -O2 and so gdb can't grok arguments well.

++DOMWINDOW == 10 (0x2a0d0f0) [pid = 10545] [serial = 10] [outer = (nil)]
++DOCSHELL 0x1e8b0c0 == 8 [pid = 10545] [id = 8]
++DOMWINDOW == 11 (0x1e8b7b0) [pid = 10545] [serial = 11] [outer = (nil)]
++DOCSHELL 0x1e8c570 == 9 [pid = 10545] [id = 9]
++DOMWINDOW == 12 (0x1e8cbf0) [pid = 10545] [serial = 12] [outer = (nil)]
++DOCSHELL 0x1e8d980 == 10 [pid = 10545] [id = 10]
++DOMWINDOW == 13 (0x1e8e0c0) [pid = 10545] [serial = 13] [outer = (nil)]
++DOCSHELL 0x2a4bfd0 == 11 [pid = 10545] [id = 11]
++DOMWINDOW == 14 (0x2a4ca10) [pid = 10545] [serial = 14] [outer = (nil)]
Chrome file doesn't exist: /NREF-COMM-CENTRAL/objdir-tb3/dist/bin/chrome/toolkit/skin/classic/global/icons/information-64.png
++DOMWINDOW == 15 (0x396ba90) [pid = 10545] [serial = 15] [outer = 0x1e8e0c0]
++DOMWINDOW == 16 (0x39e1430) [pid = 10545] [serial = 16] [outer = 0x2a0d0f0]
[New Thread 0x7fff937fe700 (LWP 10598)]
[10545] WARNING: Failed to open external DTD: publicId "-//W3C//DTD SVG 1.1//EN" systemId "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" base "file:///NREF-COMM-CENTRAL/comm-central/mail/themes/linux/mail/tabs/selected-start.svg" URL "resource://gre/res/dtd/svg11.dtd": file /NREF-COMM-CENTRAL/comm-central/mozilla/parser/htmlparser/nsExpatDriver.cpp, line 710
[10545] WARNING: Failed to open external DTD: publicId "-//W3C//DTD SVG 1.1//EN" systemId "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" base "file:///NREF-COMM-CENTRAL/comm-central/mail/themes/linux/mail/tabs/selected-end.svg" URL "resource://gre/res/dtd/svg11.dtd": file /NREF-COMM-CENTRAL/comm-central/mozilla/parser/htmlparser/nsExpatDriver.cpp, line 710

Program received signal SIGSEGV, Segmentation fault.
nsPIDOMWindow<mozIDOMWindowProxy>::GetFocusedNode (this=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsPIDOMWindowInlines.h:128
128	    return mInnerWindow->GetFocusedNode();
(gdb) where
#0  nsPIDOMWindow<mozIDOMWindowProxy>::GetFocusedNode (this=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsPIDOMWindowInlines.h:128
#1  nsFocusManager::GetFocusedDescendant (aWindow=<optimized out>, aDeep=<optimized out>, aFocusedWindow=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsFocusManager.cpp:292
#2  0x00007fffea7fd7d0 in nsDocShell::EnsureFind (this=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/docshell/base/nsDocShell.cpp:12970
#3  0x00007fffea810f18 in nsDocShell::GetInterface (this=<optimized out>, aIID=..., aSink=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/docshell/base/nsDocShell.cpp:1034
#4  0x00007fffe77e771e in nsGetInterface::operator() (this=<optimized out>, aIID=..., aInstancePtr=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/glue/nsIInterfaceRequestorUtils.cpp:18
#5  0x00007fffeabb3e1d in nsCOMPtr<nsIWebBrowserFind>::assign_from_helper (aIID=..., helper=..., this=<optimized out>)
    at /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsCOMPtr.h:1144
#6  nsCOMPtr<nsIWebBrowserFind>::operator= (aRhs=..., this=<optimized out>) at /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsCOMPtr.h:653
#7  nsTypeAheadFind::SetDocShell (this=<optimized out>, aDocShell=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp:175
#8  0x00007fffeabb27c0 in nsTypeAheadFind::Init (this=<optimized out>, aDocShell=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp:107
#9  0x00007fffe77c6533 in NS_InvokeByIndex (that=<optimized out>, methodIndex=<optimized out>, paramCount=<optimized out>, params=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:176
#10 0x00007fffe824e0a8 in CallMethodHelper::Invoke (this=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNative.cpp:2099
#11 CallMethodHelper::Call (this=<optimized out>) at /NREF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNative.cpp:1416
#12 XPCWrappedNative::CallMethod (ccx=..., mode=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNative.cpp:1383
#13 0x00007fffe8252c4a in XPC_WN_CallMethod (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1115
#14 0x00007fffeba23d22 in js::CallJSNative (cx=<optimized out>, native=<optimized out>, args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/jscntxtinlines.h:235
---Type <return> to continue, or q <return> to quit--- 
#15 0x00007fffeba1daf1 in js::Invoke (cx=<optimized out>, args=..., construct=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:475
#16 0x00007fffeba0fa98 in Interpret (cx=0x1ee34a0, state=...) at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:2799
#17 0x00007fffeba1d84f in js::RunScript (cx=<optimized out>, state=...) at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:425
#18 0x00007fffeba1ddb5 in js::Invoke (cx=<optimized out>, args=..., construct=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:493
#19 0x00007fffeba1e054 in InternalConstruct (cx=<optimized out>, args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:554
#20 0x00007fffeba1e16f in js::Construct (cx=<optimized out>, fval=..., args=..., newTarget=..., objp=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:601
#21 0x00007fffeb918547 in js::DirectProxyHandler::construct (this=<optimized out>, cx=<optimized out>, proxy=..., args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/proxy/DirectProxyHandler.cpp:96
#22 0x00007fffeb918923 in js::CrossCompartmentWrapper::construct (this=<optimized out>, cx=<optimized out>, wrapper=..., args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/proxy/CrossCompartmentWrapper.cpp:309
#23 0x00007fffeb913fd5 in js::Proxy::construct (cx=<optimized out>, proxy=..., args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/proxy/Proxy.cpp:410
#24 0x00007fffeb914094 in js::proxy_Construct (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/proxy/Proxy.cpp:692
#25 0x00007fffeba23d22 in js::CallJSNative (cx=<optimized out>, native=<optimized out>, args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/jscntxtinlines.h:235
#26 0x00007fffeba270e3 in js::CallJSNativeConstructor (cx=<optimized out>, native=<optimized out>, args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/jscntxtinlines.h:268
#27 0x00007fffeba1df94 in InternalConstruct (cx=<optimized out>, args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:564
#28 0x00007fffeba17ee0 in ConstructFromStack (args=..., cx=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:591
#29 Interpret (cx=0x1ee34a0, state=...) at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:2791
#30 0x00007fffeba1d84f in js::RunScript (cx=<optimized out>, state=...) at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:425
---Type <return> to continue, or q <return> to quit---
#31 0x00007fffeba1dc16 in js::Invoke (cx=<optimized out>, args=..., construct=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:493
#32 0x00007fffeba1e5b9 in js::Invoke (cx=<optimized out>, thisv=..., fval=..., argc=<optimized out>, argv=<optimized out>, rval=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:527
#33 0x00007fffeba1e8a4 in js::InvokeGetter (cx=<optimized out>, thisv=..., fval=..., rval=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:637
#34 0x00007fffeba1eac6 in CallGetter (vp=..., shape=..., receiver=..., obj=..., cx=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.cpp:1672
#35 GetExistingProperty<(js::AllowGC)1> (cx=<optimized out>, receiver=..., obj=..., shape=..., vp=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.cpp:1720
#36 0x00007fffeba1f32a in NativeGetPropertyInline<(js::AllowGC)1> (cx=<optimized out>, obj=..., receiver=..., id=..., nameLookup=<optimized out>, 
    vp=...) at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.cpp:1939
#37 0x00007fffeb593280 in js::GetProperty (cx=<optimized out>, obj=..., receiver=..., id=..., vp=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.h:1471
#38 0x00007fffeba2056e in js::GetProperty (vp=..., name=<optimized out>, receiver=..., obj=..., cx=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/jsobj.h:822
#39 js::GetProperty (cx=<optimized out>, v=..., name=..., vp=...) at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:4068
#40 0x00007fffeba0feb6 in GetPropertyOperation (vp=..., lval=..., pc=<optimized out>, script=..., fp=<optimized out>, cx=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:219
#41 Interpret (cx=0x1ee34a0, state=...) at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:2518
#42 0x00007fffeba1d84f in js::RunScript (cx=<optimized out>, state=...) at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:425
#43 0x00007fffeba1dc16 in js::Invoke (cx=<optimized out>, args=..., construct=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:493
#44 0x00007fffeba1e5b9 in js::Invoke (cx=<optimized out>, thisv=..., fval=..., argc=<optimized out>, argv=<optimized out>, rval=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:527
#45 0x00007fffeba20c1b in js::InvokeSetter (cx=<optimized out>, thisv=..., fval=..., v=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:646
#46 0x00007fffeba20f9f in SetExistingProperty (result=..., shape=..., pobj=..., receiver=..., v=..., id=..., obj=..., cx=<optimized out>)
---Type <return> to continue, or q <return> to quit--- 
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.cpp:2289
#47 js::NativeSetProperty (cx=<optimized out>, obj=..., id=..., value=..., receiver=..., qualified=<optimized out>, result=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.cpp:2323
#48 0x00007fffeb5932eb in js::SetProperty (cx=<optimized out>, obj=..., id=..., v=..., receiver=..., result=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.h:1488
#49 0x00007fffeba1033a in SetPropertyOperation (rval=..., id=..., lval=..., op=<optimized out>, cx=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:286
#50 Interpret (cx=0x1ee34a0, state=...) at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:2594
#51 0x00007fffeba1d84f in js::RunScript (cx=<optimized out>, state=...) at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:425
#52 0x00007fffeba1dc16 in js::Invoke (cx=<optimized out>, args=..., construct=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:493
#53 0x00007fffeba1e5b9 in js::Invoke (cx=<optimized out>, thisv=..., fval=..., argc=<optimized out>, argv=<optimized out>, rval=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:527
#54 0x00007fffeb81ee66 in JS::Call (cx=<optimized out>, thisv=..., fval=..., args=..., rval=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/jsapi.cpp:2883
#55 0x00007fffe93594e9 in (anonymous namespace)::dom::Function::Call (this=<optimized out>, cx=<optimized out>, aThisVal=..., arguments=..., 
    aRetVal=..., aRv=...) at /NREF-COMM-CENTRAL/objdir-tb3/dom/bindings/FunctionBinding.cpp:36
#56 0x00007fffe88790fb in (anonymous namespace)::dom::Function::Call<nsCOMPtr<nsISupports> > (aCompartment=<optimized out>, 
    aExceptionHandling=<optimized out>, aExecutionReason=<optimized out>, aRv=..., aRetVal=..., arguments=..., thisVal=..., this=<optimized out>)
    at /NREF-COMM-CENTRAL/objdir-tb3/dist/include/mozilla/dom/FunctionBinding.h:58
#57 nsGlobalWindow::RunTimeoutHandler (this=<optimized out>, aTimeout=<optimized out>, aScx=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsGlobalWindow.cpp:11860
#58 0x00007fffe8879d2e in nsGlobalWindow::RunTimeout (this=<optimized out>, aTimeout=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsGlobalWindow.cpp:12095
#59 0x00007fffe8879fba in nsGlobalWindow::TimerCallback (aTimer=<optimized out>, aClosure=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsGlobalWindow.cpp:12341
#60 0x00007fffe77c0d02 in nsTimerImpl::Fire (this=<optimized out>) at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/nsTimerImpl.cpp:526
#61 0x00007fffe77b6417 in nsTimerEvent::Run (this=<optimized out>) at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/TimerThread.cpp:286
---Type <return> to continue, or q <return> to quit---   
#62 0x00007fffe77b542a in nsThread::ProcessNextEvent (this=<optimized out>, aMayWait=<optimized out>, aResult=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/nsThread.cpp:995
#63 0x00007fffe77ef25d in NS_ProcessNextEvent (aThread=<optimized out>, aMayWait=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/glue/nsThreadUtils.cpp:297
#64 0x00007fffe7bfb2c3 in (anonymous namespace)::ipc::MessagePump::Run (this=<optimized out>, aDelegate=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/ipc/glue/MessagePump.cpp:95
#65 0x00007fffe7b90b6c in MessageLoop::RunInternal (this=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/ipc/chromium/src/base/message_loop.cc:234
#66 0x00007fffe7b90be8 in MessageLoop::RunHandler (this=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/ipc/chromium/src/base/message_loop.cc:227
#67 MessageLoop::Run (this=<optimized out>) at /NREF-COMM-CENTRAL/comm-central/mozilla/ipc/chromium/src/base/message_loop.cc:201
#68 0x00007fffe9f60f79 in nsBaseAppShell::Run (this=<optimized out>) at /NREF-COMM-CENTRAL/comm-central/mozilla/widget/nsBaseAppShell.cpp:156
#69 0x00007fffeab943b3 in nsAppStartup::Run (this=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/toolkit/components/startup/nsAppStartup.cpp:281
#70 0x00007fffeac18ba2 in XREMain::XRE_mainRun (this=<optimized out>) at /NREF-COMM-CENTRAL/comm-central/mozilla/toolkit/xre/nsAppRunner.cpp:4279
#71 0x00007fffeac1a217 in XREMain::XRE_main (this=<optimized out>, argc=<optimized out>, argv=<optimized out>, aAppData=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/toolkit/xre/nsAppRunner.cpp:4376
#72 0x00007fffeac1a573 in XRE_main (argc=<optimized out>, argv=<optimized out>, aAppData=<optimized out>, aFlags=<optimized out>)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/toolkit/xre/nsAppRunner.cpp:4478
#73 0x0000000000404ae9 in do_main (argc=argc@entry=1, argv=argv@entry=0x7fffffffd9d8, xreDirectory=0x465dd0)
    at /NREF-COMM-CENTRAL/comm-central/mail/app/nsMailApp.cpp:195
#74 0x0000000000404331 in main (argc=1, argv=0x7fffffffd9d8) at /NREF-COMM-CENTRAL/comm-central/mail/app/nsMailApp.cpp:332
(gdb) 

I will try a new set of patches based on comment 16-20.
I will report the result.
(I need to get C-C tryserver produce a working binary to test some other patches on platforms I don't test locally (namely I need windows binary to test on tryserver. OSX build has been busted for almost a month now. )

TIA

Jorg K (CEST = GMT+2)

Reporter

Comment 22

•

8 years ago

The patches pushed in comment #15 and comment #17 are almost identical, only some changes in the Mac code.

I see a similar crash on Linux. I'm compiling on Windows now to debug it. (I'm not familiar with gdb.)

Philip/Ratty told me on IRC that he compiled SM and he can run it. So this can't be far off.

ISHIKAWA, Chiaki

Comment 23

•

8 years ago

(In reply to Jorg K (GMT+1) from comment #22)
> The patches pushed in comment #15 and comment #17 are almost identical, only
> some changes in the Mac code.
> 
> I see a similar crash on Linux. I'm compiling on Windows now to debug it.
> (I'm not familiar with gdb.)
> 
> Philip/Ratty told me on IRC that he compiled SM and he can run it. So this
> can't be far off.

This sounds very encouraging.

(In reply to Philip Chee from comment #19)
> > -  nsCOMPtr<nsIDOMWindow> migrateWizard;
> > +  nsCOMPtr<mozIDOMWindowProxy> migrateWizard;
> >    return ww->OpenWindow(nullptr,
> 
> > -  nsCOMPtr<nsIDOMWindow> migrateWizard;
> > +  nsCOMPtr<mozIDOMWindowProxy> migrateWizard;
> >    return ww->OpenWindow(nullptr,
> 
> According to the mozilla-central patch these should be nsPIDOMWindowOuter
> (I may have made the same mistake in my patch)

(In reply to Jorg K (GMT+1) from comment #20)
> I think the compiler was asking for mozIDOMWindowProxy (I don't remember
> now).
> Anyway, back over to you. Perhaps you can get it to start ;-)

(In reply to Philip Chee from comment #19)
> > -  nsCOMPtr<nsIDOMWindow> migrateWizard;
> > +  nsCOMPtr<mozIDOMWindowProxy> migrateWizard;
> >    return ww->OpenWindow(nullptr,
> 
> > -  nsCOMPtr<nsIDOMWindow> migrateWizard;
> > +  nsCOMPtr<mozIDOMWindowProxy> migrateWizard;
> >    return ww->OpenWindow(nullptr,
> 
> According to the mozilla-central patch these should be nsPIDOMWindowOuter
> (I may have made the same mistake in my patch)

(In reply to Jorg K (GMT+1) from comment #20)
> I think the compiler was asking for mozIDOMWindowProxy (I don't remember
> now).
> Anyway, back over to you. Perhaps you can get it to start ;-)

If I set the type of migrateWizard to nsPIDOMWindowOuter, I get the following compiler error.


nsProfileMigrator.o
/NREF-COMM-CENTRAL/comm-central/mail/components/migration/src/nsProfileMigrator.cpp: In member function ‘virtual nsresult nsProfileMigrator::Migrate(nsIProfileStartup*, const nsACString_internal&)’:
/NREF-COMM-CENTRAL/comm-central/mail/components/migration/src/nsProfileMigrator.cpp:63:54: error: conversion from ‘nsGetterAddRefs<nsPIDOMWindowOuter>’ to ‘mozIDOMWindowProxy**’ is ambiguous
                         getter_AddRefs(migrateWizard));
                                                      ^
In file included from /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsComponentManagerUtils.h:11:0,
                 from /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsIServiceManager.h:138,
                 from /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsDirectoryServiceUtils.h:10,
                 from /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsIFile.h:920,
                 from /NREF-COMM-CENTRAL/comm-central/mail/components/migration/src/nsProfileMigrator.cpp:6:
/NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsCOMPtr.h:1222:3: note: candidate: nsGetterAddRefs<T>::operator T**() [with T = nsPIDOMWindowOuter] <near match>
   operator T**() { return mTargetSmartPtr.StartAssignment(); }
   ^
/NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsCOMPtr.h:1222:3: note:   no known conversion from ‘nsPIDOMWindowOuter**’ to ‘mozIDOMWindowProxy**’
/NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsCOMPtr.h:1217:3: note: candidate: nsGetterAddRefs<T>::operator void**() [with T = nsPIDOMWindowOuter] <near match>
   operator void**()
   ^
/NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsCOMPtr.h:1217:3: note:   no known conversion from ‘void**’ to ‘mozIDOMWindowProxy**’
In file included from /NREF-COMM-CENTRAL/comm-central/mail/components/migration/src/nsProfileMigrator.cpp:13:0:
/NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsIWindowWatcher.h:45:20: note:   initializing argument 6 of ‘virtual nsresult nsIWindowWatcher::OpenWindow(mozIDOMWindowProxy*, const char*, const char*, const char*, nsISupports*, mozIDOMWindowProxy**)’
   NS_IMETHOD OpenWindow(mozIDOMWindowProxy *aParent, const char * aUrl, const char * aName, const char * aFeatures, nsISupports *aArguments, mozIDOMWindowProxy * *_retval) = 0;
                    ^

In the directory  /NREF-COMM-CENTRAL/objdir-tb3/mail/components/migration/src
The following command failed to execute properly:
/usr/bin/ccache /usr/bin/g++-5 -fno-builtin-strlen -Wl,--gdb-index -Dfdatasync=fdatasync -DDEBUG_4GB_CHECK -DUSEHELGRIND=1 -o nsProfileMigrator.o -c -I/NREF-COMM-CENTRAL/objdir-tb3/dist/stl_wrappers -I/NREF-COMM-CENTRAL/objdir-tb3/dist/system_wrappers -include /NREF-COMM-CENTRAL/comm-central/mozilla/config/gcc_hidden.h -DDEBUG=1 -DTRACING=1 -DSTATIC_EXPORTABLE_JS_API -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/NREF-COMM-CENTRAL/comm-central/mail/components/migration/src -I/NREF-COMM-CENTRAL/objdir-tb3/mail/components/migration/src -I/NREF-COMM-CENTRAL/objdir-tb3/dist/include -I/NREF-COMM-CENTRAL/objdir-tb3/dist/include/nspr -I/NREF-COMM-CENTRAL/objdir-tb3/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /NREF-COMM-CENTRAL/objdir-tb3/mozilla-config.h -MD -MP -MF .deps/nsProfileMigrator.o.pp -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wtype-limits -Wunreachable-code -Wcast-align -Wno-invalid-offsetof -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -pipe -gsplit-dwarf -g -O2 -fno-omit-frame-pointer -freorder-blocks -fno-omit-frame-pointer /NREF-COMM-CENTRAL/comm-central/mail/components/migration/src/nsProfileMigrator.cpp
/NREF-COMM-CENTRAL/comm-central/mozilla/config/rules.mk:967: recipe for target 'nsProfileMigrator.o' failed
make[4]: *** [nsProfileMigrator.o] Error 1

Trying with JorgK's patch under linux64 with gcc5.3

Jorg K (CEST = GMT+2)

Reporter

Comment 24

•

8 years ago

I've been trying to find the crash when starting TB, so far without success.

Here is a more compact call stack:
xul.dll!nsPIDOMWindow<mozIDOMWindow>::GetFocusedNode() Line 128	C++
xul.dll!nsFocusManager::GetFocusedDescendant(nsPIDOMWindowOuter * aWindow, bool aDeep, nsPIDOMWindowOuter * * aFocusedWindow) Line 292	C++
xul.dll!nsDocShell::EnsureFind() Line 12970	C++
xul.dll!nsDocShell::GetInterface(const nsID & aIID, void * * aSink) Line 1035	C++
xul.dll!nsGetInterface::operator()(const nsID & aIID, void * * aInstancePtr) Line 18	C++
xul.dll!nsCOMPtr<nsIWebBrowserFind>::assign_from_helper(const nsCOMPtr_helper & helper, const nsID & aIID) Line 1144	C++
xul.dll!nsTypeAheadFind::SetDocShell(nsIDocShell * aDocShell) Line 175	C++
xul.dll!nsTypeAheadFind::Init(nsIDocShell * aDocShell) Line 109	C++
xul.dll!_NS_InvokeByIndex() Line 57	Unknown

So it crashes in the new function nsPIDOMWindow<mozIDOMWindow>::GetFocusedNode().

The call comes from JS with this stack:
0 Finder(docShell = [xpconnect wrapped nsIDocShell @ 0x9236490 (native @ 0xcf40804)]) ["resource://gre/modules/Finder.jsm":32]
    this = [object Object]
1 get_finder() ["chrome://chat/content/convbrowser.xml":115]
    this = [object XULElement]
2 set_browser(val = [object XULElement]) ["chrome://global/content/bindings/findbar.xml":279]
    this = [object XULElement]
3 findbar_XBL_Constructor/<(aSelf = [object XULElement]) ["chrome://global/content/bindings/findbar.xml":380]
    this = [object ChromeWindow]

I'm really surprised that SM starts and TB doesn't. Should the difference be in the findbar stuff?

Another question. In the patch I find many places where we convert from mozIDOMWindowProxy to nsPIDOMWindowOuter like so:
nsCOMPtr<nsPIDOMWindowOuter> win = do_QueryInterface(aWin); // where mozIDOMWindowProxy *aWin.
Would it me better like this:
nsCOMPtr<nsPIDOMWindowOuter> win = nsPIDOMWindowOuter::From(aWin);

BTW, I've looked through the patch trying to spot obvious errors, but I didn't see any.

Perhaps it's time to ask for Kyle's help :-(

Kyle, could you browse the patch and see whether you can see anything wrong. Perhaps you have an idea why we would crash.

Ah, I added a print before "if (IsOuterWindow()) {", so that moved to line 128.

The cause of the crash it reported as:
Unhandled exception at 0x5D98D0D5 (xul.dll) in thunderbird.exe: 0xC0000005: Access violation reading location 0x00000056.

Flags: needinfo?(khuey)

Kyle Huey (Exited; not receiving bugmail, old account, do not use)

Comment 25

•

8 years ago

When I wrote my patch I actually removed a null check here that didn't seem necessary in Gecko.  But apparently I was wrong.

Compare

http://hg.mozilla.org/mozilla-central/rev/e22b3043887e#l127.421
and
http://hg.mozilla.org/mozilla-central/rev/e22b3043887e#l128.129

I will r+ a patch to restore the null check on mInnerWindow in Gecko.

Flags: needinfo?(khuey)

Jorg K (CEST = GMT+2)

Reporter

Comment 26

•

8 years ago

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #25)
> I will r+ a patch to restore the null check on mInnerWindow in Gecko.
In which bug, so we can track it or apply the patch for testing locally?

Also, would you mind answering my other question:
In the TB patch I find many places where we convert from mozIDOMWindowProxy to nsPIDOMWindowOuter like so:
nsCOMPtr<nsPIDOMWindowOuter> win = do_QueryInterface(aWin); // where mozIDOMWindowProxy *aWin.
Would it be better like this?
nsCOMPtr<nsPIDOMWindowOuter> win = nsPIDOMWindowOuter::From(aWin);

Many thanks.

Flags: needinfo?(khuey)

Kyle Huey (Exited; not receiving bugmail, old account, do not use)

Comment 27

•

8 years ago

This bug, another bug, whatever.  Note I'm offering to r+ the patch you write.  If I write it I will have to find someone else to review it and it will take longer ;)

To answer your other question, it is better to do the nsPIDOMWindowOuter::From version.  But note that you *must* do any null checking before, not after.  Code like

nsFoo::Bar(nsIDOMWindow* aWindow) {
  nsCOMPtr<nsPIDOMWindow> window = do_QueryInterface(aWindow);
  NS_ENSURE_TRUE(window, NS_ERROR_FAILURE);
}

window can be null either because the do_QueryInterface failed or because aWindow was null.  That needs to change to

nsFoo::Bar(mozIDOMWindow* aWindow) {
  NS_ENSURE_TRUE(aWindow, NS_ERROR_FAILURE);
  nsCOMPtr<nsPIDOMWindowInner> window = nsPIDOMWindowInner::From(aWindow);
}

so that you do the null checking before casting.

Flags: needinfo?(khuey)

Jorg K (CEST = GMT+2)

Reporter

Comment 28

•

8 years ago

OK, now I got it ;-)
It's midnight here now. I am just compiling with

template <class T>
nsIContent*
nsPIDOMWindow<T>::GetFocusedNode() const
{
  printf ("============= nsPIDOMWindow<T>::GetFocusedNode() - this |%p|\n", this);
  if (IsOuterWindow()) {
    return mInnerWindow ? mInnerWindow->GetFocusedNode() : nullptr;
  }

  return mFocusedNode;
}

and see how I go. I'll provide the patch in another bug. You'll notice ;-)
Thanks for the explanation re. do_QueryInterface vs. nsPIDOMWindowOuter::From version.

Philip/Ratty: Would you like to revise your patch to do so or shall I do it?
I've counted seven occurrences which should use the "From".

Jorg K (CEST = GMT+2)

Reporter

Comment 29

•

8 years ago

(In reply to Jorg K (GMT+1) from comment #28)
> Philip/Ratty: Would you like to revise your patch to do so or shall I do it?
> I've counted seven occurrences which should use the "From".
Actually, there are more. You'd have to inspect all the lines that match nsPIDOMWindowOuter.*do_Query in the patch of which there are 20.

Jorg K (CEST = GMT+2)

Reporter

Updated

•

8 years ago

Depends on: 1245310

ISHIKAWA, Chiaki

Comment 30

•

8 years ago

Lacking null check as Kyle suggested seems to be culprit, indeed.

This is the gdb backtrace and listings from a few list commands under
linux64. (I recompiled C-C TB with -O0 so that I can print variable
values under gdb.) 
This is with the patch in comment 16.

Starting thunderbird in gdb 
         ...[omission]...
         
[New Thread 0x7fff63fff700 (LWP 14698)]
[Thread 0x7fff837ae700 (LWP 14693) exited]
[Thread 0x7fff5166a700 (LWP 14689) exited]
[14615] WARNING: Failed to open external DTD: publicId "-//W3C//DTD SVG 1.1//EN" systemId "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" base "file:///NREF-COMM-CENTRAL/comm-central/mail/themes/linux/mail/tabs/selected-start.svg" URL "resource://gre/res/dtd/svg11.dtd": file /NREF-COMM-CENTRAL/comm-central/mozilla/parser/htmlparser/nsExpatDriver.cpp, line 710
[14615] WARNING: Failed to open external DTD: publicId "-//W3C//DTD SVG 1.1//EN" systemId "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" base "file:///NREF-COMM-CENTRAL/comm-central/mail/themes/linux/mail/tabs/selected-end.svg" URL "resource://gre/res/dtd/svg11.dtd": file /NREF-COMM-CENTRAL/comm-central/mozilla/parser/htmlparser/nsExpatDriver.cpp, line 710
 16.
Program received signal SIGSEGV, Segmentation fault.
0x00007fffe454a5dc in nsPIDOMWindow<mozIDOMWindow>::IsInnerWindow (this=0x0)
    at /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsPIDOMWindow.h:260
260	    return mIsInnerWindow;
(gdb) print mIsInnerWindow
Cannot access memory at address 0x8e
(gdb) where
#0  0x00007fffe454a5dc in nsPIDOMWindow<mozIDOMWindow>::IsInnerWindow (
    this=0x0)
    at /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsPIDOMWindow.h:260
#1  0x00007fffe454b3e8 in nsPIDOMWindow<mozIDOMWindow>::IsOuterWindow (
    this=0x0)
    at /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsPIDOMWindow.h:265
#2  0x00007fffe60561dc in nsPIDOMWindow<mozIDOMWindow>::GetFocusedNode (
    this=0x0)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsPIDOMWindowInlines.h:127
#3  0x00007fffe605571d in nsPIDOMWindow<mozIDOMWindowProxy>::GetFocusedNode (
    this=0x285ca00)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsPIDOMWindowInlines.h:128
#4  0x00007fffe61f28d2 in nsFocusManager::GetFocusedDescendant (
    aWindow=0x285ca00, aDeep=true, aFocusedWindow=0x7fffffff2720)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsFocusManager.cpp:292
#5  0x00007fffe88b01cd in nsDocShell::EnsureFind (this=0x285bfa0)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/docshell/base/nsDocShell.cpp:12970
#6  0x00007fffe8881839 in nsDocShell::GetInterface (this=0x285bfa0, aIID=..., 
    aSink=0x7fffffff29d8)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/docshell/base/nsDocShell.cpp:1034
#7  0x00007fffe4accf96 in nsGetInterface::operator() (this=0x7fffffff2a40, 
    aIID=..., aInstancePtr=0x7fffffff29d8)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/glue/nsIInterfaceRequestorUtils.cpp:18
#8  0x00007fffe6064f8d in nsCOMPtr<nsIWebBrowserFind>::assign_from_helper (
    this=0x18b9100, helper=..., aIID=...)
    at /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsCOMPtr.h:1144
#9  0x00007fffe88be33c in nsCOMPtr<nsIWebBrowserFind>::operator= (
    this=0x18b9100, aRhs=...)
    at /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsCOMPtr.h:653
#10 0x00007fffe8d34ec1 in nsTypeAheadFind::SetDocShell (this=0x18b9040, 
    aDocShell=0x285c148)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp:175
#11 0x00007fffe8d34a94 in nsTypeAheadFind::Init (this=0x18b9040, 
    aDocShell=0x285c148)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp:107
#12 0x00007fffe4a9844c in NS_InvokeByIndex (that=0x18b9040, methodIndex=3, 
    paramCount=1, params=0x7fffffff2cf8)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:176
#13 0x00007fffe584b2cf in CallMethodHelper::Invoke (this=0x7fffffff2cb0)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNative.cpp:2099
#14 0x00007fffe5848ee1 in CallMethodHelper::Call (this=0x7fffffff2cb0)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNative.cpp:1416
#15 0x00007fffe582e422 in XPCWrappedNative::CallMethod (ccx=..., 
    mode=XPCWrappedNative::CALL_METHOD)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNative.cpp:1383
#16 0x00007fffe5836e14 in XPC_WN_CallMethod (cx=0x202f440, argc=1, 
    vp=0x9ae020)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1115
#17 0x00007fffea36827a in js::CallJSNative (cx=0x202f440, 
    native=0x7fffe5836b1e <XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*)>, args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/jscntxtinlines.h:235
#18 0x00007fffea33414c in js::Invoke (cx=0x202f440, args=..., 
    construct=js::NO_CONSTRUCT)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:475
#19 0x00007fffea34369d in Interpret (cx=0x202f440, state=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:2799
#20 0x00007fffea333dce in js::RunScript (cx=0x202f440, state=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:425
#21 0x00007fffea33421b in js::Invoke (cx=0x202f440, args=..., 
    construct=js::CONSTRUCT)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:493
#22 0x00007fffea334760 in InternalConstruct (cx=0x202f440, args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:554
#23 0x00007fffea334a31 in js::Construct (cx=0x202f440, fval=..., args=..., 
    newTarget=..., objp=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:601
#24 0x00007fffea1ff6a3 in js::DirectProxyHandler::construct (
    this=0x7fffee441600 <js::CrossCompartmentWrapper::singleton>, 
    cx=0x202f440, proxy=..., args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/proxy/DirectProxyHandler.cpp:96
#25 0x00007fffea1fd13c in js::CrossCompartmentWrapper::construct (
    this=0x7fffee441600 <js::CrossCompartmentWrapper::singleton>, 
    cx=0x202f440, wrapper=..., args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/proxy/CrossCompartmentWrapper.cpp:309
#26 0x00007fffea202b64 in js::Proxy::construct (cx=0x202f440, proxy=..., 
    args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/proxy/Proxy.cpp:410
#27 0x00007fffea203da4 in js::proxy_Construct (cx=0x202f440, argc=1, 
    vp=0x9adf90)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/proxy/Proxy.cpp:692
#28 0x00007fffea36827a in js::CallJSNative (cx=0x202f440, 
    native=0x7fffea203cd6 <js::proxy_Construct(JSContext*, unsigned int, JS::Value*)>, args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/jscntxtinlines.h:235
#29 0x00007fffea3683b4 in js::CallJSNativeConstructor (cx=0x202f440, 
    native=0x7fffea203cd6 <js::proxy_Construct(JSContext*, unsigned int, JS::Value*)>, args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/jscntxtinlines.h:268
#30 0x00007fffea334834 in InternalConstruct (cx=0x202f440, args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:564
#31 0x00007fffea334997 in ConstructFromStack (cx=0x202f440, args=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:591
#32 0x00007fffea343565 in Interpret (cx=0x202f440, state=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:2791
#33 0x00007fffea333dce in js::RunScript (cx=0x202f440, state=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:425
#34 0x00007fffea33421b in js::Invoke (cx=0x202f440, args=..., 
    construct=js::NO_CONSTRUCT)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:493
#35 0x00007fffea3344da in js::Invoke (cx=0x202f440, thisv=..., fval=..., 
    argc=0, argv=0x0, rval=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:527
#36 0x00007fffea334c74 in js::InvokeGetter (cx=0x202f440, thisv=..., 
    fval=..., rval=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:637
#37 0x00007fffea35beb2 in CallGetter (cx=0x202f440, obj=..., receiver=..., 
    shape=..., vp=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.cpp:1672
#38 0x00007fffea3613fc in GetExistingProperty<(js::AllowGC)1> (cx=0x202f440, 
    receiver=..., obj=..., shape=..., vp=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.cpp:1720
#39 0x00007fffea361942 in NativeGetPropertyInline<(js::AllowGC)1> (
    cx=0x202f440, obj=..., receiver=..., id=..., nameLookup=NotNameLookup, 
    vp=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.cpp:1939
#40 0x00007fffea35c716 in js::NativeGetProperty (cx=0x202f440, obj=..., 
    receiver=..., id=..., vp=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.cpp:1973
#41 0x00007fffe9c96816 in js::GetProperty (cx=0x202f440, obj=..., 
    receiver=..., id=..., vp=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.h:1471
#42 0x00007fffe9c9517a in js::GetProperty (cx=0x202f440, obj=..., 
    receiver=..., name=0x7fff82d086e8, vp=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/jsobj.h:822
#43 0x00007fffea34f30d in js::GetProperty (cx=0x202f440, v=..., name=..., 
    vp=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:4068
#44 0x00007fffea332934 in GetPropertyOperation (cx=0x202f440, fp=0x9ade78, 
    script=..., pc=0x36875c6 "5", lval=..., vp=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:219
#45 0x00007fffea340f59 in Interpret (cx=0x202f440, state=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:2518
#46 0x00007fffea333dce in js::RunScript (cx=0x202f440, state=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:425
#47 0x00007fffea33421b in js::Invoke (cx=0x202f440, args=..., 
    construct=js::NO_CONSTRUCT)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:493
#48 0x00007fffea3344da in js::Invoke (cx=0x202f440, thisv=..., fval=..., 
    argc=1, argv=0x7fffffff9240, rval=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:527
#49 0x00007fffea334d27 in js::InvokeSetter (cx=0x202f440, thisv=..., 
    fval=..., v=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:646
#50 0x00007fffea35dd7f in SetExistingProperty (cx=0x202f440, obj=..., id=..., 
    v=..., receiver=..., pobj=..., shape=..., result=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.cpp:2289
#51 0x00007fffea35e058 in js::NativeSetProperty (cx=0x202f440, obj=..., 
    id=..., value=..., receiver=..., qualified=js::Qualified, result=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.cpp:2323
#52 0x00007fffe9c968bf in js::SetProperty (cx=0x202f440, obj=..., id=..., 
    v=..., receiver=..., result=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/NativeObject.h:1488
#53 0x00007fffea3332d0 in SetPropertyOperation (cx=0x202f440, 
    op=JSOP_SETPROP, lval=..., id=..., rval=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:286
#54 0x00007fffea341aa4 in Interpret (cx=0x202f440, state=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:2594
#55 0x00007fffea333dce in js::RunScript (cx=0x202f440, state=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:425
#56 0x00007fffea33421b in js::Invoke (cx=0x202f440, args=..., 
    construct=js::NO_CONSTRUCT)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:493
#57 0x00007fffea3344da in js::Invoke (cx=0x202f440, thisv=..., fval=..., 
    argc=1, argv=0x7fffffffb1d0, rval=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:527
#58 0x00007fffea0be960 in JS::Call (cx=0x202f440, thisv=..., fval=..., 
    args=..., rval=...)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/js/src/jsapi.cpp:2883
#59 0x00007fffe6ed0e6e in (anonymous namespace)::dom::Function::Call (
    this=0x39c8650, cx=0x202f440, aThisVal=..., arguments=..., aRetVal=..., 
    aRv=...)
    at /NREF-COMM-CENTRAL/objdir-tb3/dom/bindings/FunctionBinding.cpp:36
#60 0x00007fffe6060032 in (anonymous namespace)::dom::Function::Call<nsCOMPtr<nsISupports> > (this=0x39c8650, thisVal=..., arguments=..., aRetVal=..., 
    aRv=..., aExecutionReason=0x7fffeae10962 "setTimeout handler", 
    aExceptionHandling=(anonymous namespace)::dom::CallbackObject::eReportExceptions, aCompartment=0x0)
    at /NREF-COMM-CENTRAL/objdir-tb3/dist/include/mozilla/dom/FunctionBinding.h:58
#61 0x00007fffe60487b0 in nsGlobalWindow::RunTimeoutHandler (this=0x2030e30, 
    aTimeout=0x39c8970, aScx=0x202f3f0)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsGlobalWindow.cpp:11860
#62 0x00007fffe60490be in nsGlobalWindow::RunTimeout (this=0x2030e30, 
    aTimeout=0x33041e0)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsGlobalWindow.cpp:12095
#63 0x00007fffe6049baa in nsGlobalWindow::TimerCallback (aTimer=0x2e50e90, 
    aClosure=0x33041e0)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsGlobalWindow.cpp:12341
#64 0x00007fffe4a8f898 in nsTimerImpl::Fire (this=0x2e50e90)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/nsTimerImpl.cpp:526
#65 0x00007fffe4a6b815 in nsTimerEvent::Run (this=0x7fff94001860)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/TimerThread.cpp:286
#66 0x00007fffe4a70cc3 in nsThread::ProcessNextEvent (this=0x50a0e0, 
    aMayWait=true, aResult=0x7fffffffbc3f)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/nsThread.cpp:995
#67 0x00007fffe4ad3630 in NS_ProcessNextEvent (aThread=0x50a0e0, 
    aMayWait=true)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/glue/nsThreadUtils.cpp:297
#68 0x00007fffe4a7064f in nsThread::Shutdown (this=0x2d7b480)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/nsThread.cpp:805
#69 0x00007fffe4a8d818 in nsRunnableMethodArguments<>::apply<nsIThread, nsresult (nsIThread::*)()>(nsIThread *, nsresult (nsIThread::*)(nsIThread * const)) (
    this=0x7fff8c008a30, o=0x2d7b480, m=&virtual nsIThread::Shutdown())
    at /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsThreadUtils.h:663
#70 0x00007fffe4a8ce5f in nsRunnableMethodImpl<nsresult (nsIThread::*)(), true>::Run (this=0x7fff8c008a00)
    at /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsThreadUtils.h:870
#71 0x00007fffe4a70cc3 in nsThread::ProcessNextEvent (this=0x50a0e0, 
    aMayWait=true, aResult=0x7fffffffbe2f)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/nsThread.cpp:995
#72 0x00007fffe4ad3630 in NS_ProcessNextEvent (aThread=0x50a0e0, 
    aMayWait=true)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/glue/nsThreadUtils.cpp:297
#73 0x00007fffe4a7064f in nsThread::Shutdown (this=0x344aba0)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/nsThread.cpp:805
#74 0x00007fffe4a8d818 in nsRunnableMethodArguments<>::apply<nsIThread, nsresult (nsIThread::*)()>(nsIThread *, nsresult (nsIThread::*)(nsIThread * const)) (
    this=0x7fff88009970, o=0x344aba0, m=&virtual nsIThread::Shutdown())
    at /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsThreadUtils.h:663
#75 0x00007fffe4a8ce5f in nsRunnableMethodImpl<nsresult (nsIThread::*)(), true>::Run (this=0x7fff88009940)
    at /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsThreadUtils.h:870
#76 0x00007fffe4a70cc3 in nsThread::ProcessNextEvent (this=0x50a0e0, 
    aMayWait=false, aResult=0x7fffffffc01f)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/nsThread.cpp:995
#77 0x00007fffe4ad3630 in NS_ProcessNextEvent (aThread=0x50a0e0, 
    aMayWait=false)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/glue/nsThreadUtils.cpp:297
#78 0x00007fffe5000e2f in (anonymous namespace)::ipc::MessagePump::Run (
    this=0x50a090, aDelegate=0x509830)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/ipc/glue/MessagePump.cpp:95
#79 0x00007fffe4f53e0d in MessageLoop::RunInternal (this=0x509830)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/ipc/chromium/src/base/message_loop.cc:234
#80 0x00007fffe4f53da0 in MessageLoop::RunHandler (this=0x509830)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/ipc/chromium/src/base/message_loop.cc:227
#81 0x00007fffe4f53d2f in MessageLoop::Run (this=0x509830)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/ipc/chromium/src/base/message_loop.cc:201
#82 0x00007fffe7e0effa in nsBaseAppShell::Run (this=0xbab620)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/widget/nsBaseAppShell.cpp:156
#83 0x00007fffe8d1035b in nsAppStartup::Run (this=0xae4ec0)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/toolkit/components/startup/nsAppStartup.cpp:281
#84 0x00007fffe8daf854 in XREMain::XRE_mainRun (this=0x7fffffffc500)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/toolkit/xre/nsAppRunner.cpp:4279
#85 0x00007fffe8dafbca in XREMain::XRE_main (this=0x7fffffffc500, argc=1, 
    argv=0x7fffffffd9a8, aAppData=0x7fffffffc6c0)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/toolkit/xre/nsAppRunner.cpp:4376
#86 0x00007fffe8dafe50 in XRE_main (argc=1, argv=0x7fffffffd9a8, 
    aAppData=0x7fffffffc6c0, aFlags=0)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/toolkit/xre/nsAppRunner.cpp:4478
#87 0x0000000000404bc5 in do_main (argc=1, argv=0x7fffffffd9a8, xreDirectory=
    0x477dd0) at /NREF-COMM-CENTRAL/comm-central/mail/app/nsMailApp.cpp:195
#88 0x0000000000404f2e in main (argc=1, argv=0x7fffffffd9a8)
    at /NREF-COMM-CENTRAL/comm-central/mail/app/nsMailApp.cpp:332
(gdb)  list

(gdb)  list
255	    return mIsInnerWindow ? mOuterWindow.get() : AsOuter();
256	  }
257	
258	  bool IsInnerWindow() const
259	  {
260	    return mIsInnerWindow;
261	  }
262	
263	  bool IsOuterWindow() const
264	  {
(gdb) up
#1  0x00007fffe454b3e8 in nsPIDOMWindow<mozIDOMWindow>::IsOuterWindow (
    this=0x0)
    at /NREF-COMM-CENTRAL/objdir-tb3/dist/include/nsPIDOMWindow.h:265
265	    return !IsInnerWindow();
(gdb) list
260	    return mIsInnerWindow;
261	  }
262	
263	  bool IsOuterWindow() const
264	  {
265	    return !IsInnerWindow();
266	  }
267	
268	  // Outer windows only.
269	  virtual bool WouldReuseInnerWindow(nsIDocument* aNewDocument) = 0;
(gdb) up
#2  0x00007fffe60561dc in nsPIDOMWindow<mozIDOMWindow>::GetFocusedNode (
    this=0x0)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsPIDOMWindowInlines.h:127
127	  if (IsOuterWindow()) {
(gdb) list
122	
123	template <class T>
124	nsIContent*
125	nsPIDOMWindow<T>::GetFocusedNode() const
126	{
127	  if (IsOuterWindow()) {
128	    return mInnerWindow->GetFocusedNode();
129	  }
130	
131	  return mFocusedNode;
(gdb) print mInnerWindow
Cannot access memory at address 0xa0
(gdb) list
132	}
(gdb) up
#3  0x00007fffe605571d in nsPIDOMWindow<mozIDOMWindowProxy>::GetFocusedNode (
    this=0x285ca00)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsPIDOMWindowInlines.h:128
128	    return mInnerWindow->GetFocusedNode();
(gdb) print mInnerWindow
$1 = (nsPIDOMWindowInner *) 0x0
(gdb) list
123	template <class T>
124	nsIContent*
125	nsPIDOMWindow<T>::GetFocusedNode() const
126	{
127	  if (IsOuterWindow()) {
128	    return mInnerWindow->GetFocusedNode();
129	  }
130	
131	  return mFocusedNode;
132	}
(gdb) up
#4  0x00007fffe61f28d2 in nsFocusManager::GetFocusedDescendant (
    aWindow=0x285ca00, aDeep=true, aFocusedWindow=0x7fffffff2720)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsFocusManager.cpp:292
292	    currentContent = window->GetFocusedNode();
(gdb) list
287	
288	  nsIContent* currentContent = nullptr;
289	  nsPIDOMWindowOuter* window = aWindow;
290	  while (window) {
291	    *aFocusedWindow = window;
292	    currentContent = window->GetFocusedNode();
293	    if (!currentContent || !aDeep)
294	      break;
295	
296	    window = GetContentWindow(currentContent);
(gdb) print window
$2 = (nsPIDOMWindowOuter *) 0x285ca00
(gdb) list
297	  }
298	
299	  NS_IF_ADDREF(*aFocusedWindow);
300	
301	  return currentContent;
302	}
303	
304	// static
305	nsIContent*
306	nsFocusManager::GetRedirectedFocus(nsIContent* aContent)
(gdb) up
#5  0x00007fffe88b01cd in nsDocShell::EnsureFind (this=0x285bfa0)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/docshell/base/nsDocShell.cpp:12970
12970	                                       getter_AddRefs(windowToSearch));
(gdb) list
12965	
12966	  // default to our window
12967	  nsCOMPtr<nsPIDOMWindowOuter> ourWindow = do_QueryInterface(scriptGO);
12968	  nsCOMPtr<nsPIDOMWindowOuter> windowToSearch;
12969	  nsFocusManager::GetFocusedDescendant(ourWindow, true,
12970	                                       getter_AddRefs(windowToSearch));
12971	
12972	  nsCOMPtr<nsIWebBrowserFindInFrames> findInFrames = do_QueryInterface(mFind);
12973	  if (!findInFrames) {
12974	    return NS_ERROR_NO_INTERFACE;
(gdb) down
#4  0x00007fffe61f28d2 in nsFocusManager::GetFocusedDescendant (
    aWindow=0x285ca00, aDeep=true, aFocusedWindow=0x7fffffff2720)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsFocusManager.cpp:292
292	    currentContent = window->GetFocusedNode();
(gdb) up
#5  0x00007fffe88b01cd in nsDocShell::EnsureFind (this=0x285bfa0)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/docshell/base/nsDocShell.cpp:12970
12970	                                       getter_AddRefs(windowToSearch));
(gdb) list
12965	
12966	  // default to our window
12967	  nsCOMPtr<nsPIDOMWindowOuter> ourWindow = do_QueryInterface(scriptGO);
12968	  nsCOMPtr<nsPIDOMWindowOuter> windowToSearch;
12969	  nsFocusManager::GetFocusedDescendant(ourWindow, true,
12970	                                       getter_AddRefs(windowToSearch));
12971	
12972	  nsCOMPtr<nsIWebBrowserFindInFrames> findInFrames = do_QueryInterface(mFind);
12973	  if (!findInFrames) {
12974	    return NS_ERROR_NO_INTERFACE;
(gdb) up
#6  0x00007fffe8881839 in nsDocShell::GetInterface (this=0x285bfa0, aIID=..., 
    aSink=0x7fffffff29d8)
    at /NREF-COMM-CENTRAL/comm-central/mozilla/docshell/base/nsDocShell.cpp:1034
1034	    nsresult rv = EnsureFind();
(gdb) list
1029	      shistory.forget(aSink);
1030	      return NS_OK;
1031	    }
1032	    return NS_NOINTERFACE;
1033	  } else if (aIID.Equals(NS_GET_IID(nsIWebBrowserFind))) {
1034	    nsresult rv = EnsureFind();
1035	    if (NS_FAILED(rv)) {
1036	      return rv;
1037	    }
1038	
(gdb)


Somewhere mInnerWindow is set to nullptr.
And afterwards, some other data are set the offset from 0x0.

Jorg K (CEST = GMT+2)

Reporter

Comment 31

•

8 years ago

(In reply to Jorg K (GMT+1) from comment #29)
> Actually, there are more. You'd have to inspect all the lines that match
> nsPIDOMWindowOuter.*do_Query in the patch of which there are 20.
Since I haven't heard, I'm revising the patch now. Update coming.

Jorg K (CEST = GMT+2)

Reporter

Comment 32

•

8 years ago

Attached patch Patch v2 - Further improvement on Philip's excellent work. (obsolete) — Details — Splinter Review

OK, I've changed all the uses of do_QueryInterface to nsPIDOMWindowInner/Outer::From.

With this patch and the patch from bug 1245310 TB starts. I've done a quick test of the following:
Folder tree, message pane, sent a message (shows progress window). All good.

When bug 1245310 lands on M-C I will send off a try run. It's not so much fun to do a try run with M-C changes, so I prefer to wait.

While testing, I spotted this on the debug console. This might be unrelated:
JavaScript error: chrome://messenger/content/messengercompose/sendProgress.xul, line 1: ReferenceError: onUnload is not defined
[1184] WARNING: '!cx', file c:/mozilla-source/comm-central/mozilla/dom/workers/WorkerRunnable.cpp, line 313

Chiaki: Thanks for confirming the fix, but please no more long stack dumps, please. It makes the bug hard to read.

Attachment #8714668 - Attachment is obsolete: true

Attachment #8715233 - Flags: review?(mkmelin+mozilla)

Attachment #8715233 - Flags: feedback?(philip.chee)

Jorg K (CEST = GMT+2)

Reporter

Comment 33

•

8 years ago

I'm assigning the bug to Philip/Ratty(*) since he had the "get up and go" to get started. I don't want to be seen as stealing anyone's work. Figuratively speaking, he got the recipe, did the shopping and baked the cake. I only added one cherry on top. So many thanks to Philip/Ratty for getting us out of this problem!

The Stainless Steel Rat Saves the World ;-)

*): However, I'm happy to see it through to completion.

Assignee: nobody → philip.chee

ISHIKAWA, Chiaki

Comment 34

•

8 years ago

(In reply to Jorg K (GMT+1) from comment #32)

> Chiaki: Thanks for confirming the fix, but please no more long stack dumps,
> please. It makes the bug hard to read.

Thank you for the effort to fix this serious breakage.

In the future, I will put the stack dump in an attachment.
(I thought the args printed by -O0 was useful, and at least it was useful for me to figure out
what was going on.)

Good to know that you could start TB with the patches.
I have a couple of PCs with different mozconfig (I disabled webrtc for TB, etc. on one PC.
On that PC, obviously something in M-C broke the unsupported mozconfig that disables certain features and
TB no longer compiles. Even after I put in the patch from bug 1241628) 

I will recheck your patch and run the local test with your patch and one in 
patch from bug 1245310 

> [1184] WARNING: '!cx', file c:/mozilla-source/comm-central/mozilla/dom/workers/WorkerRunnable.cpp, line 313

I see something about it near the termination of TB very often in mozmill test log,
and so should not be related to this bug at all.


This seems to be a symptom of ever-present sync issue of termination of various threads: some threads want to obtain services during shutdown when the services offered by other threads have already shut down.

I think the particular waning is from a Java interpreter or something that a context for the thread is no longer valid (i.e., it has become nullptr). 

The sync issue at the termination time is very visible if you look into the mozmill test of DEBUG build. I count  such serious looking errors/warnings from maybe 20-30 tests and maybe a couple triggers a run-time assertion (only enabled in DEBUG build) in the 1000+ mozmill tests. Someone ought to take a look at this issue seriously. ( I know the shortage of man-power.)

It is NOT COMFORTABLE AT ALL to see the warnings that .msf databases are not closed properly during shutdown due to such sync issues: yes, such warnings get printed a few times depending on the phase of the moon, etc. It is totally random.

I experienced crashes near the termination of TB several times in the last 3-4 years. These experience and the dire warnings/errors in mozmill log prompted me to decide to run TB always on without ever closing it down UNLESS ABSOLUTELY NECESSARY! (at the office, TB runs inside a virtual machine, so it is relatively easy to run TB application forever.) 

But I digress. 

Let us hope the current problem with the outer/inner, etc. will be over in a couple of days.

Jorg K (CEST = GMT+2)

Reporter

Comment 35

•

8 years ago

Bug 1245310 has landed. So here's the try:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a40cba69a1f1

Stefan Sitter [:ssitter]

Comment 36

•

8 years ago

Try build error could be new bustage from Bug 1235261, i.e. https://hg.mozilla.org/mozilla-central/rev/75dfe10ec44a Rename nsAutoTArray to AutoTArray

aleth [:aleth]

Comment 37

•

8 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=935eb9b49ef9

Jorg K (CEST = GMT+2)

Reporter

Comment 38

•

8 years ago

Aleth, thank for pushing again, but we're hit with yet more bustage:

c:/builds/moz2_slave/tb-try-c-cen-w32-d-00000000000/build/mailnews/extensions/smime/src/nsMsgComposeSecure.cpp(904) : error C2660: 'nsIX509CertDB::FindCertByDBKey' : function does not take 3 arguments
c:/builds/moz2_slave/tb-try-c-cen-w32-d-00000000000/build/mailnews/extensions/smime/src/nsMsgComposeSecure.cpp(926) : error C2660: 'nsIX509CertDB::FindCertByDBKey' : function does not take 3 arguments
c:/builds/moz2_slave/tb-try-c-cen-w32-d-00000000000/build/mailnews/extensions/smime/src/nsMsgComposeSecure.cpp(983) : error C2660: 'nsIX509CertDB::FindCertByEmailAddress' : function does not take 3 arguments

I'll look into it unless someone else wants to ;-)

Jorg K (CEST = GMT+2)

Reporter

Comment 39

•

8 years ago

New bustage fixed in bug 1245510.

New try:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=5d091b1d1c0c

User Story: (updated)

Jorg K (CEST = GMT+2)

Reporter

Comment 40

•

8 years ago

OK, bug 1245510 is a duplicate of bug 1243932, so let's put that patch instead:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=e4c20cff735e

Jorg K (CEST = GMT+2)

Reporter

Comment 41

•

8 years ago

Attached patch Patch v2a - Further improvement on Philip's excellent work. (obsolete) — Details — Splinter Review

Keep compilers on Mac and Linux happy.

New try, Mac and Linux only, Windows already compiled:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=76cd8090ea8a

Attachment #8715233 - Attachment is obsolete: true

Attachment #8715233 - Flags: review?(mkmelin+mozilla)

Attachment #8715233 - Flags: feedback?(philip.chee)

Attachment #8715349 - Flags: review?(mkmelin+mozilla)

Jorg K (CEST = GMT+2)

Reporter

Comment 42

•

8 years ago

Attached patch Patch v2b - Further improvement on Philip's excellent work. (obsolete) — Details — Splinter Review

Grrr, another compile error on Linux and Mac. New try:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=8f0521419d09

Attachment #8715349 - Attachment is obsolete: true

Attachment #8715349 - Flags: review?(mkmelin+mozilla)

Jorg K (CEST = GMT+2)

Reporter

Comment 43

•

8 years ago

Hmm. Lots of test failures looking at https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=e4c20cff735e.
Running TB manually, I noticed that saving a message (<ctrl>S> doesn't work. Console says:
JavaScript error: chrome://messenger/content/messengercompose/MsgComposeCommands.js, line 166: TypeError: gMsgCompose.editor is null

Formatting (<ctrl>i, <ctrl>b) works though. Sending also doesn't work. Damn, that worked before, so I made Ratty's patch worse, not better.

Jorg K (CEST = GMT+2)

Reporter

Comment 44

•

8 years ago

Ignore the last sentence. In comment #32 I state that my improved patch (v2) works at least for sending a message. So if it doesn't work now, there's something fishy.

aleth [:aleth]

Comment 45

•

8 years ago

Lots of mozmill failures on try, but looks like most of them are due to the same root cause.
MsgComposeCommands.js, line 2424: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIEditorStyleSheets.addOverrideStyleSheet]
MsgComposeCommands.js, line 166: TypeError: gMsgCompose.editor is null

Jorg K (CEST = GMT+2)

Reporter

Comment 46

•

8 years ago

Thanks for looking in the logs. MsgComposeCommands.js, line 2424 says:
2424        editorStyle.addOverrideStyleSheet("chrome://messenger/skin/messageQuotes.css");
2425        InitEditor();
So if the first call doesn't work and JS stops the execution of the function (does it not?), then the editor is not initialised which explains the rest of the symptoms.

There is another call to addOverrideStyleSheet at line 4933. With the two calls removed, TB works like it did this morning. Maybe we have more bustage ;-(

Jorg K (CEST = GMT+2)

Reporter

Comment 47

•

8 years ago

OK, while I'm looking into this problem, here a try run with the calls to addOverrideStyleSheet removed:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a6976ba47f74
Linux 64 opt only, because there usually all tests pass. That will give us an idea of where we're at.

Jorg K (CEST = GMT+2)

Reporter

Comment 48

•

8 years ago

OK, looks the sheet loading got busted by bug 1195173 comment #59.

The DOM Window stuff here is a success, but TB composition just got completely busted by the other bug.

aleth [:aleth]

Comment 49

•

8 years ago

(In reply to Jorg K (GMT+1) from comment #48)
> OK, looks the sheet loading got busted by bug 1195173 comment #59.
> 
> The DOM Window stuff here is a success, but TB composition just got
> completely busted by the other bug.

OK great, so this is ready for review? (probably jcranmer or khuey)

Jorg K (CEST = GMT+2)

Reporter

Comment 50

•

8 years ago

Joshua knows little about the windows stuff, he said so on IRC. Let's wait for the try run in comment #47. If this comes out green, we can review it.

Jorg K (CEST = GMT+2)

Reporter

Comment 51

•

8 years ago

I've just proved this by rolling M-C to before the patch that broke us:
hg up -r 5665bf204fe3

So that solves the riddle. This morning with v2 of the patch here all was well. v2b just fixes some compile errors on Linux and Mac.

Jorg K (CEST = GMT+2)

Reporter

Comment 52

•

8 years ago

The try run looks problematic:

Mozmill: One new failure:
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-image-insertion-dialog.js
I tried my local build and I can insert a picture via the menu without problem. So I'm happy to blame that on something else.

Xpcshell:
Lots of failures. I've spotted the problem.

New patch and try coming.

Jorg K (CEST = GMT+2)

Reporter

Comment 53

•

8 years ago

Attached patch Patch v2c - Further improvement on Philip's excellent work. — Details — Splinter Review

Fixed overzealous checking which destroyed all sending in xpcshell.

New try run:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=aa0b6876e89e

Attachment #8715374 - Attachment is obsolete: true

Jorg K (CEST = GMT+2)

Reporter

Comment 54

•

8 years ago

The try run looks better now:
Mozmill: One new failure: see comment #52.
I've run
mozmake SOLO_TEST=composition/test-image-insertion-dialog.js mozmill-one
locally and it works.

Xpcshell: Lots of failures, but all of them seem to have bug to pin them to. All failures are from
security/manager/ssl/tests/unit/
so they don't seem to have anything to do with the windows issue.

However, the line is worrying:
TEST-UNEXPECTED-FAIL | security/manager/ssl/tests/unit/test_toolkit_securityreporter.js | xpcshell return code: 0
TypeError: Cc['@mozilla.org/securityreporter;1'] is undefined at /builds/slave/test/build/tests/xpcshell/tests/security/manager/ssl/tests/unit/test_toolkit_securityreporter.js:28

Anyway, I ran
security/manager/ssl/tests/unit/test_toolkit_securityreporter.js and
security/manager/ssl/tests/unit/test_pinning.js
locally and they work.

So I'm shipping off another try to see whether we fare better a second time around:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=d436fba41682

Jorg K (CEST = GMT+2)

Reporter

Comment 55

•

8 years ago

Comment on attachment 8715579 [details] [diff] [review]
Patch v2c - Further improvement on Philip's excellent work.

I think this is ready for review. Sorry about the delay. But while working on this bustage, there were *four* more to fix concurrently:
Bug 1245310
Bug 1244758
Bug 1243932
Bug 1245681 (was bug 1195173).
Right now, we are still busted by bug 1245681.

Also, sorry about the r? SPAM. Who can review this?

Note: Ratty and I applied some "boy scout rules". So you will see fixed indentation and remove trailing white-space in some spots.

Attachment #8715579 - Flags: review?(rkent)

Attachment #8715579 - Flags: review?(mkmelin+mozilla)

Attachment #8715579 - Flags: review?(Pidgeot18)

Magnus Melin [:mkmelin]

Comment 56

•

8 years ago

re securityreporter - bug 1245613

Magnus Melin [:mkmelin]

Comment 57

•

8 years ago

Comment on attachment 8715579 [details] [diff] [review]
Patch v2c - Further improvement on Philip's excellent work.

Review of attachment 8715579 [details] [diff] [review]:
-----------------------------------------------------------------

rs=mkmelin since try seems ok with it. Thx for pulling this through!

Attachment #8715579 - Flags: review?(mkmelin+mozilla) → review+

Jorg K (CEST = GMT+2)

Reporter

Updated

•

8 years ago

Attachment #8715579 - Flags: review?(rkent)

Attachment #8715579 - Flags: review?(Pidgeot18)

Jorg K (CEST = GMT+2)

Reporter

Comment 58

•

8 years ago

https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=989a9aad4e24
I've pushed this so we can at least build again. Minor issues can be addressed later.

Status: NEW → RESOLVED

Closed: 8 years ago

Resolution: --- → FIXED

Target Milestone: --- → Thunderbird 47.0

Jorg K (CEST = GMT+2)

Reporter

Comment 59

•

8 years ago

(In reply to Magnus Melin from comment #56)
> re securityreporter - bug 1245613

I think you're telling me that in order to fix the test failures in security/manager/ssl/tests/unit/ we need to land bug 1245613. Let's do so!

Philip Chee

Assignee

Comment 60

•

8 years ago

Thanks for all the debugging! I couldn't figure out why my patch suddenly stopped working.

Frederic Bezies

Comment 61

•

8 years ago

Sorry to spam, but is this bug guilty for bug 1245897 ?

Jorg K (CEST = GMT+2)

Reporter

Comment 62

•

8 years ago

No.

Jorg K (CEST = GMT+2)

Reporter

Updated

•

8 years ago

Depends on: 1254596

Jorg K (CEST = GMT+2)

Reporter

Comment 63

•

8 years ago

Ratty's original patch (attachment 8714518 [details] [diff] [review]) contained only two uses of nsPIDOMWindowInner:
https://dxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgComposeContentHandler.cpp#51
https://dxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgComposeService.cpp#425

The first use I changed to "outer" before landing the bug since I was convinced that "outer" is right. I had looked at attachment 8713303 [details] [diff] [review] which is the patch that got landed on M-C for the DOM window stuff and in the majority of cases where window->GetDoc() is used, they use an outer window.

Yesterday I had to swap the other use in nsMsgComposeService.cpp to "outer" in bug 1254596.

So effectively, C-C now only uses "outer" everywhere.

What doesn't let me sleep in peace is
https://dxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgComposeContentHandler.cpp#51
where I changed Ratty's suggestion from "inner" to "outer". This is in the context handler for "application/x-mailto" and I don't have the faintest clue how to trigger that code to test it.

So can someone review just the one case.

Jorg K (CEST = GMT+2)

Reporter

Comment 64

•

8 years ago

Joshua told me that to trigger the code in questions one needs to click on a mailto: link.
I did that, debugger showed me that Joshua was right (I never doubted that) and it all worked.

Summary: All good, C-C now uses "outer" everywhere.

WIP.1 patch in progress 8 years ago Philip Chee 24.40 KB, patch		Details \| Diff \| Splinter Review
WIP.2 patch in progress mostly done 8 years ago Philip Chee 92.63 KB, patch		Details \| Diff \| Splinter Review
Patch v1 compiles out of the box. 8 years ago Philip Chee 95.29 KB, patch	jorgk-bmo : feedback+	Details \| Diff \| Splinter Review
More stuff. 8 years ago Jorg K (CEST = GMT+2) 2.41 KB, patch		Details \| Diff \| Splinter Review
Patch v1a - very minor tweaks to Philip's excellent work. 8 years ago Jorg K (CEST = GMT+2) 96.81 KB, patch		Details \| Diff \| Splinter Review
Patch v2 - Further improvement on Philip's excellent work. 8 years ago Jorg K (CEST = GMT+2) 98.56 KB, patch		Details \| Diff \| Splinter Review
Patch v2a - Further improvement on Philip's excellent work. 8 years ago Jorg K (CEST = GMT+2) 98.54 KB, patch		Details \| Diff \| Splinter Review
Patch v2b - Further improvement on Philip's excellent work. 8 years ago Jorg K (CEST = GMT+2) 98.62 KB, patch		Details \| Diff \| Splinter Review
Patch v2c - Further improvement on Philip's excellent work. 8 years ago Jorg K (CEST = GMT+2) 98.67 KB, patch	mkmelin : review+	Details \| Diff \| Splinter Review