Commit 909d320d authored by Avi Drissman's avatar Avi Drissman Committed by Commit Bot

Remove the static initializer from message_pump_mac.mm.

Static initializers are not allowed.

BUG=831951

Change-Id: I1c1d41ea66a34d38119c639c2af48183b9d1ab64
Reviewed-on: https://chromium-review.googlesource.com/1012870
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: default avatarNico Weber <thakis@chromium.org>
Reviewed-by: default avatarDirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551175}
parent 4f71dade
...@@ -25,41 +25,12 @@ ...@@ -25,41 +25,12 @@
namespace base { namespace base {
// kMessageLoopExclusiveRunLoopMode must be defined before kAllModes to generate
// a sane static initialization order.
const CFStringRef kMessageLoopExclusiveRunLoopMode = const CFStringRef kMessageLoopExclusiveRunLoopMode =
CFSTR("kMessageLoopExclusiveRunLoopMode"); CFSTR("kMessageLoopExclusiveRunLoopMode");
namespace { namespace {
// kCFRunLoopCommonModes is const but not constexpr; hence kAllModes is // Mask that determines which modes to use.
// initialized at run-time. This initialization being trivial, constant, and
// without side-effects: this is fine.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wglobal-constructors"
// AppKit RunLoop modes observed to potentially run tasks posted to Chrome's
// main thread task runner. Some are internal to AppKit but must be observed to
// keep Chrome's UI responsive. Others that may be interesting, but are not
// watched:
// - com.apple.hitoolbox.windows.transitionmode
// - com.apple.hitoolbox.windows.flushmode
const CFStringRef kAllModes[] = {
kCFRunLoopCommonModes,
// Mode that only sees Chrome work sources.
kMessageLoopExclusiveRunLoopMode,
// Process work when NSMenus are fading out.
CFSTR("com.apple.hitoolbox.windows.windowfadingmode"),
// Process work when AppKit is highlighting an item on the main menubar.
CFSTR("NSUnhighlightMenuRunLoopMode"),
};
#pragma clang diagnostic pop // -Wglobal-constructors
// Mask that determines which modes in |kAllModes| to use.
enum { kCommonModeMask = 0x1, kAllModesMask = 0xf }; enum { kCommonModeMask = 0x1, kAllModesMask = 0xf };
// Modes to use for MessagePumpNSApplication that are considered "safe". // Modes to use for MessagePumpNSApplication that are considered "safe".
...@@ -165,7 +136,33 @@ class MessagePumpCFRunLoopBase::ScopedModeEnabler { ...@@ -165,7 +136,33 @@ class MessagePumpCFRunLoopBase::ScopedModeEnabler {
CFRunLoopRemoveTimer(loop, owner_->delayed_work_timer_, mode()); CFRunLoopRemoveTimer(loop, owner_->delayed_work_timer_, mode());
} }
const CFStringRef& mode() const { return kAllModes[mode_index_]; } // This function knows about the AppKit RunLoop modes observed to potentially
// run tasks posted to Chrome's main thread task runner. Some are internal to
// AppKit but must be observed to keep Chrome's UI responsive. Others that may
// be interesting, but are not watched:
// - com.apple.hitoolbox.windows.transitionmode
// - com.apple.hitoolbox.windows.flushmode
const CFStringRef& mode() const {
static const CFStringRef modes[] = {
// The standard Core Foundation "common modes" constant. Must always be
// first in this list to match the value of kCommonModeMask.
kCFRunLoopCommonModes,
// Mode that only sees Chrome work sources.
kMessageLoopExclusiveRunLoopMode,
// Process work when NSMenus are fading out.
CFSTR("com.apple.hitoolbox.windows.windowfadingmode"),
// Process work when AppKit is highlighting an item on the main menubar.
CFSTR("NSUnhighlightMenuRunLoopMode"),
};
static_assert(arraysize(modes) == kNumModes, "mode size mismatch");
static_assert((1 << kNumModes) - 1 == kAllModesMask,
"kAllModesMask not large enough");
return modes[mode_index_];
}
private: private:
MessagePumpCFRunLoopBase* const owner_; // Weak. Owns this. MessagePumpCFRunLoopBase* const owner_; // Weak. Owns this.
...@@ -329,9 +326,7 @@ AutoreleasePoolType* MessagePumpCFRunLoopBase::CreateAutoreleasePool() { ...@@ -329,9 +326,7 @@ AutoreleasePoolType* MessagePumpCFRunLoopBase::CreateAutoreleasePool() {
} }
void MessagePumpCFRunLoopBase::SetModeMask(int mode_mask) { void MessagePumpCFRunLoopBase::SetModeMask(int mode_mask) {
static_assert(arraysize(enabled_modes_) == arraysize(kAllModes), for (size_t i = 0; i < kNumModes; ++i) {
"mode size mismatch");
for (size_t i = 0; i < arraysize(kAllModes); ++i) {
bool enable = mode_mask & (0x1 << i); bool enable = mode_mask & (0x1 << i);
if (enable == !enabled_modes_[i]) { if (enable == !enabled_modes_[i]) {
enabled_modes_[i] = enabled_modes_[i] =
...@@ -342,7 +337,7 @@ void MessagePumpCFRunLoopBase::SetModeMask(int mode_mask) { ...@@ -342,7 +337,7 @@ void MessagePumpCFRunLoopBase::SetModeMask(int mode_mask) {
int MessagePumpCFRunLoopBase::GetModeMask() const { int MessagePumpCFRunLoopBase::GetModeMask() const {
int mask = 0; int mask = 0;
for (size_t i = 0; i < arraysize(enabled_modes_); ++i) for (size_t i = 0; i < kNumModes; ++i)
mask |= enabled_modes_[i] ? (0x1 << i) : 0; mask |= enabled_modes_[i] ? (0x1 << i) : 0;
return mask; return mask;
} }
......
For instructions see This is where old perf machinery used to live, keeping track of binary sizes,
http://www.chromium.org/developers/tree-sheriffs/perf-sheriffs etc. Now that lives elsewhere and has a team to support it (see
https://www.chromium.org/developers/tree-sheriffs/perf-sheriffs). This code
remains to ensure that no static initializers get into Chromium.
Because this code has this history, it's far more complicated than it needs to
be. TODO(dpranke): Simplify it. https://crbug.com/572393
In the meanwhile, if you're trying to update perf_expectations.json, there are
no instructions for doing so, and the tools that you used to use don't work
because they rely on data files that were last updated at the end of 2015. So
here's what to do to reset the expected static initializer count value.
First, there's a bug in that on the Mac, static initializers are counted twice,
probably because it's expecting pointers to be 32 bits wide rather than 64. So
double the value.
The expected static initializer count value is in the "regress" field for the
platform. In addition, each platform has a checksum in the "sha1" field to
ensure that you properly used the magic tools. Since the magic tools don't work
anymore, dpranke added a bypass to the verification. If you run:
> tools/perf_expectations/make_expectations.py --checksum --verbose
the script will tell you what the checksum *should* be. Alter the "sha1" field
to be that value, and you can commit changes to that file.
Please see https://crbug.com/572393 for more information.
...@@ -4,6 +4,6 @@ ...@@ -4,6 +4,6 @@
"linux-release/sizes/chrome-si/initializers": {"reva": 480969, "revb": 480969, "type": "absolute", "better": "lower", "improve": 9, "regress": 9, "tolerance": 0, "sha1": "03dc3cfd"}, "linux-release/sizes/chrome-si/initializers": {"reva": 480969, "revb": 480969, "type": "absolute", "better": "lower", "improve": 9, "regress": 9, "tolerance": 0, "sha1": "03dc3cfd"},
"linux-release/sizes/nacl_helper-si/initializers": {"reva": 480969, "revb": 480969, "type": "absolute", "better": "lower", "improve": 7, "regress": 9, "sha1": "1a3c5b2b"}, "linux-release/sizes/nacl_helper-si/initializers": {"reva": 480969, "revb": 480969, "type": "absolute", "better": "lower", "improve": 7, "regress": 9, "sha1": "1a3c5b2b"},
"linux-release/sizes/nacl_helper_bootstrap-si/initializers": {"reva": 114822, "revb": 115019, "type": "absolute", "better": "lower", "improve": 0, "regress": 0, "sha1": "dd908f29"}, "linux-release/sizes/nacl_helper_bootstrap-si/initializers": {"reva": 114822, "revb": 115019, "type": "absolute", "better": "lower", "improve": 0, "regress": 0, "sha1": "dd908f29"},
"mac-release/sizes/chrome-si/initializers": {"reva": 281731, "revb": 281731, "type": "absolute", "better": "lower", "improve": 0, "regress": 4, "tolerance": 0, "sha1": "b45b7911"}, "mac-release/sizes/chrome-si/initializers": {"reva": 281731, "revb": 281731, "type": "absolute", "better": "lower", "improve": 0, "regress": 2, "tolerance": 0, "sha1": "f2370203"},
"load": true "load": true
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment