Revert "Try capturing incorrect re-entrancy into DWriteFontProxy"
This reverts e630455cf22a85822e681f0c84134193aed73433.
The detection mechanism did not detect any crashes of this kind. While
it's still very likely that the crashes are caused by Mojo sync calls
reentrancy issues, the particular theory of this change did not seem to
match what's going on. We should perhaps try to capture the re-entrancy
of font calls at the Mojo level, or move the font calls to a separate
blocking thread to avoid interference with other Mojo calls.
Bug: 561873
Change-Id: If0a1ce3b20fdf1f12afe1440db5b7989659a5e47
Reviewed-on: https://chromium-review.googlesource.com/c/1378141
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616976}diff --git a/content/child/dwrite_font_proxy/dwrite_font_proxy_win.cc b/content/child/dwrite_font_proxy/dwrite_font_proxy_win.cc
index 66c1b21..af3869e 100644
--- a/content/child/dwrite_font_proxy/dwrite_font_proxy_win.cc
+++ b/content/child/dwrite_font_proxy/dwrite_font_proxy_win.cc
@@ -102,8 +102,7 @@
return S_OK;
}
- if (!GetFontProxyScopeWrapper().GetFontProxy().FindFamily(name,
- &family_index)) {
+ if (!GetFontProxy().FindFamily(name, &family_index)) {
LogFontProxyError(FIND_FAMILY_SEND_FAILED);
return E_FAIL;
}
@@ -147,8 +146,7 @@
TRACE_EVENT0("dwrite", "FontProxy::GetFontFamilyCount");
uint32_t family_count = 0;
- if (!GetFontProxyScopeWrapper().GetFontProxy().GetFamilyCount(
- &family_count)) {
+ if (!GetFontProxy().GetFamilyCount(&family_count)) {
LogFontProxyError(GET_FAMILY_COUNT_SEND_FAILED);
return 0;
}
@@ -200,8 +198,7 @@
std::vector<base::FilePath> file_names;
std::vector<base::File> file_handles;
- if (!GetFontProxyScopeWrapper().GetFontProxy().GetFontFiles(
- *family_index, &file_names, &file_handles)) {
+ if (!GetFontProxy().GetFontFiles(*family_index, &file_names, &file_handles)) {
LogFontProxyError(GET_FONT_FILES_SEND_FAILED);
return E_FAIL;
}
@@ -322,8 +319,7 @@
TRACE_EVENT0("dwrite", "FontProxy::LoadFamilyNames");
std::vector<mojom::DWriteStringPairPtr> pairs;
- if (!GetFontProxyScopeWrapper().GetFontProxy().GetFamilyNames(family_index,
- &pairs)) {
+ if (!GetFontProxy().GetFamilyNames(family_index, &pairs)) {
return false;
}
std::vector<std::pair<base::string16, base::string16>> strings;
@@ -365,7 +361,7 @@
{base::WithBaseSyncPrimitives()}));
}
-FontProxyScopeWrapper DWriteFontCollectionProxy::GetFontProxyScopeWrapper() {
+mojom::DWriteFontProxy& DWriteFontCollectionProxy::GetFontProxy() {
if (!font_proxy_) {
mojom::DWriteFontProxyPtrInfo dwrite_font_proxy;
if (main_task_runner_->RunsTasksInCurrentSequence()) {
@@ -382,10 +378,7 @@
}
SetProxy(std::move(dwrite_font_proxy));
}
- static base::NoDestructor<base::ThreadLocalBoolean>
- font_proxy_method_in_flight;
- return FontProxyScopeWrapper(font_proxy_.get(),
- font_proxy_method_in_flight.get());
+ return **font_proxy_;
}
DWriteFontFamilyProxy::DWriteFontFamilyProxy() = default;
diff --git a/content/child/dwrite_font_proxy/dwrite_font_proxy_win.h b/content/child/dwrite_font_proxy/dwrite_font_proxy_win.h
index 63f66d27..0943acd7 100644
--- a/content/child/dwrite_font_proxy/dwrite_font_proxy_win.h
+++ b/content/child/dwrite_font_proxy/dwrite_font_proxy_win.h
@@ -22,39 +22,6 @@
class DWriteFontFamilyProxy;
-class FontProxyScopeWrapper {
- public:
- FontProxyScopeWrapper(mojom::ThreadSafeDWriteFontProxyPtr* font_proxy,
- base::ThreadLocalBoolean* is_in_flight)
- : font_proxy_(font_proxy), is_in_flight_(is_in_flight) {
- // TODO(crbug.com/561873): Turn this into a DCHECK once instances of this
- // CHECK have been found in crash reports and the referenced bug has been
- // root-caused.
- CHECK(!is_in_flight_->Get());
- is_in_flight_->Set(true);
- }
-
- ~FontProxyScopeWrapper() {
- // TODO(crbug.com/561873): Turn this into a DCHECK once instances of this
- // CHECK have been found in crash reports and the referenced bug has been
- // root-caused.
- CHECK(is_in_flight_->Get());
- is_in_flight_->Set(false);
- }
-
- content::mojom::DWriteFontProxy& GetFontProxy() const {
- return **font_proxy_;
- }
-
- FontProxyScopeWrapper(FontProxyScopeWrapper&&) = default;
- FontProxyScopeWrapper& operator=(FontProxyScopeWrapper&&) = default;
-
- private:
- mojom::ThreadSafeDWriteFontProxyPtr* font_proxy_;
- base::ThreadLocalBoolean* is_in_flight_;
- DISALLOW_COPY_AND_ASSIGN(FontProxyScopeWrapper);
-};
-
// Implements a DirectWrite font collection that uses IPC to the browser to do
// font enumeration. If a matching family is found, it will be loaded locally
// into a custom font collection.
@@ -120,7 +87,7 @@
bool CreateFamily(UINT32 family_index);
- FontProxyScopeWrapper GetFontProxyScopeWrapper();
+ mojom::DWriteFontProxy& GetFontProxy();
private:
void SetProxy(mojom::DWriteFontProxyPtrInfo);
diff --git a/content/child/dwrite_font_proxy/font_fallback_win.cc b/content/child/dwrite_font_proxy/font_fallback_win.cc
index 95c5b9f..25dbef98 100644
--- a/content/child/dwrite_font_proxy/font_fallback_win.cc
+++ b/content/child/dwrite_font_proxy/font_fallback_win.cc
@@ -100,7 +100,7 @@
mojom::MapCharactersResultPtr result;
- if (!GetFontProxyScopeWrapper().GetFontProxy().MapCharacters(
+ if (!GetFontProxy().MapCharacters(
text_chunk,
mojom::DWriteFontStyle::New(base_weight, base_style, base_stretch),
locale, source->GetParagraphReadingDirection(), base_family_name,
@@ -222,8 +222,8 @@
family_list.pop_back();
}
-FontProxyScopeWrapper FontFallback::GetFontProxyScopeWrapper() {
- return collection_->GetFontProxyScopeWrapper();
+mojom::DWriteFontProxy& FontFallback::GetFontProxy() {
+ return collection_->GetFontProxy();
}
} // namespace content
diff --git a/content/child/dwrite_font_proxy/font_fallback_win.h b/content/child/dwrite_font_proxy/font_fallback_win.h
index 0724203..ca22abc 100644
--- a/content/child/dwrite_font_proxy/font_fallback_win.h
+++ b/content/child/dwrite_font_proxy/font_fallback_win.h
@@ -65,7 +65,7 @@
const wchar_t* base_family_name);
private:
- FontProxyScopeWrapper GetFontProxyScopeWrapper();
+ mojom::DWriteFontProxy& GetFontProxy();
Microsoft::WRL::ComPtr<DWriteFontCollectionProxy> collection_;
