Fix Volume slider is captured in screenshot done in touchview mode

chromeos/audio/cras_audio_handler.cc

Index: chromeos/audio/cras_audio_handler.cc
diff --git a/chromeos/audio/cras_audio_handler.cc b/chromeos/audio/cras_audio_handler.cc
index a8f8036134ead71310d5a6748f4230045971f1f3..215d3be9698f321014d0c6989bfa9a485d561e12 100644
--- a/chromeos/audio/cras_audio_handler.cc
+++ b/chromeos/audio/cras_audio_handler.cc
@@ -25,6 +25,11 @@ namespace chromeos {
 
 namespace {
 
+enum NonUserVolumeChangeReason {

[Daniel Erat, 2016/07/28 23:57:52]

nit: s/NonUser/Automated/

(then also update comments and variable in header)

[Qiang(Joe) Xu, 2016/07/29 04:30:31]

Done.

+  INITIALIZING_AUDIO_STATE = 1 << 0,

[Daniel Erat, 2016/07/28 23:57:52]

can you make this be an enum class instead or does it make that hard to mask it?
if not, give these a more descriptive prefix, like
VOLUME_CHANGE_INITIALIZING_AUDIO_STATE

[Qiang(Joe) Xu, 2016/07/29 04:30:32]

done by adding prefix

+  MAXIMIZE_MODE_SCREENSHOT = 1 << 1,
+};
+
 // Default value for unmuting, as a percent in the range [0, 100].
 // Used when sound is unmuted, but volume was less than kMuteThresholdPercent.
 const int kDefaultUnmuteVolumePercent = 4;
@@ -376,6 +381,15 @@ void CrasAudioHandler::SetOutputVolumePercent(int volume_percent) {
   }
 }
 
+void CrasAudioHandler::SetOutputVolumePercentInQuietMode(int volume_percent) {
+  // Currently SetOutputVolumePercentInQuietMode is only called from
+  // maximize mode screenshot in PowerButtonController. Everytime this method
+  // is called, DCHECK if MAXIMIZE_MODE_SCREENSHOT is already released.
+  DCHECK(!(volume_change_not_by_user_ & MAXIMIZE_MODE_SCREENSHOT));
+  volume_change_not_by_user_ |= MAXIMIZE_MODE_SCREENSHOT;
+  SetOutputVolumePercent(volume_percent);
+}
+
 // TODO: Rename the 'Percent' to something more meaningful.
 void CrasAudioHandler::SetInputGainPercent(int gain_percent) {
   // TODO(jennyz): Should we set all input devices' gain to the same level?
@@ -613,22 +627,31 @@ void CrasAudioHandler::OutputNodeVolumeChanged(uint64_t node_id, int volume) {
   output_volume_ = volume;
   audio_pref_handler_->SetVolumeGainValue(*device, volume);
 
-  if (initializing_audio_state_) {
-    // Do not notify the observers for volume changed event if CrasAudioHandler
-    // is initializing its state, i.e., the volume change event is in responding
-    // to SetOutputNodeVolume request from intializaing audio state, not
-    // from user action, no need to notify UI to pop uo the volume slider bar.
-    if (init_node_id_ == node_id && init_volume_ == volume) {
-      init_volume_count_--;
-      if (!init_volume_count_)
-        initializing_audio_state_ = false;
+  if (volume_change_not_by_user_) {
+    if (volume_change_not_by_user_ & INITIALIZING_AUDIO_STATE) {

[Daniel Erat, 2016/07/28 23:57:52]

using a mask for this seems a bit confusing. is this called asynchronously? if
not, why do you need to track multiple reasons? when you track multiple reasons,
it looks like you just unmask one of them with each call here. how do you know
that you're handling them in the right order?

[Qiang(Joe) Xu, 2016/07/29 04:30:31]

Yes. They are called asynchronously by observing cras_audio_client. I update the
logic a little bit so that the order doesn't matter. As far as there is an
AutomatedVolumeChangeReason, we should not notify observers and reset each
reasons.

+      // Do not notify the observers for volume changed event if
+      // CrasAudioHandler is initializing its state, i.e., the volume change
+      // event is in responding to SetOutputNodeVolume request from intializaing
+      // audio state, not from user action, no need to notify UI to pop up the
+      // volume slider bar.
+      if (init_node_id_ == node_id && init_volume_ == volume) {
+        init_volume_count_--;
+        if (!init_volume_count_)
+          volume_change_not_by_user_ ^= INITIALIZING_AUDIO_STATE;
+        return;
+      } else {
+        // Reset the INITIALIZING_AUDIO_STATE in case SetOutputNodeVolume
+        // request is lost by cras due to cras is not ready when
+        // CrasAudioHandler is being initialized.
+        volume_change_not_by_user_ ^= INITIALIZING_AUDIO_STATE;
+        init_volume_count_ = 0;
+      }
+    } else if (volume_change_not_by_user_ & MAXIMIZE_MODE_SCREENSHOT) {
+      // Do not notify the observers for the volume changed event if that is
+      // coming from a restoring volume after a maximize mode screenshot is
+      // taken. Reset the MAXIMIZE_MODE_SCREENSHOT.
+      volume_change_not_by_user_ ^= MAXIMIZE_MODE_SCREENSHOT;
       return;
-    } else {
-      // Reset the initializing_audio_state_ in case SetOutputNodeVolume request
-      // is lost by cras due to cras is not ready when CrasAudioHandler is being
-      // initialized.
-      initializing_audio_state_ = false;
-      init_volume_count_ = 0;
     }
   }
 
@@ -738,7 +761,7 @@ void CrasAudioHandler::SetupAudioOutputState() {
 
   SetOutputMuteInternal(output_mute_on_);
 
-  if (initializing_audio_state_) {
+  if (volume_change_not_by_user_ & INITIALIZING_AUDIO_STATE) {
     // During power up, InitializeAudioState() could be called twice, first
     // by CrasAudioHandler constructor, then by cras server restarting signal,
     // both sending SetOutputNodeVolume requests, and could lead to two
@@ -774,7 +797,7 @@ void CrasAudioHandler::SetupAdditionalActiveAudioNodeState(uint64_t node_id) {
 }
 
 void CrasAudioHandler::InitializeAudioState() {
-  initializing_audio_state_ = true;
+  volume_change_not_by_user_ |= INITIALIZING_AUDIO_STATE;
   ApplyAudioPolicy();
 
   // Defer querying cras for GetNodes until cras service becomes available.