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

Issue 2874013002: Add GetWakeLockWithoutContext() in WakeLockContextProvider mojo interface. (Closed)

Created:
3 years, 7 months ago by ke.he
Modified:
3 years, 7 months ago
CC:
chromium-reviews, David Trainor- moved to gerrit, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, jam, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin-cc_chromium.org, darin (slow to review)
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Add GetWakeLockWithoutContext() in WakeLockContextProvider mojo interface. 1) Add GetWakeLockWithoutContext in WakeLockContextProvider. 2) Rename WakeLockContextProvider to WakeLockProvider as it provides both WakeLockService and WakeLockContext. 3) Convert download_request_core to be client of wakelock. BUG=689410 TBR=jam@chromium.org, for mechanical changes in //content/browser/wake_lock/wake_lock_context_host{.h|.cc} Review-Url: https://codereview.chromium.org/2874013002 Cr-Commit-Position: refs/heads/master@{#471629} Committed: https://chromium.googlesource.com/chromium/src/+/2d8b867dfc360643d87634931521e9c5a39594fc

Patch Set 1 #

Patch Set 2 : code rebase #

Patch Set 3 : Add GetUnassociatedWakeLockContext in WakeLockContextProvider mojo interface. #

Total comments: 5

Patch Set 4 : GetUnassociatedWakeLockContext -> GetWakeLockWithoutContext. #

Patch Set 5 : Add GetWakeLockWithoutContext in WakeLockContextProvider mojo interface. #

Total comments: 12

Patch Set 6 : Addressed Colin's comments. #

Total comments: 2

