795209 - System Message API: Fail to fire 'icc-stkcommand' system messages

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

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

The Patch for Bug 787175 added a system message 'icc-stkcommand',
but I found that sometimes this system message is sent too early before gaia is able to handle that.
So we need to tell ICC to send this proactive command later.

Comment 1

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

This is blocker because sometimes Gaia STK app cannot receive stk commands.

Comment 2

[Philipp von Weitershausen [:philikon]]

(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #0)
> The Patch for Bug 787175 added a system message 'icc-stkcommand',
> but I found that sometimes this system message is sent too early before gaia
> is able to handle that.

ORLY? We don't even turn on the radio until we receive 'system-message-listener-ready'. So how come we get proactive STK commands from the ICC already?

Please see also bug 783149.

Comment 3

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

Attachment: log

Hi, philikon
I attached the log.
Yes, the radio is not on yet,
but it's already in SIM-Ready state.

line 517: UNSOL_RIL_CONNECTED

line 565: Got first proactive command 

line 928: proactive command sent to content

line 942: Got system-message-listener-ready.

Comment 4

[Philipp von Weitershausen [:philikon]]

Oof. Maybe we need to re-think the 'system-message-listener-ready' approach then.

The buffering in bug 783149 should help us, though, no?

Comment 5

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

(In reply to Philipp von Weitershausen [:philikon] from comment #4)
> Oof. Maybe we need to re-think the 'system-message-listener-ready' approach
> then.
> 
> The buffering in bug 783149 should help us, though, no?
Hi, philikon

After discussed with Gene, when SystemMessageInternal received this message, the handlers for this message are still empty, might be this message is arrived too early. He will take a look into system message part.

Thanks

Comment 6

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

After some tests I found SIM app won't send the proactive command anymore, 
I think we should try to see if SystemMessageManager can broadcast this early message or not.

Reset assignee to default to let someone who knows system message better to handle this.

Thanks

Comment 7

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

Gene says he will take this bug.

Comment 8

[Andrew Overholt [:overholt]]

Need STK for v1.

Comment 9

[Gene Lian (I already quit Mozilla)]

This issue is basically affected by multiple causes. I'll upload a patch to address this later. ;)

Comment 10

[Gene Lian (I already quit Mozilla)]

Attachment: Patch

Hi Fabrice :)

This issue is due to the following causes:

1. The "icc-stkcommand" system message is fired super early when booting up, even before the webapps' registration process is done, thus having incorrect pages to be broadcasted. We need to buffer such "early-fired" messages and let SystemMessageInternal listen to the "webapps-registry-ready" observer event to know the right time to fire them.

2. Even if the #1 is fixed, the "icc-stkcommand" is still fired early than the homescreen activity message, thus occupying the home and disable the homescreen app. To fix this, we need to delay the 10 seconds to 20 seconds. Well, I know this is dirty but is fine because these messages don't care about the exact time they are going to fire (just "near" the booting-up time, though).

Comment 11

[[:fabrice] Fabrice Desré]

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

Review of attachment 666175 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not a big fan of buffering for 20s on the shell.js side. It looks like only the settings app is listening for the 'icc-stkcommand' message, so why do we need to wait that long?
What I don't understand in your comment is the "thus occupying the home and disable the homescreen app" part.

::: dom/messages/SystemMessageInternal.js
@@ +61,5 @@
> +                                   msg: aMessage,
> +                                   pageURI: aPageURI,
> +                                   manifestURI: aManifestURI });
> +      return;
> +    }

Nit: add a blank line

@@ +103,5 @@
> +      this._bufferedSysMsgs.push({ how: "broadcast",
> +                                   type: aType,
> +                                   msg: aMessage });
> +      return;
> +    }

Nit: add a blank line

@@ +223,5 @@
> +              this.broadcastMessage(aSysMsg.type, aSysMsg.msg);
> +              break;
> +          }
> +        }).bind(this));
> +        break;    

nit: whitespace

Comment 12

[Gene Lian (I already quit Mozilla)]

Attachment: Patch, V1.1

Hi Fabrice,

I just did more tests and cannot reproduce the issue #2 at comment #10 anymore. I did at least 10 times and it always works well. However, it's very close to the homescreen app loaded (at 9 seconds). Do you think we need to make 10 seconds longer?

Btw, what I mentioned: "thus occupying the home and disable the homescreen app" means if an app is asked to be opened *earlier* then the homescreen app, then such the first application opened would be considered as the homescreen app [1]. This would mess up the homescreen.

[1] Please see https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/window_manager.js#L1078

Comment 13

[Gene Lian (I already quit Mozilla)]

Attachment: Patch 1.2

Rebase. r=fabrice.

Comment 14

[Gene Lian (I already quit Mozilla)]

Attachment: Patch 1.3

Clean up the buffer after sending the buffered system messages.

Rebase. r=fabrice.

Comment 15

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/integration/mozilla-inbound/rev/97362b531a38

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/97362b531a38