Update the current frame count in a safe place

Source/modules/webaudio/AudioContext.h

 /*
  * Copyright (C) 2010, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1.  Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 88 matching lines @@
     virtual void trace(Visitor*) override;
 
     bool isInitialized() const { return m_isInitialized; }
     bool isOfflineContext() { return m_isOfflineContext; }
 
     // Document notification
     virtual void stop() override final;
     virtual bool hasPendingActivity() const override;
 
     AudioDestinationNode* destination() { return m_destinationNode.get(); }
+    // currentSampleFrame() returns the current sample frame. It should only be called from the
+    // audio thread.
     size_t currentSampleFrame() const { return m_destinationNode->currentSampleFrame(); }
+    // sampleFrame() is like currentSampleFrame() but must be called from the main thread to get the
+    // sample frame. It might be slightly behind curentSampleFrame() due to locking.
+    size_t sampleFrame() const;

[hongchan, 2014/12/03 18:50:47]

Just a thought - can't this be shadowCurrentSampleFrame()?

I think the method name is a bit general for its behavior and having 'shadow' in
the method name clarifies it.

[Raymond Toy, 2014/12/03 19:04:44]

I'm terrible at names.  m_shadowCurrentSampleFrame isn't a great name, and I
think shadowCurrentSampleFrame() seems to reflect the implementation too much
because it's really meant to be the currentSampleFrame of the destination that
the main thread can use.

Maybe sampleFrame() should be called currentSampleFrame() and
currentSampleFrame() should be renamed to destinationCurrentSampleFrame() or
currentDestinationSampleFrame()?
  

[hongchan, 2014/12/03 19:13:29]

I agree that it is too much.

How about using the word 'cached' in the issue description instead? I think it
is a good name for the frame value introduced in this patch.

currentSampleFrame() -> m_currentSampleFrame (no change)
cachedSampleFrame() -> m_cachedSampleFrame

I believe the name 'cached' implies that it is behind the actual current sample
time and also thread-safe. Maybe a tad better than 'shadow' or 'destination.'

[Raymond Toy, 2014/12/03 19:34:21]

Sounds good.

     double currentTime() const { return m_destinationNode->currentTime(); }
     float sampleRate() const { return m_destinationNode->sampleRate(); }
     String state() const;
 
     AudioBuffer* createBuffer(unsigned numberOfChannels, size_t numberOfFrames, float sampleRate, ExceptionState&);
 
     // Asynchronous audio file data decoding.
     void decodeAudioData(DOMArrayBuffer*, AudioBufferCallback*, AudioBufferCallback*, ExceptionState&);
 
     AudioListener* listener() { return m_listener.get(); }
@@ skipping 269 matching lines @@
     AudioContextState m_contextState;
     void setContextState(AudioContextState);
 
     AsyncAudioDecoder m_audioDecoder;
 
     // Collection of nodes where the channel count mode has changed. We want the channel count mode
     // to change in the pre- or post-rendering phase so as not to disturb the running audio thread.
     GC_PLUGIN_IGNORE("http://crbug.com/404527")
     HashSet<AudioNode*> m_deferredCountModeChange;
 
+    // Follows the destinations currentSampleFrame, but might be slightly behind due to locking.
+    size_t m_shadowCurrentSampleFrame;
+
     // This is considering 32 is large enough for multiple channels audio.
     // It is somewhat arbitrary and could be increased if necessary.
     enum { MaxNumberOfChannels = 32 };
 };
 
 } // namespace blink
 
 #endif // AudioContext_h