[inspector] Expose GetPossibleBreakpoints for wasm

src/inspector/wasm-translation.cc

Index: src/inspector/wasm-translation.cc
diff --git a/src/inspector/wasm-translation.cc b/src/inspector/wasm-translation.cc
index 825341e122d06ec0efa9377d5c53da77a8df93b2..e9cefc21f4a7b6a84527e8a6d8b6f10d953bf08a 100644
--- a/src/inspector/wasm-translation.cc
+++ b/src/inspector/wasm-translation.cc
@@ -33,6 +33,12 @@ class WasmTranslation::TranslatorImpl {
           column(column) {}
   };
 
+  TranslatorImpl(WasmTranslation* translation, int script_id) {
+    DCHECK_EQ(0, translation->wasm_translators_.count(script_id));
+    translation->wasm_translators_.insert(
+        std::make_pair(script_id, std::unique_ptr<TranslatorImpl>(this)));
+  }
+
   virtual void Translate(TransLocation *loc) = 0;
   virtual void TranslateBack(TransLocation *loc) = 0;
 
@@ -43,6 +49,8 @@ class WasmTranslation::TranslatorImpl {
 class WasmTranslation::TranslatorImpl::RawTranslator
     : public WasmTranslation::TranslatorImpl {
  public:
+  RawTranslator(WasmTranslation* translation, Local<debug::WasmScript> script)
+      : TranslatorImpl(translation, script->Id()) {}
   void Translate(TransLocation *loc) {}
   void TranslateBack(TransLocation *loc) {}
 };
@@ -52,10 +60,10 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
   using OffsetTable = debug::WasmDisassembly::OffsetTable;
 
  public:
-  DisassemblingTranslator(Isolate *isolate, Local<debug::WasmScript> script,
-                          WasmTranslation *translation,
-                          V8DebuggerAgentImpl *agent)
-      : script_(isolate, script) {
+  DisassemblingTranslator(Isolate* isolate, Local<debug::WasmScript> script,
+                          WasmTranslation* translation,
+                          V8DebuggerAgentImpl* agent)
+      : TranslatorImpl(translation, script->Id()), script_(isolate, script) {
     // Register fake scripts for each function in this wasm module/script.
     int num_functions = script->NumFunctions();
     int num_imported_functions = script->NumImportedFunctions();
@@ -101,6 +109,7 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
     const OffsetTable *reverse_table = GetReverseTable(func_index);
     if (!reverse_table) return;
     DCHECK(!reverse_table->empty());
+    v8::Isolate* isolate = loc->translation->isolate_;
 
     // Binary search for the given line and column.
     unsigned left = 0;                                              // inclusive
@@ -119,15 +128,24 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
     int found_byte_offset = 0;
     // If we found an exact match, use it. Otherwise check whether the next
     // bigger entry is still in the same line. Report that one then.
+    // Otherwise we might have hit the special case of pointing after the last
+    // line, which is translated to the end of the function (one byte after the
+    // last function byte).
     if ((*reverse_table)[left].line == loc->line &&
         (*reverse_table)[left].column == loc->column) {
       found_byte_offset = (*reverse_table)[left].byte_offset;
     } else if (left + 1 < reverse_table->size() &&
                (*reverse_table)[left + 1].line == loc->line) {
       found_byte_offset = (*reverse_table)[left + 1].byte_offset;
+    } else if (left == reverse_table->size() - 1 &&
+               (*reverse_table)[left].line == loc->line - 1 &&
+               loc->column == 0) {
+      std::pair<int, int> func_range =
+          script_.Get(isolate)->GetFunctionRange(func_index);
+      DCHECK_LE(func_range.first, func_range.second);
+      found_byte_offset = func_range.second - func_range.first;
     }
 
-    v8::Isolate *isolate = loc->translation->isolate_;
     loc->script_id = String16::fromInteger(script_.Get(isolate)->Id());
     loc->line = func_index;
     loc->column = found_byte_offset;
@@ -177,8 +195,9 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
     String16 source(disassembly.disassembly.data(),
                     disassembly.disassembly.length());
     std::unique_ptr<V8DebuggerScript> fake_script =
-        V8DebuggerScript::CreateWasm(isolate, script, fake_script_id,
-                                     std::move(fake_script_url), source);
+        V8DebuggerScript::CreateWasm(isolate, translation, script,
+                                     fake_script_id, std::move(fake_script_url),
+                                     source);
 
     translation->AddFakeScript(fake_script->scriptId(), this);
     agent->didParseSource(std::move(fake_script), true);
@@ -240,20 +259,21 @@ WasmTranslation::~WasmTranslation() { Clear(); }
 
 void WasmTranslation::AddScript(Local<debug::WasmScript> script,
                                 V8DebuggerAgentImpl *agent) {
-  int script_id = script->Id();
-  DCHECK_EQ(0, wasm_translators_.count(script_id));
-  std::unique_ptr<TranslatorImpl> impl;
+  // Instantiate new TranslatorImpls via new, the base class will register them
+  // in wasm_translators_, taking ownership. This early registration is
+  // necessary since the DisassemblingTranslator constructor already registers
+  // the fake scripts, which triggers setting breakpoints.
   switch (mode_) {
     case Raw:
-      impl.reset(new TranslatorImpl::RawTranslator());
+      new TranslatorImpl::RawTranslator(this, script);

[kozy, 2017/01/25 18:34:19]

Who will free this memory?

[Clemens Hammacher, 2017/01/25 20:23:54]

As documented above: the WasmTranslation takes ownership by putting them in
wasm_translators_.
The problem is that the constructor already triggers generation of the fake
scripts, which in turn triggers setting breakpoints. So the TranslatorImpl needs
to be registered right at the beginning of the constructor.
Alternatively, we could split it into a constructor and a init function, which
then creates the fake scripts. I thought about both options and decided for the
one implemented.

       break;
     case Disassemble:
-      impl.reset(new TranslatorImpl::DisassemblingTranslator(isolate_, script,
-                                                             this, agent));
+      new TranslatorImpl::DisassemblingTranslator(isolate_, script, this,
+                                                  agent);
       break;
+    default:

[kozy, 2017/01/25 18:34:19]

It's redundant and will silence compiler warning about not covered enum values.

[Clemens Hammacher, 2017/01/25 20:23:54]

Fixed.

+      UNREACHABLE();
   }
-  DCHECK(impl);
-  wasm_translators_.insert(std::make_pair(script_id, std::move(impl)));
 }
 
 void WasmTranslation::Clear() {