Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(8)

Issue 2881143003: Convert //content/browser/media to be clients of WakeLock mojo service. (Closed)

Created:
3 years, 7 months ago by ke.he
Modified:
3 years, 7 months ago
Reviewers:
Sergey Ulanov
CC:
chromium-reviews, chfremer+watch_chromium.org, jam, feature-media-reviews_chromium.org, darin-cc_chromium.org, xjz+watch_chromium.org, mfoltz+watch_chromium.org, miu+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Convert clients of PowerSaveBlocker to be clients of WakeLock mojo service. Wake Lock is a Mojo interface that wraps PowerSaveBlocker, As part of the creation of standalone Device Service, all browser-side clients of PowerSaveBlocker should be converted to be clients of WakeLock mojo interface instead. This CL converts clients of PowerSaveBlocker in //content/browser/media/ BUG=689410 Review-Url: https://codereview.chromium.org/2881143003 Cr-Commit-Position: refs/heads/master@{#472967} Committed: https://chromium.googlesource.com/chromium/src/+/bfff87457dc144fb7cc3d3a34d6c21c43369f867

Patch Set 1 #

Patch Set 2 : Convert clients of PowerSaveBlocker to be clients of WakeLock mojo service. #

Patch Set 3 : clone() should happen in UI threads. #

Total comments: 19

Patch Set 4 : Addressed Sergeyu@'s comments. #

Patch Set 5 : wake lock bindinterface should only be executed once. #

Total comments: 4

Patch Set 6 : Revert patchset 5, add DCHECK(!wake_lock_). #

Patch Set 7 : code rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+70 lines, -27 lines) Patch
M content/browser/media/capture/aura_window_capture_machine.h View 1 2 3 4 5 6 3 chunks +3 lines, -6 lines 0 comments Download
M content/browser/media/capture/aura_window_capture_machine.cc View 1 2 3 4 5 6 3 chunks +23 lines, -8 lines 0 comments Download
M content/browser/media/capture/desktop_capture_device.cc View 1 2 3 4 5 6 8 chunks +44 lines, -13 lines 0 comments Download

Messages

Total messages: 52 (44 generated)
ke.he
Hi, Sergeyu@, PTAL. thanks.
3 years, 7 months ago (2017-05-16 10:04:38 UTC) #15
Do not use (sergeyu)
https://codereview.chromium.org/2881143003/diff/40001/content/browser/media/capture/aura_window_capture_machine.cc File content/browser/media/capture/aura_window_capture_machine.cc (right): https://codereview.chromium.org/2881143003/diff/40001/content/browser/media/capture/aura_window_capture_machine.cc#newcode99 content/browser/media/capture/aura_window_capture_machine.cc:99: connector->BindInterface(device::mojom::kServiceName, Can this operation fail? https://codereview.chromium.org/2881143003/diff/40001/content/browser/media/capture/aura_window_capture_machine.cc#newcode104 content/browser/media/capture/aura_window_capture_machine.cc:104: "DesktopCaptureDevice is ...
3 years, 7 months ago (2017-05-16 19:29:44 UTC) #17
ke.he
Hi, Sergey, thanks very much! Besides your comments, I also make changes to make sure ...
3 years, 7 months ago (2017-05-17 10:25:52 UTC) #29
Sergey Ulanov
https://codereview.chromium.org/2881143003/diff/40001/content/browser/media/capture/desktop_capture_device.cc File content/browser/media/capture/desktop_capture_device.cc (left): https://codereview.chromium.org/2881143003/diff/40001/content/browser/media/capture/desktop_capture_device.cc#oldcode143 content/browser/media/capture/desktop_capture_device.cc:143: // TODO(jiayl): Remove power_save_blocker_ when there is an API ...
3 years, 7 months ago (2017-05-17 18:47:55 UTC) #32
ke.he
Sergey, Thanks! CL is updated. PTAL. https://codereview.chromium.org/2881143003/diff/40001/content/browser/media/capture/desktop_capture_device.cc File content/browser/media/capture/desktop_capture_device.cc (left): https://codereview.chromium.org/2881143003/diff/40001/content/browser/media/capture/desktop_capture_device.cc#oldcode143 content/browser/media/capture/desktop_capture_device.cc:143: // TODO(jiayl): Remove ...
3 years, 7 months ago (2017-05-18 05:32:58 UTC) #44
Sergey Ulanov
lgtm
3 years, 7 months ago (2017-05-18 17:16:57 UTC) #47
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2881143003/140001
3 years, 7 months ago (2017-05-18 23:14:44 UTC) #49
commit-bot: I haz the power
3 years, 7 months ago (2017-05-18 23:30:43 UTC) #52
Message was sent while issue was closed.
Committed patchset #7 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/bfff87457dc144fb7cc3d3a34d6c...

Powered by Google App Engine
This is Rietveld 408576698