I am a broken record, but I strongly feel like issues like this should be fixed at as low a level as possible instead of worked around by adding more complexity at a higher layer. :-P

It sounds from the other bug like the cause of the suspend "failures" isn't fully understood, right? Is there a possibility that the failure can be detected in the kernel or below?

I'd rather not have powerd second-guess what the kernel tells it, due to the risk of false positives in the future (perhaps on hardware that isn't even susceptible to the issue that we'd be working around here).

Oh, I absolutely agree that this is a bug that "shouldn't happen" and needs to be investigated. But of course software isn't always perfect, especially when it comes to all the weird edge cases possible with resume signalling. Note that this isn't a regular occurrence on this board that I'm trying to paper over... we just have this one report of this one dogfood device that suspended and resumed in a tight loop for days (and we don't yet know why).

I'm just saying that I think this is exactly the same thing as the "suspend failures" that we already handle. Those shouldn't happen either, especially not 10 times in a row. Yet they sometimes do in the field, and then it's nice to not burn down a user's battery and fill their SSD to the brim with logs just because they ran into some freak driver bug that we've never seen before.

I think the differentiation should be a pretty safe bet here, since there's generally nothing that should give a Chromebook a (non-dark-resume) wakeup while the lid is closed. People probably won't open/close the lid in quick succession 10 times in a row.

Sorry for the incredibly slow reply here!

Is this still popping up? I'm willing to try to handle it more gracefully in powerd if we think it's actually likely to help users, but the code will probably be a bit on the ugly/hacky side so I'd prefer to avoid adding it otherwise.

I think I should look into doing this per issue 767053. Shutting down will be a poor user experience, but still better than heating up and draining the battery.

Uploaded https://crrev.com/c/784050 for review.

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

commit 154a5231cf7749a253e7922ed31ef746cc259512
Author: Daniel Erat <derat@chromium.org>
Date: Tue Nov 28 21:34:28 2017

power: Clean up time usage in suspend tests.

Update unit tests for powerd's policy::Suspender class to be
a bit clearer with regard to time. Add support for advancing
the clock for each suspend attempt instead of requiring
tests to register a callback that does this. Also
(hopefully) improve confusing dark resume wake data testing.

No functional changes.

BUG=chromium:624203
TEST=tests pass

Change-Id: I82fd2258f8732e9b07cbce7edb4a7235ed0c2ec3
Reviewed-on: https://chromium-review.googlesource.com/789804
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/154a5231cf7749a253e7922ed31ef746cc259512/power_manager/common/clock.h
[modify] https://crrev.com/154a5231cf7749a253e7922ed31ef746cc259512/power_manager/powerd/policy/suspender.cc
[modify] https://crrev.com/154a5231cf7749a253e7922ed31ef746cc259512/power_manager/powerd/policy/suspender.h
[modify] https://crrev.com/154a5231cf7749a253e7922ed31ef746cc259512/power_manager/powerd/policy/suspender_unittest.cc

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

commit 87ef87790a2a24172e997d48ae548b41b5af18e9
Author: Daniel Erat <derat@chromium.org>
Date: Sat Dec 02 01:57:56 2017

power: Refactor Suspender::Suspend.

Break up powerd's policy::Suspender class's Suspend() method
into smaller pieces.

Also update lockfile paths in docs/suspend_resume.md.

BUG=chromium:624203,chromium:790898
TEST=existing unit tests pass; also verified that normal
     suspend/resume works as before

Change-Id: Ib9ef3204041c850e96301497d9e65f1d7ff76eda
Reviewed-on: https://chromium-review.googlesource.com/798640
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>

[modify] https://crrev.com/87ef87790a2a24172e997d48ae548b41b5af18e9/power_manager/powerd/policy/suspender.cc
[modify] https://crrev.com/87ef87790a2a24172e997d48ae548b41b5af18e9/power_manager/docs/suspend_resume.md
[modify] https://crrev.com/87ef87790a2a24172e997d48ae548b41b5af18e9/power_manager/powerd/policy/suspender.h