Replicating LayoutDumpFlags across OOPIFs.

components/test_runner/tracked_dictionary.h

+// Copyright 2016 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 COMPONENTS_TEST_RUNNER_TRACKED_DICTIONARY_H_
+#define COMPONENTS_TEST_RUNNER_TRACKED_DICTIONARY_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/values.h"
+#include "components/test_runner/test_runner_export.h"
+
+namespace test_runner {
+
+class TEST_RUNNER_EXPORT TrackedDictionary {
+ public:
+  TrackedDictionary();
+
+  // Current value of the tracked dictionary.
+  const base::DictionaryValue& current_values() const {
+    return current_values_;
+  }
+
+  // Subset of |current_values| that have been changed (i.e. via Set method)
+  // since the last call to ResetChangeTracking.
+  const base::DictionaryValue& changed_values() const {
+    return changed_values_;
+  }
+
+  // Clears out |changed_values|.
+  void ResetChangeTracking();
+
+  // Overwrites |current_values| with values present in |new_changes|.
+  // Application of the new values is not tracked - they will not appear in
+  // |changed_values|.
+  void ApplyUntrackedChanges(const base::DictionaryValue& new_changes);
+
+  // Sets a value in |current_values| and tracks the change in |changed_values|.
+  void Set(const std::string& path, scoped_ptr<base::Value> new_value);
+
+  // Type-specific setter for convenience.
+  void SetBoolean(const std::string& path, bool new_value);
+
+ private:
+  base::DictionaryValue current_values_;
+  base::DictionaryValue changed_values_;
+
+  DISALLOW_COPY_AND_ASSIGN(TrackedDictionary);
+};
+
+}  // namespace test_runner
+
+#endif  // COMPONENTS_TEST_RUNNER_TRACKED_DICTIONARY_H_

[Łukasz Anforowicz, 2016/03/03 21:46:34]

I like the TrackedDictionary approach:

- This avoids browser->layout_dump_flags.h dependency - the browser can treat
base::DictionaryValue more or less opaquely (more-or-less, because of the single
call to MergeDictionary - I think this is still pretty opaque / oblivious to
what actual values are allowed+stored in the dictionary).  This benefit would be
gone with optional<T> and shared-memory approach (although in shared-memory
approach only sizeof(LayoutDumpFlags) is needed).

- Usage of base::DictionaryValue instead of introducing optional<T> to Chromium
already has a precedent in remoting::PolicyWatcher::PolicyUpdatedCallback
(disclaimer: I touched this code, but I didn’t introduce usage of the
DictionaryValue [and I did think about introducing optional<T> for
remoting::PolicyWatcher but decided that it would be just a bit of busywork with
no big benefit]).

- At one point I thought that shared memory approach (i.e.
https://codereview.chromium.org/1736353002/) might be cleaner, but dcheng@
rightfully pointed out that I haven’t thought about the need to use volatile and
memory fences and that even with those the semantics/safety is not clear.

- BTW: I use base::DictionaryValue rather than std::map<std::string, bool>,
because in the future LayoutDumpFlags might also contain other data types -
custom_text_output, http_headers_to_clear, audio_data, tooltip_text, etc.