Closed Bug 870598 Opened 12 years ago Closed 12 years ago

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

Categories

(Firefox OS Graveyard :: General, defect)

defect
Not set
normal

Tracking

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

RESOLVED FIXED
blocking-b2g tef+
Tracking Status
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed

People

(Reporter: justin.lebar+bug, Assigned: justin.lebar+bug, NeedInfo)

References

Details

Attachments

(1 file)

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. :)
Attached patch Patch, v1 (deleted) — Splinter Review
Attachment #747660 - Flags: review?(bent.mozilla)
Blocks a blocker.
Blocks: 847592
blocking-b2g: --- → tef+
Assignee: nobody → justin.lebar+bug
Attachment #747660 - Flags: review?(khuey)
Attachment #747660 - Flags: review?(bent.mozilla)
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Can you please provide steps to verify this fix - as we can perform blackbox testing from the UI?
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.

Attachment

General

Created:
Updated:
Size: