Closed Bug 959897 Opened 10 years ago Closed 10 years ago

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

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

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

RESOLVED WONTFIX
Tracking Status
b2g-v1.2 --- affected
b2g-v1.3 --- fixed
b2g-v1.4 --- fixed

People

(Reporter: bkelly, Unassigned)

Details

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.
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.
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
Closed: 10 years ago
Resolution: --- → WONTFIX
(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.