Fix Volume slider is captured in screenshot done in touchview mode

chromeos/audio/cras_audio_handler.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chromeos/audio/cras_audio_handler.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/sys_info.h"
 #include "base/system_monitor/system_monitor.h"
 #include "chromeos/audio/audio_devices_pref_handler_stub.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 
 using std::max;
 using std::min;
 
 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;
 
 // Volume value which should be considered as muted in range [0, 100].
 const int kMuteThresholdPercent = 1;
 
 // The duration of HDMI output re-discover grace period in milliseconds.
 const int kHDMIRediscoverGracePeriodDurationInMs = 2000;
 
@@ skipping 331 matching lines @@
   // Set all active devices to the same volume.
   for (AudioDeviceMap::const_iterator it = audio_devices_.begin();
        it != audio_devices_.end();
        it++) {
     const AudioDevice& device = it->second;
     if (!device.is_input && device.active)
       SetOutputNodeVolumePercent(device.id, 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?
   for (AudioDeviceMap::const_iterator it = audio_devices_.begin();
        it != audio_devices_.end();
        it++) {
     const AudioDevice& device = it->second;
     if (device.is_input && device.active)
       SetInputNodeGainPercent(active_input_node_id_, gain_percent);
   }
@@ skipping 217 matching lines @@
   }
 
   // Sync internal volume state and notify UI for the change. We trust cras
   // signal to report the volume state of the device, no matter which source
   // set the volume, i.e., volume could be set from non-chrome source, like
   // Bluetooth headset, etc. Assume all active output devices share a single
   // 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;
     }
   }
 
   FOR_EACH_OBSERVER(AudioObserver, observers_,
                     OnOutputNodeVolumeChanged(node_id, volume));
 }
 
 void CrasAudioHandler::ActiveOutputNodeChanged(uint64_t node_id) {
   if (active_output_node_id_ == node_id)
     return;
@@ skipping 89 matching lines @@
   if (hdmi_rediscovering_ && !IsHDMIPrimaryOutputDevice()) {
     VLOG(1) << "Mute the output during HDMI re-discovering grace period";
     output_mute_on_ = true;
   } else {
     output_mute_on_ = audio_pref_handler_->GetMuteValue(*device);
   }
   output_volume_ = audio_pref_handler_->GetOutputVolumeValue(device);
 
   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
     // OutputNodeVolumeChanged signals.
     init_volume_count_++;
     init_node_id_ = active_output_node_id_;
     init_volume_ = output_volume_;
   }
   SetOutputNodeVolume(active_output_node_id_, output_volume_);
 }
@@ skipping 15 matching lines @@
   // devices. For input devices, we don't restore their gain value so far.
   // TODO(jennyz): crbug.com/417418, track the status for the decison if
   // we should persist input gain value in prefs.
   if (!device->is_input) {
     audio_pref_handler_->SetMuteValue(*device, IsOutputMuted());
     SetOutputNodeVolumePercent(node_id, GetOutputVolumePercent());
   }
 }
 
 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.
   cras_service_available_ = false;
   chromeos::DBusThreadManager::Get()
       ->GetCrasAudioClient()
       ->WaitForServiceToBeAvailable(base::Bind(
           &CrasAudioHandler::InitializeAudioAfterCrasServiceAvailable,
           weak_ptr_factory_.GetWeakPtr()));
 }
@@ skipping 584 matching lines @@
                      hdmi_rediscover_grace_period_duration_in_ms_),
       this, &CrasAudioHandler::UpdateAudioAfterHDMIRediscoverGracePeriod);
 }
 
 void CrasAudioHandler::SetHDMIRediscoverGracePeriodForTesting(
     int duration_in_ms) {
   hdmi_rediscover_grace_period_duration_in_ms_ = duration_in_ms;
 }
 
 }  // namespace chromeos