From 8ae463d736c1a85d66babf2593fd4397162a80f0 Mon Sep 17 00:00:00 2001
From: yangguo <yangguo@chromium.org>
Date: Thu, 2 Feb 2017 06:22:15 -0800
Subject: [PATCH] [debugger] remove mirror cache and v8::Debug::GetMirror.

R=jgruber@chromium.org
BUG=v8:5530

Review-Url: https://codereview.chromium.org/2670823002
Cr-Commit-Position: refs/heads/master@{#42893}
---
 include/v8-debug.h                       |   7 --
 src/api.cc                               |  20 ----
 src/debug/debug.cc                       |  15 ---
 src/debug/debug.h                        |   3 -
 src/debug/debug.js                       |   7 --
 src/debug/mirrors.js                     | 122 ++---------------------
 src/inspector/debugger-script.js         |   9 +-
 src/inspector/debugger_script_externs.js |   7 +-
 test/cctest/test-debug.cc                |   2 +-
 test/mjsunit/regress/regress-618608.js   |  36 -------
 10 files changed, 12 insertions(+), 216 deletions(-)

diff --git a/include/v8-debug.h b/include/v8-debug.h
index bd8568072e..797d23314e 100644
--- a/include/v8-debug.h
+++ b/include/v8-debug.h
@@ -194,13 +194,6 @@ class V8_EXPORT Debug {
                                 v8::Local<v8::Function> fun,
                                 Local<Value> data = Local<Value>());
 
-  /**
-   * Returns a mirror object for the given object.
-   */
-  V8_DEPRECATED("No longer supported",
-                static MaybeLocal<Value> GetMirror(Local<Context> context,
-                                                   v8::Local<v8::Value> obj));
-
   // This is now a no-op.
   V8_DEPRECATED("No longer supported",
                 static void ProcessDebugMessages(Isolate* isolate));
diff --git a/src/api.cc b/src/api.cc
index 7c81839ae7..8dd84c9055 100644
--- a/src/api.cc
+++ b/src/api.cc
@@ -8925,26 +8925,6 @@ MaybeLocal<Value> Debug::Call(Local<Context> context,
 }
 
 
-MaybeLocal<Value> Debug::GetMirror(Local<Context> context,
-                                   v8::Local<v8::Value> obj) {
-  PREPARE_FOR_EXECUTION(context, Debug, GetMirror, Value);
-  i::Debug* isolate_debug = isolate->debug();
-  has_pending_exception = !isolate_debug->Load();
-  RETURN_ON_FAILED_EXECUTION(Value);
-  i::Handle<i::JSObject> debug(isolate_debug->debug_context()->global_object());
-  auto name = isolate->factory()->NewStringFromStaticChars("MakeMirror");
-  auto fun_obj = i::JSReceiver::GetProperty(debug, name).ToHandleChecked();
-  auto v8_fun = Utils::CallableToLocal(i::Handle<i::JSFunction>::cast(fun_obj));
-  const int kArgc = 1;
-  v8::Local<v8::Value> argv[kArgc] = {obj};
-  Local<Value> result;
-  has_pending_exception =
-      !v8_fun->Call(context, Utils::ToLocal(debug), kArgc, argv)
-           .ToLocal(&result);
-  RETURN_ON_FAILED_EXECUTION(Value);
-  RETURN_ESCAPED(result);
-}
-
 void Debug::ProcessDebugMessages(Isolate* isolate) {}
 
 Local<Context> Debug::GetDebugContext(Isolate* isolate) {
diff --git a/src/debug/debug.cc b/src/debug/debug.cc
index b701cb0c5d..b732303fc8 100644
--- a/src/debug/debug.cc
+++ b/src/debug/debug.cc
@@ -1599,13 +1599,6 @@ bool Debug::IsDebugGlobal(JSGlobalObject* global) {
 }
 
 
-void Debug::ClearMirrorCache() {
-  PostponeInterruptsScope postpone(isolate_);
-  HandleScope scope(isolate_);
-  CallFunction("ClearMirrorCache", 0, NULL);
-}
-
-
 Handle<FixedArray> Debug::GetLoadedScripts() {
   isolate_->heap()->CollectAllGarbage(Heap::kFinalizeIncrementalMarkingMask,
                                       GarbageCollectionReason::kDebugger);
@@ -2314,14 +2307,6 @@ DebugScope::DebugScope(Debug* debug)
 
 
 DebugScope::~DebugScope() {
-  if (!failed_ && prev_ == NULL) {
-    // Clear mirror cache when leaving the debugger. Skip this if there is a
-    // pending exception as clearing the mirror cache calls back into
-    // JavaScript. This can happen if the v8::Debug::Call is used in which
-    // case the exception should end up in the calling code.
-    if (!isolate()->has_pending_exception()) debug_->ClearMirrorCache();
-  }
-
   // Leaving this debugger entry.
   base::NoBarrier_Store(&debug_->thread_local_.current_debug_scope_,
                         reinterpret_cast<base::AtomicWord>(prev_));
diff --git a/src/debug/debug.h b/src/debug/debug.h
index 7af4502f8b..802e630e13 100644
--- a/src/debug/debug.h
+++ b/src/debug/debug.h
@@ -501,9 +501,6 @@ class Debug {
   MUST_USE_RESULT MaybeHandle<Object> MakeAsyncTaskEvent(Handle<Smi> type,
                                                          Handle<Smi> id);
 
-  // Mirror cache handling.
-  void ClearMirrorCache();
-
   void ProcessCompileEvent(v8::DebugEvent event, Handle<Script> script);
   void ProcessDebugEvent(v8::DebugEvent event, Handle<JSObject> event_data);
 
diff --git a/src/debug/debug.js b/src/debug/debug.js
index 88579c2108..6993274f09 100644
--- a/src/debug/debug.js
+++ b/src/debug/debug.js
@@ -12,18 +12,11 @@ var FrameMirror = global.FrameMirror;
 var GlobalArray = global.Array;
 var GlobalRegExp = global.RegExp;
 var IsNaN = global.isNaN;
-var LookupMirror = global.LookupMirror;
 var MakeMirror = global.MakeMirror;
 var MathMin = global.Math.min;
 var Mirror = global.Mirror;
-var MirrorType;
-var ParseInt = global.parseInt;
 var ValueMirror = global.ValueMirror;
 
-utils.Import(function(from) {
-  MirrorType = from.MirrorType;
-});
-
 //----------------------------------------------------------------------------
 
 // Default number of frames to include in the response to backtrace request.
diff --git a/src/debug/mirrors.js b/src/debug/mirrors.js
index 8b050500ff..94e45dff82 100644
--- a/src/debug/mirrors.js
+++ b/src/debug/mirrors.js
@@ -77,59 +77,15 @@ var MirrorType = {
   GENERATOR_TYPE : 'generator',
 }
 
-
-// Handle id counters.
-var next_handle_ = 0;
-var next_transient_handle_ = -1;
-
-// Mirror cache.
-var mirror_cache_ = [];
-var mirror_cache_enabled_ = true;
-
-
-function MirrorCacheIsEmpty() {
-  return next_handle_ == 0 && mirror_cache_.length == 0;
-}
-
-
-function ToggleMirrorCache(value) {
-  mirror_cache_enabled_ = value;
-  ClearMirrorCache();
-}
-
-
-function ClearMirrorCache(value) {
-  next_handle_ = 0;
-  mirror_cache_ = [];
-}
-
-
 /**
  * Returns the mirror for a specified value or object.
  *
  * @param {value or Object} value the value or object to retrieve the mirror for
- * @param {boolean} transient indicate whether this object is transient and
- *    should not be added to the mirror cache. The default is not transient.
  * @returns {Mirror} the mirror reflects the passed value or object
  */
-function MakeMirror(value, opt_transient) {
+function MakeMirror(value) {
   var mirror;
 
-  // Look for non transient mirrors in the mirror cache.
-  if (!opt_transient && mirror_cache_enabled_) {
-    for (var id in mirror_cache_) {
-      mirror = mirror_cache_[id];
-      if (mirror.value() === value) {
-        return mirror;
-      }
-      // Special check for NaN as NaN == NaN is false.
-      if (mirror.isNumber() && IsNaN(mirror.value()) &&
-          typeof value == 'number' && IsNaN(value)) {
-        return mirror;
-      }
-    }
-  }
-
   if (IS_UNDEFINED(value)) {
     mirror = new UndefinedMirror();
   } else if (IS_NULL(value)) {
@@ -165,29 +121,13 @@ function MakeMirror(value, opt_transient) {
   } else if (IS_GENERATOR(value)) {
     mirror = new GeneratorMirror(value);
   } else {
-    mirror = new ObjectMirror(value, MirrorType.OBJECT_TYPE, opt_transient);
+    mirror = new ObjectMirror(value, MirrorType.OBJECT_TYPE);
   }
 
-  if (mirror_cache_enabled_) mirror_cache_[mirror.handle()] = mirror;
   return mirror;
 }
 
 
-/**
- * Returns the mirror for a specified mirror handle.
- *
- * @param {number} handle the handle to find the mirror for
- * @returns {Mirror or undefiend} the mirror with the requested handle or
- *     undefined if no mirror with the requested handle was found
- */
-function LookupMirror(handle) {
-  if (!mirror_cache_enabled_) {
-    throw %make_error(kDebugger, "Mirror cache is disabled");
-  }
-  return mirror_cache_[handle];
-}
-
-
 /**
  * Returns the mirror for the undefined value.
  *
@@ -491,23 +431,6 @@ Mirror.prototype.isIterator = function() {
 };
 
 
-/**
- * Allocate a handle id for this object.
- */
-Mirror.prototype.allocateHandle_ = function() {
-  if (mirror_cache_enabled_) this.handle_ = next_handle_++;
-};
-
-
-/**
- * Allocate a transient handle id for this object. Transient handles are
- * negative.
- */
-Mirror.prototype.allocateTransientHandle_ = function() {
-  this.handle_ = next_transient_handle_--;
-};
-
-
 Mirror.prototype.toText = function() {
   // Simpel to text which is used when on specialization in subclass.
   return "#<" + this.constructor.name + ">";
@@ -518,28 +441,16 @@ Mirror.prototype.toText = function() {
  * Base class for all value mirror objects.
  * @param {string} type The type of the mirror
  * @param {value} value The value reflected by this mirror
- * @param {boolean} transient indicate whether this object is transient with a
- *    transient handle
  * @constructor
  * @extends Mirror
  */
-function ValueMirror(type, value, transient) {
+function ValueMirror(type, value) {
   %_Call(Mirror, this, type);
   this.value_ = value;
-  if (!transient) {
-    this.allocateHandle_();
-  } else {
-    this.allocateTransientHandle_();
-  }
 }
 inherits(ValueMirror, Mirror);
 
 
-Mirror.prototype.handle = function() {
-  return this.handle_;
-};
-
-
 /**
  * Check whether this is a primitive value.
  * @return {boolean} True if the mirror reflects a primitive value
@@ -684,14 +595,12 @@ SymbolMirror.prototype.toText = function() {
 /**
  * Mirror object for objects.
  * @param {object} value The object reflected by this mirror
- * @param {boolean} transient indicate whether this object is transient with a
- *    transient handle
  * @constructor
  * @extends ValueMirror
  */
-function ObjectMirror(value, type, transient) {
+function ObjectMirror(value, type) {
   type = type || MirrorType.OBJECT_TYPE;
-  %_Call(ValueMirror, this, type, value, transient);
+  %_Call(ValueMirror, this, type, value);
 }
 inherits(ObjectMirror, ValueMirror);
 
@@ -2313,13 +2222,10 @@ ScopeMirror.prototype.scopeType = function() {
 
 
 ScopeMirror.prototype.scopeObject = function() {
-  // For local, closure and script scopes create a transient mirror
+  // For local, closure and script scopes create a mirror
   // as these objects are created on the fly materializing the local
   // or closure scopes and therefore will not preserve identity.
-  var transient = this.scopeType() == ScopeType.Local ||
-                  this.scopeType() == ScopeType.Closure ||
-                  this.scopeType() == ScopeType.Script;
-  return MakeMirror(this.details_.object(), transient);
+  return MakeMirror(this.details_.object());
 };
 
 
@@ -2338,7 +2244,6 @@ function ScriptMirror(script) {
   %_Call(Mirror, this, MirrorType.SCRIPT_TYPE);
   this.script_ = script;
   this.context_ = new ContextMirror(script.context_data);
-  this.allocateHandle_();
 }
 inherits(ScriptMirror, Mirror);
 
@@ -2454,7 +2359,6 @@ ScriptMirror.prototype.toText = function() {
 function ContextMirror(data) {
   %_Call(Mirror, this, MirrorType.CONTEXT_TYPE);
   this.data_ = data;
-  this.allocateHandle_();
 }
 inherits(ContextMirror, Mirror);
 
@@ -2468,9 +2372,6 @@ ContextMirror.prototype.data = function() {
 
 utils.InstallFunctions(global, DONT_ENUM, [
   "MakeMirror", MakeMirror,
-  "LookupMirror", LookupMirror,
-  "ToggleMirrorCache", ToggleMirrorCache,
-  "MirrorCacheIsEmpty", MirrorCacheIsEmpty,
 ]);
 
 utils.InstallConstants(global, [
@@ -2505,13 +2406,4 @@ utils.InstallConstants(global, [
   "FrameDetails", FrameDetails,
 ]);
 
-// Functions needed by the debugger runtime.
-utils.InstallFunctions(utils, DONT_ENUM, [
-  "ClearMirrorCache", ClearMirrorCache
-]);
-
-// Export to debug.js
-utils.Export(function(to) {
-  to.MirrorType = MirrorType;
-});
 })
diff --git a/src/inspector/debugger-script.js b/src/inspector/debugger-script.js
index c9ede086ac..34b2effbdb 100644
--- a/src/inspector/debugger-script.js
+++ b/src/inspector/debugger-script.js
@@ -107,7 +107,7 @@ DebuggerScript.getGeneratorScopes = function(gen)
  */
 DebuggerScript.getGeneratorObjectLocation = function(object)
 {
-    var mirror = MakeMirror(object, true /* transient */);
+    var mirror = MakeMirror(object);
     if (!mirror.isGenerator())
         return null;
     var generatorMirror = /** @type {!GeneratorMirror} */(mirror);
@@ -132,7 +132,7 @@ DebuggerScript.getGeneratorObjectLocation = function(object)
  */
 DebuggerScript.getCollectionEntries = function(object)
 {
-    var mirror = MakeMirror(object, true /* transient */);
+    var mirror = MakeMirror(object);
     if (mirror.isMap())
         return /** @type {!MapMirror} */(mirror).entries();
     if (mirror.isSet() || mirror.isIterator()) {
@@ -548,7 +548,7 @@ DebuggerScript._buildScopeObject = function(scopeType, scopeObject)
         // the same properties.
         // Reset scope object prototype to null so that the proto properties
         // don't appear in the local scope section.
-        var properties = /** @type {!ObjectMirror} */(MakeMirror(scopeObject, true /* transient */)).properties();
+        var properties = /** @type {!ObjectMirror} */(MakeMirror(scopeObject)).properties();
         // Almost always Script scope will be empty, so just filter out that noise.
         // Also drop empty Block, Eval and Script scopes, should we get any.
         if (!properties.length && (scopeType === ScopeType.Script ||
@@ -573,8 +573,5 @@ DebuggerScript._buildScopeObject = function(scopeType, scopeObject)
     return result;
 }
 
-// We never resolve Mirror by its handle so to avoid memory leaks caused by Mirrors in the cache we disable it.
-ToggleMirrorCache(false);
-
 return DebuggerScript;
 })();
diff --git a/src/inspector/debugger_script_externs.js b/src/inspector/debugger_script_externs.js
index e2006309c4..478cd11f3b 100644
--- a/src/inspector/debugger_script_externs.js
+++ b/src/inspector/debugger_script_externs.js
@@ -238,16 +238,11 @@ FrameDetails.prototype.returnValue = function() {}
 /** @return {number} */
 FrameDetails.prototype.scopeCount = function() {}
 
-
-/** @param {boolean} value */
-function ToggleMirrorCache(value) {}
-
 /**
  * @param {*} value
- * @param {boolean=} transient
  * @return {!Mirror}
  */
-function MakeMirror(value, transient) {}
+function MakeMirror(value) {}
 
 
 /** @interface */
diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc
index e053a4e604..ce069884cb 100644
--- a/test/cctest/test-debug.cc
+++ b/test/cctest/test-debug.cc
@@ -4422,7 +4422,7 @@ TEST(HiddenPrototypePropertyMirror) {
             .FromJust());
 
   // The prototype (__proto__) for o0 should be o3 as o1 and o2 are hidden.
-  CHECK(CompileRun("o0_mirror.protoObject() == o3_mirror")
+  CHECK(CompileRun("o0_mirror.protoObject().value() == o3_mirror.value()")
             ->BooleanValue(context)
             .FromJust());
 }
diff --git a/test/mjsunit/regress/regress-618608.js b/test/mjsunit/regress/regress-618608.js
index 02e2cdf3c2..742cc6e32e 100644
--- a/test/mjsunit/regress/regress-618608.js
+++ b/test/mjsunit/regress/regress-618608.js
@@ -1238,7 +1238,6 @@ gc();
     "use asm";
     var __v_9 = new stdlib.Float32Array(heap);
     var __v_13 = stdlib.Math.fround;
-    assertEquals("number", debug.LookupMirror(__v_112).type());
     function __f_73() {
       var __v_23 = __v_13(1.0);
       var __v_25 = __v_13(2.0);
@@ -1433,38 +1432,3 @@ gc();
   assertEquals(0x87654321, __v_30.__f_50());
 })();
 } catch(e) { print("Caught: " + e); }
-try {
-var __v_112 = debug.MakeMirror(123).handle();
-assertEquals("number", debug.LookupMirror(__v_112).type());
-debug.ToggleMirrorCache(false);
-var __v_114 = debug.MakeMirror(123).handle();
-gc();
-assertEquals(undefined, __v_114);
-assertThrows(function() { debug.LookupMirror(__v_114) });
-debug.ToggleMirrorCache(true);
-var __v_113 = debug.MakeMirror(123).handle();
-assertEquals("number", debug.LookupMirror(__v_113).type());
-} catch(e) { print("Caught: " + e); }
-try {
-var Debug = debug.Debug;
-var __v_25 = null;
-var __v_113 = true;
-} catch(e) { print("Caught: " + e); }
-function __f_112(event, exec_state, event_data, data) {
-  if (event != Debug.DebugEvent.Break) return;
-  try {
-    assertTrue(exec_state.frame(0).sourceLineText().indexOf("BREAK") > 0);
-  } catch (e) {
-    __v_0 = e;
-  }
-}
-function __f_113() {
-  return 1;
-}
-try {
-Debug.setListener(__f_112);
-nop();
-__f_113();
-Debug.setListener(null);
-assertNull(__v_112);
-} catch(e) { print("Caught: " + e); }