Architecture for cross-process memory notification suppressing

content/browser/memory/memory_message_filter.h

Index: content/browser/memory/memory_message_filter.h
diff --git a/content/browser/memory/memory_message_filter.h b/content/browser/memory/memory_message_filter.h
new file mode 100644
index 0000000000000000000000000000000000000000..30003f1537e2b77dcb4858fc54f8ff891c8b6b37
--- /dev/null
+++ b/content/browser/memory/memory_message_filter.h
@@ -0,0 +1,47 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_BROWSER_MEMORY_MEMORY_MESSAGE_FILTER_H_
+#define CONTENT_BROWSER_MEMORY_MEMORY_MESSAGE_FILTER_H_
+
+#include <set>
+
+#include "base/lazy_instance.h"
+#include "content/public/browser/browser_message_filter.h"
+
+namespace content {
+namespace memory {
+
+// This class sends and receives memory messages on the browser process.
+// See also: child_memory_message_filter.h
+class MemoryMessageFilter : public BrowserMessageFilter {
+ public:
+  MemoryMessageFilter();
+
+  // BrowserMessageFilter implementation.
+  void OnFilterAdded(IPC::Sender* sender) override;
+  void OnFilterRemoved() override;
+  bool OnMessageReceived(const IPC::Message& message) override;
+
+  static void SendSetNotificationsSuppressedToAllProcesses(bool suppressed);

[Sami, 2015/09/08 18:06:56]

I'm slightly queasy about making this a static method (b/c testing), especially
if we're going to add more stuff here. I wonder if we should have something like
TracingControllerImpl on top:

https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/tr...

WDYT? Of course, YAGNI applies and what you have here is nice and simple.

[petrcermak, 2015/09/11 10:43:50]

Yeah, this is something that I thought about and I'm still not sure
what is the "right" thing to do here...

I totally agree that having a singleton object like MemoryController
would be better in many ways including testing. The only issue I see
with it is that it will create the following situation:

  Browser process:

    MemoryController (singleton)
      - suppresses memory pressure notifications in the process (*)
      - keeps a list of memory message filters for all child processes

    MemoryMessageFilter (per child process)
      - sends IPC messages to the child process

  Child processes:

    ChildMemoryMessageFilter (effectively singleton)
      - receives IPC messages from the browser process
      - suppresses memory pressure notifications in the process (**)

The problem is that the code for suppressing memory pressure
notifications is now triggered from two very different places:
the controller in the browser process (*) and the filter in the
child processes (**). The natural solution would be to have
controllers on both sides that have a common base class:

                       +------------------+
                       | MemoryController |
                       +------------------+
                                ^
                                |
               +----------------+----------------+
               |                                 |
  +-------------------------+       +-----------------------+
  | BrowserMemoryController |       | ChildMemoryController |
  +-------------------------+       +-----------------------+
               :                                 ^
               V                                 :
    +---------------------+        +--------------------------+
    | MemoryMessageFilter | =====> | ChildMemoryMessageFilter |
    +---------------------+  IPC   +--------------------------+

Maybe we should just go ahead with this to avoid refactors in the
future. WDYT?

[Sami, 2015/09/11 11:37:42]

As soon as you need ascii art in a code review comment you should discuss IRL --
which we did :)

[petrcermak, 2015/09/14 12:46:22]

I replaced the static behavior with a MemoryPressureController singleton.

+
+ protected:
+  ~MemoryMessageFilter() override;
+
+ private:
+  // Set of all memory message filters in the browser process. Always accessed
+  // on the UI thread.
+  typedef std::set<scoped_refptr<MemoryMessageFilter>> MemoryMessageFilterSet;
+  static base::LazyInstance<MemoryMessageFilterSet>::Leaky
+      memory_message_filters_;
+
+  void SendSetNotificationsSuppressed(bool suppressed);
+
+  DISALLOW_COPY_AND_ASSIGN(MemoryMessageFilter);
+};
+
+}  // namespace memory
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_MEMORY_MEMORY_MESSAGE_FILTER_H_