Print preview is probably leaking WebLocalFrames |
||||||||
Issue descriptionhttps://codereview.chromium.org/2369613003 changed how we cleaned up frames on detach, to avoid a bunch of boilerplate in tests. One of these changes was pushing boilerplate frame cleanup into the base interface. https://codereview.chromium.org/2874133002 deleted the default implementation but print web view helper did not get the default methods added back.
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ba508351c5ea1cfdc468bb1c4a92ea4bf38c0556 commit ba508351c5ea1cfdc468bb1c4a92ea4bf38c0556 Author: Daniel Cheng <dcheng@chromium.org> Date: Fri Jun 23 08:49:36 2017 Don't leak WebLocalFrame in print preview. Unfortunately, Blink currently relies on the embedder to make sure these objects don't leak. Bug: 735860 Change-Id: Ia213d4360c5f95627de70a1dfe343880144f9ff6 Reviewed-on: https://chromium-review.googlesource.com/544367 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#481829} [modify] https://crrev.com/ba508351c5ea1cfdc468bb1c4a92ea4bf38c0556/components/printing/renderer/print_web_view_helper.cc
,
Jun 23 2017
I'm not sure how significant the leak is, but it's a simple fix and doesn't hurt.
,
Jun 23 2017
,
Jun 24 2017
This bug requires manual review: M60 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 26 2017
Approving merge to M60. Regression, simple fix.
,
Jun 30 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 3 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 10 2017
Please merge this to M60 ASAP. branch:3112
,
Jul 17 2017
Sorry. I missed the email because I was OOO. Merging now.
,
Jul 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/09d8b19232cec8603cc3618f977d91103277e571 commit 09d8b19232cec8603cc3618f977d91103277e571 Author: Daniel Cheng <dcheng@chromium.org> Date: Mon Jul 17 10:07:34 2017 Don't leak WebLocalFrame in print preview. Unfortunately, Blink currently relies on the embedder to make sure these objects don't leak. TBR=dcheng@chromium.org (cherry picked from commit ba508351c5ea1cfdc468bb1c4a92ea4bf38c0556) Bug: 735860 Change-Id: Ia213d4360c5f95627de70a1dfe343880144f9ff6 Reviewed-on: https://chromium-review.googlesource.com/544367 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Daniel Cheng <dcheng@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#481829} Reviewed-on: https://chromium-review.googlesource.com/573881 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/branch-heads/3112@{#616} Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897} [modify] https://crrev.com/09d8b19232cec8603cc3618f977d91103277e571/components/printing/renderer/print_web_view_helper.cc
,
Jul 19 2017
dcheng@ - Could you please provide reproducible steps with expected and actual results to verify the issue from TE-end. Thanks...!!
,
Jul 19 2017
I don't know that there's good repro steps. I suspect that if you open and close print preview enough times on one page with headers and footers enabled, you'll be able to see memory usage increasing over time (before my change) and hopefully remaining roughly constant afterwards. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by dcheng@chromium.org
, Jun 22 2017