Patch Set 7 : Add GetWakeLockWithoutContext in WakeLockContextProvider mojo interface. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+128 lines, -154 lines) Patch
M content/browser/download/download_request_core.h View 1 2 3 4 5 2 chunks +5 lines, -8 lines 0 comments Download
M content/browser/download/download_request_core.cc View 1 2 3 4 5 6 4 chunks +22 lines, -6 lines 0 comments Download
M content/browser/wake_lock/wake_lock_context_host.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M content/browser/wake_lock/wake_lock_context_host.cc View 1 2 3 2 chunks +4 lines, -4 lines 0 comments Download
M device/wake_lock/BUILD.gn View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M device/wake_lock/public/interfaces/BUILD.gn View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M device/wake_lock/public/interfaces/README.md View 1 2 3 4 5 2 chunks +6 lines, -1 line 0 comments Download
M device/wake_lock/public/interfaces/wake_lock_context.mojom View 1 2 3 1 chunk +4 lines, -1 line 0 comments Download
M device/wake_lock/public/interfaces/wake_lock_context_provider.mojom View 1 2 3 1 chunk +0 lines, -15 lines 0 comments Download
A device/wake_lock/public/interfaces/wake_lock_provider.mojom View 1 2 3 1 chunk +25 lines, -0 lines 0 comments Download
M device/wake_lock/wake_lock_context_provider.h View 1 2 3 1 chunk +0 lines, -44 lines 0 comments Download
M device/wake_lock/wake_lock_context_provider.cc View 1 2 3 1 chunk +0 lines, -40 lines 0 comments Download
A + device/wake_lock/wake_lock_provider.h View 1 2 3 2 chunks +17 lines, -12 lines 0 comments Download
A + device/wake_lock/wake_lock_provider.cc View 1 2 3 1 chunk +20 lines, -7 lines 0 comments Download
M device/wake_lock/wake_lock_service_context.h View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M device/wake_lock/wake_lock_service_context.cc View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M device/wake_lock/wake_lock_service_impl.cc View 1 2 3 4 5 1 chunk +6 lines, -0 lines 0 comments Download
M services/device/device_service.h View 1 2 3 2 chunks +3 lines, -3 lines 0 comments Download
M services/device/device_service.cc View 1 2 3 3 chunks +7 lines, -8 lines 0 comments Download
M services/device/manifest.json View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 61 (46 generated)
ke.he
Colin, PTAL. Not sure should I split the download client into a separate CL? Thanks.
3 years, 7 months ago (2017-05-11 12:39:47 UTC) #15
blundell
Thanks! https://codereview.chromium.org/2874013002/diff/40001/device/wake_lock/public/interfaces/wake_lock_context_provider.mojom File device/wake_lock/public/interfaces/wake_lock_context_provider.mojom (right): https://codereview.chromium.org/2874013002/diff/40001/device/wake_lock/public/interfaces/wake_lock_context_provider.mojom#newcode13 device/wake_lock/public/interfaces/wake_lock_context_provider.mojom:13: // Android (see WakeLockContextCallback). Let's note that context_ids ...
3 years, 7 months ago (2017-05-11 15:42:40 UTC) #16
ke.he
https://codereview.chromium.org/2874013002/diff/40001/device/wake_lock/public/interfaces/wake_lock_context_provider.mojom File device/wake_lock/public/interfaces/wake_lock_context_provider.mojom (right): https://codereview.chromium.org/2874013002/diff/40001/device/wake_lock/public/interfaces/wake_lock_context_provider.mojom#newcode17 device/wake_lock/public/interfaces/wake_lock_context_provider.mojom:17: GetUnassociatedWakeLockContext(WakeLockContext& context); On 2017/05/11 15:42:40, blundell wrote: > Looking ...
3 years, 7 months ago (2017-05-11 16:08:05 UTC) #17
blundell
On 2017/05/11 16:08:05, ke.he wrote: > https://codereview.chromium.org/2874013002/diff/40001/device/wake_lock/public/interfaces/wake_lock_context_provider.mojom > File device/wake_lock/public/interfaces/wake_lock_context_provider.mojom > (right): > > https://codereview.chromium.org/2874013002/diff/40001/device/wake_lock/public/interfaces/wake_lock_context_provider.mojom#newcode17 ...
3 years, 7 months ago (2017-05-11 16:13:26 UTC) #18
ke.he
Colin, PTAL. Thanks. https://codereview.chromium.org/2874013002/diff/40001/device/wake_lock/public/interfaces/wake_lock_context_provider.mojom File device/wake_lock/public/interfaces/wake_lock_context_provider.mojom (right): https://codereview.chromium.org/2874013002/diff/40001/device/wake_lock/public/interfaces/wake_lock_context_provider.mojom#newcode13 device/wake_lock/public/interfaces/wake_lock_context_provider.mojom:13: // Android (see WakeLockContextCallback). On 2017/05/11 ...
3 years, 7 months ago (2017-05-12 12:24:24 UTC) #36
blundell
LGTM! https://codereview.chromium.org/2874013002/diff/80001/content/browser/download/download_request_core.cc File content/browser/download/download_request_core.cc (right): https://codereview.chromium.org/2874013002/diff/80001/content/browser/download/download_request_core.cc#newcode35 content/browser/download/download_request_core.cc:35: #include "device/wake_lock/public/interfaces/wake_lock_context.mojom.h" Not needed now. https://codereview.chromium.org/2874013002/diff/80001/content/browser/download/download_request_core.cc#newcode207 content/browser/download/download_request_core.cc:207: if ...
3 years, 7 months ago (2017-05-12 12:42:18 UTC) #37
ke.he
> https://codereview.chromium.org/2874013002/diff/80001/device/wake_lock/wake_lock_service_impl.cc > File device/wake_lock/wake_lock_service_impl.cc (right): > > https://codereview.chromium.org/2874013002/diff/80001/device/wake_lock/wake_lock_service_impl.cc#newcode137 > device/wake_lock/wake_lock_service_impl.cc:137: return; > Maybe we ...
3 years, 7 months ago (2017-05-12 15:32:43 UTC) #38
ke.he
On 2017/05/12 15:32:43, ke.he wrote: > > > https://codereview.chromium.org/2874013002/diff/80001/device/wake_lock/wake_lock_service_impl.cc > > File device/wake_lock/wake_lock_service_impl.cc (right): > ...
3 years, 7 months ago (2017-05-12 15:53:18 UTC) #39
ke.he
Hi, Dtrainor@, PTAL on the //content/browser/download. TOM, PTAL on the mojom files. Thanks! https://codereview.chromium.org/2874013002/diff/80001/content/browser/download/download_request_core.cc File ...
3 years, 7 months ago (2017-05-12 16:09:39 UTC) #43
Tom Sepez
lgtm
3 years, 7 months ago (2017-05-12 16:24:39 UTC) #46
jam
lgtm
3 years, 7 months ago (2017-05-12 19:52:22 UTC) #49
David Trainor- moved to gerrit
content/browser/download lgtm % nit https://codereview.chromium.org/2874013002/diff/100001/content/browser/download/download_request_core.cc File content/browser/download/download_request_core.cc (right): https://codereview.chromium.org/2874013002/diff/100001/content/browser/download/download_request_core.cc#newcode206 content/browser/download/download_request_core.cc:206: // |connector| might be nullptr ...
3 years, 7 months ago (2017-05-14 05:52:40 UTC) #50
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/2874013002/120001
3 years, 7 months ago (2017-05-14 12:04:23 UTC) #57
ke.he
Thanks. https://codereview.chromium.org/2874013002/diff/100001/content/browser/download/download_request_core.cc File content/browser/download/download_request_core.cc (right): https://codereview.chromium.org/2874013002/diff/100001/content/browser/download/download_request_core.cc#newcode206 content/browser/download/download_request_core.cc:206: // |connector| might be nullptr in some testing ...
3 years, 7 months ago (2017-05-14 12:05:00 UTC) #58
commit-bot: I haz the power
3 years, 7 months ago (2017-05-14 12:10:00 UTC) #61
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/2d8b867dfc360643d87634931521...

Powered by Google App Engine
This is Rietveld 408576698