From 5b0c1c65343b0a12e989d34897ee20d1c4ab712a Mon Sep 17 00:00:00 2001
From: "Ben L. Titzer" <titzer@chromium.org>
Date: Fri, 18 Aug 2017 10:22:47 +0200
Subject: [PATCH] [frames] Unpack FrameSummary early in FrameInspector.

This CL is a precursor to the callback-based enumeration of frame summaries.
It removes the reliance of FrameInspector on having a cached copy of the
FrameSummary, instead unpacking it to instance variables so that clients
of FrameInspector do not need to get information from two sources
(the FrameSummary and the FrameInspector itself).

R=yangguo@chromium.org

Bug:
Change-Id: Ib388566c2e1a1147ee0a581323932982a29ae4ff
Reviewed-on: https://chromium-review.googlesource.com/618334
Commit-Queue: Ben Titzer <titzer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47420}
---
 src/debug/debug-frames.cc               | 43 ++++++++++++-------------
 src/debug/debug-frames.h                | 30 ++++++++++-------
 src/debug/debug-stack-trace-iterator.cc | 26 ++++++++-------
 src/runtime/runtime-debug.cc            |  8 ++---
 4 files changed, 58 insertions(+), 49 deletions(-)

diff --git a/src/debug/debug-frames.cc b/src/debug/debug-frames.cc
index 39c5b3b329..d646dc9cf1 100644
--- a/src/debug/debug-frames.cc
+++ b/src/debug/debug-frames.cc
@@ -14,13 +14,24 @@ namespace internal {
 FrameInspector::FrameInspector(StandardFrame* frame, int inlined_frame_index,
                                Isolate* isolate)
     : frame_(frame),
-      frame_summary_(FrameSummary::Get(frame, inlined_frame_index)),
       isolate_(isolate) {
+  // Extract the relevant information from the frame summary and discard it.
+  FrameSummary summary = FrameSummary::Get(frame, inlined_frame_index);
+
+  is_constructor_ = summary.is_constructor();
+  source_position_ = summary.SourcePosition();
+  function_name_ = summary.FunctionName();
+  script_ = Handle<Script>::cast(summary.script());
+  receiver_ = summary.receiver();
+
+  if (summary.IsJavaScript()) {
+    function_ = summary.AsJavaScript().function();
+  }
+
   JavaScriptFrame* js_frame =
       frame->is_java_script() ? javascript_frame() : nullptr;
   DCHECK(js_frame || frame->is_wasm());
   has_adapted_arguments_ = js_frame && js_frame->has_adapted_arguments();
-  is_bottommost_ = inlined_frame_index == 0;
   is_optimized_ = frame_->is_optimized();
   is_interpreted_ = frame_->is_interpreted();
 
@@ -38,10 +49,8 @@ FrameInspector::FrameInspector(StandardFrame* frame, int inlined_frame_index,
         js_frame, inlined_frame_index, isolate));
   } else if (frame_->is_wasm_interpreter_entry()) {
     wasm_interpreted_frame_ =
-        frame_summary_.AsWasm()
-            .wasm_instance()
-            ->debug_info()
-            ->GetInterpretedFrame(frame_->fp(), inlined_frame_index);
+        summary.AsWasm().wasm_instance()->debug_info()->GetInterpretedFrame(
+            frame_->fp(), inlined_frame_index);
     DCHECK(wasm_interpreted_frame_);
   }
 }
@@ -58,14 +67,6 @@ int FrameInspector::GetParametersCount() {
   return frame_->ComputeParametersCount();
 }
 
