Convert SetContentSettingRules to use mojo, part 1/2.

chrome/common/renderer_configuration_struct_traits.cc

Index: chrome/common/renderer_configuration_struct_traits.cc
diff --git a/chrome/common/renderer_configuration_struct_traits.cc b/chrome/common/renderer_configuration_struct_traits.cc
new file mode 100644
index 0000000000000000000000000000000000000000..84aec065e349b8b2ec177855829d3968af8a86af
--- /dev/null
+++ b/chrome/common/renderer_configuration_struct_traits.cc
@@ -0,0 +1,169 @@
+// 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.
+
+#include "chrome/common/renderer_configuration_struct_traits.h"
+
+namespace mojo {
+
+// static
+
+chrome::mojom::ContentSetting
+EnumTraits<chrome::mojom::ContentSetting, ContentSetting>::ToMojom(
+    ContentSetting setting) {
+  switch (setting) {
+    case CONTENT_SETTING_DEFAULT:
+      return chrome::mojom::ContentSetting::CONTENT_SETTING_DEFAULT;
+    case CONTENT_SETTING_ALLOW:
+      return chrome::mojom::ContentSetting::CONTENT_SETTING_ALLOW;
+    case CONTENT_SETTING_BLOCK:
+      return chrome::mojom::ContentSetting::CONTENT_SETTING_BLOCK;
+    case CONTENT_SETTING_ASK:
+      return chrome::mojom::ContentSetting::CONTENT_SETTING_ASK;
+    case CONTENT_SETTING_SESSION_ONLY:
+      return chrome::mojom::ContentSetting::CONTENT_SETTING_SESSION_ONLY;
+    case CONTENT_SETTING_DETECT_IMPORTANT_CONTENT:
+      return chrome::mojom::ContentSetting::
+          CONTENT_SETTING_DETECT_IMPORTANT_CONTENT;
+    // XXX: Should we really catch the "NUM_SETTINGS" dummy value that defines

[Sam McNally, 2016/12/19 00:17:51]

I think I would leave it out of the mojom enum entirely.

[nigeltao1, 2016/12/22 04:09:46]

Done.

+    // a convenient constant for the number of enum values, not a semantically
+    // valid enum value in its own right? Ditto below.
+    case CONTENT_SETTING_NUM_SETTINGS:
+      return chrome::mojom::ContentSetting::CONTENT_SETTING_NUM_SETTINGS;
+    default:

[Sam McNally, 2016/12/19 00:17:51]

Put the catchall outside the switch instead of using default. That way the
compiler will enforce that new values are handled.

[nigeltao1, 2016/12/22 04:09:46]

Done.

+      // XXX: The NOTREACHED here mimics the one in
+      // ui/base/mojo/window_open_disposition_enum_traits.h
+      //
+      // Can we really assert NOTREACHED if Mojo messages can come from
+      // malicious renderers? Ditto below.

[Sam McNally, 2016/12/19 00:17:51]

Yes. This NOTREACHED would be hit in the sender of invalid values.

[nigeltao1, 2016/12/22 04:09:46]

Acknowledged.

+      NOTREACHED();
+      return chrome::mojom::ContentSetting::CONTENT_SETTING_DEFAULT;
+  }
+}
+
+// static
+bool EnumTraits<chrome::mojom::ContentSetting, ContentSetting>::FromMojom(
+    chrome::mojom::ContentSetting setting,
+    ContentSetting* out) {
+  switch (setting) {
+    case chrome::mojom::ContentSetting::CONTENT_SETTING_DEFAULT:
+      *out = CONTENT_SETTING_DEFAULT;
+      return true;
+    case chrome::mojom::ContentSetting::CONTENT_SETTING_ALLOW:
+      *out = CONTENT_SETTING_ALLOW;
+      return true;
+    case chrome::mojom::ContentSetting::CONTENT_SETTING_BLOCK:
+      *out = CONTENT_SETTING_BLOCK;
+      return true;
+    case chrome::mojom::ContentSetting::CONTENT_SETTING_ASK:
+      *out = CONTENT_SETTING_ASK;
+      return true;
+    case chrome::mojom::ContentSetting::CONTENT_SETTING_SESSION_ONLY:
+      *out = CONTENT_SETTING_SESSION_ONLY;
+      return true;
+    case chrome::mojom::ContentSetting::
+        CONTENT_SETTING_DETECT_IMPORTANT_CONTENT:
+      *out = CONTENT_SETTING_DETECT_IMPORTANT_CONTENT;
+      return true;
+    case chrome::mojom::ContentSetting::CONTENT_SETTING_NUM_SETTINGS:
+      *out = CONTENT_SETTING_NUM_SETTINGS;
+      return true;
+    default:
+      NOTREACHED();

[Sam McNally, 2016/12/19 00:17:51]

Again, move this out of the switch. I think it makes sense to remove this
NOTREACHED(); receiving invalid messages should just fail validation by
returning false.

[nigeltao1, 2016/12/22 04:09:45]

Done.

+      return false;
+  }
+}
+
+// Note that the {primary,secondary}_pattern methods aren't simple getters.
+// They serialize the ContentSettingsPattern to a string.
+
+// static
+const std::string

[Sam McNally, 2016/12/19 00:17:51]

This is inefficient. This accessor is called twice. If you do need to construct
a string, you'll want to use a context to store the temporary string.

[nigeltao1, 2016/12/22 04:09:45]

No longer applicable, as I serialize the underlying data structure, not a
string.

+StructTraits<chrome::mojom::ContentSettingPatternSourceDataView,
+             ContentSettingPatternSource>::
+    primary_pattern(const ContentSettingPatternSource& r) {
+  return r.primary_pattern.ToString();
+}
+
+// static
+const std::string
+StructTraits<chrome::mojom::ContentSettingPatternSourceDataView,
+             ContentSettingPatternSource>::
+    secondary_pattern(const ContentSettingPatternSource& r) {
+  return r.secondary_pattern.ToString();
+}
+
+// static
+ContentSetting StructTraits<chrome::mojom::ContentSettingPatternSourceDataView,
+                            ContentSettingPatternSource>::
+    setting(const ContentSettingPatternSource& r) {
+  return r.setting;
+}
+
+// static
+const std::string& StructTraits<
+    chrome::mojom::ContentSettingPatternSourceDataView,
+    ContentSettingPatternSource>::source(const ContentSettingPatternSource& r) {
+  return r.source;
+}
+
+// static
+bool StructTraits<chrome::mojom::ContentSettingPatternSourceDataView,
+                  ContentSettingPatternSource>::
+    incognito(const ContentSettingPatternSource& r) {
+  return r.incognito;
+}
+
+// static
+bool StructTraits<chrome::mojom::ContentSettingPatternSourceDataView,
+                  ContentSettingPatternSource>::
+    Read(chrome::mojom::ContentSettingPatternSourceDataView data,
+         ContentSettingPatternSource* out) {
+  std::string primary_pattern;
+  std::string secondary_pattern;
+  if (!data.ReadPrimaryPattern(&primary_pattern) ||
+      !data.ReadSecondaryPattern(&secondary_pattern) ||
+      !data.ReadSetting(&out->setting) || !data.ReadSource(&out->source))
+    return false;
+  out->primary_pattern = ContentSettingsPattern::FromString(primary_pattern);
+  out->secondary_pattern =
+      ContentSettingsPattern::FromString(secondary_pattern);
+  out->incognito = data.incognito();
+  return true;
+}
+
+// static
+const std::vector<ContentSettingPatternSource>& StructTraits<
+    chrome::mojom::RendererContentSettingRulesDataView,
+    RendererContentSettingRules>::image_rules(const RendererContentSettingRules&
+                                                  r) {
+  return r.image_rules;
+}
+
+// static
+const std::vector<ContentSettingPatternSource>&
+StructTraits<chrome::mojom::RendererContentSettingRulesDataView,
+             RendererContentSettingRules>::
+    script_rules(const RendererContentSettingRules& r) {
+  return r.script_rules;
+}
+
+// static
+const std::vector<ContentSettingPatternSource>&
+StructTraits<chrome::mojom::RendererContentSettingRulesDataView,
+             RendererContentSettingRules>::
+    autoplay_rules(const RendererContentSettingRules& r) {
+  return r.autoplay_rules;
+}
+
+// static
+bool StructTraits<chrome::mojom::RendererContentSettingRulesDataView,
+                  RendererContentSettingRules>::
+    Read(chrome::mojom::RendererContentSettingRulesDataView data,
+         RendererContentSettingRules* out) {
+  return data.ReadImageRules(&out->image_rules) &&
+         data.ReadScriptRules(&out->script_rules) &&
+         data.ReadAutoplayRules(&out->autoplay_rules);
+}
+
+}  // namespace mojo