793419 - Add Gecko:Ready event notification to pageloader.js

Status: RESOLVED WONTFIX

(Testing :: Talos, defect)

Description

[Scott Johnson (:jwir3)]

(From bug 716575, comment 46):

> Scott and I ran some experiments on Try, and discovered that the Talos crashes
> are caused because screen->GetRect is returning a 0-size rect (which ultimately 
> causes a divide-by-zero in GetViewportInfo).

> I believe this happens because the Android Java code does not send the screen 
> size to Gecko until it receives a Gecko:Ready message.  This message is usually 
> sent when browser.xul loads, but the Talos pageloader extension replaces 
> browser.xul with its own pageloader.xul.

> The crash can be avoided by adding a zero check before dividing by aDisplayWidth 
> or aDisplayHeight.  The lack of screen information in Talos might be fixed just 
> by sending a Gecko:Ready message from pageloader.js when running in native 
> Android Fennec.

We should add the capability for pageloader.js to send a Gecko:Ready event, so we don't run into the problem of screen->GetRect() being 0 in the future.

Comment 1

[Scott Johnson (:jwir3)]

Joel:

To fix this, would it make sense to add the line:
>      sendAsyncMessage('Gecko:Ready', {});
after window.resizeTo(10,10) in pageloader.js? 

(I'm not 100% sure I understand how events are passed between the browser in pageloader). Alternatively, I could put the code from browser.js that gets the android bridge and sends a message that way, but it would require that we check to make sure it's on mobile.

Comment 2

[Matt Brubeck (:mbrubeck)]

(In reply to Scott Johnson (:jwir3) from comment #1)
> To fix this, would it make sense to add the line:
> >      sendAsyncMessage('Gecko:Ready', {});
> after window.resizeTo(10,10) in pageloader.js? 
> 
> (I'm not 100% sure I understand how events are passed between the browser in
> pageloader). Alternatively, I could put the code from browser.js that gets
> the android bridge and sends a message that way, but it would require that
> we check to make sure it's on mobile.

You'll need to use the Android bridge method to send a message from Gecko to the Android/Java code.  (sendAsyncMessage will only send messages between e10s content and chrome processes.)

Comment 3

[Scott Johnson (:jwir3)]

(In reply to Matt Brubeck (:mbrubeck) from comment #2)
> You'll need to use the Android bridge method to send a message from Gecko to
> the Android/Java code.  (sendAsyncMessage will only send messages between
> e10s content and chrome processes.)

Thanks for the heads-up. It's possible that this is why this wasn't working. (jmaher tried what I suggested this morning).

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

I don't have my build available anymore for this.  Can you link me to a build that can reproduce this problem?

Comment 5

[Scott Johnson (:jwir3)]

(In reply to Joel Maher (:jmaher) from comment #4)
> I don't have my build available anymore for this.  Can you link me to a
> build that can reproduce this problem?

I'm not sure how to link to an actual build, but here's a try revision that was exhibiting the problem:
https://hg.mozilla.org/try/rev/b3c5811f5463

Comment 6

[Scott Johnson (:jwir3)]

Matt:

jmaher added this code to pageloader.js, but it seems that it didn't fix the crash:

http://pastebin.mozilla.org/1842427

Comment 7

[Matt Brubeck (:mbrubeck)]

> dumpLine(bridge.handleGeckoMessage(JSON.stringify({'Gecko:Ready': {}})));

This should be:

> dumpLine(bridge.handleGeckoMessage(JSON.stringify({gecko: {type: 'Gecko:Ready'}})));

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

thanks for clarifying that.  I did this and had no luck, same old crash.