1140275 - System Message API: message is sent to an app page even if not registered in manifest

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Kan-Ru Chen [:kanru] (UTC+9)]

Steps to reproduce:

  1. Edit gaia/apps/clock/manifest.webapp and remove the "messages" section
  2. Re-install gaia (make reset-gaia)
  3. Open clock app and schedule a short alarm

Expected result:
  Alarm does not alarm

Actual result:
  Alarm alarms at the specified time

However if we kill the clock app after step 3, the alarm does not alarm.

See also bug 1137722 comment 10

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] seanyhlin:bug-1140275 > mozilla-b2g:master

Comment 2

[Sean Lin [:seanlin] (inactive from Mar 2016)]

Attachment: Patch Gaia

Gaia patch - Allow required system messages for mochitest.

Comment 3

[Sean Lin [:seanlin] (inactive from Mar 2016)]

Attachment: Patch Gecko, v1

Patch Gecko v1: Only send messages to pages registered in manifests.

Comment 4

[[:fabrice] Fabrice Desré]

Sean, can you explain the changes?

Comment 5

[Sean Lin [:seanlin] (inactive from Mar 2016)]

According to bug 1137722 comment 10, when we strictly prevent system messages from sending to app pages not registered in manifests, it would cause test failures on B2G emulators because the page URLs of those tests are not put in the manifest of the mochitest container app. So the Gaia patch is to whitelist those test pages.

However, the URLs of those test pages may end with query strings like 

?autorun=1&timeout=300&closeWhenDone=1&logFile=%2Fdata%2Flocal%2Ftests%2Flog%2Fmochitest.log&fileLevel=INFO&consoleLevel=INFO&hideResultsTable=1&dumpOutputDirectory=%2Ftmp&manifestFile=tests.json

Thus the patch also trims the tailing query string in the page URL registered for system messages. And it looks compatible with the actual usage, such as /dialer/index.html#keyboard-view, in Gaia apps.

Comment 6

[[:fabrice] Fabrice Desré]

I don't agree with doing that. It's perfectly valid to have different query strings for different messages with the same path. We need a better fix.

Also make sure that this addresses the issue from comment #0 which looks slightly different to me.

Comment 7

[Sean Lin [:seanlin] (inactive from Mar 2016)]

Attachment: Patch, v2

Prevent system messages from being sent to app pages not registered in manifests. And fix relevant tests. (In comment 0, the generated system message to fire alarms may still be sent to the alarm page even when the page URL hasn't been listed in the app manifest.)

Comment 8

[Sean Lin [:seanlin] (inactive from Mar 2016)]

Attachment: Patch, v3

Fixing two more requestsync tests.

Comment 9

[[:fabrice] Fabrice Desré]

Comment on attachment 8642276 [details] [diff] [review]
Patch, v3

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

Thanks!

Comment 10

[Sean Lin [:seanlin] (inactive from Mar 2016)]

Try run. FYI.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f563e0cc98b0

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f85097eaf255

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f85097eaf255