-Handle<Script> FrameInspector::GetScript() {
-  return Handle<Script>::cast(frame_summary_.script());
-}
-
-Handle<JSFunction> FrameInspector::GetFunction() {
-  return frame_summary_.AsJavaScript().function();
-}
-
 Handle<Object> FrameInspector::GetParameter(int index) {
   if (is_optimized_) return deoptimized_frame_->GetParameter(index);
   // TODO(clemensh): Handle wasm_interpreted_frame_.
@@ -83,17 +84,15 @@ Handle<Object> FrameInspector::GetExpression(int index) {
                        : handle(frame_->GetExpression(index), isolate_);
 }
 
-int FrameInspector::GetSourcePosition() {
-  return frame_summary_.SourcePosition();
-}
-
-bool FrameInspector::IsConstructor() { return frame_summary_.is_constructor(); }
-
 Handle<Object> FrameInspector::GetContext() {
-  return is_optimized_ ? deoptimized_frame_->GetContext()
-                       : handle(frame_->context(), isolate_);
+  return deoptimized_frame_ ? deoptimized_frame_->GetContext()
+                            : handle(frame_->context(), isolate_);
 }
 
+bool FrameInspector::IsWasm() { return frame_->is_wasm(); }
+
+bool FrameInspector::IsJavaScript() { return frame_->is_java_script(); }
+
 // To inspect all the provided arguments the frame might need to be
 // replaced with the arguments frame.
 void FrameInspector::SetArgumentsFrame(StandardFrame* frame) {
diff --git a/src/debug/debug-frames.h b/src/debug/debug-frames.h
index 2c9e43ff88..0a0d4329b9 100644
--- a/src/debug/debug-frames.h
+++ b/src/debug/debug-frames.h
@@ -25,16 +25,20 @@ class FrameInspector {
 
   ~FrameInspector();
 
-  FrameSummary& summary() { return frame_summary_; }
-
   int GetParametersCount();
-  Handle<JSFunction> GetFunction();
-  Handle<Script> GetScript();
+  Handle<JSFunction> GetFunction() { return function_; }
+  Handle<Script> GetScript() { return script_; }
   Handle<Object> GetParameter(int index);
   Handle<Object> GetExpression(int index);
-  int GetSourcePosition();
-  bool IsConstructor();
+  int GetSourcePosition() { return source_position_; }
+  bool IsConstructor() { return is_constructor_; }
   Handle<Object> GetContext();
+  Handle<Object> GetReceiver() { return receiver_; }
+
+  Handle<String> GetFunctionName() { return function_name_; }
+
+  bool IsWasm();
+  bool IsJavaScript();
 
   inline JavaScriptFrame* javascript_frame() {
     return frame_->is_arguments_adaptor() ? ArgumentsAdaptorFrame::cast(frame_)
@@ -58,14 +62,18 @@ class FrameInspector {
                                          Handle<String> parameter_name);
 
   StandardFrame* frame_;
-  FrameSummary frame_summary_;
   std::unique_ptr<DeoptimizedFrameInfo> deoptimized_frame_;
   std::unique_ptr<wasm::InterpretedFrame> wasm_interpreted_frame_;
   Isolate* isolate_;
-  bool is_optimized_;
-  bool is_interpreted_;
-  bool is_bottommost_;
-  bool has_adapted_arguments_;
+  Handle<Script> script_;
+  Handle<Object> receiver_;
+  Handle<JSFunction> function_;
+  Handle<String> function_name_;
+  int source_position_ = -1;
+  bool is_optimized_ = false;
+  bool is_interpreted_ = false;
+  bool has_adapted_arguments_ = false;
+  bool is_constructor_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(FrameInspector);
 };
diff --git a/src/debug/debug-stack-trace-iterator.cc b/src/debug/debug-stack-trace-iterator.cc
index caea1df9af..24b0f365a0 100644
--- a/src/debug/debug-stack-trace-iterator.cc
+++ b/src/debug/debug-stack-trace-iterator.cc
@@ -66,14 +66,18 @@ void DebugStackTraceIterator::Advance() {
 
 int DebugStackTraceIterator::GetContextId() const {
   DCHECK(!Done());
-  Object* value =
-      frame_inspector_->summary().native_context()->debug_context_id();
-  return (value->IsSmi()) ? Smi::ToInt(value) : 0;
+  Handle<Object> context = frame_inspector_->GetContext();
+  if (context->IsContext()) {
+    Object* value =
+        Context::cast(*context)->native_context()->debug_context_id();
+    if (value->IsSmi()) return Smi::ToInt(value);
+  }
+  return 0;
 }
 
 v8::Local<v8::Value> DebugStackTraceIterator::GetReceiver() const {
   DCHECK(!Done());
-  Handle<Object> value = frame_inspector_->summary().receiver();
+  Handle<Object> value = frame_inspector_->GetReceiver();
   if (value.is_null() || (value->IsSmi() || !value->IsTheHole(isolate_))) {
     return Utils::ToLocal(value);
   }
@@ -82,7 +86,7 @@ v8::Local<v8::Value> DebugStackTraceIterator::GetReceiver() const {
 
 v8::Local<v8::Value> DebugStackTraceIterator::GetReturnValue() const {
   DCHECK(!Done());
-  if (frame_inspector_->summary().IsWasm()) return v8::Local<v8::Value>();
+  if (frame_inspector_->IsWasm()) return v8::Local<v8::Value>();
   bool is_optimized = iterator_.frame()->is_optimized();
   if (is_optimized || !is_top_frame_ ||
       !isolate_->debug()->IsBreakAtReturn(iterator_.javascript_frame())) {
@@ -93,12 +97,12 @@ v8::Local<v8::Value> DebugStackTraceIterator::GetReturnValue() const {
 
 v8::Local<v8::String> DebugStackTraceIterator::GetFunctionName() const {
   DCHECK(!Done());
-  return Utils::ToLocal(frame_inspector_->summary().FunctionName());
+  return Utils::ToLocal(frame_inspector_->GetFunctionName());
 }
 
 v8::Local<v8::debug::Script> DebugStackTraceIterator::GetScript() const {
   DCHECK(!Done());
-  Handle<Object> value = frame_inspector_->summary().script();
+  Handle<Object> value = frame_inspector_->GetScript();
   if (!value->IsScript()) return v8::Local<v8::debug::Script>();
   return ToApiHandle<debug::Script>(Handle<Script>::cast(value));
 }
@@ -107,15 +111,13 @@ debug::Location DebugStackTraceIterator::GetSourceLocation() const {
   DCHECK(!Done());
   v8::Local<v8::debug::Script> script = GetScript();
   if (script.IsEmpty()) return v8::debug::Location();
-  return script->GetSourceLocation(
-      frame_inspector_->summary().SourcePosition());
+  return script->GetSourceLocation(frame_inspector_->GetSourcePosition());
 }
 
 v8::Local<v8::Function> DebugStackTraceIterator::GetFunction() const {
   DCHECK(!Done());
-  if (!frame_inspector_->summary().IsJavaScript())
-    return v8::Local<v8::Function>();
-  return Utils::ToLocal(frame_inspector_->summary().AsJavaScript().function());
+  if (!frame_inspector_->IsJavaScript()) return v8::Local<v8::Function>();
+  return Utils::ToLocal(frame_inspector_->GetFunction());
 }
 
 std::unique_ptr<v8::debug::ScopeIterator>
diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc
index 7f112e7fdc..06e30030c6 100644
--- a/src/runtime/runtime-debug.cc
+++ b/src/runtime/runtime-debug.cc
@@ -502,7 +502,7 @@ RUNTIME_FUNCTION(Runtime_GetFrameDetails) {
   Handle<Object> frame_id(DebugFrameHelper::WrapFrameId(it.frame()->id()),
                           isolate);
 
-  if (frame_inspector.summary().IsWasm()) {
+  if (frame_inspector.IsWasm()) {
     // Create the details array (no dynamic information for wasm).
     Handle<FixedArray> details =
         isolate->factory()->NewFixedArray(kFrameDetailsFirstDynamicIndex);
@@ -511,7 +511,7 @@ RUNTIME_FUNCTION(Runtime_GetFrameDetails) {
     details->set(kFrameDetailsFrameIdIndex, *frame_id);
 
     // Add the function name.
-    Handle<String> func_name = frame_inspector.summary().FunctionName();
+    Handle<String> func_name = frame_inspector.GetFunctionName();
     details->set(kFrameDetailsFunctionIndex, *func_name);
 
     // Add the script wrapper
@@ -526,7 +526,7 @@ RUNTIME_FUNCTION(Runtime_GetFrameDetails) {
     details->set(kFrameDetailsLocalCountIndex, Smi::kZero);
 
     // Add the source position.
-    int position = frame_inspector.summary().SourcePosition();
+    int position = frame_inspector.GetSourcePosition();
     details->set(kFrameDetailsSourcePositionIndex, Smi::FromInt(position));
 
     // Add the constructor information.
@@ -724,7 +724,7 @@ RUNTIME_FUNCTION(Runtime_GetFrameDetails) {
   }
 
   // Add the receiver (same as in function frame).
-  Handle<Object> receiver = frame_inspector.summary().receiver();
+  Handle<Object> receiver = frame_inspector.GetReceiver();
   DCHECK(function->shared()->IsUserJavaScript());
   // Optimized frames only restore the receiver as best-effort (see
   // OptimizedFrame::Summarize).