Correctly annotate v8::StackTrace and v8::StackFrame API methods

src/api.cc

Index: src/api.cc
diff --git a/src/api.cc b/src/api.cc
index c7ec5722e4f6e52c89fe8355cd4f9a130df9737e..753be0c99b6ce25482a7eca6aac396876666abbc 100644
--- a/src/api.cc
+++ b/src/api.cc
@@ -2841,7 +2841,7 @@ void Message::PrintCurrentStackTrace(Isolate* isolate, FILE* out) {
 
 Local<StackFrame> StackTrace::GetFrame(uint32_t index) const {
   i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();
-  ENTER_V8(isolate);
+  ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
   EscapableHandleScope scope(reinterpret_cast<Isolate*>(isolate));
   auto self = Utils::OpenHandle(this);
   auto obj = i::JSReceiver::GetElement(isolate, self, index).ToHandleChecked();
@@ -2865,8 +2865,7 @@ Local<StackTrace> StackTrace::CurrentStackTrace(
     int frame_limit,
     StackTraceOptions options) {
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
-  ENTER_V8(i_isolate);
-  // TODO(dcarney): remove when ScriptDebugServer is fixed.

[mmoroz, 2017/03/21 13:12:58]

Looks like dcarney@ is not at Google anymore. Should we go through other his
TODO / FIXME comments? There are ~35 of them.

[jochen (gone - plz use gerrit), 2017/03/22 09:59:06]

yeah, eventually we should do that... however, we'll need to address them /
assign to a new owner. Just deleting is no good :/

[mmoroz, 2017/03/22 11:55:43]

Yeah, I didn't mean to simply delete others. Sorry, I should have been more
clear. I can try to go through TODOs and address those ones that I can resolve +
reassign all the rest.

[jochen (gone - plz use gerrit), 2017/03/22 12:45:33]

great! I'd do that in separate CLs, however.

in this instance, you can probably move kExposeFramesAcrossSecurityOrigins to
https://cs.chromium.org/chromium/src/v8/src/inspector/v8-stack-trace-impl.cc?...

[mmoroz, 2017/03/22 13:34:17]

Done.

+  ENTER_V8_NO_SCRIPT_NO_EXCEPTION(i_isolate);
   options = static_cast<StackTraceOptions>(
       static_cast<int>(options) | kExposeFramesAcrossSecurityOrigins);

[mmoroz, 2017/03/21 13:12:58]

Why do we need to expose frames across security origins? I've tried to remove
this, and lots of tests failed after that. So it feels that this is important
flag, but I don't understand the logic behind it. Why do we want to expose
cross-origin stack frames?

[jochen (gone - plz use gerrit), 2017/03/22 09:59:06]

when we expose a stack to javascript, e.g. via new Error().stack, we can't
expose cross origin frames. The debugger, however, will want to expose such
stacks via devtools.

[mmoroz, 2017/03/22 11:55:43]

Acknowledged.

   i::Handle<i::JSArray> stackTrace =
@@ -2880,7 +2879,7 @@ Local<StackTrace> StackTrace::CurrentStackTrace(
 static int getIntProperty(const StackFrame* f, const char* propertyName,
                           int defaultValue) {
   i::Isolate* isolate = Utils::OpenHandle(f)->GetIsolate();
-  ENTER_V8(isolate);
+  ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
   i::HandleScope scope(isolate);
   i::Handle<i::JSObject> self = Utils::OpenHandle(f);
   i::Handle<i::Object> obj =
@@ -2907,7 +2906,7 @@ int StackFrame::GetScriptId() const {
 static Local<String> getStringProperty(const StackFrame* f,
                                        const char* propertyName) {
   i::Isolate* isolate = Utils::OpenHandle(f)->GetIsolate();
-  ENTER_V8(isolate);
+  ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
   EscapableHandleScope scope(reinterpret_cast<Isolate*>(isolate));
   i::Handle<i::JSObject> self = Utils::OpenHandle(f);
   i::Handle<i::Object> obj =
@@ -2935,7 +2934,7 @@ Local<String> StackFrame::GetFunctionName() const {
 
 static bool getBoolProperty(const StackFrame* f, const char* propertyName) {
   i::Isolate* isolate = Utils::OpenHandle(f)->GetIsolate();
-  ENTER_V8(isolate);
+  ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
   i::HandleScope scope(isolate);
   i::Handle<i::JSObject> self = Utils::OpenHandle(f);
   i::Handle<i::Object> obj =