Commit 5111bdc8 authored by Dominik Röttsches's avatar Dominik Röttsches Committed by Commit Bot

Speculative fix for crash in CSSFontFace::FontLoaded

When font scanning finished, Blink receives a Mojo IPC callback that
local font matching is now available in
notifies all WeakPtr's to LocalFontFaceSource's if those WeakPtrs are
individually still valid.

After consulting with haraken@ it turns out that using a WeakPtr and
WeakPtrFactory on a heap object is bad practice, as an object may remain
allocated for a little while after being marked for GC - but the WeakPtr
state does not reflect that yet. So when we have callbacks bound to the
WeakPtr they still get executed.

Instead, use WrapWeakPersistent() to in the Bind call in order to drop
the callbacks if the target object for the callback has been collected
by GC.

This should address the assumed GC race condition which is most likely
the underlying issue for issue 1017078.


FontUniqueNameLookupWin: :ReceiveReadOnlySharedMemoryRegion, then it
Bug: 1017078
Change-Id: I9812ec292f2986fc15a3d14cbbfe5381788634d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1895334Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712128}
parent cf08a578
......@@ -13,21 +13,12 @@
#include "third_party/blink/renderer/platform/fonts/font_selector.h"
#include "third_party/blink/renderer/platform/fonts/font_unique_name_lookup.h"
#include "third_party/blink/renderer/platform/fonts/simple_font_data.h"
#include "third_party/blink/renderer/platform/heap/persistent.h"
#include "third_party/blink/renderer/platform/instrumentation/histogram.h"
#include "third_party/blink/renderer/platform/wtf/functional.h"
namespace blink {
namespace {
void NotifyFontUniqueNameLookupReadyWeakPtr(
base::WeakPtr<LocalFontFaceSource> local_font_face_source) {
if (local_font_face_source)
local_font_face_source->NotifyFontUniqueNameLookupReady();
}
} // namespace
LocalFontFaceSource::LocalFontFaceSource(CSSFontFace* css_font_face,
FontSelector* font_selector,
const String& font_name)
......@@ -110,7 +101,8 @@ void LocalFontFaceSource::BeginLoadIfNeeded() {
FontGlobalContext::Get()->GetFontUniqueNameLookup();
DCHECK(unique_name_lookup);
unique_name_lookup->PrepareFontUniqueNameLookup(
WTF::Bind(&NotifyFontUniqueNameLookupReadyWeakPtr, GetWeakPtr()));
WTF::Bind(&LocalFontFaceSource::NotifyFontUniqueNameLookupReady,
WrapWeakPersistent(this)));
face_->DidBeginLoad();
}
......@@ -122,10 +114,6 @@ void LocalFontFaceSource::NotifyFontUniqueNameLookupReady() {
}
}
base::WeakPtr<LocalFontFaceSource> LocalFontFaceSource::GetWeakPtr() {
return weak_factory_.GetWeakPtr();
}
bool LocalFontFaceSource::IsLoaded() const {
return IsLocalNonBlocking();
}
......
......@@ -48,8 +48,6 @@ class LocalFontFaceSource final : public CSSFontFaceSource,
void NotifyFontUniqueNameLookupReady();
base::WeakPtr<LocalFontFaceSource> GetWeakPtr();
protected:
scoped_refptr<SimpleFontData> CreateLoadingFallbackFontData(
const FontDescription&);
......@@ -75,7 +73,6 @@ class LocalFontFaceSource final : public CSSFontFaceSource,
AtomicString font_name_;
LocalFontHistograms histograms_;
base::WeakPtrFactory<LocalFontFaceSource> weak_factory_{this};
};
} // namespace blink
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment