Closed Bug 1470510 Opened 2 years ago Closed Last month

Merge nsXULWindow and nsWebShellWindow

Categories

(Core :: Window Management, enhancement, P5)

enhancement

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: bdahl, Assigned: bdahl)

References

Details

Attachments

(2 files, 1 obsolete file)

This looks like a reasonable cleanup for two reasons:
1) nsWebShellWindow is the only class that extends nsXULWindow
2) nsXULWindow is going to be more and more of a misnomer as nsXULWindows can now be tied to HTML windows and nothing in nsXULWindow is XUL specific anymore.
Priority: -- → P5

nsWebShellWindow is the only class that extends nsXULWindow and only
nsWebShellWindows are ever instantiated.

nsXULWindow is no longer XUL specific and is somewhat confusing name.

Depends on D51155

Pushed by bdahl@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/067a556bb614
Merge nsWebShellWindow into nsXULWindow r=smaug
https://hg.mozilla.org/integration/autoland/rev/5967bf633574
Rename nsXULWindow and nsIXULWindow to AppWindow and nsIAppWindow. r=smaug
Pushed by bdahl@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4da64790094d
Merge nsWebShellWindow into nsXULWindow r=smaug
https://hg.mozilla.org/integration/autoland/rev/a343f30c34a3
Rename nsXULWindow and nsIXULWindow to AppWindow and nsIAppWindow. r=smaug

Backed out 2 changesets for causing bustages in widget/cocoa/nsChildView.mm

Backout link: https://hg.mozilla.org/integration/autoland/rev/fe74a5208f32d653b7bf24b4173e0bba3210e738

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception&tochange=fe74a5208f32d653b7bf24b4173e0bba3210e738&fromchange=955256297d6dc83d52eb77a202c9c320622c0d13&searchStr=os%2Cx%2Cbuild&selectedJob=274555801

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=274555818&repo=autoland&lineNumber=30045

[task 2019-11-05T07:14:36.391Z] 07:14:36 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/widget/cocoa'
[task 2019-11-05T07:14:36.391Z] 07:14:36 INFO - widget/cocoa/nsChildView.o
[task 2019-11-05T07:14:36.391Z] 07:14:36 INFO - /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ -isysroot /builds/worker/workspace/build/src/MacOSX10.11.sdk --target=x86_64-apple-darwin -o nsChildView.o -c -fvisibility=hidden -fvisibility-inlines-hidden -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_MACOSX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/widget/cocoa -I/builds/worker/workspace/build/src/obj-firefox/widget/cocoa -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/dom/media/platforms/apple -I/builds/worker/workspace/build/src/layout/base -I/builds/worker/workspace/build/src/layout/forms -I/builds/worker/workspace/build/src/layout/generic -I/builds/worker/workspace/build/src/layout/style -I/builds/worker/workspace/build/src/layout/xul -I/builds/worker/workspace/build/src/widget -I/builds/worker/workspace/build/src/widget/headless -I/builds/worker/workspace/build/src/gfx/skia -I/builds/worker/workspace/build/src/gfx/skia/skia -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -fno-common -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-sized-deallocation -fno-aligned-new -fsanitize=address -fcrash-diagnostics-dir=/builds/worker/artifacts -fcrash-diagnostics-dir=/builds/worker/artifacts -U_FORTIFY_SOURCE -fno-common -fno-exceptions -fno-strict-aliasing -stdlib=libc++ -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O2 -fno-omit-frame-pointer -funwind-tables -Werror -MD -MP -MF .deps/nsChildView.o.pp -x objective-c++ -fobjc-exceptions /builds/worker/workspace/build/src/widget/cocoa/nsChildView.mm
[task 2019-11-05T07:14:36.391Z] 07:14:36 ERROR - /builds/worker/workspace/build/src/widget/cocoa/nsChildView.mm:619:3: error: use of undeclared identifier 'GetXULWindowWidget'; did you mean 'GetAppWindowWidget'?
[task 2019-11-05T07:14:36.391Z] 07:14:36 INFO - GetXULWindowWidget()->SuppressAnimation(aSuppress);

Apparently Thunderbird doesn't compile with your changes, see bug 1593469 comment #3:

EDIT: I compiled it myself and got:
33:29.13 In file included from c:/mozilla-source/comm-central/xpfe/appshell/AppWindow.cpp:23:
33:29.13 c:/mozilla-source/comm-central/dom/base\nsGlobalWindowOuter.h(465,32): error: member access into incomplete type 'mozilla::dom::BrowsingContext'
33:29.13 return GetBrowsingContext()->HadOriginalOpener();
33:29.13 ^
33:29.13 c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist/include\nsIWindowProvider.h(22,7): note: forward declaration of 'mozilla::dom::BrowsingContext'
33:29.13 class BrowsingContext; /* webidl BrowsingContext */
33:29.13 ^

Could you please check that you haven't forgotten an include file somewhere.

Attached patch add-missing-include.patch (obsolete) — Splinter Review

Please merge this into your patch set (of course with removal for real).

Assignee: nobody → jorgk
Attachment #9106542 - Flags: feedback?(bugs)
Attachment #9106542 - Flags: feedback?(bdahl)
Comment on attachment 9106542 [details] [diff] [review]
add-missing-include.patch

Letting bdahl to deal with this.
Attachment #9106542 - Flags: feedback?(bugs)
Assignee: jorgk → bdahl
Status: NEW → ASSIGNED
Pushed by bdahl@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/67f2b416b270
Merge nsWebShellWindow into nsXULWindow r=smaug
https://hg.mozilla.org/integration/autoland/rev/0e3e0d51aaf6
Rename nsXULWindow and nsIXULWindow to AppWindow and nsIAppWindow. r=smaug

Third time's the charm... Added the header and fixed the in-air collision.

Flags: needinfo?(bdahl)
Comment on attachment 9106542 [details] [diff] [review]
add-missing-include.patch

> Third time's the charm... Added the header and fixed the in-air collision.

Thanks.
Attachment #9106542 - Attachment is obsolete: true
Attachment #9106542 - Flags: feedback?(bdahl)
Status: ASSIGNED → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
You need to log in before you can comment on or make changes to this bug.