Commit f397777d authored by Xiaocheng Hu's avatar Xiaocheng Hu Committed by Commit Bot

Revert "Make CapurePageText capture textContent only on dirty layout"

This reverts commit 802df057.

Reason for revert: suspected for causing memory regressions crbug.com/828026 and crbug.com/828027

Original change's description:
> 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/984474
> Reviewed-by: Kent Tamura <tkent@chromium.org>
> Reviewed-by: Rachel Blum <groby@chromium.org>
> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#547334}

TBR=groby@chromium.org,tkent@chromium.org,xiaochengh@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 826174, 828026, 828027
Change-Id: I4c3200e5c7de0e6b3838ddcc40d53e751538f915
Reviewed-on: https://chromium-review.googlesource.com/992818Reviewed-by: default avatarXiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547796}
parent 72e90130
...@@ -306,18 +306,9 @@ WebString WebFrameContentDumper::DeprecatedDumpFrameTreeAsText( ...@@ -306,18 +306,9 @@ WebString WebFrameContentDumper::DeprecatedDumpFrameTreeAsText(
size_t max_chars) { size_t max_chars) {
if (!frame) if (!frame)
return WebString(); 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; StringBuilder text;
FrameContentAsPlainText(max_chars, local_frame, FrameContentAsPlainText(max_chars, ToWebLocalFrameImpl(frame)->GetFrame(),
needs_layout ? TextDumpOption::kDumpTextContent TextDumpOption::kDumpTextContent, text);
: TextDumpOption::kDumpInnerText,
text);
return text.ToString(); 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