Closed Bug 1470510 Opened 2 years ago Closed 11 months ago

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

Blocks: 1593469
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: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
You need to log in before you can comment on or make changes to this bug.