Oxide

Merge lp://qastaging/~osomon/oxide/prevent-concurrent-power-save-blocks into lp://qastaging/~oxide-developers/oxide/oxide.trunk

  1. prevent-concurrent-power-save-blocks
  2. Merge into oxide.trunk
Proposed by Olivier Tilloy
Status: Merged
Merged at revision: 1220
Proposed branch: lp://qastaging/~osomon/oxide/prevent-concurrent-power-save-blocks
Merge into: lp://qastaging/~oxide-developers/oxide/oxide.trunk
Diff against target: 40 lines (+11/-5)
1 file modified
shared/browser/oxide_power_save_blocker.cc (+11/-5)
To merge this branch: bzr merge lp://qastaging/~osomon/oxide/prevent-concurrent-power-save-blocks
Reviewer Review Type Date Requested Status
Chris Coulson Approve
Review via email: mp+274675@code.qastaging.launchpad.net

Commit message

Ensure that a power save block cannot be issued twice in a row.

To post a comment you must log in.
Revision history for this message
Chris Coulson (chrisccoulson) wrote :

I've left a comment inline. Also, there's a thread-safety issue with the current implementation which probably should be addressed here - unity_cookie_ is read on the UI thread and written on the file thread with no locking. The access on the UI thread should probably just be removed.

review: Needs Fixing
lp://qastaging/~osomon/oxide/prevent-concurrent-power-save-blocks updated
1217. By Olivier Tilloy

Remove an unneeded guard, because MessageLoop prevents reentry by default whilst executing a task.

1218. By Olivier Tilloy

Do not read unity_cookie_ on the UI thread, this wouldn’t be thread-safe (as it’s being written on the file thread),
and it’s useless anyway as the same check is performed later on the file thread.

Revision history for this message
Olivier Tilloy (osomon) wrote :

Thanks for the review Chris.
I wasn’t aware that MessageLoop prevents reentry by default (it totally makes sense, I should have checked before introducing this useless guard).
Updated to fix both comments.

Revision history for this message
Chris Coulson (chrisccoulson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches