NOTIMPLEMENTED_POLICY will affect more than the current file with jumbo, or no file at all |
||
Issue descriptionNOTIMPLEMENTED_POLICY is an optional define that can be set to an integer 0-5 before including base/logging.h. That will affect how NOTIMPLEMENTED() is implemented between No-op, to "log once per run" using a local static variable. With jumbo it will always get the default value (4 except on Android which has 0) unless the very first file in the jumbo chunk happens to redefine it. I suggest replacing NOTIMPLEMENTED_POLICY with different NOTIMPLEMENTED() macros. 5 could be NOTIMPLEMENTED_TERSE()
,
Oct 24 2017
Looking through the whole tree, I can see four files (one in //chrome and three in //ui) where it is set to 5 and two build files that do the same (//ash/mus and //services/ui/gpu) and that is all. Those can use a new macro. 1, 2, 3 are not used. 4 is default on desktop, 0 on Android so I imagine that the current macro can cover those files. So this doesn't look too bad at all.
,
Nov 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ff54119409d44ed3ba74d62cea3c2a81e7d36483 commit ff54119409d44ed3ba74d62cea3c2a81e7d36483 Author: Daniel Bratell <bratell@opera.com> Date: Thu Nov 02 14:22:28 2017 Replace the NOTIMPLEMENTED_POLICY macro with an explicit macro NOTIMPLEMENTED_POLICY could be used to control what the NOTIMPLEMENTED macro did, but *only* if set before the first include of base/logging.h. That made it hard to use and not at all compatible with jumbo builds. There were only a few users of NOTIMPLEMENTED_POLICY and they all set NOTIMPLEMENTED_POLICY to 5 so a better solution seems to be to have NOTIMPLEMENTED and NOTIMPLEMENTED_LOG_ONCE for places where spam is feared. Bug: 777852 Change-Id: Id1b454200a64d43e27df39b2c9a02f07ba4710cf Reviewed-on: https://chromium-review.googlesource.com/749143 Commit-Queue: Daniel Bratell <bratell@opera.com> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: danakj <danakj@chromium.org> Cr-Commit-Position: refs/heads/master@{#513481} [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/ash/mus/BUILD.gn [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/ash/mus/accelerators/accelerator_controller_delegate_mus.cc [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/ash/mus/ash_window_tree_host_mus.cc [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/ash/mus/bridge/immersive_handler_factory_mus.cc [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/ash/mus/bridge/shell_port_mash.cc [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/ash/mus/drag_window_resizer.cc [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/ash/mus/network_connect_delegate_mus.cc [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/ash/mus/shell_delegate_mus.cc [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/ash/mus/wallpaper_delegate_mus.cc [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/base/logging.h [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/chrome/browser/ui/views/ime_driver/remote_text_input_client.cc [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/components/viz/service/main/BUILD.gn [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/media/capture/video/linux/video_capture_device_factory_linux.cc [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/ui/display/screen_base.cc [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/ui/views/mus/screen_mus.cc [modify] https://crrev.com/ff54119409d44ed3ba74d62cea3c2a81e7d36483/ui/wm/core/easy_resize_window_targeter.cc
,
Nov 6 2017
And it's gone! (Closing as fixed) |
||
►
Sign in to add a comment |
||
Comment 1 by most...@vewd.com
, Oct 24 2017