Don't send the frameloader-visibility-changed notification when the frameloader's visibility didn't actually change

RESOLVED FIXED
(NeedInfo fromanyone)

Status

Firefox OS
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Justin Lebar (not reading bugmail), Assigned: Justin Lebar (not reading bugmail), NeedInfo)

Tracking

unspecified

Firefox Tracking Flags

(blocking-b2g:tef+, firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
The patch will make clear what I'm doing; it's simply a question of not firing an observer notification if JS does

  var x = frameLoader.visible;
  frameLoader.visible = x;

The interesting part is, why do we care?

The reason is that when the ParticularProcessPriorityManager gets this notification, it immediately resets the process's priority.  We have grace periods baked in to the code, but this notification bypasses the grace period.

But if we end up setting frameLoader.visible to itself, we end up skirting this grace period when we really shouldn't be, because no visibility change has happened.  And in fact we do set frameLoader.visible to itself, after a round-trip between the parent and child.  It's a recipe for race conditions.

Anyway if you don't follow that (I barely do), at least you'll believe that this patch leaves us no worse than we were.  :)
(Assignee)

Comment 1

5 years ago
Created attachment 747660 [details] [diff] [review]
Patch, v1
Attachment #747660 - Flags: review?(bent.mozilla)
(Assignee)

Comment 2

5 years ago
Blocks a blocker.
Blocks: 847592
blocking-b2g: --- → tef+
(Assignee)

Updated

5 years ago
Assignee: nobody → justin.lebar+bug
(Assignee)

Updated

5 years ago
Attachment #747660 - Flags: review?(khuey)
(Assignee)

Updated

5 years ago
Attachment #747660 - Flags: review?(bent.mozilla)
https://hg.mozilla.org/mozilla-central/rev/aee46daa2262
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-b2g18/rev/3db526e95362
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/78de4587a287
status-b2g18: --- → fixed
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → fixed
status-firefox21: --- → wontfix
status-firefox22: --- → wontfix
status-firefox23: --- → fixed

Comment 6

5 years ago
Can you please provide steps to verify this fix - as we can perform blackbox testing from the UI?

Comment 7

5 years ago
Can you please provide steps to verify this fix - as we can perform blackbox testing from the UI?
Flags: needinfo?
You need to log in before you can comment on or make changes to this bug.