Redesign scope API, separate declarative and object scopes.

plugins/org.chromium.sdk/src/org/chromium/sdk/internal/v8native/value/JsScopeImpl.java

Index: plugins/org.chromium.sdk/src/org/chromium/sdk/internal/v8native/value/JsScopeImpl.java
diff --git a/plugins/org.chromium.sdk/src/org/chromium/sdk/internal/v8native/value/JsScopeImpl.java b/plugins/org.chromium.sdk/src/org/chromium/sdk/internal/v8native/value/JsScopeImpl.java
index 7062dee2e0a4a5614476d50cb3d0ec5bc67af969..7a3a062f6eb09b5722fd60bb237ea93d864ecf2e 100644
--- a/plugins/org.chromium.sdk/src/org/chromium/sdk/internal/v8native/value/JsScopeImpl.java
+++ b/plugins/org.chromium.sdk/src/org/chromium/sdk/internal/v8native/value/JsScopeImpl.java
@@ -28,7 +28,7 @@ import org.chromium.sdk.util.MethodIsBlockingException;
 /**
  * A generic implementation of the JsScope interface.
  */
-public abstract class JsScopeImpl<D> implements JsScope {
+public abstract class JsScopeImpl<D extends JsScopeImpl.DataBase> implements JsScope {
 
   /**
    * An abstraction over object that hosts the scope. It could be either call frame or function.
@@ -69,10 +69,10 @@ public abstract class JsScopeImpl<D> implements JsScope {
 
   public static JsScopeImpl<?> create(Host host, ScopeRef scopeRef) {
     Type type = convertType((int) scopeRef.type());
-    if (type == Type.WITH) {
-      return new With(host, type, (int) scopeRef.index());
+    if (type == Type.WITH || type == Type.GLOBAL) {
+      return new ObjectBasedImpl(host, type, (int) scopeRef.index());
     } else {
-      return new NoWith(host, type, (int) scopeRef.index());
+      return new DeclarativeImpl(host, type, (int) scopeRef.index());
     }
   }
 
@@ -87,11 +87,6 @@ public abstract class JsScopeImpl<D> implements JsScope {
     return type;
   }
 
-  @Override
-  public List<? extends JsVariable> getVariables() throws MethodIsBlockingException {
-    return getVariables(getDeferredData());
-  }
-
   protected D getDeferredData() throws MethodIsBlockingException {
     AsyncFuture<D> future = deferredDataRef.get();
     ValueLoaderImpl valueLoader = host.getInternalContext().getValueLoader();
@@ -102,8 +97,7 @@ public abstract class JsScopeImpl<D> implements JsScope {
       restartOperation = false;
     } else {
       D result = future.getSync();
-      int dataCacheState = getDataCacheState(result);
-      if (dataCacheState == cacheState) {
+      if (!result.isCacheObsolete(cacheState)) {
         return result;
       }
       restartOperation = true;
@@ -119,11 +113,6 @@ public abstract class JsScopeImpl<D> implements JsScope {
   protected abstract SyncOperation<D> createLoadDataOperation(ValueLoaderImpl valueLoader,
       int cacheState);
 
-  protected abstract List<? extends JsVariable> getVariables(D data)
-      throws MethodIsBlockingException;
-
-  protected abstract int getDataCacheState(D data);
-
   protected ObjectValueHandle loadScopeObject(ValueLoaderImpl valueLoader)
       throws MethodIsBlockingException {
     return valueLoader.loadScopeFields(scopeIndex, host.getFactoryParameter());
@@ -137,24 +126,25 @@ public abstract class JsScopeImpl<D> implements JsScope {
     return type;
   }
 
-  private static class NoWith extends JsScopeImpl<NoWith.DeferredData> {
-    NoWith(Host host, Type type, int scopeIndex) {
+  private static class DeclarativeImpl extends JsScopeImpl<DeclarativeImpl.DeferredData>
+      implements JsScope.Declarative {
+    DeclarativeImpl(Host host, Type type, int scopeIndex) {
       super(host, type, scopeIndex);
     }
 
-    @Override
-    public WithScope asWithScope() {
+    @Override public Declarative asDeclarativeScope() {
+      return this;
+    }
+    @Override public ObjectBased asObjectBased() {
       return null;
     }
-
-    @Override
-    protected List<? extends JsVariable> getVariables(DeferredData data) {
-      return data.variables;
+    @Override public <R> R accept(Visitor<R> visitor) {
+      return visitor.visitDeclarative(this);
     }
 
     @Override
-    protected int getDataCacheState(DeferredData data) {
-      return data.cacheState;
+    public List<? extends JsVariable> getVariables() throws MethodIsBlockingException {
+      return getDeferredData().variables;
     }
 
     @Override
@@ -189,30 +179,34 @@ public abstract class JsScopeImpl<D> implements JsScope {
       return properties;
     }
 
-    static class DeferredData {
+    static class DeferredData extends DataBase {
       final List<? extends JsVariable> variables;
-      final int cacheState;
+      private final int cacheState;
 
       DeferredData(List<? extends JsVariable> variables, int cacheState) {
         this.variables = variables;
         this.cacheState = cacheState;
       }
+      @Override boolean isCacheObsolete(int newCacheState) {
+        return cacheState != newCacheState;
+      }
     }
   }
 
-  private static class With extends JsScopeImpl<With.DeferredData> implements JsScope.WithScope {
-    With(Host host, Type type, int scopeIndex) {
+  private static class ObjectBasedImpl extends JsScopeImpl<ObjectBasedImpl.DeferredData>
+      implements JsScope.ObjectBased {
+    ObjectBasedImpl(Host host, Type type, int scopeIndex) {
       super(host, type, scopeIndex);
     }
 
-    @Override
-    public WithScope asWithScope() {
+    @Override public Declarative asDeclarativeScope() {
+      return null;
+    }
+    @Override public ObjectBased asObjectBased() {

[apavlov, 2012/12/27 08:02:16]

ditto

[Peter Rybin, 2013/01/09 15:30:31]

Done.

       return this;
     }
-
-    @Override
-    public JsValue getWithArgument() throws MethodIsBlockingException {
-      return getDeferredData().jsValue;
+    @Override public <R> R accept(Visitor<R> visitor) {

[apavlov, 2012/12/27 08:02:16]

ditto

[Peter Rybin, 2013/01/09 15:30:31]

Done.

+      return visitor.visitObject(this);
     }
 
     @Override
@@ -231,44 +225,33 @@ public abstract class JsScopeImpl<D> implements JsScope {
       ObjectValueHandle scopeObject = loadScopeObject(valueLoader);
       ValueMirror mirror = valueLoader.addDataToMap(scopeObject.getSuper());
       JsValue jsValue = JsVariableBase.createValue(valueLoader, mirror);
-      return new DeferredData(jsValue, cacheState);
+      return new DeferredData(jsValue);
     }
 
     @Override
-    protected List<? extends JsVariable> getVariables(DeferredData data)
-        throws MethodIsBlockingException {
-      if (data.orderedProperties == null) {
-        List<? extends JsVariable> list;
-        JsObject jsObject = data.jsValue.asObject();
-        if (jsObject == null) {
-          list = Collections.emptyList();
-        } else {
-          list = new ArrayList<JsVariable>(jsObject.getProperties());
-        }
-        data.orderedProperties = list;
+    public JsObject getScopeObject() throws MethodIsBlockingException {
+      JsObject asObject = getDeferredData().jsValue.asObject();
+      if (asObject == null) {
+        throw new RuntimeException("Received scope object value is not an object");
       }
-      return data.orderedProperties;
-    }
-
-    @Override
-    protected int getDataCacheState(DeferredData data) {
-      return data.cacheState;
+      return asObject;
     }
 
-    static class DeferredData {
+    static class DeferredData extends DataBase {
       final JsValue jsValue;
-      final int cacheState;
-
-      // Stores properties in the list -- the order is unspecified, but fixed.
-      volatile List<? extends JsVariable> orderedProperties = null;
-
-      DeferredData(JsValue jsValue, int cacheState) {
+      DeferredData(JsValue jsValue) {
         this.jsValue = jsValue;
-        this.cacheState = cacheState;
+      }
+      @Override boolean isCacheObsolete(int newCacheState) {

[apavlov, 2012/12/27 08:02:16]

ditto

[Peter Rybin, 2013/01/09 15:30:31]

Done.

+        return false;
       }
     }
   }
 
+  static abstract class DataBase {
+    abstract boolean isCacheObsolete(int newCacheState);
+  }
+
   private static final Map<Integer, Type> CODE_TO_TYPE;
   static {
     CODE_TO_TYPE = new HashMap<Integer, Type>();