[inspector] added type of break location into getPossibleBreakpoints output

src/api.cc

 // Copyright 2012 the V8 project 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 "src/api.h"
 
 #include <string.h>  // For memcpy, strlen.
 #ifdef V8_USE_ADDRESS_SANITIZER
 #include <sanitizer/asan_interface.h>
 #endif  // V8_USE_ADDRESS_SANITIZER
@@ skipping 9222 matching lines @@
 }
 
 namespace {
 int GetSmiValue(i::Handle<i::FixedArray> array, int index) {
   return i::Smi::cast(array->get(index))->value();
 }
 }  // namespace
 
 bool debug::Script::GetPossibleBreakpoints(
     const debug::Location& start, const debug::Location& end,
-    bool restrict_to_function, std::vector<debug::Location>* locations) const {
+    bool restrict_to_function,
+    std::vector<debug::BreakLocation>* locations) const {
   CHECK(!start.IsEmpty());
   i::Handle<i::Script> script = Utils::OpenHandle(this);
   if (script->type() == i::Script::TYPE_WASM) {
     i::Handle<i::WasmCompiledModule> compiled_module(
         i::WasmCompiledModule::cast(script->wasm_compiled_module()));
     return compiled_module->GetPossibleBreakpoints(start, end, locations);
   }
 
   i::Script::InitLineEnds(script);
   CHECK(script->line_ends()->IsFixedArray());
   i::Isolate* isolate = script->GetIsolate();
   i::Handle<i::FixedArray> line_ends =
       i::Handle<i::FixedArray>::cast(i::handle(script->line_ends(), isolate));
   CHECK(line_ends->length());
 
   int start_offset = GetSourceOffset(start);
   int end_offset = end.IsEmpty()
                        ? GetSmiValue(line_ends, line_ends->length() - 1) + 1
                        : GetSourceOffset(end);
   if (start_offset >= end_offset) return true;
 
-  std::set<int> offsets;
+  std::vector<i::BreakLocation> v8_locations;
   if (!isolate->debug()->GetPossibleBreakpoints(
-          script, start_offset, end_offset, restrict_to_function, &offsets)) {
+          script, start_offset, end_offset, restrict_to_function,
+          &v8_locations)) {
     return false;
   }
 
+  std::map<int, i::BreakLocation> unique_locations;
+  for (const auto& location : v8_locations) {

[Yang, 2017/03/02 20:06:57]

Some comments would be great. What issue is this supposed to work around? From
what I can tell, we are deduplicating breaks with the same source location, but
prefer call/return/debugger locations over normal statement positions? I don't
think we need to do this for debugger or return statements. Because these are
already statements too.

Also, I'd prefer if this kind of logic is either put in the debugger or in the
inspector, not in api.cc where we should mostly only have glue code.

[kozy, 2017/03/02 22:38:09]

Added a comment and DCHECK.
From my perspective of view it's a glue code:
- on one side we have v8::Debug with all possible locations and with a way to
set breakpoint by each of them, there are no reason to merge locations there,
- on another side we have inspector and currently we can put breakpoint only by
first break location at current source location - it means there are no reason
to report all break locations to inspector.

+    auto it = unique_locations.find(location.position());
+    if (it != unique_locations.end() &&
+        (it->second.IsCall() || it->second.IsDebuggerStatement() ||
+         it->second.IsReturn())) {
+      continue;
+    }
+    if (it == unique_locations.end()) {
+      unique_locations.insert(std::make_pair(location.position(), location));
+    } else {

[Yang, 2017/03/02 20:06:57]

Can we DCHECK that this has to be a call location?

[kozy, 2017/03/02 22:38:09]

Done.

+      it->second = location;
+    }
+  }
   int current_line_end_index = 0;
-  for (const auto& it : offsets) {
-    int offset = it;
+  for (const auto& it : unique_locations) {
+    int offset = it.first;
     while (offset > GetSmiValue(line_ends, current_line_end_index)) {
       ++current_line_end_index;
       CHECK(current_line_end_index < line_ends->length());
     }
     int line_offset = 0;
 
     if (current_line_end_index > 0) {
       line_offset = GetSmiValue(line_ends, current_line_end_index - 1) + 1;
     }
-    locations->push_back(debug::Location(
+    locations->emplace_back(
         current_line_end_index + script->line_offset(),
         offset - line_offset +
-            (current_line_end_index == 0 ? script->column_offset() : 0)));
+            (current_line_end_index == 0 ? script->column_offset() : 0),
+        it.second.IsCall(), it.second.IsReturn(),
+        it.second.IsDebuggerStatement());
   }
   return true;
 }
 
 int debug::Script::GetSourceOffset(const debug::Location& location) const {
   i::Handle<i::Script> script = Utils::OpenHandle(this);
   if (script->type() == i::Script::TYPE_WASM) {
     // TODO(clemensh): Return the proper thing for wasm.
     return 0;
   }
@@ skipping 982 matching lines @@
   Address callback_address =
       reinterpret_cast<Address>(reinterpret_cast<intptr_t>(callback));
   VMState<EXTERNAL> state(isolate);
   ExternalCallbackScope call_scope(isolate, callback_address);
   callback(info);
 }
 
 
 }  // namespace internal
 }  // namespace v8