[profiling] auto-settings for v8_interpreted_frames

Make sure we can set js=False without having to set
v8_interpreted_frames as well. This makes it easier to quickly try
out a C++-only profile.

Drive-by-fix:
- Convert empty string "" to None in ObjectParser.optional_bool

Change-Id: I0715f11c03085b937b1d5e205c320be5fe6873c0
Reviewed-on: https://chromium-review.googlesource.com/c/crossbench/+/7801017
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Victor Vianna <victorvianna@google.com>

crossbench/parse.py

diff --git a/crossbench/parse.py b/crossbench/parse.py
index f41f870..a77d426 100644
--- a/crossbench/parse.py
+++ b/crossbench/parse.py
@@ -550,7 +550,7 @@
                     value: Any,
                     name: str = "value",
                     strict: bool = False) -> Optional[bool]:
-    if value is None:
+    if value is None or value == "":
       return None
     return cls.bool(value, name, strict)
 

crossbench/probes/profiling/system_profiling.py

diff --git a/crossbench/probes/profiling/system_profiling.py b/crossbench/probes/profiling/system_profiling.py
index 4d6cf37..879102e 100644
--- a/crossbench/probes/profiling/system_profiling.py
+++ b/crossbench/probes/profiling/system_profiling.py
@@ -87,8 +87,8 @@
               "for fewer uninteresting processes."))
     parser.add_argument(
         "v8_interpreted_frames",
-        type=bool,
-        default=True,
+        type=ObjectParser.optional_bool,
+        default=None,
         help=(
             f"Chrome-only: Sets the {V8_INTERPRETED_FRAMES_FLAG} flag for "
             "V8, which exposes interpreted frames as native frames. "
@@ -198,7 +198,7 @@
   def __init__(
       self,
       js: bool = True,
-      v8_interpreted_frames: bool = True,
+      v8_interpreted_frames: Optional[bool] = None,
       pprof: bool = True,
       cleanup: CleanupMode = CleanupMode.AUTO,
       browser_process: bool = False,
@@ -220,6 +220,8 @@
     self._spare_renderer_process: bool = spare_renderer_process
     self._run_pprof: bool = pprof
     self._cleanup_mode = cleanup
+    if v8_interpreted_frames is None:
+      v8_interpreted_frames = js
     self._expose_v8_interpreted_frames: bool = v8_interpreted_frames
     if v8_interpreted_frames:
       assert js, "Cannot expose V8 interpreted frames without js profiling."
@@ -261,6 +263,10 @@
     return self._sample_js
 
   @property
+  def expose_v8_interpreted_frames(self) -> bool:
+    return self._expose_v8_interpreted_frames
+
+  @property
   def sample_browser_process(self) -> bool:
     return self._sample_browser_process
 

tests/crossbench/probes/profiling/test_system_profiling.py

diff --git a/tests/crossbench/probes/profiling/test_system_profiling.py b/tests/crossbench/probes/profiling/test_system_profiling.py
index 2880bb5..009b91f 100644
--- a/tests/crossbench/probes/profiling/test_system_profiling.py
+++ b/tests/crossbench/probes/profiling/test_system_profiling.py
@@ -313,6 +313,26 @@
     self.assertEqual(probe.grouped_events, ("cache-references", "cache-misses"))
     self.assertEqual(probe.add_counters, ("aa", "bb"))
 
+  def test_v8_interpreted_frames_default(self):
+    probe = ProfilingProbe()
+    self.assertTrue(probe.expose_v8_interpreted_frames)
+
+    probe = ProfilingProbe(js=False)
+    self.assertFalse(probe.expose_v8_interpreted_frames)
+
+    probe = ProfilingProbe(js=True, v8_interpreted_frames=False)
+    self.assertFalse(probe.expose_v8_interpreted_frames)
+
+    with self.assertRaisesRegex(AssertionError,
+                                "Cannot expose V8 interpreted frames"):
+      ProfilingProbe(js=False, v8_interpreted_frames=True)
+
+    probe = ProfilingProbe.parse_dict({"js": False})
+    self.assertFalse(probe.expose_v8_interpreted_frames)
+
+    probe = ProfilingProbe.parse_dict({"js": True})
+    self.assertTrue(probe.expose_v8_interpreted_frames)
+
   def test_create_custom_frequency(self):
     probe = ProfilingProbe.parse_dict({"freq": "max"})
     self.assertEqual(probe.frequency, "max")

tests/crossbench/test_parse.py

diff --git a/tests/crossbench/test_parse.py b/tests/crossbench/test_parse.py
index 267e0fb..ca77219 100644
--- a/tests/crossbench/test_parse.py
+++ b/tests/crossbench/test_parse.py
@@ -634,12 +634,13 @@
 
   def test_parse_optional_bool(self):
     self.assertIsNone(ObjectParser.optional_bool(None))
+    self.assertIsNone(ObjectParser.optional_bool(""))
     self.assertIs(ObjectParser.optional_bool("true"), True)
     self.assertIs(ObjectParser.optional_bool("false"), False)
 
   def test_parse_optional_bool_invalid(self):
     invalid: Any
-    for invalid in (1, 0, "1", "0", "", [], ()):
+    for invalid in (1, 0, "1", "0", [], ()):
       with self.assertRaises(argparse.ArgumentTypeError):
         ObjectParser.optional_bool(invalid)
         ObjectParser.optional_bool(invalid, strict=True)