Commit 802df057 authored by Xiaocheng Hu's avatar Xiaocheng Hu Committed by Commit Bot

Make CapurePageText capture textContent only on dirty layout

This is a partial revert for r536969 Change TranslateHelper to use a
TextContent-like text dump.

r536969 is a workaround for a lifecycle violation bug that layout is
still dirty after running layout update.

It turns out that textContent is not a good text dump for
TranslateHelper, as pages may hide arbitrary content (e.g., JSON) in
invisible text nodes, which can't be reliably filtered without checking
element style or layout.

Since we don't always run CapturePageText() with dirty layout, there is
no need to always dump textContent. Hence, this patch partially reverts
r536969 that, we still dump innerText as long as layout is clean. We
dump textContent only when layout is dirty, which is enough to get
around the lifecycle violation bug.

Bug: 826174
Change-Id: I63e97832caa9858e67992847fb9a22f6405183cb
Reviewed-on: https://chromium-review.googlesource.com/984474Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Reviewed-by: default avatarRachel Blum <groby@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547334}
parent 24a946aa
......@@ -306,9 +306,18 @@ WebString WebFrameContentDumper::DeprecatedDumpFrameTreeAsText(
size_t max_chars) {
if (!frame)
return WebString();
LocalFrame* local_frame = ToWebLocalFrameImpl(frame)->GetFrame();
// TODO(crbug.com/586241): We shoulndn't reach here with dirty layout as the
// function is called in DidMeaningfulLayout(); However, we still see this in
// the wild. As a workaround, we dump |textContent| instead when layout is
// dirty. We should always dump |innerText| when the bug is fixed.
bool needs_layout = local_frame->View()->NeedsLayout() ||
local_frame->GetDocument()->NeedsLayoutTreeUpdate();
StringBuilder text;
FrameContentAsPlainText(max_chars, ToWebLocalFrameImpl(frame)->GetFrame(),
TextDumpOption::kDumpTextContent, text);
FrameContentAsPlainText(max_chars, local_frame,
needs_layout ? TextDumpOption::kDumpTextContent
: TextDumpOption::kDumpInnerText,
text);
return text.ToString();
}
......
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