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

Issue 679030 link

Starred by 3 users
Status: Fixed
Owner:
eroman@chromium.org
Closed: Jul 2017
Cc:
█ xunji...@chromium.org
█ rdsmith@chromium.org
█ wangyix@chromium.org
eroman@chromium.org
mmenke@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Use bounded-size NetLog writer everywhere

Project Member Reported by eroman@chromium.org, Jan 6 2017 
Currently NetLog files written to disk (either by chrome://net-export or command line switch) are unbounded. The resulting file will continue growing until logging is stopped.

@dconnol added support as part of  issue 500543 . This needs to be wired up as the default implementation.

Comment 1 by eroman@chromium.org, Jan 6 2017

Cc: wangyix@chromium.org
+wangyix who has been doing work that unifies the implementations.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 28 2017 

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

commit 8ae679d977a7eb3c784d7bc6483cdf74400b52bd
Author: wangyix <wangyix@google.com>
Date: Sat Jan 28 01:47:40 2017

Move net-export thread-hopping code into NetLogFileWriter and add IO polled data.

(1) Use net::FileNetLogObserver in place of net::WriteToFileNetLogObserver for chrome://net-export (which is a big part of  crbug.com/679030 ).

(2) Move NetLogFileWriter's public interface from the FILE_USER_BLOCKING to the UI thread (internalizing the post tasks to file thread/UI thread to simplify things)

(3) Add plumbing for passing polled data, which will be collected on both the UI thread and IO thread ( crbug.com/438656 ). Some of the refactor done will also facilitate  crbug.com/679021 , which is just a few extra lines. In this CL, only the polled data from the IO thread will be retrieved; the UI thread polled data will be added in a follow-up CL.

BUG= 679030 , 438656 , 679021 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2603523002
Cr-Commit-Position: refs/heads/master@{#446875}

[modify] https://crrev.com/8ae679d977a7eb3c784d7bc6483cdf74400b52bd/chrome/browser/ui/webui/net_export_ui.cc
[modify] https://crrev.com/8ae679d977a7eb3c784d7bc6483cdf74400b52bd/components/cronet/android/cronet_url_request_context_adapter.cc
[modify] https://crrev.com/8ae679d977a7eb3c784d7bc6483cdf74400b52bd/components/net_log/BUILD.gn
[modify] https://crrev.com/8ae679d977a7eb3c784d7bc6483cdf74400b52bd/components/net_log/net_log_file_writer.cc
[modify] https://crrev.com/8ae679d977a7eb3c784d7bc6483cdf74400b52bd/components/net_log/net_log_file_writer.h
[modify] https://crrev.com/8ae679d977a7eb3c784d7bc6483cdf74400b52bd/components/net_log/net_log_file_writer_unittest.cc
[modify] https://crrev.com/8ae679d977a7eb3c784d7bc6483cdf74400b52bd/components/net_log/resources/net_export.js
[modify] https://crrev.com/8ae679d977a7eb3c784d7bc6483cdf74400b52bd/ios/chrome/browser/ui/webui/net_export/BUILD.gn
[modify] https://crrev.com/8ae679d977a7eb3c784d7bc6483cdf74400b52bd/ios/chrome/browser/ui/webui/net_export/net_export_ui.cc
[modify] https://crrev.com/8ae679d977a7eb3c784d7bc6483cdf74400b52bd/net/log/file_net_log_observer.cc
[modify] https://crrev.com/8ae679d977a7eb3c784d7bc6483cdf74400b52bd/net/log/file_net_log_observer.h
[modify] https://crrev.com/8ae679d977a7eb3c784d7bc6483cdf74400b52bd/net/log/file_net_log_observer_unittest.cc

Comment 3 by eroman@chromium.org, Jul 14 2017

Owner: eroman@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 18 2017 

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

commit 345b03dfacf9088d81ff47c63eef73d762670314
Author: Eric Roman <eroman@chromium.org>
Date: Tue Jul 18 20:08:37 2017

Unify the bounded and unbounded versions of FileNetLogObserver.

The directory-based file layout for bounded mode logging is a now an internal detail rather than part of the API contract. The result of logging, whether in bounded mode or unbounded mode, is now a single file using the same format.

This makes it possible to use either bounded or unbounded mode and get a compatible result.

Bug:  679030 
Change-Id: I501911678a98c14a3907d1407a91aabcbfa31d62
Reviewed-on: https://chromium-review.googlesource.com/570887
Commit-Queue: Eric Roman <eroman@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487574}
[modify] https://crrev.com/345b03dfacf9088d81ff47c63eef73d762670314/components/cronet/android/cronet_url_request_context_adapter.cc
[modify] https://crrev.com/345b03dfacf9088d81ff47c63eef73d762670314/net/log/file_net_log_observer.cc
[modify] https://crrev.com/345b03dfacf9088d81ff47c63eef73d762670314/net/log/file_net_log_observer.h
[modify] https://crrev.com/345b03dfacf9088d81ff47c63eef73d762670314/net/log/file_net_log_observer_unittest.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 18 2017 

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

commit 85c39b61393117d622bcda06ec4ff9e47fe6bfb1
Author: Eric Roman <eroman@chromium.org>
Date: Tue Jul 18 20:34:54 2017

Use a more efficient implementation for flattening a bounded-size NetLog to a single file.

Only read 64KiB of the events file at a time.

Bug:  679030 
Change-Id: If063b547d547240f79024350c1cead13e0f6426e
Reviewed-on: https://chromium-review.googlesource.com/572185
Commit-Queue: Eric Roman <eroman@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487591}
[modify] https://crrev.com/85c39b61393117d622bcda06ec4ff9e47fe6bfb1/net/log/file_net_log_observer.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 18 2017 

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

