Brian Smith (:briansmith, :bsmith, use NEEDINFO?) Reporter
	Description • 11 years ago

+++ This bug was initially created as a clone of Bug #900515 +++

xul.dll!EnsureNSSInitialized(EnsureNSSOperator op) Line 146	C++
xul.dll!`anonymous namespace'::nsCryptoConstructor(nsISupports * aOuter, const nsID & aIID, void * * aResult) Line 193	C++
xul.dll!mozilla::GenericFactory::CreateInstance(nsISupports * aOuter, const nsID & aIID, void * * aResult) Line 16	C++
xul.dll!nsComponentManagerImpl::CreateInstanceByContractID(const char * aContractID, nsISupports * aDelegate, const nsID & aIID, void * * aResult) Line 1097	C++
xul.dll!CallCreateInstance(const char * aContractID, nsISupports * aDelegate, const nsID & aIID, void * * aResult) Line 139	C++
xul.dll!nsCreateInstanceByContractID::operator()(const nsID & aIID, void * * aInstancePtr) Line 178	C++
xul.dll!nsCOMPtr<nsIDOMCrypto>::assign_from_helper(const nsCOMPtr_helper & helper, const nsID & aIID) Line 1259	C++
xul.dll!nsCOMPtr<nsIDOMCrypto>::operator=(const nsCOMPtr_helper & rhs) Line 728	C++
xul.dll!nsGlobalWindow::GetCrypto(nsIDOMCrypto * * aCrypto) Line 3890	C++
xul.dll!nsGlobalWindow::GetCrypto(nsIDOMCrypto * * aCrypto) Line 3881	C++

NS_IMETHODIMP
nsGlobalWindow::GetCrypto(nsIDOMCrypto** aCrypto)
{
  FORWARD_TO_OUTER(GetCrypto, (aCrypto), NS_ERROR_NOT_INITIALIZED);

  if (!mCrypto) {
#ifndef MOZ_DISABLE_CRYPTOLEGACY
    mCrypto = do_CreateInstance(NS_CRYPTO_CONTRACTID);
#else
    mCrypto = new Crypto();
#endif
  }
  NS_IF_ADDREF(*aCrypto = mCrypto);
  return NS_OK;
}

We should change the condition to:

#ifndef MOZ_DISABLE_CRYPTOLEGACY
  // XXX: The legacy crypto API is not available in child
  // processes. See bug XXXXXX about removing this hack.
  if (geckoProcessType_Default == XRE_GetProcessType()) {
    mCrypto = do_CreateInstance(NS_CRYPTO_CONTRACTID);
  } else {
#endif
    mCrypto = new Crypto();
#ifndef MOZ_DISABLE_CRYPTOLEGACY
  }
#endif

	Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]
	Comment 1 • 11 years ago

Hi, Brian.
Bug 883741 is landed, and it has checked for the do_CreateInstance(...), I think that can prevent the assertion you met.

Do we still need to add the process type checking ?

Thanks

	Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]
	Comment 2 • 11 years ago

Hi, Brian
I saw you are away from Bugzilla now, ni? to you in case you miss my Comment 1, thanks.

	Brian Smith (:briansmith, :bsmith, use NEEDINFO?) Reporter
	Comment 3 • 11 years ago

Yoshi, I am not sure I understand your question correctly. AFAICT, the process type checking is still needed after the change in bug 883741 too. Am I overlooking something?

	Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]
	Comment 4 • 11 years ago

Hi, Brian
I mean if we try to initialize NSS in a non-chrome process,
it will return fail in http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#3861

So won't get the assertion you said.

	Brian Smith (:briansmith, :bsmith, use NEEDINFO?) Reporter
	Comment 5 • 11 years ago

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #4)
> Hi, Brian
> I mean if we try to initialize NSS in a non-chrome process,
> it will return fail in
> http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.
> cpp#3861
> 
> So won't get the assertion you said.

I expect that do_CreateInstance(NS_CRYPTO_CONTRACTID, &rv) is what causes the assertion anyway. window.crypto.getRandomValues() and similar things should still work in the non-Chrome process, but with the current code there won't be any window.crypto object because of the NS_ENSURE_SUCCESS(rv, rv) will fail. Or, in other words, I really think it is better to do as I suggested in comment 0. I can write it as a patch.

	Chris Peterson [:cpeterson]
	Comment 6 • 10 years ago

Mass tracking-e10s flag change. Filter bugmail on "2be0fcce-e36a-4e2c-aa80-0e3d33eb5406".