New issue
Advanced search Search tips

Issue 780677 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 702449



Sign in to add a comment

Change mojo conversion helpers in services/device/wake_lock/wake_lock.cc to use TypeConverter template

Project Member Reported by dcheng@chromium.org, Nov 2 2017

Issue description

This helps us audit where things are being converted to/from Mojo types. Another good alternative is to just use Mojo types all the way through, but I'm not sure how hard that'd be here.
 

Comment 1 by derat@chromium.org, Nov 3 2017

Status: Started (was: Assigned)
I think that PowerSaveBlocker can just use the Mojo enums. I've sent https://crrev.com/c/754169 to do that.
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c2e8ef036e4661ff593d4b3431244d52d4e1026e

commit c2e8ef036e4661ff593d4b3431244d52d4e1026e
Author: Daniel Erat <derat@chromium.org>
Date: Sat Nov 04 01:06:13 2017

Remove redundant device::PowerSaveBlocker enums.

Make device::PowerSaveBlocker just use
device::mojom::WakeLockType and
device::mojom::WakeLockReason rather than converting to its
own PowerSaveBlockerType and Reason enums.

Bug: 780677
Change-Id: I84ee0be57a6e631f6c2134f9d9488099d1010984
Reviewed-on: https://chromium-review.googlesource.com/754169
Reviewed-by: Zijie He <zijiehe@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514000}
[modify] https://crrev.com/c2e8ef036e4661ff593d4b3431244d52d4e1026e/chromeos/dbus/power_policy_controller.h
[modify] https://crrev.com/c2e8ef036e4661ff593d4b3431244d52d4e1026e/remoting/host/DEPS
[modify] https://crrev.com/c2e8ef036e4661ff593d4b3431244d52d4e1026e/remoting/host/host_power_save_blocker.cc
[modify] https://crrev.com/c2e8ef036e4661ff593d4b3431244d52d4e1026e/remoting/host/host_power_save_blocker.h
[modify] https://crrev.com/c2e8ef036e4661ff593d4b3431244d52d4e1026e/services/device/wake_lock/power_save_blocker/BUILD.gn
[modify] https://crrev.com/c2e8ef036e4661ff593d4b3431244d52d4e1026e/services/device/wake_lock/power_save_blocker/DEPS
[modify] https://crrev.com/c2e8ef036e4661ff593d4b3431244d52d4e1026e/services/device/wake_lock/power_save_blocker/power_save_blocker.h
[modify] https://crrev.com/c2e8ef036e4661ff593d4b3431244d52d4e1026e/services/device/wake_lock/power_save_blocker/power_save_blocker_android.cc
[modify] https://crrev.com/c2e8ef036e4661ff593d4b3431244d52d4e1026e/services/device/wake_lock/power_save_blocker/power_save_blocker_chromeos.cc
[modify] https://crrev.com/c2e8ef036e4661ff593d4b3431244d52d4e1026e/services/device/wake_lock/power_save_blocker/power_save_blocker_mac.cc
[modify] https://crrev.com/c2e8ef036e4661ff593d4b3431244d52d4e1026e/services/device/wake_lock/power_save_blocker/power_save_blocker_ozone.cc
[modify] https://crrev.com/c2e8ef036e4661ff593d4b3431244d52d4e1026e/services/device/wake_lock/power_save_blocker/power_save_blocker_win.cc
[modify] https://crrev.com/c2e8ef036e4661ff593d4b3431244d52d4e1026e/services/device/wake_lock/power_save_blocker/power_save_blocker_x11.cc
[modify] https://crrev.com/c2e8ef036e4661ff593d4b3431244d52d4e1026e/services/device/wake_lock/wake_lock.cc

Comment 3 by derat@chromium.org, Nov 4 2017

Blockedon: 702449
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/28922ba386cbc4d88455aee948cc8bde10503ed3

commit 28922ba386cbc4d88455aee948cc8bde10503ed3
Author: Daniel Erat <derat@chromium.org>
Date: Wed Nov 08 16:18:26 2017

services/device: Fix enum naming in wake_lock.mojom.

Add 'k' prefix to values in device::mojom::WakeLockType and
WakeLockReason, and remove redundant "Reason" prefix in
device::mojom::WakeLockReason::kReason* values.

Bug: 780677
Change-Id: Id016c576b1a7366360ddce43c1657df587fd4340
Reviewed-on: https://chromium-review.googlesource.com/755884
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514851}
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/chrome/browser/media/cast_transport_host_filter.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/chrome/browser/renderer_host/pepper/pepper_flash_browser_host.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/chrome/browser/ui/webui/chromeos/login/encryption_migration_screen_handler.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/components/arc/power/arc_power_bridge.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/components/arc/power/arc_power_bridge_unittest.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/components/drive/drive_uploader.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/content/browser/devtools/render_frame_devtools_agent_host.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/content/browser/download/download_request_core.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/content/browser/loader/wake_lock_resource_throttle.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/content/browser/media/capture/aura_window_capture_machine.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/content/browser/media/capture/desktop_capture_device.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/content/browser/media/media_web_contents_observer.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/content/browser/webrtc/webrtc_internals.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/extensions/browser/api/power/power_api.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/extensions/browser/api/power/power_api_unittest.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/remoting/host/host_power_save_blocker.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/services/device/public/interfaces/wake_lock.mojom
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/services/device/wake_lock/power_save_blocker/power_save_blocker.h
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/services/device/wake_lock/power_save_blocker/power_save_blocker_chromeos.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/services/device/wake_lock/power_save_blocker/power_save_blocker_mac.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/services/device/wake_lock/power_save_blocker/power_save_blocker_win.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/services/device/wake_lock/power_save_blocker/power_save_blocker_x11.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/services/device/wake_lock/wake_lock.cc
[modify] https://crrev.com/28922ba386cbc4d88455aee948cc8bde10503ed3/services/device/wake_lock/wake_lock_unittest.cc

Comment 5 by derat@chromium.org, Jan 29 2018

Is this still something I should do? TypeConverter has a comment saying it's deprecated. I see mojo/public/cpp/bindings/enum_traits.h, but I'm not sure if it's the right tool for the job -- I believe that we currently only convert from mojom::WakeLockReason to chromeos::PowerPolicyController::WakeLockReason in services/device/wake_lock/power_save_blocker/power_save_blocker_chromeos.cc, and that we never convert in the opposite direction.

Sign in to add a comment