commit c2fadc96f9678d8b8aa87efd8f6cf1d30ee3ebbd
Author: Eric Roman <eroman@chromium.org>
Date: Tue Jul 18 21:02:37 2017

Enable bounded mode logging on chrome://net-export.

There is an option to configure the maximum size of the NetLog. The
default is 100MiB.

Bug:  679030 
Change-Id: I9260d7ee895d589c4789a350ca4fe1dfc8617b65
Reviewed-on: https://chromium-review.googlesource.com/571514
Commit-Queue: Eric Roman <eroman@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487602}
[modify] https://crrev.com/c2fadc96f9678d8b8aa87efd8f6cf1d30ee3ebbd/chrome/browser/ui/webui/net_export_ui.cc
[modify] https://crrev.com/c2fadc96f9678d8b8aa87efd8f6cf1d30ee3ebbd/components/net_log/net_export_file_writer.cc
[modify] https://crrev.com/c2fadc96f9678d8b8aa87efd8f6cf1d30ee3ebbd/components/net_log/net_export_file_writer.h
[modify] https://crrev.com/c2fadc96f9678d8b8aa87efd8f6cf1d30ee3ebbd/components/net_log/net_export_file_writer_unittest.cc
[modify] https://crrev.com/c2fadc96f9678d8b8aa87efd8f6cf1d30ee3ebbd/components/net_log/resources/net_export.css
[modify] https://crrev.com/c2fadc96f9678d8b8aa87efd8f6cf1d30ee3ebbd/components/net_log/resources/net_export.html
[modify] https://crrev.com/c2fadc96f9678d8b8aa87efd8f6cf1d30ee3ebbd/components/net_log/resources/net_export.js
[modify] https://crrev.com/c2fadc96f9678d8b8aa87efd8f6cf1d30ee3ebbd/ios/chrome/browser/ui/webui/net_export/net_export_ui.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 18 2017 

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

commit ab7f8ce792fc043b8a2a80b4fae0efba89328b07
Author: Eric Roman <eroman@chromium.org>
Date: Tue Jul 18 21:50:56 2017

Fix cronet bounded NetLog integration tests.

Bug:  679030 
Change-Id: I431e32e5287602fe3321e0ae803a0314e3d87ea2
Reviewed-on: https://chromium-review.googlesource.com/576364
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487618}
[modify] https://crrev.com/ab7f8ce792fc043b8a2a80b4fae0efba89328b07/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java

Comment 8 by eroman@chromium.org, Jul 18 2017

Status: Fixed (was: Assigned)
Marking it as fixed based on the milestone that chrome://net-export/ defaults to bounded mode now.

There is some more work that can be done for defaulting or configuring the log size from the various other consumers (command line and cronet). But it should be a simple matter of passing the appropriate file size bound to FileNetLogObserver now to change those policies.
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 18 2017 

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

commit 651b27378a324517d3cf31792d93f39307a5178b
Author: Eric Roman <eroman@chromium.org>
Date: Tue Jul 18 23:09:02 2017

Rename FileNetLogObserver::BoundedFileWriter to FileWriter.

Also removes the interface, since there is only the one implementation
now.

Bug:  679030 
Change-Id: I5a758c29284464a5341b2eae6665aca5d2cf2674
Reviewed-on: https://chromium-review.googlesource.com/576200
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487651}
[modify] https://crrev.com/651b27378a324517d3cf31792d93f39307a5178b/net/log/file_net_log_observer.cc
[modify] https://crrev.com/651b27378a324517d3cf31792d93f39307a5178b/net/log/file_net_log_observer.h
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 19 2017 

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

commit 5602e852d482212446cab44097f2907d64297217
Author: Eric Stevenson <estevenson@chromium.org>
Date: Wed Jul 19 17:24:09 2017

Revert "Enable bounded mode logging on chrome://net-export."

