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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Justin Lebar (not reading bugmail)]

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.  :)

Comment 1

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v1

Comment 2

[Justin Lebar (not reading bugmail)]

Blocks a blocker.

Comment 3

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/projects/birch/rev/aee46daa2262

Comment 4

[Ryan VanderMeulen [:RyanVM]]

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

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/3db526e95362
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/78de4587a287

Comment 6

[Deepa Subramanian]

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

Comment 7

[Deepa Subramanian]

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