As smart dim model is enabled in experiments only, we will also need to check the experiment flag UserActivityPrediction (https://cs.chromium.org/chromium/src/chrome/common/chrome_features.cc?g=0&l=589) to decide whether to change the factor to 1. Specifically, we change the factor to 1, iff 
(i). user profile pref has power.smart_dim_enabled set to true, and
(ii). the experiment is enabled.

I think it's ok to check the flag in //src/ash/system/power/power_prefs.cc or //src/chromeos/dbus/power_policy_controller.cc, but I haven't seen anyone checking flags in these prefs files. I guess it's because these prefs should not be dependent on flags?

Alternatively, what do you think if we let the model decide whether to set the factor to 1 and notify powerd?

Thanks

All power management policy goes through PowerPolicyController, so trying to notify powerd from somewhere else isn't really an option.

By "checking flags" do you mean checking features? I think it's probably okay to check them from anywhere, but you may have trouble checking the power.smart_dim_enabled pref while you're registering another one.

How about adding power.smart_dim_enabled to the prefs struct that gets passed into PowerPolicyController and putting the logic there? It feels like a natural fit since that file is already responsible for consolidating prefs into the protobuf.

Thanks a lot, this SGTM.

The feature is declared in //src/chrome/common/chrome_features.h. This means we need to add dep on chrome/common:chrome_features to either //src/chromeos/BUILD.gn or //src/ash/BUILD.gn. 

I don't see either BUILD.gn has any dep on chrome/, and I suspect dep isn't allowed or at least not preferred as it'd cause circular dep. 

I think there are two solutions

1. Always set factor=1 if profile has smart dim enabled, regardless if the user has been put on to the experiment group.
2. Let the model set delay factor via PowerManagerClient.

(2) would require a lot of modifications. I think (1) is probably better, especially considering we're only running experiments for a limited period of time. What do you think?

I can think of a few more options:

- Move the feature definition to somewhere under //chromeos so that //chromeos/dbus can check it.

- Add a bool field to PowerPolicyController::PrefValue describing the state of the feature (even though it technically isn't a pref) so that PPC can check it.

I don't have a strong preference between those.

Thanks a lot, I've prepared a cl that moves the feature to chromeos and check the field in PPC.

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

commit 5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf
Author: Jia <jiameng@chromium.org>
Date: Wed Sep 26 00:22:19 2018

Disable idle delay doubling when smart dim is enabled.

In order to check if an experiment is enabled from //chromeos, this CL
also moves the feature flag from //chrome/common/chrome_features to
//chromeos/chromeos_features.

Bug:  888392 
Change-Id: I05c6f7f096c8f1ed98e867ac147da3607c043776
Reviewed-on: https://chromium-review.googlesource.com/1242253
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Jia Meng <jiameng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594157}
[modify] https://crrev.com/5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf/ash/system/power/power_prefs.cc
[modify] https://crrev.com/5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf/chrome/browser/chromeos/power/ml/smart_dim/BUILD.gn
[modify] https://crrev.com/5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf/chrome/browser/chromeos/power/ml/smart_dim/model_impl.cc
[modify] https://crrev.com/5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf/chrome/browser/chromeos/power/ml/smart_dim/model_unittest.cc
[modify] https://crrev.com/5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf/chrome/browser/chromeos/power/ml/user_activity_manager.cc
[modify] https://crrev.com/5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf/chrome/browser/chromeos/power/ml/user_activity_manager_unittest.cc
[modify] https://crrev.com/5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf/chrome/common/chrome_features.cc
[modify] https://crrev.com/5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf/chrome/common/chrome_features.h
[modify] https://crrev.com/5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf/chromeos/chromeos_features.cc
[modify] https://crrev.com/5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf/chromeos/chromeos_features.h
[modify] https://crrev.com/5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf/chromeos/dbus/DEPS
[modify] https://crrev.com/5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf/chromeos/dbus/power_policy_controller.cc
[modify] https://crrev.com/5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf/chromeos/dbus/power_policy_controller.h
[modify] https://crrev.com/5f9ed29fc5a55d0c3614c73d88f099d1ea4f48bf/chromeos/dbus/power_policy_controller_unittest.cc

I've tested it on my machine and it is working as intended now. Hence closing the ticket. Thanks.