This reverts commit c2fadc96f9678d8b8aa87efd8f6cf1d30ee3ebbd.

Reason for revert: Added static initializer to MonochromePublic.apk.

Original change's description:
> Enable bounded mode logging on chrome://net-export.
> 
> There is an option to configure the maximum size of the NetLog. The
> default is 100MiB.
> 
> Bug:  679030 
> Change-Id: I9260d7ee895d589c4789a350ca4fe1dfc8617b65
> Reviewed-on: https://chromium-review.googlesource.com/571514
> Commit-Queue: Eric Roman <eroman@chromium.org>
> Reviewed-by: Helen Li <xunjieli@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#487602}

TBR=eroman@chromium.org,xunjieli@chromium.org

Change-Id: I7d8fb8daee69e381097b441f3ed42bd24ae252d1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  679030 ,  746430 
Reviewed-on: https://chromium-review.googlesource.com/577197
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487900}
[modify] https://crrev.com/5602e852d482212446cab44097f2907d64297217/chrome/browser/ui/webui/net_export_ui.cc
[modify] https://crrev.com/5602e852d482212446cab44097f2907d64297217/components/net_log/net_export_file_writer.cc
[modify] https://crrev.com/5602e852d482212446cab44097f2907d64297217/components/net_log/net_export_file_writer.h
[modify] https://crrev.com/5602e852d482212446cab44097f2907d64297217/components/net_log/net_export_file_writer_unittest.cc
[modify] https://crrev.com/5602e852d482212446cab44097f2907d64297217/components/net_log/resources/net_export.css
[modify] https://crrev.com/5602e852d482212446cab44097f2907d64297217/components/net_log/resources/net_export.html
[modify] https://crrev.com/5602e852d482212446cab44097f2907d64297217/components/net_log/resources/net_export.js
[modify] https://crrev.com/5602e852d482212446cab44097f2907d64297217/ios/chrome/browser/ui/webui/net_export/net_export_ui.cc
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 19 2017 

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

commit ca671fdcaaa15767c96a363c0ee9547683ff7f60
Author: Eric Roman <eroman@chromium.org>
Date: Wed Jul 19 20:20:52 2017

Add more descriptive message to in-progress (bounded mode) NetLog files

Bug:  679030 
Change-Id: If8794e2432994b88deb28e0edc2b38a986f07cbb
Reviewed-on: https://chromium-review.googlesource.com/576747
Commit-Queue: Eric Roman <eroman@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487948}
[modify] https://crrev.com/ca671fdcaaa15767c96a363c0ee9547683ff7f60/net/log/file_net_log_observer.cc
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 19 2017 

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

commit 5f82c9706c81766b4c444e336530ad721bc9d2ef
Author: Eric Roman <eroman@chromium.org>
Date: Wed Jul 19 20:44:55 2017

Enable bounded mode logging on chrome://net-export.

There is an option to configure the maximum size of the NetLog. The
default is 100MiB.

This is a re-land of https://chromium-review.googlesource.com/c/571514/

Bug:  679030 
Change-Id: I79eb7e6012deb0979b94a1c0f487068e7800105b
Reviewed-on: https://chromium-review.googlesource.com/578048
Commit-Queue: Eric Roman <eroman@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487965}
[modify] https://crrev.com/5f82c9706c81766b4c444e336530ad721bc9d2ef/chrome/browser/ui/webui/net_export_ui.cc
[modify] https://crrev.com/5f82c9706c81766b4c444e336530ad721bc9d2ef/components/net_log/net_export_file_writer.cc
[modify] https://crrev.com/5f82c9706c81766b4c444e336530ad721bc9d2ef/components/net_log/net_export_file_writer.h
[modify] https://crrev.com/5f82c9706c81766b4c444e336530ad721bc9d2ef/components/net_log/net_export_file_writer_unittest.cc
[modify] https://crrev.com/5f82c9706c81766b4c444e336530ad721bc9d2ef/components/net_log/resources/net_export.css
[modify] https://crrev.com/5f82c9706c81766b4c444e336530ad721bc9d2ef/components/net_log/resources/net_export.html
[modify] https://crrev.com/5f82c9706c81766b4c444e336530ad721bc9d2ef/components/net_log/resources/net_export.js
[modify] https://crrev.com/5f82c9706c81766b4c444e336530ad721bc9d2ef/ios/chrome/browser/ui/webui/net_export/net_export_ui.cc
[modify] https://crrev.com/5f82c9706c81766b4c444e336530ad721bc9d2ef/net/log/file_net_log_observer.cc
[modify] https://crrev.com/5f82c9706c81766b4c444e336530ad721bc9d2ef/net/log/file_net_log_observer.h
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 20 2017 

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

