Honza Bambas (:mayhemer) Reporter
	Description • 14 years ago

After mozilla-central gets merged with the electrolysis tree, security UI of Fennec will stop working.

After discussion with Benjamin Smedberg we need to change browser.xml here: http://hg.mozilla.org/mozilla-central/annotate/d4e5f5f19777/toolkit/content/widgets/browser.xml#l538

New class in the e10s tree TabParent (IFrameEmbedding) will implement nsISecureBrowserUI after bug 536301 lands.  browser.xml shall then bind to that implementation.

	Honza Bambas (:mayhemer) Reporter
	Comment 1 • 14 years ago

Sorry, wrong source reference, the correct is:

http://hg.mozilla.org/mobile-browser/annotate/4cfb4164ba20/chrome/content/browser.js#l3198

and as I can see, if browser.webProgress is TabParent, then this bug seems not to be valid as Fennec is not using nsISecureBrowserUI at all.

	Benjamin Smedberg
	Comment 2 • 14 years ago

Sure it is: http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.js#2174

	Honza Bambas (:mayhemer) Reporter
	Comment 3 • 14 years ago

Ah, I was searching for the interface.  I'm not much familiar with binding and xul, so where is the securityUI property instantiated?  By the code from mozilla-central's browser.xml?

	Benjamin Smedberg
	Comment 4 • 14 years ago

Yes. browser.xml is what implements the <xul:browser> element. After you fix this bug I think Firefox should "just work" (or at least the basic larry stuff: the more complex certificate viewer UI may not).

	Honza Bambas (:mayhemer) Reporter
	Comment 5 • 14 years ago

Thanks for clearing it for me.  

I think it's backwards: after I fix the Firefox bug (browser.xul code) this one should 'just work', mobile browser depends on it.

The cert detail UI is probably another bug, not needed for Fennec (?).

	Benjamin Smedberg
	Comment 6 • 14 years ago

The Firefox/Fennec bugs are probably dups. And yes, the cert detail UI is a different bug: I don't know whether Fennec has any UI like that or not.

	Mark Finkle (:mfinkle) (use needinfo?)
	Comment 7 • 14 years ago

Fennec had originally planned on moving the security data collection into the
content process. If the securityUI will be accessible in chrome, then we might
save ourselves some work.

No cert UI for Fennec

	Benjamin Smedberg
	Comment 8 • 14 years ago

Well, you'll need at least some cooperation with chrome since only chrome will have EV data.

	Honza Bambas (:mayhemer) Reporter
	Comment 9 • 14 years ago

Security data collection is made on the content process as by of means of checking web progress notifications and security information of each top level request and all their sub-requests (as is being made in the single process model).

The security info is being passed between parent and child processes because it is determined on the parent process but needs to be handled on the content process.

Security UI (nsISecureBrowserUI interface) will be accessible on the chrome process.  It will be implemented by the TabParent class.

	Honza Bambas (:mayhemer) Reporter
	Comment 10 • 14 years ago

And EV data is processed on the chrome as well (in TabParent).

	Mark Finkle (:mfinkle) (use needinfo?)
	Comment 11 • 14 years ago

Sounds like we will not need to change much with the current mobile code. Everything should just work, once bug 536301 lands.

If we need to do some small cleanup, it won't be a problem.

	Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
	Comment 12 • 13 years ago

We should remove all the security UI processing from the content process, and instead have the content process notify the chrome process when something happens that should modify the security indicators. Then we can avoid serializing nsNSSSocketInfo and nsSSLStatus for IPC between the processes. These structures can be pretty large (several KB) because each one embeds a SSL certificate in its serialization. (And, nsNSSSocketInfo embeds an nSSSLStatus, so we're sending *two* serialized certs across processes.)

Also, because none of these classes (nsNSSCertificate, nsSSLStatus, nsNSSSocketInfo) serialize the EV status of the cert, we end up doing certificate validation multiple times, and sometimes on the main thread. This causes the main thread to block on network IO (OCSP responses). We can change the serialization/deserialization logic to store/restore the EV status, but this gets tricky because the HTTP disk cache also uses the same serialization for these structures; changing the serialization would thus cause compatibility problems. (OTOH, we probably need to serialize/deserialize the EV status anyway, to properly handle WYCIWYG, especially for session restore.)

Once we avoid sending nsNSSSocketInfo/nsSSLStatus across processes, we can remove the serialization code from those two classes. (For the cache, we should use specialized serialization logic instead of the current and inefficient generic serialization of these structures.)

	BMO Automation
	Comment 13 • 5 years ago

Closing all opened bug in a graveyard component