Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(762)

Unified Diff: third_party/WebKit/Source/modules/fetch/Response.cpp

Issue 1539803002: [Fetch API] Fix a memory leak with a Response constructed with a ReadableStream (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@response-constructed-with-stream
Patch Set: Created 5 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/modules/fetch/Response.cpp
diff --git a/third_party/WebKit/Source/modules/fetch/Response.cpp b/third_party/WebKit/Source/modules/fetch/Response.cpp
index 478426227a827e71def320de9bc3118152d37998..0aabf677a440bc6ea4c4527da065ed72fea45ac9 100644
--- a/third_party/WebKit/Source/modules/fetch/Response.cpp
+++ b/third_party/WebKit/Source/modules/fetch/Response.cpp
@@ -14,11 +14,13 @@
#include "bindings/core/v8/V8Binding.h"
#include "bindings/core/v8/V8Blob.h"
#include "bindings/core/v8/V8FormData.h"
+#include "bindings/core/v8/V8HiddenValue.h"
#include "core/dom/DOMArrayBuffer.h"
#include "core/dom/DOMArrayBufferView.h"
#include "core/fileapi/Blob.h"
#include "core/html/FormData.h"
#include "modules/fetch/BodyStreamBuffer.h"
+#include "modules/fetch/DataConsumerHandleUtil.h"
#include "modules/fetch/FetchBlobDataConsumerHandle.h"
#include "modules/fetch/FetchFormDataConsumerHandle.h"
#include "modules/fetch/ReadableStreamDataConsumerHandle.h"
@@ -112,6 +114,7 @@ Response* Response::create(ScriptState* scriptState, ExceptionState& exceptionSt
Response* Response::create(ScriptState* scriptState, ScriptValue bodyValue, const Dictionary& init, ExceptionState& exceptionState)
{
v8::Local<v8::Value> body = bodyValue.v8Value();
+ ScriptValue reader;
v8::Isolate* isolate = scriptState->isolate();
ExecutionContext* executionContext = scriptState->executionContext();
@@ -132,7 +135,14 @@ Response* Response::create(ScriptState* scriptState, ScriptValue bodyValue, cons
contentType = AtomicString("multipart/form-data; boundary=", AtomicString::ConstructFromLiteral) + formData->boundary().data();
bodyHandle = FetchFormDataConsumerHandle::create(executionContext, formData.release());
} else if (RuntimeEnabledFeatures::responseConstructedWithReadableStreamEnabled() && ReadableStreamOperations::isReadableStream(scriptState, body)) {
- bodyHandle = ReadableStreamDataConsumerHandle::create(scriptState, body);
+ reader = ReadableStreamOperations::getReader(scriptState, body, exceptionState);
+ if (exceptionState.hadException()) {
+ reader = ScriptValue();
+ bodyHandle = createFetchDataConsumerHandleFromWebHandle(createUnexpectedErrorDataConsumerHandle());
+ exceptionState.clearException();
+ } else {
+ bodyHandle = ReadableStreamDataConsumerHandle::create(scriptState, reader.v8Value().As<v8::Object>());
bashi 2016/01/06 05:39:17 just to clarify: reader.v8Value() is always an Obj
yhirano 2016/01/26 05:09:32 Deleted, but yes, ReadableStreamOperations::isRead
+ }
} else {
String string = toUSVString(isolate, body, exceptionState);
if (exceptionState.hadException())
@@ -141,7 +151,17 @@ Response* Response::create(ScriptState* scriptState, ScriptValue bodyValue, cons
contentType = "text/plain;charset=UTF-8";
}
// TODO(yhirano): Add the URLSearchParams case.
- return create(executionContext, bodyHandle.release(), contentType, ResponseInit(init, exceptionState), exceptionState);
+ Response* response = create(executionContext, bodyHandle.release(), contentType, ResponseInit(init, exceptionState), exceptionState);
+ if (!reader.isEmpty()) {
+ // Add a hidden reference so that the weak persistent in the
+ // ReadableStreamDataConsumerHandle will be valid as long as the
+ // the Response is valid.
haraken 2016/01/06 05:49:06 the the
yhirano 2016/01/26 05:09:32 Done.
+ v8::Local<v8::Value> wrapper = toV8(response, scriptState->context()->Global(), scriptState->isolate());
+ ASSERT(!wrapper.IsEmpty());
haraken 2016/01/06 05:49:06 toV8 can return an empty handle.
yhirano 2016/01/26 05:09:32 Done.
+ ASSERT(wrapper->IsObject());
+ V8HiddenValue::setHiddenValue(scriptState, wrapper.As<v8::Object>(), V8HiddenValue::readableStreamReaderInResponse(scriptState->isolate()), reader.v8Value());
+ }
+ return response;
}
Response* Response::create(ExecutionContext* context, PassOwnPtr<FetchDataConsumerHandle> bodyHandle, const String& contentType, const ResponseInit& init, ExceptionState& exceptionState)

Powered by Google App Engine
This is Rietveld 408576698