[b2g][system][v1.2] all apps get mozapptype set to 'critical'

RESOLVED WONTFIX

Status

RESOLVED WONTFIX
5 years ago
5 years ago

People

(Reporter: bkelly, Unassigned)

Tracking

unspecified
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(b2g-v1.2 affected, b2g-v1.3 fixed, b2g-v1.4 fixed)

Details

(Reporter)

Description

5 years ago
While investigating bug 957346 I discovered that v1.2 has a bug which results in the mozapptype being incorrectly set to 'critical' for all apps.  This is caused by by this code:

  https://github.com/mozilla-b2g/gaia/blob/v1.2/apps/system/js/browser_frame.js#L69

Specifically, the following function call will always return true:

      config.url.startsWith(window.location.protocol +
                              '//communications.gaiamobile.org' +
                              window.location.port ?
                              '' : (':' + window.location.port) +
                              '/dialer')

This is due to order of operations on the inline conditional.  The intention was for |window.location.port| to be used in the conditional statement to control if the port is appended or not.  Unfortunately, though, the string concatenation binds more tightly and the condition triggers on the entire |window.location.protocol + '//communications.gaiamobile.org' + window.location.port|.  This of course is truthy which results in |config.url.startsWith('')| being executed.  This will always return true.

This bug was fixed as a side effect of bug 905116 with this commit:

  https://github.com/mozilla-b2g/gaia/commit/afd555ae68dd12429bd9162d7826b8329f251f90

I'm writing this bug in case we want to fix this on v1.2 before its frozen.  Also, it may be useful information when diagnosing differences between v1.2 and v1.3.
(Reporter)

Comment 1

5 years ago
My personal opinion is that it would be dangerous to try fixing this in v1.2 now.  It would potentially change how OOM killing performs which in turn might suddenly make a lot of test cases behave differently.
(Reporter)

Comment 2

5 years ago
Alive, what do you think?  Should we try to fix the setting of mozapptype in v1.2?
Flags: needinfo?(alive)
I don't think we can fix this at this point - we're done with 1.2 & are not planning to ship this branch to any operator.
What Jason said. :(
Flags: needinfo?(alive)
The side effect of this bug is the process priority would be the same -- as callscreen.
Moving to WONT FIX.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
(Reporter)

Comment 7

5 years ago
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #5)
> The side effect of this bug is the process priority would be the same -- as
> callscreen.

I think we just need to keep this in mind when debugging background OOM kill "regressions" compared to v1.2.
You need to log in before you can comment on or make changes to this bug.