New issue
Advanced search Search tips

Issue 833609 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Layout tests which navigate after notifyDone() are considered leaking

Project Member Reported by dgozman@chromium.org, Apr 16 2018

Issue description

http/tests/history/back-during-beforeunload.html
fast/forms/form-associated-element-crash2.html

Example build: https://test-results.appspot.com/data/layout_results/WebKit_Linux_Trusty_Leak/17757/layout-test-results/results.html
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 16 2018

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

commit e11ee5a27087548b93dd63124a270de72c67bd7b
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Mon Apr 16 21:56:00 2018

Add two tests to LeakExpectations

These were exposed by leak detector change, dgozman@ is going to fix them.

Bug:  833609 
Change-Id: I068afd816b67158a0d1ee5aef515525685398069
Reviewed-on: https://chromium-review.googlesource.com/1014462
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551135}
[modify] https://crrev.com/e11ee5a27087548b93dd63124a270de72c67bd7b/third_party/WebKit/LayoutTests/LeakExpectations

More flakes:

17757:
* fast/forms/form-associated-element-crash2.html
* http/tests/history/back-during-beforeunload.html
* virtual/mouseevent_fractional/fast/events/hr-timestamp/input-events.html
17758:
* fast/forms/form-associated-element-crash2.html
* fast/webgl/canvas-toDataURL-premul-overflow.html
* http/tests/history/back-during-beforeunload.html
17763:
* fast/forms/button/button-type-change.html
* fast/forms/form-associated-element-crash2.html
* fast/forms/form-submission-create-crash.xhtml
* http/tests/history/back-during-beforeunload.html

Reverting for now.
Project Member

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

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e11ee5a27087548b93dd63124a270de72c67bd7b

commit e11ee5a27087548b93dd63124a270de72c67bd7b
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Mon Apr 16 21:56:00 2018

Add two tests to LeakExpectations

These were exposed by leak detector change, dgozman@ is going to fix them.

Bug:  833609 
Change-Id: I068afd816b67158a0d1ee5aef515525685398069
Reviewed-on: https://chromium-review.googlesource.com/1014462
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551135}
[modify] https://crrev.com/e11ee5a27087548b93dd63124a270de72c67bd7b/third_party/WebKit/LayoutTests/LeakExpectations

Project Member

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

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

commit 9dbe74cf985f307fc28f93ee6e4fa744b752c141
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Wed Apr 18 23:51:14 2018

Layout Tests: fix race between leak detection and browser-side navigation

When the test navigates around the time testRunner.notifyDone is called,
we might end up sending request for navigation to the browser process.

Meanwhile, layout test harness navigates to about:blank and starts leak
detection. If browser side navigation returns to the renderer with actual
navigation after about:blank has been loaded, but before leak detection
did finish, we report leaked documents, nodes, etc.

To fix this race, we can call WebContents::Stop before asking renderer
to reset and prepare for leak detection. This will cancel any potential
navigation requests left over from the test.

Bug:  833609 
Change-Id: I845c4c2b16fa0805696efae5a8a95a548b988e33
Reviewed-on: https://chromium-review.googlesource.com/1015221
Reviewed-by: vmpstr <vmpstr@chromium.org>
Reviewed-by: Yuzu Saijo <yuzus@chromium.org>
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551882}
[modify] https://crrev.com/9dbe74cf985f307fc28f93ee6e4fa744b752c141/content/shell/browser/layout_test/blink_test_controller.cc
[modify] https://crrev.com/9dbe74cf985f307fc28f93ee6e4fa744b752c141/third_party/WebKit/LayoutTests/LeakExpectations

dgozman@ can this issue be marked as fixed now?
Let me land https://chromium-review.googlesource.com/c/chromium/src/+/1016021 and if there is no more flakiness, I'll mark this as fixed.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 20 2018

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

commit d016d8844cf4d787705983dd22d5026296adaf48
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Fri Apr 20 01:55:24 2018

Reland: Onion-soupify blink leak detector

Original patch exposed an issue in layout test controller, which was
fixed by https://chromium-review.googlesource.com/c/chromium/src/+/1015221.

