Do we know how many devices have this battery status? 

chromebox and chromebase specifically are expected to (fievel, tiger).
received report from two devices that had broken power reporting.

In theory if they have a broken battery it needs to be plugged in, right? I'm ok with migrating these devices as you describe above.

OK, I'll update the UI to start migration anyway when battery does not exist or work.

This should have been fixed as a part of issue 723103.

I'll close this once I confirm that the migration completes on a device which does not have battery.

did you get around to verify?

I haven't verified it using a real device.
Can we run N on fievel?

No need for N, just enabling dircrypto on the device should be sufficient for testing.

I tested it with Fievel with direncryption enabled.
The migration did not start, since the battery percent is reported as -1% and wait it forever.

+derat@:
In the power status update callback, has_battery_percent() returns true even when the battery is not present.
https://codesearch.chromium.org/chromium/src/chrome/browser/ui/webui/chromeos/login/encryption_migration_screen_handler.cc?rcl=d5c6448dc428ce9bb22cc14f8b45bf929cebadd6&l=371
Is this WAI?

We can handle this situation since battery_status() returns PowerSupplyProperties_BatteryState_NOT_PRESENT as expected, but I'd like to confirm the expected behavior just in case.

I agree that it'd be cleaner for powerd to not set battery_percent in the protobuf if the battery isn't present. But yes, callers should check battery_status before they try to use the percent. :-)

I have a powerd change to not set other battery-related fields when battery_state is PowerSupplyProperties_BatteryState_NOT_PRESENT, but I'm a bit anxious since I suspect that some code that consumes these protobufs reads these fields without checking that the battery is there.

Looking through Chrome:

In ash/system/power/power_status.cc, various PowerStatus methods appear to read battery fields unconditionally.

In chrome/browser/chromeos/power/power_data_collector.cc, PowerDataCollector::PowerChanged reads battery_percent and battery_discharge_rate unconditionally.

In chrome/browser/ui/webui/chromeos/emulator/device_emulator_message_handler.cc, DeviceEmulatorMessageHandler::PowerObserver::PowerChanged reads battery_percent and battery_time_to_*_sec unconditionally.

I think I'll probably set default values for these fields that match what powerd is currently setting and then change powerd to not actually set the fields. Sound reasonable?

I'm marking this bug M-62 since M-62 is possibly the first version to migrate a batteryless device.

Re: #13
The approach sounds good to me.

I understand the new behavior as follows:
If the battery does not present, has_battery_percent() returns false and battery_percent() returns -1.
Please correct me if I'm wrong. Thanks!

#15: Yes, that's correct.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/87d717a9e67756a26a49c4c23592d88760d11a5a

commit 87d717a9e67756a26a49c4c23592d88760d11a5a
Author: Daniel Erat <derat@chromium.org>
Date: Sun Aug 06 05:07:57 2017

system_api: battery_percent default in PowerSupplyProperties

Update powerd's PowerSupplyProperties protocol buffer to
specify a default of -1.0 for its battery_percent field.
powerd has historically set this field to -1 when a battery
isn't present, but it's probably more correct to leave
battery-related fields unset in this case. Chrome reads
these fields unconditionally in some cases, though, so we
want to maintain the same user-facing behavior.

BUG= chromium:724903 
TEST=none

Change-Id: I7cf0b9e46e7e90742185c7785a9b2133cd6af1ff
Reviewed-on: https://chromium-review.googlesource.com/598512
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>

[modify] https://crrev.com/87d717a9e67756a26a49c4c23592d88760d11a5a/dbus/power_manager/power_supply_properties.proto

With the CL in comment 17, is this now effectively fixed for tiger and fievel?

#18: No. I still need to:

a) Update Chrome to use an updated revision of system_api that includes the change from #17.
b) Wait around a week for the Chrome PFQ to pull an updated version of Chrome into the OS.
c) Update powerd to stop setting battery-related fields when the battery is missing (https://crrev.com/c/599504).

I'd recommend that someone (fukino@?) also make the Chrome change described in #11. That'll be faster and also feels like the correct thing for Chrome to be doing.

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

commit e5c2edced0ebeb814574f4e6fa8f3c3e60cb6e89
Author: Daniel Erat <derat@chromium.org>
Date: Tue Aug 08 00:37:39 2017

Roll src/third_party/cros_system_api/ 3eac6aef2..87d717a9e (2 commits)

https://chromium.googlesource.com/chromiumos/platform/system_api.git/+log/3eac6aef29eb..87d717a9e677

$ git log 3eac6aef2..87d717a9e --date=short --no-merges --format='%ad %ae %s'
2017-08-02 derat system_api: battery_percent default in PowerSupplyProperties
2017-08-03 isandrk system_api: Add kCryptohomeTpmGetVersion.

Created with:
  roll-dep src/third_party/cros_system_api

BUG= 724903 

Change-Id: Ib0702a200d31caae7527af9f638c619074b6383c
Reviewed-on: https://chromium-review.googlesource.com/604393
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Commit-Queue: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492472}
[modify] https://crrev.com/e5c2edced0ebeb814574f4e6fa8f3c3e60cb6e89/DEPS

So it seems like after we land https://chromium-review.googlesource.com/#/c/chromiumos/platform2/+/599504/ we should be clear?

Maybe, but I still feel pretty strongly about this: "I'd recommend that someone (fukino@?) also make the Chrome change described in #11. That'll be faster and also feels like the correct thing for Chrome to be doing."

Is that going to happen?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/ca495c6af9dbf80d16b1802564b1f40fc5d4ed6d

commit ca495c6af9dbf80d16b1802564b1f40fc5d4ed6d
Author: Daniel Erat <derat@chromium.org>
Date: Tue Aug 15 21:54:13 2017

power: Don't set battery protobuf fields when not present.

Make powerd avoid setting the battery_percent,
battery_time_to_empty_sec, battery_time_to_full_sec,
is_calculating_battery_time, and battery_discharge_rate
fields in the PowerSupplyProperties protocol buffers that it
emits when a battery isn't present.

BUG= chromium:724903 
TEST=added test
CQ-DEPEND=I7cf0b9e46e7e90742185c7785a9b2133cd6af1ff

Change-Id: I763b12238e148b86d59685cb131e7815fa06ee2e
Reviewed-on: https://chromium-review.googlesource.com/599504
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>

[modify] https://crrev.com/ca495c6af9dbf80d16b1802564b1f40fc5d4ed6d/power_manager/powerd/system/power_supply.cc
[modify] https://crrev.com/ca495c6af9dbf80d16b1802564b1f40fc5d4ed6d/power_manager/powerd/system/power_supply_unittest.cc

Naoki-san, are you looking at implementing what is described in comment 11 and requested in comment 23?

Can anyone confirm that after comment 24 this bug is at least worked around? 

I'm going to implement the behavior in #11 as requested in #23.
Commit #24 might already have worked around the issue, but I haven't confirmed yet. (I will check it tomorrow)

I confirmed that the migration starts and finishes on fievel, with the commit #24.

== How it was tested ==
1) Patch chrome os to use direncryption by https://chrome-internal-review.googlesource.com/#/c/chromeos/overlays/overlay-variant-veyron-fievel-private/+/423908/
2) Build chrome os test image and flash it to fievel
3) Edit /etc/init/cryptohomed.conf to disable direncryption and reboot
4) Create a profile
5) Edit /etc/init/cryptohomed.conf to enable direncryption again and reboot
6) Try to log in to the created profile
7) On migration screen, press "Update"

The implementation mentioned in comment #11 is nice to have, but it is not a blocker to enable migration on fievel anymore.