MozReview-Commit-ID: BpNykQdha7L

MozReview-Commit-ID: 2fsVRy6INr3

MozReview-Commit-ID: EtDEaaeoARB

Comment on attachment 8924672 [details] [diff] [review] Part 2: Add FORWARD_TO_DOCSHELL macros to nsGlobalWindow.cpp ># HG changeset patch ># User Nika Layzell <nika@thelayzells.com> > >Bug 1405805 - Part 2: Add FORWARD_TO_DOCSHELL macros to nsGlobalWindow.cpp, r=smaug > >MozReview-Commit-ID: 2fsVRy6INr3 > >diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp >index fe0519c85ee3..08692a0037ae 100644 >--- a/dom/base/nsGlobalWindow.cpp >+++ b/dom/base/nsGlobalWindow.cpp >@@ -410,16 +410,73 @@ static uint32_t gThrottledIdlePeriodLength; > if (!mInnerWindow) { \ > return err_rval; \ > } \ > } \ > return GetCurrentInnerWindowInternal()->method args; \ > } \ > PR_END_MACRO > >+// NOTE: This macro doesn't take the fallback as an argument. Instead, the >+// fallback is the block of code which is placed after the macro invocation, as >+// if the forwarding was successful we will return from the current function. >+#define FORWARD_TO_DOCSHELL_WITH_FALLBACK(method, args) \ the naming is weird. There is no fallback there. Perhaps something like TRY_TO_FORWARD_TO_DOCSHELL >+ >+#define FORWARD_TO_DOCSHELL(method, args, err_rval) \ >+ PR_BEGIN_MACRO \ >+ FORWARD_TO_DOCSHELL_WITH_FALLBACK(method, args); \ >+ return err_rval; \ >+ PR_END_MACRO >+ >+#define FORWARD_TO_DOCSHELL_OR_THROW(method, args, errorresult, err_rval) \ >+ PR_BEGIN_MACRO \ >+ FORWARD_TO_DOCSHELL_WITH_FALLBACK(method, args); \ >+ errorresult.Throw(NS_ERROR_XPC_SECURITY_MANAGER_VETO); \ >+ return err_rval; \ >+ PR_END_MACRO align \s >+ >+#define FORWARD_TO_DOCSHELL_VOID(method, args, err_rval) \ what? _VOID taking an err_rval which is then returned? Something wrong here

Comment on attachment 8924673 [details] [diff] [review] Part 3: Move window closing logic into nsDocShell >+class nsCloseEvent : public Runnable { Since you're modifying the coding style anyhow, could you fix this. { to its own line >+ PostCloseEvent(nsDocShell* aDocShell, bool aIndirect) { Ditto, and same also with many other methods

Comment on attachment 8924671 [details] [diff] [review] Part 1: Remove some unused macros & fields from nsGlobalWindow The DialogValueHolder class can go away altogether. Mind doing that? Looks like these bits were missed in bug 1374460.

Taking a different approach with the nsGlobalWindow split, so marking wontfix.