875191 - Add support for BrowserElementPromptService to show prompt for NSS

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: General, defect)

Description

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Currently in Bug 867899 we're trying to use NSS on B2G, but I found out that BrowserElementPromptService (which is enabled on B2G) will intercept the nsIPromptFactory.getPrompt call and throw error when we successfully imported a certificate. (NSS will try to show the message 'Successfully restored your security certificate(s) and private key(s).' to user)

The error is thrown because those function arguments from NSS don't contain a window object.  See [1]

But on B2G, we don't have native prompt, nor some XUL components to display this prompt to user, so the phone crashed immediately. 

So for BrowserElementPromptService, we should add support for showing prompts from NSS.


[1] : http://mxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementPromptService.jsm#486

Comment 1

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Is an NSS prompt opened to a specific mozbrowser? (i.e. this alert is for mozbrowser A, that confirm is for mozbrowser B, ...etc) If not, probably mozChromeEvent is more suitable for this case.

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Thanks, Patrick.
I'll try to see how to implement this by mozChromeEvent.

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 761951 [details] [diff] [review]
Patch

Hi Brian and Justin
This bug is trying to show 'prompt' for NSS on B2G.
Please also see https://bugzilla.mozilla.org/show_bug.cgi?id=875191#c0 for the detail.

Can you guys help to give me some feedback on this?
f? to Brian for the usage of PromptService,
also f? to Justin for the ChromeEvent

Your comments will be highly appreciated.

Thanks

Comment 5

[Justin Lebar (not reading bugmail)]

Sorry to keep you waiting for six days, Yoshi.

You've stepped into code that is actually a lot more complex than it appears at first.  Bear with me here; I'm going to try to explain how it works.

I think your patch is only going to work if the alert happens in the main process.

If it happens in a child process, we'll never forward the alert up to the main process.  I'm not sure what Services.wm.getMostRecentWindow("navigator:browser"); is going to return in a child process, but I'm pretty sure it can't be the window you want.

It's OK with me if we don't make this work in child processes for now, if we know that we only ever get this NSS message in the main process.  But then we need to throw an error in the child process case, instead of trying and possibly doing the wrong thing.

Another problem is that alert() is a blocking call.  That is, if code does

  alert();
  dump('alert done');

the 'alert done' shouldn't get printed until after the user clicks 'ok' on the alert.  As this is written, that doesn't happen; we send the chrome event and then would immediately print 'alert done'.

Note that mozbrowser is the only consumer the prompt.allowNative pref, so if you're setting it to true on B2G, it's effectively true everywhere, and we should probably get rid of it entirely.

To determine whether we're in the main process or a child process, you can do something like

  var appInfoSvc = Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULRuntime);
  if (!appInfo || appInfoSvc.processType == Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT) {
    // main process
  } else {
    // child process
  }

To make alert() blocking, you probably want to do something similar to what we do for alert() inside mozbrowser.  Here's what happens when a page inside <iframe mozbrowser> calls alert():

  1. page (in child process) calls alert()
  2. BrowserElementChildPreload.js sends a message to the parent indicating
     that we're in an alert()
  3. BrowserElementChildPreload.js::waitForResult spins in a nested event loop
     waiting for a message back from the parent.
  4. BrowserElementParent.jsm receives the message about the alert() and
     dispatches a mozbrowsershowmodalprompt event to the embedder.  This event
     contains a method called unblock(), which is relevant below.
  5. Once the event is done dispatching,
     a. If preventDefault() was not called on the event, the parent immediately
	sends a message down to the child telling the child to exit its nested
	event loop.
     b. If preventDefault() was called on the event, the child stays in its
        nested event loop until unblock() is called.

I think what you want to do with the chrome event is very similar to what we do here, except instead of disaptching an event to the embedder (the page in the parent process which contains <iframe mozbrowser>), you're going to dispatch a chrome event.

