New issue
Advanced search Search tips

Issue 715060 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocking:
issue 717103



Sign in to add a comment

Chrome watcher process doesn't initialize logging

Project Member Reported by grt@chromium.org, Apr 25 2017

Issue description

This means that any logging done from the watcher goes to debug.log next to chrome.exe rather than to chrome_debug.log in the user data dir.

For reference, other Chrome processes initialize logging in ChromeMainDelegate::SandboxInitialized.
 
Cc: pastarmovj@chromium.org
Status: Started (was: Assigned)
Status: Fixed (was: Started)
Blocking: 717103
Owner: brucedaw...@chromium.org
Status: Assigned (was: Fixed)
Unfortunately this wasn't fixed, as far as I can tell, because the logging switches were not passed along to the chrome_watcher process, so it never actually initialized itself properly. I'm fixing that as part of 717103.
Project Member

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

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

commit 5c9466ebd9a5800fa1631cadddda6c7b1ce65ae5
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Thu Jul 13 20:54:52 2017

Fix chrome_watcher logging size regression

When crrev.com/2850863002 added a call to InitChromeLogging() it had to
pull in //chrome/common:common, and that necessarily pulls in several MB
of code that is not wanted. The tendrils of dependencies are many and
varied and since a sword was disallowed there was no practical way to
cut this Gordian knot. See the investigation in  bug 717103  for details.

So, InitChromeLogging() was copied, named as InitChromeWatcherLogging(),
and made to work. This gets the size-cost of the logging code down to
~25 KB.

A non_code_constants target was added to //chrome/common because the
existing constants target was actually the main cause of excess code
being pulled in.

While testing this change it was discovered that the command-line
logging flags were not passed to the chrome_watcher process, so that
logging was not actually being enabled. This was also fixed.

It may be possible to delete more of the copied code, depending on what
logging features are actually needed. Thoughts?

Bug:  717103 ,  715060 
Change-Id: I536b0813fff32e513dfd3e0bbb8fc3cb715546c1
Reviewed-on: https://chromium-review.googlesource.com/567030
Reviewed-by: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486473}
[modify] https://crrev.com/5c9466ebd9a5800fa1631cadddda6c7b1ce65ae5/chrome/app/chrome_watcher_command_line_win.cc
[modify] https://crrev.com/5c9466ebd9a5800fa1631cadddda6c7b1ce65ae5/chrome/chrome_watcher/BUILD.gn
[modify] https://crrev.com/5c9466ebd9a5800fa1631cadddda6c7b1ce65ae5/chrome/chrome_watcher/chrome_watcher_main.cc
[modify] https://crrev.com/5c9466ebd9a5800fa1631cadddda6c7b1ce65ae5/chrome/common/BUILD.gn

Status: Fixed (was: Assigned)
I meant to delete the last paragraph from the commit message. Oops. Oh well. Fixed.
Ugh. Thank you.

Sign in to add a comment