BodyStreamBuffer should release WebDataConsumerHandle::Reader eagerly

third_party/WebKit/Source/modules/fetch/BodyStreamBuffer.h

 // Copyright 2014 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.
 
 #ifndef BodyStreamBuffer_h
 #define BodyStreamBuffer_h
 
 #include "core/dom/DOMException.h"
 #include "core/streams/ReadableByteStream.h"
 #include "core/streams/ReadableByteStreamReader.h"
 #include "core/streams/UnderlyingSource.h"
 #include "modules/ModulesExport.h"
 #include "modules/fetch/FetchDataConsumerHandle.h"
 #include "modules/fetch/FetchDataLoader.h"
 #include "platform/heap/Handle.h"
 #include "public/platform/WebDataConsumerHandle.h"
 #include "wtf/OwnPtr.h"
 #include "wtf/PassOwnPtr.h"
 
 namespace blink {
 
 class EncodedFormData;
 class ExecutionContext;
 
 class MODULES_EXPORT BodyStreamBuffer final : public GarbageCollectedFinalized<BodyStreamBuffer>, public UnderlyingSource, public WebDataConsumerHandle::Client {
     WTF_MAKE_NONCOPYABLE(BodyStreamBuffer);
     USING_GARBAGE_COLLECTED_MIXIN(BodyStreamBuffer);
+    USING_PRE_FINALIZER(BodyStreamBuffer, stop);

[sof, 2016/02/19 06:17:11]

This prefinalizer (and its registration) is correct, but the action itself isn't
very interesting (wrt access of other Oilpan objects when finalizing), hence
adding a smaller, but avoidable, overhead during GC.

I'd argue that EAGERLY_FINALIZE() is what you want here instead.

[yhirano, 2016/02/19 06:36:22]

Sorry I don't understand EAGERLY_FINALIZE: Can you tell me what it is? What is
the difference between them?

 public:
     // |handle| cannot be null and cannot be locked.
     explicit BodyStreamBuffer(PassOwnPtr<FetchDataConsumerHandle> /* handle */);
 
     ReadableByteStream* stream() { return m_stream; }
 
     // Callable only when not locked.
     PassRefPtr<BlobDataHandle> drainAsBlobDataHandle(FetchDataConsumerHandle::Reader::BlobSizePolicy);
     PassRefPtr<EncodedFormData> drainAsFormData();
     void startLoading(ExecutionContext*, FetchDataLoader*, FetchDataLoader::Client* /* client */);
@@ skipping 25 matching lines @@
 private:
     class LoaderClient;
     void close();
     void error();
     void processData();
     void endLoading();
     void stopLoading();
 
     OwnPtr<FetchDataConsumerHandle> m_handle;
     OwnPtr<FetchDataConsumerHandle::Reader> m_reader;
-    Member<ReadableByteStream> m_stream;
+    const Member<ReadableByteStream> m_stream;

[sof, 2016/02/19 06:17:11]

( what's the concern? )

[yhirano, 2016/02/19 17:32:22]

Done.

     // We need this member to keep it alive while loading.
     Member<FetchDataLoader> m_loader;
     bool m_streamNeedsMore;
 };
 
 } // namespace blink
 
 #endif // BodyStreamBuffer_h