> Onion-soupify blink leak detector
>
> - Expose LeakDetector interface in render process, remove unneeded plumbing.
> - Switch from IPCs to Mojo in layout test runner.
> - Move implementation to controller.
> - Migrate inspector usage to browser.
> - Merged two methods of LeakDetector into a single one.
> - Fixed leaks in existing tests.
>
> Needs a followup to not register fetchers in leak detector,
> but instead have a collection in ResourceFetcher.
>
> Reviewed-on: https://chromium-review.googlesource.com/999170
> Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
> Reviewed-by: Yuzu Saijo <yuzus@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#551012}

TBR=kolos,yuzus,dcheng,haraken
NOPRESUBMIT=true

Bug:  833609 
Change-Id: I6723ffc4aa6a88e6d6d88cc3850410dcd42aa51c
Reviewed-on: https://chromium-review.googlesource.com/1016021
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552228}
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/chrome/renderer/autofill/page_passwords_analyser_browsertest.cc
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/browser/DEPS
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/browser/devtools/protocol/memory_handler.cc
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/browser/devtools/protocol/memory_handler.h
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/browser/devtools/protocol_config.json
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/public/app/mojo/content_renderer_manifest.json
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/public/test/render_view_test.cc
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/public/test/render_view_test.h
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/shell/BUILD.gn
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/shell/browser/layout_test/blink_test_controller.cc
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/shell/browser/layout_test/blink_test_controller.h
[add] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/shell/browser/layout_test/leak_detector.cc
[add] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/shell/browser/layout_test/leak_detector.h
[delete] https://crrev.com/349171ef2357c024bb84faa43d842e52141b20ff/content/shell/common/leak_detection_result.h
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/shell/common/shell_messages.h
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/shell/renderer/layout_test/blink_test_runner.cc
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/shell/renderer/layout_test/blink_test_runner.h
[delete] https://crrev.com/349171ef2357c024bb84faa43d842e52141b20ff/content/shell/renderer/layout_test/leak_detector.cc
[delete] https://crrev.com/349171ef2357c024bb84faa43d842e52141b20ff/content/shell/renderer/layout_test/leak_detector.h
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/content/shell/test_runner/web_frame_test_client.cc
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/public/BUILD.gn
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/public/mojom/BUILD.gn
[add] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/public/mojom/leak_detector/OWNERS
[add] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/public/mojom/leak_detector/leak_detector.mojom
[delete] https://crrev.com/349171ef2357c024bb84faa43d842e52141b20ff/third_party/blink/public/web/web_leak_detector.h
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/renderer/controller/BUILD.gn
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/renderer/controller/blink_initializer.cc
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/renderer/controller/blink_initializer.h
[rename] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/renderer/controller/blink_leak_detector.cc
[add] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/renderer/controller/blink_leak_detector.h
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/renderer/core/BUILD.gn
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/renderer/core/core_initializer.h
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/renderer/core/css/css_default_style_sheets.h
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/renderer/core/dom/document.cc
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/renderer/core/inspector/InspectorMemoryAgent.cpp
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/renderer/core/inspector/InspectorMemoryAgent.h
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/renderer/core/inspector/inspector_protocol_config.json
[delete] https://crrev.com/349171ef2357c024bb84faa43d842e52141b20ff/third_party/blink/renderer/core/leak_detector/BUILD.gn
[delete] https://crrev.com/349171ef2357c024bb84faa43d842e52141b20ff/third_party/blink/renderer/core/leak_detector/README.md
[delete] https://crrev.com/349171ef2357c024bb84faa43d842e52141b20ff/third_party/blink/renderer/core/leak_detector/blink_leak_detector.h
[delete] https://crrev.com/349171ef2357c024bb84faa43d842e52141b20ff/third_party/blink/renderer/core/leak_detector/blink_leak_detector_client.h
[delete] https://crrev.com/349171ef2357c024bb84faa43d842e52141b20ff/third_party/blink/renderer/core/leak_detector/web_leak_detector.cc
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/renderer/core/loader/frame_fetch_context.cc
[modify] https://crrev.com/d016d8844cf4d787705983dd22d5026296adaf48/third_party/blink/renderer/modules/modules_initializer.cc

Status: Fixed (was: Assigned)
No new issues found, closing this one.

Sign in to add a comment