New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 828027 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 828026
Owner: ----
Closed: Apr 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

4%-9.5% regression in system_health.memory_desktop at 547248:547380

Project Member Reported by pmeenan@chromium.org, Apr 2 2018

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=828027

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=a642db7e3ebbd0300ac8fe8554c71558da2a1494be9da53ebe41f533a7cbea35


Bot(s) for this bug's original alert(s):

chromium-rel-mac11-air
chromium-rel-mac11-pro
chromium-rel-mac12
chromium-rel-win8-dual
📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/1694993f440000
Cc: tkent@chromium.org groby@chromium.org xiaoche...@chromium.org
Owner: xiaoche...@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/1694993f440000

Make CapurePageText capture textContent only on dirty layout by xiaochengh@chromium.org
https://chromium.googlesource.com/chromium/src/+/802df0571195f064c2bd77b513b86950eeb9f52a

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f397777d8c4e6855f580f0f2d1a1ddd9b0eec217

commit f397777d8c4e6855f580f0f2d1a1ddd9b0eec217
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Tue Apr 03 19:53:06 2018

Revert "Make CapurePageText capture textContent only on dirty layout"

This reverts commit 802df0571195f064c2bd77b513b86950eeb9f52a.

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/992818
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547796}
[modify] https://crrev.com/f397777d8c4e6855f580f0f2d1a1ddd9b0eec217/third_party/WebKit/Source/core/exported/WebFrameContentDumper.cpp

Mergedinto: 828026
Owner: ----
Status: Duplicate (was: Assigned)

Sign in to add a comment