commit 8ec117da69bd48a21dfc724477e5620407e7fe1e
Author: Jan Wilken Dörrie <jdoerrie@chromium.org>
Date: Thu Jul 20 15:34:04 2017

Revert "Enable bounded mode logging on chrome://net-export."

This reverts commit 5f82c9706c81766b4c444e336530ad721bc9d2ef.

Reason for revert: Likely caused https://bugs.chromium.org/p/chromium/issues/detail?id=746813

Original change's description:
> Enable bounded mode logging on chrome://net-export.
> 
> There is an option to configure the maximum size of the NetLog. The
> default is 100MiB.
> 
> This is a re-land of https://chromium-review.googlesource.com/c/571514/
> 
> Bug:  679030 
> Change-Id: I79eb7e6012deb0979b94a1c0f487068e7800105b
> Reviewed-on: https://chromium-review.googlesource.com/578048
> Commit-Queue: Eric Roman <eroman@chromium.org>
> Reviewed-by: Helen Li <xunjieli@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#487965}

TBR=eroman@chromium.org,xunjieli@chromium.org

Change-Id: I4f955d9c289eef907b72aa637395ce56e36c4adf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  679030 
Reviewed-on: https://chromium-review.googlesource.com/579767
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488254}
[modify] https://crrev.com/8ec117da69bd48a21dfc724477e5620407e7fe1e/chrome/browser/ui/webui/net_export_ui.cc
[modify] https://crrev.com/8ec117da69bd48a21dfc724477e5620407e7fe1e/components/net_log/net_export_file_writer.cc
[modify] https://crrev.com/8ec117da69bd48a21dfc724477e5620407e7fe1e/components/net_log/net_export_file_writer.h
[modify] https://crrev.com/8ec117da69bd48a21dfc724477e5620407e7fe1e/components/net_log/net_export_file_writer_unittest.cc
[modify] https://crrev.com/8ec117da69bd48a21dfc724477e5620407e7fe1e/components/net_log/resources/net_export.css
[modify] https://crrev.com/8ec117da69bd48a21dfc724477e5620407e7fe1e/components/net_log/resources/net_export.html
[modify] https://crrev.com/8ec117da69bd48a21dfc724477e5620407e7fe1e/components/net_log/resources/net_export.js
[modify] https://crrev.com/8ec117da69bd48a21dfc724477e5620407e7fe1e/ios/chrome/browser/ui/webui/net_export/net_export_ui.cc
[modify] https://crrev.com/8ec117da69bd48a21dfc724477e5620407e7fe1e/net/log/file_net_log_observer.cc
[modify] https://crrev.com/8ec117da69bd48a21dfc724477e5620407e7fe1e/net/log/file_net_log_observer.h
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 20 2017 

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

commit 94bed167487c90db4d6c445ada0170d45397a4e0
Author: Eric Roman <eroman@chromium.org>
Date: Thu Jul 20 20:06:03 2017

Enable bounded mode logging on chrome://net-export.

There is an option to configure the maximum size of the NetLog. The
default is 100MiB.

This is a re-land that was originally reviewed at https://chromium-review.googlesource.com/c/571514/

TBR=xunjieli@chromium.org

Bug:  679030 
Change-Id: Ibf4fd80bca9ecd2d7899bc7039a930bbfc70ffa0
Reviewed-on: https://chromium-review.googlesource.com/580030
Reviewed-by: Eric Roman <eroman@chromium.org>
Commit-Queue: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488371}
[modify] https://crrev.com/94bed167487c90db4d6c445ada0170d45397a4e0/chrome/browser/ui/webui/net_export_ui.cc
[modify] https://crrev.com/94bed167487c90db4d6c445ada0170d45397a4e0/components/net_log/net_export_file_writer.cc
[modify] https://crrev.com/94bed167487c90db4d6c445ada0170d45397a4e0/components/net_log/net_export_file_writer.h
[modify] https://crrev.com/94bed167487c90db4d6c445ada0170d45397a4e0/components/net_log/net_export_file_writer_unittest.cc
[modify] https://crrev.com/94bed167487c90db4d6c445ada0170d45397a4e0/components/net_log/resources/net_export.css
[modify] https://crrev.com/94bed167487c90db4d6c445ada0170d45397a4e0/components/net_log/resources/net_export.html
[modify] https://crrev.com/94bed167487c90db4d6c445ada0170d45397a4e0/components/net_log/resources/net_export.js
[modify] https://crrev.com/94bed167487c90db4d6c445ada0170d45397a4e0/ios/chrome/browser/ui/webui/net_export/net_export_ui.cc
[modify] https://crrev.com/94bed167487c90db4d6c445ada0170d45397a4e0/net/log/file_net_log_observer.cc
[modify] https://crrev.com/94bed167487c90db4d6c445ada0170d45397a4e0/net/log/file_net_log_observer.h
►

Sign in to add a comment