Always-on Document Leak Detector |
||||||||||||||||||
Issue descriptionWe're now implementing always-on leak detector with a UMA, that counts a Document's life time between detaching and destroying. The unit of time is GC count. The CL is https://chromium-review.googlesource.com/c/601569 (Add UMAs Document.OutLiveTimeAfterShutdown.GCCount). We assume that a non-leaky document should be destroyed soon after detached, but we found there are some cases that a document lives longer after detaching, because e.g. a document is held by the opened window via window.opener. Thus, the UMA still includes a lot of false positives and we need to improve this. Other thing is that UMA can't track URLs and we can't know where the leak happens. We're now thinking of other way than UMA. Thanks to this leak detector, we found a Document leak caused by find-in-page and fixed ( crbug.com/752902 ). This issue is for tracking tasks to improve the always-on leak detector.
,
Aug 21 2017
Awesome, nice work! Very excited to see this happen. :)
,
Aug 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/07a35cfbbc39af902fa53bbe5c251a013243d3c9 commit 07a35cfbbc39af902fa53bbe5c251a013243d3c9 Author: Hajime Hoshi <hajimehoshi@chromium.org> Date: Tue Aug 22 07:25:00 2017 Start counting document outlive time after all the related DOMWrapper disappear Now the UMA Document.OutLiveTimeAfterShutdown.GCCount counts GC age after the document is detached to detect leaks, but the problem is that not all documents die soon after detaching (e.g. when the document is held by a different window), and the UMA includes a lot of false positives as a leak detector. This CL reduces most of false positives by delaying to start count after the docuemnt-related DOM wrapper in all the worlds disappear. Bug: 757374 Change-Id: Icff6a0156899d03f9516477407ae780d73ba183c Reviewed-on: https://chromium-review.googlesource.com/623046 Reviewed-by: Hitoshi Yoshida <peria@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org> Cr-Commit-Position: refs/heads/master@{#496240} [modify] https://crrev.com/07a35cfbbc39af902fa53bbe5c251a013243d3c9/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/07a35cfbbc39af902fa53bbe5c251a013243d3c9/third_party/WebKit/Source/platform/bindings/DOMWrapperWorld.cpp [modify] https://crrev.com/07a35cfbbc39af902fa53bbe5c251a013243d3c9/third_party/WebKit/Source/platform/bindings/DOMWrapperWorld.h
,
Sep 13 2017
,
Sep 13 2017
,
Sep 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/55364a64915fe7aca37fc3e02d09a9fa667df006 commit 55364a64915fe7aca37fc3e02d09a9fa667df006 Author: Hajime Hoshi <hajimehoshi@chromium.org> Date: Fri Sep 29 03:24:39 2017 Add UKM Document.OutliveTimeAfterShutdown This CL adds a new UKM Document.OutliveTimeAfterShutdown, that is recorded when a Document object survives 5, 10, 20 or 50 garbage collections after detached. If a document outlives such long time, the document might be leaked. The UKM would be very useful to know where such leaky documents exist and to fix them. Design doc: https://docs.google.com/document/d/1fbs5smdd-pBLLMpq7u8EkyddZILtI7CZPJlo_AA1kak/edit?usp=sharing Bug: 757374 Change-Id: Idc05b03f625db11ab54c5203d18344b1ca72b4eb Reviewed-on: https://chromium-review.googlesource.com/647050 Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org> Reviewed-by: Kent Tamura <tkent@chromium.org> Reviewed-by: Chris Palmer <palmer@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Reviewed-by: Brett Wilson <brettw@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#505275} [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/content/public/app/mojo/content_renderer_manifest.json [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/services/metrics/public/cpp/ukm_recorder.h [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/third_party/WebKit/PRESUBMIT.py [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/third_party/WebKit/Source/core/DEPS [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/third_party/WebKit/Source/core/dom/BUILD.gn [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/third_party/WebKit/Source/core/dom/Document.h [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/third_party/WebKit/Source/platform/weborigin/KURL.cpp [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/third_party/WebKit/Source/platform/weborigin/KURL.h [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/merge_results.py [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/merge_results_unittest.py [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/tools/metrics/ukm/ukm.xml [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/url/url_canon.h [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/url/url_canon_etc.cc [modify] https://crrev.com/55364a64915fe7aca37fc3e02d09a9fa667df006/url/url_export.h
,
Sep 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/028cc7690869fa370c1cbaf49de17d5da2305f7c commit 028cc7690869fa370c1cbaf49de17d5da2305f7c Author: Brett Wilson <brettw@chromium.org> Date: Fri Sep 29 19:50:14 2017 Revert "Add UKM Document.OutliveTimeAfterShutdown" This reverts commit 55364a64915fe7aca37fc3e02d09a9fa667df006. Reason for revert: Broke component build https://build.chromium.org/p/chromium.gpu.fyi/builders/GPU%20Win%20Clang%20Builder%20%28dbg%29/builds/16604 While that's just an FYI bot, this also broke my local x86 Windows debug builds. Original change's description: > Add UKM Document.OutliveTimeAfterShutdown > > This CL adds a new UKM Document.OutliveTimeAfterShutdown, that is > recorded when a Document object survives 5, 10, 20 or 50 garbage > collections after detached. If a document outlives such long time, the > document might be leaked. The UKM would be very useful to know where such > leaky documents exist and to fix them. > > Design doc: https://docs.google.com/document/d/1fbs5smdd-pBLLMpq7u8EkyddZILtI7CZPJlo_AA1kak/edit?usp=sharing > > Bug: 757374 > Change-Id: Idc05b03f625db11ab54c5203d18344b1ca72b4eb > Reviewed-on: https://chromium-review.googlesource.com/647050 > Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org> > Reviewed-by: Kent Tamura <tkent@chromium.org> > Reviewed-by: Chris Palmer <palmer@chromium.org> > Reviewed-by: Steven Holte <holte@chromium.org> > Reviewed-by: Brett Wilson <brettw@chromium.org> > Reviewed-by: Kentaro Hara <haraken@chromium.org> > Reviewed-by: Nico Weber <thakis@chromium.org> > Cr-Commit-Position: refs/heads/master@{#505275} TBR=palmer@chromium.org,thakis@chromium.org,brettw@chromium.org,erikchen@chromium.org,hajimehoshi@chromium.org,haraken@chromium.org,tkent@chromium.org,holte@chromium.org Change-Id: Ie3e3e3ec3ce401d934cd5af4453e0ca92a1d8a63 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 757374 Reviewed-on: https://chromium-review.googlesource.com/692840 Reviewed-by: Brett Wilson <brettw@chromium.org> Commit-Queue: Brett Wilson <brettw@chromium.org> Cr-Commit-Position: refs/heads/master@{#505448} [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/content/public/app/mojo/content_renderer_manifest.json [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/services/metrics/public/cpp/ukm_recorder.h [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/third_party/WebKit/PRESUBMIT.py [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/third_party/WebKit/Source/core/DEPS [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/third_party/WebKit/Source/core/dom/BUILD.gn [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/third_party/WebKit/Source/core/dom/Document.h [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/third_party/WebKit/Source/platform/weborigin/KURL.cpp [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/third_party/WebKit/Source/platform/weborigin/KURL.h [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/merge_results.py [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/merge_results_unittest.py [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/tools/metrics/ukm/ukm.xml [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/url/url_canon.h [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/url/url_canon_etc.cc [modify] https://crrev.com/028cc7690869fa370c1cbaf49de17d5da2305f7c/url/url_export.h
,
Oct 2 2017
,
Oct 12 2017
,
Oct 18 2017
,
Oct 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/71c0f5b542ba0e51a699d84c22d5eed63799466b commit 71c0f5b542ba0e51a699d84c22d5eed63799466b Author: Hajime Hoshi <hajimehoshi@chromium.org> Date: Thu Oct 19 08:07:34 2017 Reland: Add UKM Document.OutliveTimeAfterShutdown This is basically same as https://chromium-review.googlesource.com/c/chromium/src/+/647050. This CL tries to reland the change after rebasing. This CL adds a new UKM Document.OutliveTimeAfterShutdown, that is recorded when a Document object survives 5, 10, 20 or 50 garbage collections after detached. If a document outlives such long time, the document might be leaked. The UKM would be very useful to know where such leaky documents exist and to fix them. Design doc: https://docs.google.com/document/d/1fbs5smdd-pBLLMpq7u8EkyddZILtI7CZPJlo_AA1kak/edit?usp=sharing Bug: 757374 Change-Id: I1f64d2a9260d898c386bb948dd25a1c5586f8eb7 Reviewed-on: https://chromium-review.googlesource.com/722301 Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Chris Palmer <palmer@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Cr-Commit-Position: refs/heads/master@{#510029} [modify] https://crrev.com/71c0f5b542ba0e51a699d84c22d5eed63799466b/third_party/WebKit/Source/core/dom/BUILD.gn [modify] https://crrev.com/71c0f5b542ba0e51a699d84c22d5eed63799466b/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/71c0f5b542ba0e51a699d84c22d5eed63799466b/third_party/WebKit/Source/core/dom/Document.h [modify] https://crrev.com/71c0f5b542ba0e51a699d84c22d5eed63799466b/tools/metrics/ukm/ukm.xml
,
Oct 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/91880a5748379dad1d7218ef183ce8828fad2e6f commit 91880a5748379dad1d7218ef183ce8828fad2e6f Author: Dirk Pranke <dpranke@chromium.org> Date: Thu Oct 19 23:18:55 2017 Revert "Reland: Add UKM Document.OutliveTimeAfterShutdown" This reverts commit 71c0f5b542ba0e51a699d84c22d5eed63799466b. Reason for revert: This CL logs to stderr on every test, causing the archive_webkit_layout_tests step to explode. See crbug.com/776334. Original change's description: > Reland: Add UKM Document.OutliveTimeAfterShutdown > > This is basically same as https://chromium-review.googlesource.com/c/chromium/src/+/647050. > This CL tries to reland the change after rebasing. > > This CL adds a new UKM Document.OutliveTimeAfterShutdown, that is > recorded when a Document object survives 5, 10, 20 or 50 garbage > collections after detached. If a document outlives such long time, the > document might be leaked. The UKM would be very useful to know where such > leaky documents exist and to fix them. > > Design doc: https://docs.google.com/document/d/1fbs5smdd-pBLLMpq7u8EkyddZILtI7CZPJlo_AA1kak/edit?usp=sharing > > Bug: 757374 > Change-Id: I1f64d2a9260d898c386bb948dd25a1c5586f8eb7 > Reviewed-on: https://chromium-review.googlesource.com/722301 > Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org> > Reviewed-by: Kentaro Hara <haraken@chromium.org> > Reviewed-by: Chris Palmer <palmer@chromium.org> > Reviewed-by: Steven Holte <holte@chromium.org> > Cr-Commit-Position: refs/heads/master@{#510029} TBR=palmer@chromium.org,hajimehoshi@chromium.org,haraken@chromium.org,holte@chromium.org Change-Id: I59155476ced5620a87e027225f9f017c296813e2 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 757374 Reviewed-on: https://chromium-review.googlesource.com/729462 Commit-Queue: Dirk Pranke <dpranke@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#510247} [modify] https://crrev.com/91880a5748379dad1d7218ef183ce8828fad2e6f/third_party/WebKit/Source/core/dom/BUILD.gn [modify] https://crrev.com/91880a5748379dad1d7218ef183ce8828fad2e6f/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/91880a5748379dad1d7218ef183ce8828fad2e6f/third_party/WebKit/Source/core/dom/Document.h [modify] https://crrev.com/91880a5748379dad1d7218ef183ce8828fad2e6f/tools/metrics/ukm/ukm.xml
,
Oct 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f5c45c745a3259e9b8ddafcb25f4348d2a87067f commit f5c45c745a3259e9b8ddafcb25f4348d2a87067f Author: Dirk Pranke <dpranke@chromium.org> Date: Thu Oct 19 23:40:03 2017 Try to fix botched merge of the revert in r510247. TBR=haraken@chromium.org BUG=757374, 776334 NOTRY=true NOTREECHECKS=true NOPRESUBMIT=true Change-Id: I36c3f44119b547c8c85efa7a03c66357c3f1a118 Reviewed-on: https://chromium-review.googlesource.com/729302 Commit-Queue: Dirk Pranke <dpranke@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Cr-Commit-Position: refs/heads/master@{#510248} [modify] https://crrev.com/f5c45c745a3259e9b8ddafcb25f4348d2a87067f/third_party/WebKit/Source/core/dom/Document.cpp
,
Oct 23 2017
It looks like the CL was reverted again due to the added log (crbug.com/776334) like below: [130790:130817:1023/182648.337334:1832551877662:ERROR:service_manager.cc(157)] Connection InterfaceProviderSpec prevented service: content_renderer from binding interface: ukm::mojom::UkmRecorderInterface exposed by: content_browser [130790:130817:1023/182648.467990:1832552008326:ERROR:service_manager.cc(157)] Connection InterfaceProviderSpec prevented service: content_renderer from binding interface: ukm::mojom::UkmRecorderInterface exposed by: content_browser
,
Oct 25 2017
,
Nov 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3c722d4240550989f41e517912c974798c49c4ed commit 3c722d4240550989f41e517912c974798c49c4ed Author: Hajime Hoshi <hajimehoshi@chromium.org> Date: Thu Nov 09 04:30:51 2017 Re-Reland: Add UKM Document.OutliveTimeAfterShutdown This is basically same as https://chromium-review.googlesource.com/c/chromium/src/+/722301 This fix originally caused a log explosion (crbug/776334). Now UKM is moved from a browser to a service, this problem has been fixed (crbug/733995). This CL adds a new UKM Document.OutliveTimeAfterShutdown, that is recorded when a Document object survives 5, 10, 20 or 50 garbage collections after detached. If a document outlives such long time, the document might be leaked. The UKM would be very useful to know where such leaky documents exist and to fix them. Design doc: https://docs.google.com/document/d/1fbs5smdd-pBLLMpq7u8EkyddZILtI7CZPJlo_AA1kak/edit?usp=sharing Bug: 757374 Change-Id: Ib8e3fdb0e8ed06842588d73a6e235208ccbcfbba Reviewed-on: https://chromium-review.googlesource.com/722301 Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Chris Palmer <palmer@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#510029} Reviewed-on: https://chromium-review.googlesource.com/757885 Cr-Commit-Position: refs/heads/master@{#515098} [modify] https://crrev.com/3c722d4240550989f41e517912c974798c49c4ed/third_party/WebKit/Source/core/dom/BUILD.gn [modify] https://crrev.com/3c722d4240550989f41e517912c974798c49c4ed/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/3c722d4240550989f41e517912c974798c49c4ed/third_party/WebKit/Source/core/dom/Document.h [modify] https://crrev.com/3c722d4240550989f41e517912c974798c49c4ed/tools/metrics/ukm/ukm.xml
,
Nov 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/768cf73afc5361a1203c422f98cf18b41dedff0c commit 768cf73afc5361a1203c422f98cf18b41dedff0c Author: John Budorick <jbudorick@chromium.org> Date: Thu Nov 09 17:14:27 2017 Revert "Re-Reland: Add UKM Document.OutliveTimeAfterShutdown" This reverts commit 3c722d4240550989f41e517912c974798c49c4ed. Reason for revert: Causing another log explosion: https://bugs.chromium.org/p/chromium/issues/detail?id=783221 Original change's description: > Re-Reland: Add UKM Document.OutliveTimeAfterShutdown > > This is basically same as https://chromium-review.googlesource.com/c/chromium/src/+/722301 > This fix originally caused a log explosion (crbug/776334). Now UKM is > moved from a browser to a service, this problem has been fixed > (crbug/733995). > > This CL adds a new UKM Document.OutliveTimeAfterShutdown, that is > recorded when a Document object survives 5, 10, 20 or 50 garbage > collections after detached. If a document outlives such long time, the > document might be leaked. The UKM would be very useful to know where such > leaky documents exist and to fix them. > > Design doc: https://docs.google.com/document/d/1fbs5smdd-pBLLMpq7u8EkyddZILtI7CZPJlo_AA1kak/edit?usp=sharing > > Bug: 757374 > Change-Id: Ib8e3fdb0e8ed06842588d73a6e235208ccbcfbba > Reviewed-on: https://chromium-review.googlesource.com/722301 > Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org> > Reviewed-by: Kentaro Hara <haraken@chromium.org> > Reviewed-by: Chris Palmer <palmer@chromium.org> > Reviewed-by: Steven Holte <holte@chromium.org> > Cr-Original-Commit-Position: refs/heads/master@{#510029} > Reviewed-on: https://chromium-review.googlesource.com/757885 > Cr-Commit-Position: refs/heads/master@{#515098} TBR=palmer@chromium.org,hajimehoshi@chromium.org,haraken@chromium.org,holte@chromium.org Change-Id: Ic0be7a1aac8f420ac37290682d5251b0a80b0de3 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 757374 Reviewed-on: https://chromium-review.googlesource.com/760398 Reviewed-by: John Budorick <jbudorick@chromium.org> Commit-Queue: John Budorick <jbudorick@chromium.org> Cr-Commit-Position: refs/heads/master@{#515194} [modify] https://crrev.com/768cf73afc5361a1203c422f98cf18b41dedff0c/third_party/WebKit/Source/core/dom/BUILD.gn [modify] https://crrev.com/768cf73afc5361a1203c422f98cf18b41dedff0c/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/768cf73afc5361a1203c422f98cf18b41dedff0c/third_party/WebKit/Source/core/dom/Document.h [modify] https://crrev.com/768cf73afc5361a1203c422f98cf18b41dedff0c/tools/metrics/ukm/ukm.xml
,
Nov 9 2017
+holte Hm, I thought the log problem was fixed after holte@ moved the UKM to service, but the problem was not fixed...
,
Nov 9 2017
,
Nov 9 2017
+palmer
,
Nov 9 2017
I think the issue is that your code is trying to get the interface via: frame_->GetInterfaceProvider().GetInterface(mojo::MakeRequest(&interface)); and that InterfaceProvider always requests content_renderer->content_browser I think you should be using: ukm::MojoUkmRecorder::Create(Platform::Current()->GetConnector()); As in: https://chromium-review.googlesource.com/c/chromium/src/+/611560/19/third_party/WebKit/Source/core/dom/Document.cpp
,
Nov 28 2017
+dpranke I'm trying the third trial https://chromium-review.googlesource.com/c/chromium/src/+/763187/6 but I found there are some errors on uploading log on mac_chromium_rel_ng bot and sometimes other bots: https://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/598782 Traceback (most recent call last): File "/b/rr/tmp_dTqni/rw/checkout/scripts/slave/recipe_modules/test_results/resources/upload_test_results.py", line 200, in <module> sys.exit(main(sys.argv[1:])) File "/b/rr/tmp_dTqni/rw/checkout/scripts/slave/recipe_modules/test_results/resources/upload_test_results.py", line 195, in main options.test_results_server, attrs, files, 120) File "/b/rr/tmp_dTqni/rw/checkout/scripts/slave/recipe_modules/test_results/resources/test_results_uploader.py", line 51, in upload_test_results timeout_secs) File "/b/rr/tmp_dTqni/rw/checkout/scripts/slave/recipe_modules/test_results/resources/test_results_uploader.py", line 135, in _retry_exp_backoff % (e.code, e.filename, e.read())) test_results_uploader.PermanentError: Received HTTP status 409 loading "https://test-results.appspot.com/testfile/upload": build number conflict step returned non-zero exit code: 1 I have no idea what is going on. dpranke@, could you give me any suggestions? Thanks,
,
Nov 28 2017
+dpranke
,
Nov 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/676ba628b79464bb399d475157acad5039fd5e3c commit 676ba628b79464bb399d475157acad5039fd5e3c Author: Hajime Hoshi <hajimehoshi@chromium.org> Date: Tue Nov 28 09:06:01 2017 Re^3 land Add UKM Document.OutliveTimeAfterShutdown This CL is the third trial to add the UKM Document. OutliveTimeAfterShutdown. This CL uses the existing Document:: UKMRecorder() and avoids requesting from content renderer to content browser. Actually, I have confirmed that this CL doesn't emit the problematic log "Connection InterfaceProviderSpec prevented service: content_renderer from binding interface: ukm::mojom::UkmRecorderInterface exposed by: content_browser" on my local machine. This CL adds a new UKM Document.OutliveTimeAfterShutdown, that is recorded when a Document object survives 5, 10, 20 or 50 garbage collections after detached. If a document outlives such long time, the document might be leaked. The UKM would be very useful to know where such leaky documents exist and to fix them. Design doc: https://docs.google.com/document/d/1fbs5smdd-pBLLMpq7u8EkyddZILtI7CZPJlo_AA1kak/edit?usp=sharing Bug: 757374 Change-Id: I4615f170626faab7151444b4a898c40761fbb6e1 Reviewed-on: https://chromium-review.googlesource.com/763187 Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#519618} [modify] https://crrev.com/676ba628b79464bb399d475157acad5039fd5e3c/components/ukm/test_ukm_recorder.cc [modify] https://crrev.com/676ba628b79464bb399d475157acad5039fd5e3c/components/ukm/test_ukm_recorder.h [modify] https://crrev.com/676ba628b79464bb399d475157acad5039fd5e3c/third_party/WebKit/Source/core/dom/BUILD.gn [modify] https://crrev.com/676ba628b79464bb399d475157acad5039fd5e3c/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/676ba628b79464bb399d475157acad5039fd5e3c/third_party/WebKit/Source/core/dom/Document.h [modify] https://crrev.com/676ba628b79464bb399d475157acad5039fd5e3c/third_party/WebKit/Tools/Scripts/audit-non-blink-usage.py [modify] https://crrev.com/676ba628b79464bb399d475157acad5039fd5e3c/tools/metrics/ukm/ukm.xml
,
Nov 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/26b03d9ee62594c971b9dabccdf0447047abf42f commit 26b03d9ee62594c971b9dabccdf0447047abf42f Author: Katie Thomas <katthomas@google.com> Date: Tue Nov 28 17:17:37 2017 Revert "Re^3 land Add UKM Document.OutliveTimeAfterShutdown" This reverts commit 676ba628b79464bb399d475157acad5039fd5e3c. Reason for revert: This breaks archive_webkit_tests Original change's description: > Re^3 land Add UKM Document.OutliveTimeAfterShutdown > > This CL is the third trial to add the UKM Document. > OutliveTimeAfterShutdown. This CL uses the existing Document:: > UKMRecorder() and avoids requesting from content renderer to content > browser. Actually, I have confirmed that this CL doesn't emit the > problematic log "Connection InterfaceProviderSpec prevented service: > content_renderer from binding interface: ukm::mojom::UkmRecorderInterface > exposed by: content_browser" on my local machine. > > This CL adds a new UKM Document.OutliveTimeAfterShutdown, that is > recorded when a Document object survives 5, 10, 20 or 50 garbage > collections after detached. If a document outlives such long time, the > document might be leaked. The UKM would be very useful to know where such > leaky documents exist and to fix them. > > Design doc: https://docs.google.com/document/d/1fbs5smdd-pBLLMpq7u8EkyddZILtI7CZPJlo_AA1kak/edit?usp=sharing > > Bug: 757374 > Change-Id: I4615f170626faab7151444b4a898c40761fbb6e1 > Reviewed-on: https://chromium-review.googlesource.com/763187 > Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org> > Reviewed-by: Steven Holte <holte@chromium.org> > Reviewed-by: Kentaro Hara <haraken@chromium.org> > Cr-Commit-Position: refs/heads/master@{#519618} TBR=hajimehoshi@chromium.org,haraken@chromium.org,holte@chromium.org Change-Id: I29014cd0266fe2f8d7c0137c3a1c5a8d419c5505 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 757374 Reviewed-on: https://chromium-review.googlesource.com/794152 Reviewed-by: John Budorick <jbudorick@chromium.org> Commit-Queue: Katie Thomas <katthomas@google.com> Cr-Commit-Position: refs/heads/master@{#519716} [modify] https://crrev.com/26b03d9ee62594c971b9dabccdf0447047abf42f/components/ukm/test_ukm_recorder.cc [modify] https://crrev.com/26b03d9ee62594c971b9dabccdf0447047abf42f/components/ukm/test_ukm_recorder.h [modify] https://crrev.com/26b03d9ee62594c971b9dabccdf0447047abf42f/third_party/WebKit/Source/core/dom/BUILD.gn [modify] https://crrev.com/26b03d9ee62594c971b9dabccdf0447047abf42f/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/26b03d9ee62594c971b9dabccdf0447047abf42f/third_party/WebKit/Source/core/dom/Document.h [modify] https://crrev.com/26b03d9ee62594c971b9dabccdf0447047abf42f/third_party/WebKit/Tools/Scripts/audit-non-blink-usage.py [modify] https://crrev.com/26b03d9ee62594c971b9dabccdf0447047abf42f/tools/metrics/ukm/ukm.xml
,
Nov 28 2017
troopers, could we get an assist on c#22? """ test_results_uploader.PermanentError: Received HTTP status 409 loading "https://test-results.appspot.com/testfile/upload": build number conflict """ katthomas, when you revert CLs, could you include a link to the failures? I don't know which master to look at, and which build is the relevant failure for archive_webkit_tests. Any chance you still remember the failure and could provide a link?
,
Nov 28 2017
How is this related to Blink>MemoryAllocator? What do you need me to help with?
,
Nov 28 2017
Re: c#22 / c#26 - that's kind of a strange error and my guess is that it was transient; are you getting that errors consistently?
,
Nov 28 2017
Here is a link to the bug with the relevant failures: https://bugs.chromium.org/p/chromium/issues/detail?id=789133 Here is a good example: https://uberchromegw.corp.google.com/i/chromium.mac/builders/Mac10.12%20Tests/builds/7723 The step is killed by logdog after 40 minutes with no output. You can see that the archive has been uploaded to storage, and that the compressed version is about 2x the size it was in the last successful build.
,
Nov 28 2017
Thanks for the links! Looking at the logs, it looks like tests are still emitting errors due to service_manager-related issues, this time just a *different* error. @hajimehoshi: When you run the tests locally, do you get output like this? Looking at Hajime's CL, nothing explicitly references a "metrics" service name. At a guess, the line in question is coming from: https://cs.chromium.org/chromium/src/content/public/app/mojo/content_renderer_manifest.json?type=cs&q=metrics+file:manifest.*json+-file:out/debug&sq=package:chromium&l=30 which is supposed to reference: https://cs.chromium.org/chromium/src/services/metrics/manifest.json?q=metrics+file:manifest.*json&sq=package:chromium&l=2&dr=C This latter gets registered in ChromeContentBrowserClient: https://cs.chromium.org/chromium/src/chrome/browser/chrome_content_browser_client.cc?type=cs&l=3152 which won't be included in Chromium builds. holte@, can you take a look at this?
,
Nov 28 2017
,
Nov 29 2017
FYI: part of the problem here is that I was supposed to add a check to run-webkit-tests after the last time this happened, to fail the build if too many tests have stderr results. I'll try to get to that now-ish.
,
Nov 29 2017
Sorry for the similar problem again... > @hajimehoshi: When you run the tests locally, do you get output like this? Yes, now I confirmed there is a difference of the layout test results. For example: $ ./out/Default/content_shell --run-layout-test fast/dom/icon-url-property.html Before the CL: [11140:11171:1129/132505.512037:ERROR:instance.cc(49)] Unable to locate service manifest for metrics [11140:11171:1129/132505.512128:ERROR:service_manager.cc(890)] Failed to resolve service name: metrics #READY DevTools listening on ws://127.0.0.1:33665/devtools/browser/49bb073d-5876-4b4e-ae8f-be4fd223fbae [11181:11181:1129/132506.357790:ERROR:gpu_info.cc(103)] No active GPU found, returning primary GPU. [11140:11171:1129/132507.741469:ERROR:instance.cc(49)] Unable to locate service manifest for metrics [11140:11171:1129/132507.741525:ERROR:service_manager.cc(890)] Failed to resolve service name: metrics main frame - didChangeIcons main frame - didChangeIcons main frame - didChangeIcons Content-Type: text/plain Original iconURL is: http://test.com/oldfavicon.ico Setting new icon URL to: http://test.com/newfavicon.ico New iconURL is: http://test.com/newfavicon.ico PASS - URL list matches expected #EOF #EOF After the CL: [4342:4371:1129/132340.742996:ERROR:instance.cc(49)] Unable to locate service manifest for metrics [4342:4371:1129/132340.743094:ERROR:service_manager.cc(890)] Failed to resolve service name: metrics #READY DevTools listening on ws://127.0.0.1:37088/devtools/browser/fec47d46-59db-4ceb-b837-9b4eae79ed33 [4378:4378:1129/132341.574124:ERROR:gpu_info.cc(103)] No active GPU found, returning primary GPU. [4342:4371:1129/132342.955021:ERROR:instance.cc(49)] Unable to locate service manifest for metrics [4342:4371:1129/132342.955092:ERROR:service_manager.cc(890)] Failed to resolve service name: metrics [4342:4371:1129/132343.033322:ERROR:instance.cc(49)] Unable to locate service manifest for metrics [4342:4371:1129/132343.033384:ERROR:service_manager.cc(890)] Failed to resolve service name: metrics main frame - didChangeIcons main frame - didChangeIcons main frame - didChangeIcons Content-Type: text/plain Original iconURL is: http://test.com/oldfavicon.ico Setting new icon URL to: http://test.com/newfavicon.ico New iconURL is: http://test.com/newfavicon.ico PASS - URL list matches expected #EOF #EOF [4342:4371:1129/132343.060400:ERROR:instance.cc(49)] Unable to locate service manifest for metrics [4342:4371:1129/132343.060455:ERROR:service_manager.cc(890)] Failed to resolve service name: metrics #EOF
,
Nov 29 2017
> How is this related to Blink>MemoryAllocator? It's because this (original) issue is about introducing UKM for memory leak detector.
,
Nov 29 2017
> [11140:11171:1129/132505.512037:ERROR:instance.cc(49)] Unable to locate service manifest for metrics > [11140:11171:1129/132505.512128:ERROR:service_manager.cc(890)] Failed to resolve service name: metrics Without the CL, IIUC this is caused by an attempt to use MojoUKMRecorder at components/viz (https://chromium-review.googlesource.com/c/chromium/src/+/744544). I was wondering if this error is expected and why the CL 744544 was OK (just amount of stderr mattered?).
,
Nov 29 2017
Another cause of the logs is https://chromium-review.googlesource.com/c/chromium/src/+/611560. As a result, the CL 611560 added 2 lines to stderr, the CL 744544 added 2 lines and lastly my CL 763187 added more 4 lines.
,
Nov 29 2017
This should be fixed by https://chromium-review.googlesource.com/c/chromium/src/+/795362. Can you patch it in and check?
,
Nov 30 2017
Confirmed that all logs (8 lines in total) were disappeared with holte@'s CL, however, there are some new warning 'Can't create service metrics.' with my UKM CL. hajimehoshi@hajimehoshi-z840:~/chromium/src$ ./out/Default/content_shell --run-layout-test fast/dom/icon-url-property.html [132615:132636:1130/130314.934105:ERROR:service_manager_connection_impl.cc(289)] Can't create service metrics. No handler found. #READY DevTools listening on ws://127.0.0.1:39506/devtools/browser/6e3165e6-5cad-4a6b-8222-e0d2379dbced [132642:132642:1130/130315.783524:ERROR:gpu_info.cc(103)] No active GPU found, returning primary GPU. [132615:132636:1130/130317.159810:ERROR:service_manager_connection_impl.cc(289)] Can't create service metrics. No handler found. main frame - didChangeIcons main frame - didChangeIcons main frame - didChangeIcons Content-Type: text/plain Original iconURL is: http://test.com/oldfavicon.ico Setting new icon URL to: http://test.com/newfavicon.ico New iconURL is: http://test.com/newfavicon.ico PASS - URL list matches expected #EOF #EOF #EOF
,
Nov 30 2017
> there are some new warning 'Can't create service metrics.' with my UKM CL. The above warnings happened with holte@'s CL but without my UKM CL. Additionally, this happens not only layout tests but also regular chrome. I'll leave my comment on the CL.
,
Dec 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1fe496ee541ede1f5a53539c5f73dff51ea59261 commit 1fe496ee541ede1f5a53539c5f73dff51ea59261 Author: Steven Holte <holte@google.com> Date: Sat Dec 09 01:49:05 2017 Move metrics service embedding to content. This should make a no-op version of the service available in layout_tests. Bug: 757374 Change-Id: I1f06e6db8af84ffd97be83c6f14242cfaa74f05d Reviewed-on: https://chromium-review.googlesource.com/795362 Reviewed-by: Ken Rockot <rockot@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Erik Chen <erikchen@chromium.org> Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Commit-Queue: Steven Holte <holte@chromium.org> Cr-Commit-Position: refs/heads/master@{#522958} [modify] https://crrev.com/1fe496ee541ede1f5a53539c5f73dff51ea59261/chrome/app/BUILD.gn [modify] https://crrev.com/1fe496ee541ede1f5a53539c5f73dff51ea59261/chrome/browser/BUILD.gn [modify] https://crrev.com/1fe496ee541ede1f5a53539c5f73dff51ea59261/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/1fe496ee541ede1f5a53539c5f73dff51ea59261/content/browser/BUILD.gn [modify] https://crrev.com/1fe496ee541ede1f5a53539c5f73dff51ea59261/content/browser/service_manager/service_manager_context.cc [modify] https://crrev.com/1fe496ee541ede1f5a53539c5f73dff51ea59261/content/public/app/BUILD.gn [modify] https://crrev.com/1fe496ee541ede1f5a53539c5f73dff51ea59261/services/metrics/manifest.json
,
Dec 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e48a05cca8abdd3ddc0e9a14825237da9beda971 commit e48a05cca8abdd3ddc0e9a14825237da9beda971 Author: Hajime Hoshi <hajimehoshi@chromium.org> Date: Wed Dec 13 07:15:23 2017 Re^4 land Add UKM Document.OutliveTimeAfterShutdown This is the 4th trial to add the UKM Document.OutliveTimeAfterShutdown. This change caused a problem that logs became huge, but now the problem has been fixed by holte@'s fix (https://chromium-review.googlesource.com/c/chromium/src/+/795362). This CL adds a new UKM Document.OutliveTimeAfterShutdown, that is recorded when a Document object survives 5, 10, 20 or 50 garbage collections after detached. If a document outlives such long time, the document might be leaked. The UKM would be very useful to know where such leaky documents exist and to fix them. Design doc: https://docs.google.com/document/d/1fbs5smdd-pBLLMpq7u8EkyddZILtI7CZPJlo_AA1kak/edit?usp=sharing Bug: 757374 Change-Id: I20c390c8f3856ed8a9cd733ab6ac18d667b12d95 Reviewed-on: https://chromium-review.googlesource.com/818676 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org> Cr-Commit-Position: refs/heads/master@{#523708} [modify] https://crrev.com/e48a05cca8abdd3ddc0e9a14825237da9beda971/components/ukm/test_ukm_recorder.cc [modify] https://crrev.com/e48a05cca8abdd3ddc0e9a14825237da9beda971/components/ukm/test_ukm_recorder.h [modify] https://crrev.com/e48a05cca8abdd3ddc0e9a14825237da9beda971/third_party/WebKit/Source/core/dom/BUILD.gn [modify] https://crrev.com/e48a05cca8abdd3ddc0e9a14825237da9beda971/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/e48a05cca8abdd3ddc0e9a14825237da9beda971/third_party/WebKit/Source/core/dom/Document.h [modify] https://crrev.com/e48a05cca8abdd3ddc0e9a14825237da9beda971/third_party/WebKit/Tools/Scripts/audit-non-blink-usage.py [modify] https://crrev.com/e48a05cca8abdd3ddc0e9a14825237da9beda971/tools/metrics/ukm/ukm.xml
,
Oct 17
|
||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||
Comment 1 by hayato@chromium.org
, Aug 21 2017