Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge branch '4-2-x' into miniak/liftoff-correctly-unuse-labels-4-2-x
- Loading branch information
Showing
10 changed files
with
310 additions
and
10 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
4.2.3 | ||
4.2.4 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
134 changes: 134 additions & 0 deletions
134
patches/common/chromium/fix_uap_in_imagebitmaploader_filereaderloader.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,134 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Marijn Kruisselbrink <mek@chromium.org> | ||
Date: Thu, 13 Dec 2018 17:09:55 +0000 | ||
Subject: Fix UAP in ImageBitmapLoader/FileReaderLoader | ||
|
||
FileReaderLoader stores its client as a raw pointer, so in cases like | ||
ImageBitmapLoader where the FileReaderLoaderClient really is garbage | ||
collected we have to make sure to destroy the FileReaderLoader when | ||
the ExecutionContext that owns it is destroyed. | ||
|
||
Bug: 913970 | ||
Change-Id: I40b02115367cf7bf5bbbbb8e9b57874d2510f861 | ||
Reviewed-on: https://chromium-review.googlesource.com/c/1374511 | ||
Reviewed-by: Jeremy Roman <jbroman@chromium.org> | ||
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org> | ||
Cr-Commit-Position: refs/heads/master@{#616342} | ||
|
||
diff --git a/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.cc b/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.cc | ||
index 076b9beeaa6675c5e858a1c7f5a321ab57c606fb..40898b704891c3723574d3afa4aebf72f5cdce5c 100644 | ||
--- a/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.cc | ||
+++ b/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.cc | ||
@@ -238,27 +238,31 @@ void ImageBitmapFactories::DidFinishLoading(ImageBitmapLoader* loader) { | ||
pending_loaders_.erase(loader); | ||
} | ||
|
||
+void ImageBitmapFactories::Trace(blink::Visitor* visitor) { | ||
+ visitor->Trace(pending_loaders_); | ||
+ Supplement<LocalDOMWindow>::Trace(visitor); | ||
+ Supplement<WorkerGlobalScope>::Trace(visitor); | ||
+} | ||
+ | ||
ImageBitmapFactories::ImageBitmapLoader::ImageBitmapLoader( | ||
ImageBitmapFactories& factory, | ||
base::Optional<IntRect> crop_rect, | ||
ScriptState* script_state, | ||
const ImageBitmapOptions& options) | ||
- : loader_( | ||
+ : ContextLifecycleObserver(ExecutionContext::From(script_state)), | ||
+ loader_( | ||
FileReaderLoader::Create(FileReaderLoader::kReadAsArrayBuffer, this)), | ||
factory_(&factory), | ||
resolver_(ScriptPromiseResolver::Create(script_state)), | ||
crop_rect_(crop_rect), | ||
options_(options) {} | ||
|
||
-void ImageBitmapFactories::ImageBitmapLoader::LoadBlobAsync( | ||
- Blob* blob) { | ||
+void ImageBitmapFactories::ImageBitmapLoader::LoadBlobAsync(Blob* blob) { | ||
loader_->Start(blob->GetBlobDataHandle()); | ||
} | ||
|
||
-void ImageBitmapFactories::Trace(blink::Visitor* visitor) { | ||
- visitor->Trace(pending_loaders_); | ||
- Supplement<LocalDOMWindow>::Trace(visitor); | ||
- Supplement<WorkerGlobalScope>::Trace(visitor); | ||
+ImageBitmapFactories::ImageBitmapLoader::~ImageBitmapLoader() { | ||
+ DCHECK(!loader_); | ||
} | ||
|
||
void ImageBitmapFactories::ImageBitmapLoader::RejectPromise( | ||
@@ -277,11 +281,20 @@ void ImageBitmapFactories::ImageBitmapLoader::RejectPromise( | ||
default: | ||
NOTREACHED(); | ||
} | ||
+ loader_.reset(); | ||
factory_->DidFinishLoading(this); | ||
} | ||
|
||
+void ImageBitmapFactories::ImageBitmapLoader::ContextDestroyed( | ||
+ ExecutionContext*) { | ||
+ if (loader_) | ||
+ factory_->DidFinishLoading(this); | ||
+ loader_.reset(); | ||
+} | ||
+ | ||
void ImageBitmapFactories::ImageBitmapLoader::DidFinishLoading() { | ||
DOMArrayBuffer* array_buffer = loader_->ArrayBufferResult(); | ||
+ loader_.reset(); | ||
if (!array_buffer) { | ||
RejectPromise(kAllocationFailureImageBitmapRejectionReason); | ||
return; | ||
@@ -359,6 +372,7 @@ void ImageBitmapFactories::ImageBitmapLoader::ResolvePromiseOnOriginalThread( | ||
} | ||
|
||
void ImageBitmapFactories::ImageBitmapLoader::Trace(blink::Visitor* visitor) { | ||
+ ContextLifecycleObserver::Trace(visitor); | ||
visitor->Trace(factory_); | ||
visitor->Trace(resolver_); | ||
} | ||
diff --git a/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.h b/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.h | ||
index 25bdf1ffd1e85a05ab4ee46aaa81c61294fd7d1a..726012a20f7250ea1166ec03875ebaa10f352a49 100644 | ||
--- a/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.h | ||
+++ b/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.h | ||
@@ -36,6 +36,7 @@ | ||
#include "third_party/blink/renderer/bindings/core/v8/image_bitmap_source.h" | ||
#include "third_party/blink/renderer/bindings/core/v8/script_promise.h" | ||
#include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h" | ||
+#include "third_party/blink/renderer/core/dom/context_lifecycle_observer.h" | ||
#include "third_party/blink/renderer/core/fileapi/file_reader_loader.h" | ||
#include "third_party/blink/renderer/core/fileapi/file_reader_loader_client.h" | ||
#include "third_party/blink/renderer/core/frame/local_dom_window.h" | ||
@@ -103,7 +104,10 @@ class ImageBitmapFactories final | ||
private: | ||
class ImageBitmapLoader final | ||
: public GarbageCollectedFinalized<ImageBitmapLoader>, | ||
+ public ContextLifecycleObserver, | ||
public FileReaderLoaderClient { | ||
+ USING_GARBAGE_COLLECTED_MIXIN(ImageBitmapLoader); | ||
+ | ||
public: | ||
static ImageBitmapLoader* Create(ImageBitmapFactories& factory, | ||
base::Optional<IntRect> crop_rect, | ||
@@ -115,9 +119,9 @@ class ImageBitmapFactories final | ||
void LoadBlobAsync(Blob*); | ||
ScriptPromise Promise() { return resolver_->Promise(); } | ||
|
||
- void Trace(blink::Visitor*); | ||
+ void Trace(blink::Visitor*) override; | ||
|
||
- ~ImageBitmapLoader() override = default; | ||
+ ~ImageBitmapLoader() override; | ||
|
||
private: | ||
ImageBitmapLoader(ImageBitmapFactories&, | ||
@@ -140,6 +144,9 @@ class ImageBitmapFactories final | ||
const String& color_space_conversion_option); | ||
void ResolvePromiseOnOriginalThread(sk_sp<SkImage>); | ||
|
||
+ // ContextLifecycleObserver | ||
+ void ContextDestroyed(ExecutionContext*) override; | ||
+ | ||
// FileReaderLoaderClient | ||
void DidStartLoading() override {} | ||
void DidReceiveData() override {} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
164 changes: 164 additions & 0 deletions
164
patches/common/v8/valueserializer_report_if_buffer_expansion_fails_during.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,164 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Jeremy Roman <jbroman@chromium.org> | ||
Date: Thu, 13 Dec 2018 18:43:00 -0500 | ||
Subject: ValueSerializer: Report if buffer expansion fails during | ||
WriteHostObject. | ||
|
||
Also fail early if we detect that we've previously run out of memory and thus | ||
corrupted the buffer. | ||
|
||
Add a unit test for this kind of case. | ||
|
||
Bug: chromium:914731 | ||
Change-Id: Iaaf3927209bffeab6fe8ba462d9dd9dad8cbbe2f | ||
Reviewed-on: https://chromium-review.googlesource.com/c/1377449 | ||
Reviewed-by: Yang Guo <yangguo@chromium.org> | ||
Commit-Queue: Jeremy Roman <jbroman@chromium.org> | ||
Cr-Commit-Position: refs/heads/master@{#58248} | ||
|
||
diff --git a/src/value-serializer.cc b/src/value-serializer.cc | ||
index 4c9c9a9aa2d9ce6017bb02448fa474478c9f6308..8c85bd9f5466612937a71f8bfdcb82120f7fb9c7 100644 | ||
--- a/src/value-serializer.cc | ||
+++ b/src/value-serializer.cc | ||
@@ -344,7 +344,11 @@ void ValueSerializer::TransferArrayBuffer(uint32_t transfer_id, | ||
} | ||
|
||
Maybe<bool> ValueSerializer::WriteObject(Handle<Object> object) { | ||
- out_of_memory_ = false; | ||
+ // There is no sense in trying to proceed if we've previously run out of | ||
+ // memory. Bail immediately, as this likely implies that some write has | ||
+ // previously failed and so the buffer is corrupt. | ||
+ if (V8_UNLIKELY(out_of_memory_)) return ThrowIfOutOfMemory(); | ||
+ | ||
if (object->IsSmi()) { | ||
WriteSmi(Smi::cast(*object)); | ||
return ThrowIfOutOfMemory(); | ||
@@ -930,8 +934,10 @@ Maybe<bool> ValueSerializer::WriteHostObject(Handle<JSObject> object) { | ||
Maybe<bool> result = | ||
delegate_->WriteHostObject(v8_isolate, Utils::ToLocal(object)); | ||
RETURN_VALUE_IF_SCHEDULED_EXCEPTION(isolate_, Nothing<bool>()); | ||
+ USE(result); | ||
DCHECK(!result.IsNothing()); | ||
- return result; | ||
+ DCHECK(result.ToChecked()); | ||
+ return ThrowIfOutOfMemory(); | ||
} | ||
|
||
Maybe<uint32_t> ValueSerializer::WriteJSObjectPropertiesSlow( | ||
diff --git a/test/unittests/value-serializer-unittest.cc b/test/unittests/value-serializer-unittest.cc | ||
index 92603b588ad13480460a166b556be18e57fc8ad6..bc738c84fc8b15bdfcdb0cc995f8002362292a4c 100644 | ||
--- a/test/unittests/value-serializer-unittest.cc | ||
+++ b/test/unittests/value-serializer-unittest.cc | ||
@@ -119,7 +119,10 @@ class ValueSerializerTest : public TestWithIsolate { | ||
} | ||
std::pair<uint8_t*, size_t> buffer = serializer.Release(); | ||
std::vector<uint8_t> result(buffer.first, buffer.first + buffer.second); | ||
- free(buffer.first); | ||
+ if (auto* delegate = GetSerializerDelegate()) | ||
+ delegate->FreeBufferMemory(buffer.first); | ||
+ else | ||
+ free(buffer.first); | ||
return Just(std::move(result)); | ||
} | ||
|
||
@@ -138,6 +141,10 @@ class ValueSerializerTest : public TestWithIsolate { | ||
return buffer; | ||
} | ||
|
||
+ std::vector<uint8_t> EncodeTest(const char* source) { | ||
+ return EncodeTest(EvaluateScriptForInput(source)); | ||
+ } | ||
+ | ||
v8::Local<v8::Message> InvalidEncodeTest(Local<Value> input_value) { | ||
Context::Scope scope(serialization_context()); | ||
TryCatch try_catch(isolate()); | ||
@@ -2698,5 +2705,89 @@ TEST_F(ValueSerializerTestWithWasm, DecodeWasmModuleWithInvalidDataLength) { | ||
InvalidDecodeTest({0xFF, 0x09, 0x3F, 0x00, 0x57, 0x79, 0x00, 0x7F}); | ||
} | ||
|
||
+class ValueSerializerTestWithLimitedMemory : public ValueSerializerTest { | ||
+ protected: | ||
+// GMock doesn't use the "override" keyword. | ||
+#if __clang__ | ||
+#pragma clang diagnostic push | ||
+#pragma clang diagnostic ignored "-Winconsistent-missing-override" | ||
+#endif | ||
+ | ||
+ class SerializerDelegate : public ValueSerializer::Delegate { | ||
+ public: | ||
+ explicit SerializerDelegate(ValueSerializerTestWithLimitedMemory* test) | ||
+ : test_(test) {} | ||
+ | ||
+ ~SerializerDelegate() { EXPECT_EQ(nullptr, last_buffer_); } | ||
+ | ||
+ void SetMemoryLimit(size_t limit) { memory_limit_ = limit; } | ||
+ | ||
+ void* ReallocateBufferMemory(void* old_buffer, size_t size, | ||
+ size_t* actual_size) override { | ||
+ EXPECT_EQ(old_buffer, last_buffer_); | ||
+ if (size > memory_limit_) return nullptr; | ||
+ *actual_size = size; | ||
+ last_buffer_ = realloc(old_buffer, size); | ||
+ return last_buffer_; | ||
+ } | ||
+ | ||
+ void FreeBufferMemory(void* buffer) override { | ||
+ EXPECT_EQ(buffer, last_buffer_); | ||
+ last_buffer_ = nullptr; | ||
+ free(buffer); | ||
+ } | ||
+ | ||
+ void ThrowDataCloneError(Local<String> message) override { | ||
+ test_->isolate()->ThrowException(Exception::Error(message)); | ||
+ } | ||
+ | ||
+ MOCK_METHOD2(WriteHostObject, | ||
+ Maybe<bool>(Isolate* isolate, Local<Object> object)); | ||
+ | ||
+ private: | ||
+ ValueSerializerTestWithLimitedMemory* test_; | ||
+ void* last_buffer_ = nullptr; | ||
+ size_t memory_limit_ = 0; | ||
+ }; | ||
+ | ||
+#if __clang__ | ||
+#pragma clang diagnostic pop | ||
+#endif | ||
+ | ||
+ ValueSerializer::Delegate* GetSerializerDelegate() override { | ||
+ return &serializer_delegate_; | ||
+ } | ||
+ | ||
+ void BeforeEncode(ValueSerializer* serializer) override { | ||
+ serializer_ = serializer; | ||
+ } | ||
+ | ||
+ SerializerDelegate serializer_delegate_{this}; | ||
+ ValueSerializer* serializer_ = nullptr; | ||
+}; | ||
+ | ||
+TEST_F(ValueSerializerTestWithLimitedMemory, FailIfNoMemoryInWriteHostObject) { | ||
+ EXPECT_CALL(serializer_delegate_, WriteHostObject(isolate(), _)) | ||
+ .WillRepeatedly(Invoke([this](Isolate*, Local<Object>) { | ||
+ static const char kDummyData[1024] = {}; | ||
+ serializer_->WriteRawBytes(&kDummyData, sizeof(kDummyData)); | ||
+ return Just(true); | ||
+ })); | ||
+ | ||
+ // If there is enough memory, things work. | ||
+ serializer_delegate_.SetMemoryLimit(2048); | ||
+ EncodeTest("new ExampleHostObject()"); | ||
+ | ||
+ // If not, we get a graceful failure, rather than silent misbehavior. | ||
+ serializer_delegate_.SetMemoryLimit(1024); | ||
+ InvalidEncodeTest("new ExampleHostObject()"); | ||
+ | ||
+ // And we definitely don't continue to serialize other things. | ||
+ serializer_delegate_.SetMemoryLimit(1024); | ||
+ EvaluateScriptForInput("gotA = false"); | ||
+ InvalidEncodeTest("[new ExampleHostObject, {get a() { gotA = true; }}]"); | ||
+ EXPECT_TRUE(EvaluateScriptForInput("gotA")->IsFalse()); | ||
+} | ||
+ | ||
} // namespace | ||
} // namespace v8 |