debug build failure in nsContentUtils.cpp

I tried to create a debug build using the 5.0a3 Tor Browser code from tor-browser-38.1.0esr-5.0-1 and encountered a compile error in nsContentUtils.cpp. A patch is attached.

added component::applications/tor browser owner::tbb-team priority::medium resolution::fixed status::closed type::defect labels

Trac:
nsContentUtils-patch.txt

a possible fix

What is causing this given that Mozilla does not need to remove this to have debug builds? What options did you choose for the debug build? Could you add the compile error for posterity?

Trac:
Status: new to needs_information
Cc: arthuredelstein, brade to arthuredelstein, brade, gk

The compilation error (from clang) is:

tor-browser/dom/base/nsContentUtils.cpp:3000:19: error: 
      use of undeclared identifier 'aLoadingDocument'; did you mean
      'mozilla::LoadInfo::GetLoadingDocument'?
  NS_PRECONDITION(aLoadingDocument, "Must have a document");
                  ^~~~~~~~~~~~~~~~
                  mozilla::LoadInfo::GetLoadingDocument
../../dist/include/nsDebug.h:93:49: note: expanded from macro 'NS_PRECONDITION'
#define NS_PRECONDITION(expr, str) NS_ASSERTION(expr, str)
                                                ^
../../dist/include/nsDebug.h:82:11: note: expanded from macro 'NS_ASSERTION'
    if (!(expr)) {                                            \
          ^
../../dist/include/mozilla/LoadInfo.h:33:3: note: 
      'mozilla::LoadInfo::GetLoadingDocument' declared here
  NS_DECL_NSILOADINFO
  ^
../../dist/include/nsILoadInfo.h:137:14: note: expanded from macro
      'NS_DECL_NSILOADINFO'
  NS_IMETHOD GetLoadingDocument(nsIDOMDocument * *aLoadingDocument) override; \
             ^
/Users/brade/dev/tor/tor-browser/dom/base/nsContentUtils.cpp:3000:19: error: 
      call to non-static member function without an object argument
  NS_PRECONDITION(aLoadingDocument, "Must have a document");
                  ^~~~~~~~~~~~~~~~
../../dist/include/nsDebug.h:93:49: note: expanded from macro 'NS_PRECONDITION'
#define NS_PRECONDITION(expr, str) NS_ASSERTION(expr, str)
                                                ^
../../dist/include/nsDebug.h:82:11: note: expanded from macro 'NS_ASSERTION'
    if (!(expr)) {                                            \
          ^
2 errors generated.
make[1]: *** [nsContentUtils.o] Error 1

It is caused by one of our patches: ac7de4b0b756891a3835e738ebe534c44d4855c4 (for #13670 (moved)). That patch replaces the aLoadingDocument parameter with aLoadingNode. Since it is OK for aLoadingNode to be NULL, I do not think we need or want an NS_ASSERTION() for aLoadingNode.

The bottom line: the patch I attached to this bug is a fixup for the #13670 (moved) patch (I am sorry I did not mention that before).

So, to understand this fully: This is no error just happening in a debug build. Rather, this is actually a thing clang does not like (while GCC and mingw-w64 + our cross-compiler don't have an issue with it)? If so, what clang version did you use?

Replying to gk:

So, to understand this fully: This is no error just happening in a debug build. Rather, this is actually a thing clang does not like (while GCC and mingw-w64 + our cross-compiler don't have an issue with it)?

No, I am 99% sure this is a problem that affects all debug builds. I only mentioned clang because the error text that it generates are different than those that gcc generates (never mind). Let me try to explain more clearly.

If you look at the #13670 (moved) patch, here: https://gitweb.torproject.org/tor-browser.git/diff/dom/base/nsContentUtils.cpp?h=tor-browser-38.1.0esr-5.0-1&id=ac7de4b0b756891a3835e738ebe534c44d4855c4 you will see that Arthur removed the aLoadingDocument parameter from nsContentUtils::LoadImage() but he neglected to remove the NS_PRECONDITION() that references it. Does this fix make sense now?

Also, I pushed this fix to the brade tor-browser repo to make it easier to review and merge: https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug16497-01&id=bfdfcccae99d1af38b3320eb7409377bd0ebc154

Trac:
Status: needs_information to needs_review

Replying to mcs:

If you look at the #13670 (moved) patch, here: https://gitweb.torproject.org/tor-browser.git/diff/dom/base/nsContentUtils.cpp?h=tor-browser-38.1.0esr-5.0-1&id=ac7de4b0b756891a3835e738ebe534c44d4855c4 you will see that Arthur removed the aLoadingDocument parameter from nsContentUtils::LoadImage() but he neglected to remove the NS_PRECONDITION() that references it. Does this fix make sense now?

Yes, thanks.

Also, I pushed this fix to the brade tor-browser repo to make it easier to review and merge: https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug16497-01&id=bfdfcccae99d1af38b3320eb7409377bd0ebc154

Pushed to tor-browser-38.1.0esr-5.0-1.

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

closed

moved to tpo/applications/tor-browser#16497 (closed)