Last Comment Bug 763694 - Add <iframe mozbrowser> securitychange event
: Add <iframe mozbrowser> securitychange event
Status: RESOLVED FIXED
: dev-doc-complete
Product: Firefox OS
Classification: Client Software
Component: General (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Justin Lebar (not reading bugmail)
:
Mentors:
Depends on: 766586 764248 764496 768001 774807
Blocks: browser-api
  Show dependency treegraph
 
Reported: 2012-06-11 14:19 PDT by Justin Lebar (not reading bugmail)
Modified: 2013-06-27 06:08 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Part 1: Implement securityChange event in <iframe mozbrowser>. (asserts) (4.03 KB, patch)
2012-06-11 17:10 PDT, Justin Lebar (not reading bugmail)
no flags Details | Diff | Splinter Review
Part 2: Tests (5.76 KB, patch)
2012-06-11 17:10 PDT, Justin Lebar (not reading bugmail)
bugs: review+
Details | Diff | Splinter Review
Part 1, v1.1 (rebased atop bug 764248) (4.02 KB, patch)
2012-06-13 13:30 PDT, Justin Lebar (not reading bugmail)
bugs: review+
Details | Diff | Splinter Review

Description Justin Lebar (not reading bugmail) 2012-06-11 14:19:11 PDT
As I understand, Ben would be very happy if I could give him a lock icon.
Comment 1 Justin Lebar (not reading bugmail) 2012-06-11 15:56:21 PDT
Hm, can't test EV certs due to bug 458727.
Comment 2 Justin Lebar (not reading bugmail) 2012-06-11 16:44:28 PDT
Sigh, turns out this is more complicated than I expected because iframes don't have an instance of nsSecureBrowserUIImpl instantiated for them.
Comment 3 Justin Lebar (not reading bugmail) 2012-06-11 17:02:39 PDT
I get the following assertion when I try to read the is-EV-cert property OOP:

> [Child 29324] ###!!! ASSERTION: nsSSLStatus has null mServerCert or was called in the content process: 'Error', file ../../../../../src/security/manager/ssl/src/nsIdentityChecking.cpp, line 1085
> nsSSLStatus::GetIsExtendedValidation(bool*) (/home/jlebar/code/moz/ff-git/debug/security/manager/ssl/src/../../../../../src/security/manager/ssl/src/nsIdentityChecking.cpp:1086)
> nsSecureBrowserUIImpl::EvaluateAndUpdateSecurityState(nsIRequest*, nsISupports*, bool) (/home/jlebar/code/moz/ff-git/debug/security/manager/boot/src/../../../../../src/security/manager/boot/src/nsSecureBrowserUIImpl.cpp:502)
> nsSecureBrowserUIImpl::OnLocationChange(nsIWebProgress*, nsIRequest*, nsIURI*, unsigned int) (/home/jlebar/code/moz/ff-git/debug/security/manager/boot/src/../../../../../src/security/manager/boot/src/nsSecureBrowserUIImpl.cpp:1613)
> nsDocLoader::FireOnLocationChange(nsIWebProgress*, nsIRequest*, nsIURI*, unsigned int) (/home/jlebar/code/moz/ff-git/debug/uriloader/base/../../../src/uriloader/base/nsDocLoader.cpp:1390)
> nsDocShell::CreateContentViewer(char const*, nsIRequest*, nsIStreamListener**) (/home/jlebar/code/moz/ff-git/debug/docshell/base/../../../src/docshell/base/nsDocShell.cpp:7633)
> nsDSURIContentListener::DoContent(char const*, bool, nsIRequest*, nsIStreamListener**, bool*) (/home/jlebar/code/moz/ff-git/debug/docshell/base/../../../src/docshell/base/nsDSURIContentListener.cpp:132)
> nsDocumentOpenInfo::TryContentListener(nsIURIContentListener*, nsIChannel*) (/home/jlebar/code/moz/ff-git/debug/uriloader/base/../../../src/uriloader/base/nsURILoader.cpp:677)
> nsDocumentOpenInfo::DispatchContent(nsIRequest*, nsISupports*) (/home/jlebar/code/moz/ff-git/debug/uriloader/base/../../../src/uriloader/base/nsURILoader.cpp:374)
> nsDocumentOpenInfo::OnStartRequest(nsIRequest*, nsISupports*) (/home/jlebar/code/moz/ff-git/debug/uriloader/base/../../../src/uriloader/base/nsURILoader.cpp:262)
> mozilla::net::HttpChannelChild::OnStartRequest(nsHttpResponseHead const&, bool const&, nsHttpHeaderArray const&, bool const&, bool const&, unsigned int const&, nsCString const&, nsCString const&, PRNetAddr const&, PRNetAddr const&) (/home/jlebar/code/moz/ff-git/debug/netwerk/protocol/http/../../../../src/netwerk/protocol/http/HttpChannelChild.cpp:270)
> mozilla::net::HttpChannelChild::RecvOnStartRequest(nsHttpResponseHead const&, bool const&, nsHttpHeaderArray const&, bool const&, bool const&, unsigned int const&, nsCString const&, nsCString const&, PRNetAddr const&, PRNetAddr const&) (/home/jlebar/code/moz/ff-git/debug/netwerk/protocol/http/../../../../src/netwerk/protocol/http/HttpChannelChild.cpp:231)
> mozilla::net::PHttpChannelChild::OnMessageReceived(IPC::Message const&) (/home/jlebar/code/moz/ff-git/debug/ipc/ipdl/PHttpChannelChild.cpp:531)
> mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) (/home/jlebar/code/moz/ff-git/debug/ipc/ipdl/PContentChild.cpp:1641)
> mozilla::ipc::AsyncChannel::OnDispatchMessage(IPC::Message const&) (/home/jlebar/code/moz/ff-git/debug/ipc/glue/../../../src/ipc/glue/AsyncChannel.cpp:463)
> mozilla::ipc::RPCChannel::OnMaybeDequeueOne() (/home/jlebar/code/moz/ff-git/debug/ipc/glue/../../../src/ipc/glue/RPCChannel.cpp:404)
> void DispatchToMethod<mozilla::ipc::RPCChannel, bool (mozilla::ipc::RPCChannel::*)()>(mozilla::ipc::RPCChannel*, bool (mozilla::ipc::RPCChannel::*)(), Tuple0 const&) (/home/jlebar/code/moz/ff-git/debug/ipc/glue/../../../src/ipc/chromium/src/base/tuple.h:384)
> RunnableMethod<mozilla::ipc::RPCChannel, bool (mozilla::ipc::RPCChannel::*)(), Tuple0>::Run() (/home/jlebar/code/moz/ff-git/debug/ipc/glue/../../../src/ipc/chromium/src/base/task.h:308)
> mozilla::ipc::RPCChannel::RefCountedTask::Run() (/home/jlebar/code/moz/ff-git/debug/ipc/glue/../../dist/include/mozilla/ipc/RPCChannel.h:430)
> mozilla::ipc::RPCChannel::DequeueTask::Run() (/home/jlebar/code/moz/ff-git/debug/ipc/glue/../../dist/include/mozilla/ipc/RPCChannel.h:453)
> MessageLoop::RunTask(Task*) (/home/jlebar/code/moz/ff-git/debug/ipc/chromium/../../../src/ipc/chromium/src/base/message_loop.cc:319)
> MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) (/home/jlebar/code/moz/ff-git/debug/ipc/chromium/../../../src/ipc/chromium/src/base/message_loop.cc:329)
> MessageLoop::DoWork() (/home/jlebar/code/moz/ff-git/debug/ipc/chromium/../../../src/ipc/chromium/src/base/message_loop.cc:426)
> mozilla::ipc::DoWorkRunnable::Run() (/home/jlebar/code/moz/ff-git/debug/ipc/glue/../../../src/ipc/glue/MessagePump.cpp:43)
> nsThread::ProcessNextEvent(bool, bool*) (/home/jlebar/code/moz/ff-git/debug/xpcom/threads/../../../src/xpcom/threads/nsThread.cpp:624)
> NS_ProcessNextEvent_P(nsIThread*, bool) (/home/jlebar/code/moz/ff-git/debug/xpcom/build/nsThreadUtils.cpp:213)
> mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (/home/jlebar/code/moz/ff-git/debug/ipc/glue/../../../src/ipc/glue/MessagePump.cpp:79)
> mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) (/home/jlebar/code/moz/ff-git/debug/ipc/glue/../../../src/ipc/glue/MessagePump.cpp:209)
> MessageLoop::RunInternal() (/home/jlebar/code/moz/ff-git/debug/ipc/chromium/../../../src/ipc/chromium/src/base/message_loop.cc:209)
> MessageLoop::RunHandler() (/home/jlebar/code/moz/ff-git/debug/ipc/chromium/../../../src/ipc/chromium/src/base/message_loop.cc:202)
> MessageLoop::Run() (/home/jlebar/code/moz/ff-git/debug/ipc/chromium/../../../src/ipc/chromium/src/base/message_loop.cc:175)
> nsBaseAppShell::Run() (/home/jlebar/code/moz/ff-git/debug/widget/xpwidgets/../../../src/widget/xpwidgets/nsBaseAppShell.cpp:165)
> XRE_RunAppShell (/home/jlebar/code/moz/ff-git/debug/toolkit/xre/../../../src/toolkit/xre/nsEmbedFunctions.cpp:641)
> mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) (/home/jlebar/code/moz/ff-git/debug/ipc/glue/../../../src/ipc/glue/MessagePump.cpp:194)
> MessageLoop::RunInternal() (/home/jlebar/code/moz/ff-git/debug/ipc/chromium/../../../src/ipc/chromium/src/base/message_loop.cc:209)
> MessageLoop::RunHandler() (/home/jlebar/code/moz/ff-git/debug/ipc/chromium/../../../src/ipc/chromium/src/base/message_loop.cc:202)
> MessageLoop::Run() (/home/jlebar/code/moz/ff-git/debug/ipc/chromium/../../../src/ipc/chromium/src/base/message_loop.cc:175)
> XRE_InitChildProcess (/home/jlebar/code/moz/ff-git/debug/toolkit/xre/../../../src/toolkit/xre/nsEmbedFunctions.cpp:484)
> main (/home/jlebar/code/moz/ff-git/debug/ipc/app/../../../src/ipc/app/MozillaRuntimeMain.cpp:48)
Comment 4 Justin Lebar (not reading bugmail) 2012-06-11 17:08:30 PDT
> I get the following assertion when I try to read the is-EV-cert property OOP:

Actually, I can a backtrace -- that assertion happens before I try to read the is-ev property.

Brian, can you comment on how I'm supposed to handle this?  I'm clearly not doing things as expected.  I'll post my patch in a moment.
Comment 5 Justin Lebar (not reading bugmail) 2012-06-11 17:10:35 PDT
Created attachment 632071 [details] [diff] [review]
Part 1: Implement securityChange event in <iframe mozbrowser>. (asserts)
Comment 6 Justin Lebar (not reading bugmail) 2012-06-11 17:10:43 PDT
Created attachment 632072 [details] [diff] [review]
Part 2: Tests
Comment 7 Justin Lebar (not reading bugmail) 2012-06-11 17:13:52 PDT
Comment on attachment 632071 [details] [diff] [review]
Part 1: Implement securityChange event in <iframe mozbrowser>. (asserts)

f? because I'd like to know what's the right thing to do here.
Comment 8 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-06-12 20:34:27 PDT
In general, PSM must only be used in chrome processes because NSS is only used in chrome processes. If you want to use nsSecureBrowserUIImpl as-is, then the nsSecureBrowserUIImpl needs to be instantiated in the chrome process, and then you need to proxy the information you need from the nsSecureBrowserUIImpl to the content process.

In other words, don't instantiate nsSecureBrowserUIImpl within a content process.
Comment 9 Justin Lebar (not reading bugmail) 2012-06-13 07:14:15 PDT
> If you want to use nsSecureBrowserUIImpl as-is, then the nsSecureBrowserUIImpl needs to be 
> instantiated in the chrome process