Because it's so similar to what you're trying to do, and because BrowserElementChildPreload::waitForEvent is pretty complicated, it might be simpler for you to try to reuse the the existing machinery.  When there's no window in the alert() call, you could pick an arbitrary BrowserElementChild and ask it to forward up to its parent and then wait for a response from the parent.  We'd want to avoid the window-specific calls in waitForEvent, but the new code would be pretty similar.

Anyway, you have some options here.  Have a look through the code and let me know if this helps you understand!

Comment 6

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 761951 [details] [diff] [review]
Patch

Thanks Justin for the valuable comments,
it helps a lot, and I really appreciated it. :)

I'll check how this should work for 'alert' from child process.

Thanks again.

Comment 7

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Hi Justin

Sorry I was busy on other bugs before, and I could work on this bug again.

(In reply to Justin Lebar [:jlebar] from comment #5)
> 
> It's OK with me if we don't make this work in child processes for now, if we
> know that we only ever get this NSS message in the main process.  But then
> we need to throw an error in the child process case, instead of trying and
> possibly doing the wrong thing.
> 
Okay, I think I should focus on Parent process now, and I will try to throw the error if that happens in Child Process.

> 
> Note that mozbrowser is the only consumer the prompt.allowNative pref, so if
> you're setting it to true on B2G, it's effectively true everywhere, and we
> should probably get rid of it entirely.
> 
Okay, I will turn it to false.


> 
> I think what you want to do with the chrome event is very similar to what we
> do here, except instead of disaptching an event to the embedder (the page in
> the parent process which contains <iframe mozbrowser>), you're going to
> dispatch a chrome event.
> 
> Because it's so similar to what you're trying to do, and because
> BrowserElementChildPreload::waitForEvent is pretty complicated, it might be
> simpler for you to try to reuse the the existing machinery.  When there's no
> window in the alert() call, you could pick an arbitrary BrowserElementChild
> and ask it to forward up to its parent and then wait for a response from the
> parent. 

What you said here is for showing NSS message in Child process, right?

But like I said, if I'd like to do it for Parent Process first,
and currently I saw BrowserElementPrompt is associated with BrowserElementChild, can I add a BrowserElementPrompt for Parent?
And do the alert-blocking in that BrowserElementPrompt for Parent?


> We'd want to avoid the window-specific calls in waitForEvent, but
> the new code would be pretty similar.
> 
I think this is also for shoing NSS message in Child process, right?
But I don't quite understand here, the waitForResult code is full of window-specific calls, and there are some data structure in the window object.
If I were to change my mind to show NSS message in Child, how do I reuse waitForResult here without a proper window object?

Thanks

Comment 8

[Justin Lebar (not reading bugmail)]

> What you said here is for showing NSS message in Child process, right?

Yes.

> But like I said, if I'd like to do it for Parent Process first,

That's fine, but note that we probably cannot check that in.

> and currently I saw BrowserElementPrompt is associated with BrowserElementChild, can I add a 
> BrowserElementPrompt for Parent?
> And do the alert-blocking in that BrowserElementPrompt for Parent?

There are a lot of different ways you can do this parent-process-only.  I'm not sure I can help much in this respect.  But that's OK because we're not going to check in this code.  So you can do it however you want.

> If I were to change my mind to show NSS message in Child, how do I reuse waitForResult here 
> without a proper window object?

Why don't we table this until you're ready to work on the child-process part?

Comment 9

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Justin Lebar [:jlebar] from comment #8)
> 
> > But like I said, if I'd like to do it for Parent Process first,
> 
> That's fine, but note that we probably cannot check that in.
> 

Hi, Justin,
Thanks for the reply, but I don't understand what you mean by "check that in" here?

Can you explain that again ?

Thank you :)

Comment 10

[Justin Lebar (not reading bugmail)]

I mean, a solution which works only in the parent process is fine as a stepping stone, but would not get r+.  We would not be able to commit that code into mozilla-central.