The relevant window exists only in the content process, so I don't see how that would work.

It looks like XUL Fennec does basically the same thing as this patch: It listens to securitychange events in the child process.  I don't see it explicitly initializing the SecureBrowserUI, but I expect that's happening somehow, because SecureBrowserUI is the only thing I can find that triggers securitychange events.

Maybe I'm misunderstanding this?  http://hg.mozilla.org/mozilla-central/file/bdbed29aaaa6/mobile/xul/chrome/content/bindings/browser.js#l82
Comment 10 Justin Lebar (not reading bugmail) 2012-06-13 11:20:44 PDT
Brian and I talked on IRC and figured out that, aside from the EV problem, this is all fine.

I'll file a separate bug on the EV business.  It's harmless for our purposes; just means that we'll get spew in debug builds and that we won't be able to detect EV certs.

I'm going to continue sending |event.detail.extendedValidation == false|, so that the API is clear even if it doesn't work.  I'll add a comment to the patch pointing to the new EV bug, but aside from that, this is ready for review.
Comment 11 Justin Lebar (not reading bugmail) 2012-06-13 11:25:29 PDT
The EV bug is 764496.  Once that is fixed, I think EV should Just Work in mozbrowser.
Comment 12 Justin Lebar (not reading bugmail) 2012-06-13 13:27:48 PDT
New patch based atop bug 764248 in a moment.
Comment 13 Justin Lebar (not reading bugmail) 2012-06-13 13:30:13 PDT
Created attachment 632845 [details] [diff] [review]
Part 1, v1.1 (rebased atop bug 764248)

I guess this wasn't really necessary; it's a change only to irrelevant context.
Comment 15 Ed Morley [:emorley] 2012-06-20 02:30:07 PDT
Backed out for native Android failures:
(This bug's push didn't get builds, next one did)
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=4a44ebe3e8ff

https://tbpl.mozilla.org/php/getParsedLog.php?id=12816346&tree=Mozilla-Inbound
{
699 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/browser-element/mochitest/test_browserElement_inproc_SecurityChange.html | Test timed out.
916 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/browser-element/mochitest/test_browserElement_oop_SecurityChange.html | Test timed out.
}

https://hg.mozilla.org/integration/mozilla-inbound/rev/6cb8b0d62bbd
Comment 16 Justin Lebar (not reading bugmail) 2012-06-20 08:58:57 PDT
Given the choice between wasting a day fixing this and, alternatively, landing this now for B2G, that's an easy choice.  We can figure out what the heck is wrong with native Android later.  Unless anyone has objections, I'll disable this test on native Android and push again.
Comment 17 Justin Lebar (not reading bugmail) 2012-06-20 09:18:10 PDT
Filed bug 766586 on things not working on native android, and pushed again with the tests disabled there.

https://hg.mozilla.org/integration/mozilla-inbound/rev/a81245912f3f
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c6c55befb22
Comment 19 Jeremie Patonnier :Jeremie 2013-06-27 06:08:52 PDT
Documentation available here:
https://developer.mozilla.org/en-US/docs/Web/Reference/Events/mozbrowsersecuritychange

Note You need to log in before you can comment on or make changes to this bug.