Move setValue operation into a separate JsDeclarativeVariable interface (issue 12300043)

[peter...@gmail.com]

Reviewers: apavlov,

Description:
Move setValue operation into a separate JsDeclarativeVariable interface

– Introduce JsDeclarativeVariable type, move setValue and isMutable methods
there;
– stop pretending SDK can implement setValue for object variables;
– completely drop isReadable.

Please review this at https://codereview.chromium.org/12300043/

SVN Base: https://chromedevtools.googlecode.com/svn/trunk

Affected files:
M
plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/Variable.java
M
plugins/org.chromium.sdk.wipbackend.dev/src/org/chromium/sdk/internal/wip/WipContextBuilder.java
M
plugins/org.chromium.sdk.wipbackend.dev/src/org/chromium/sdk/internal/wip/WipValueBuilder.java
M
plugins/org.chromium.sdk.wipbackend.dev/src/org/chromium/sdk/internal/wip/WipValueLoader.java
A plugins/org.chromium.sdk/src/org/chromium/sdk/JsDeclarativeVariable.java
M plugins/org.chromium.sdk/src/org/chromium/sdk/JsScope.java
M plugins/org.chromium.sdk/src/org/chromium/sdk/JsVariable.java
M
plugins/org.chromium.sdk/src/org/chromium/sdk/internal/v8native/CallFrameImpl.java
M
plugins/org.chromium.sdk/src/org/chromium/sdk/internal/v8native/value/JsObjectBase.java
M
plugins/org.chromium.sdk/src/org/chromium/sdk/internal/v8native/value/JsScopeImpl.java
M
plugins/org.chromium.sdk/src/org/chromium/sdk/internal/v8native/value/JsVariableBase.java

[apa...@chromium.org]

LGTM with comments


https://codereview.chromium.org/12300043/diff/2001/plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/Variable.java
File
plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/Variable.java
(right):

https://codereview.chromium.org/12300043/diff/2001/plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/Variable.java#newcode54
plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/Variable.java:54:
// to parallel if for several properties.
"parallel if" -> "parallelize it"

https://codereview.chromium.org/12300043/diff/2001/plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/Variable.java#newcode300
plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/Variable.java:300:
throw new UnsupportedOperationException();
Can it happen under normal conditions? Should it be handled gracefully
in the Eclipse plugin code?

https://codereview.chromium.org/12300043/diff/2001/plugins/org.chromium.sdk.wipbackend.dev/src/org/chromium/sdk/internal/wip/WipValueBuilder.java
File
plugins/org.chromium.sdk.wipbackend.dev/src/org/chromium/sdk/internal/wip/WipValueBuilder.java
(right):

https://codereview.chromium.org/12300043/diff/2001/plugins/org.chromium.sdk.wipbackend.dev/src/org/chromium/sdk/internal/wip/WipValueBuilder.java#newcode157
plugins/org.chromium.sdk.wipbackend.dev/src/org/chromium/sdk/internal/wip/WipValueBuilder.java:157:
JsValue jsValue = wrap(valueData);
is it necessary?

https://codereview.chromium.org/12300043/diff/2001/plugins/org.chromium.sdk/src/org/chromium/sdk/JsDeclarativeVariable.java
File
plugins/org.chromium.sdk/src/org/chromium/sdk/JsDeclarativeVariable.java
(right):

https://codereview.chromium.org/12300043/diff/2001/plugins/org.chromium.sdk/src/org/chromium/sdk/JsDeclarativeVariable.java#newcode8
plugins/org.chromium.sdk/src/org/chromium/sdk/JsDeclarativeVariable.java:8:
* A variable from JavaScript declarative scope. This variable is backed
a property of any object,
backed BY a property ?

https://codereview.chromium.org/12300043/

[peter...@gmail.com]

https://codereview.chromium.org/12300043/diff/2001/plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/Variable.java
File
plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/Variable.java
(right):

https://codereview.chromium.org/12300043/diff/2001/plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/Variable.java#newcode54
plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/Variable.java:54:
// to parallel if for several properties.

On 2013/02/20 09:02:02, apavlov wrote:
> "parallel if" -> "parallelize it"

Done.

https://codereview.chromium.org/12300043/diff/2001/plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/Variable.java#newcode300
plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/Variable.java:300:
throw new UnsupportedOperationException();

On 2013/02/20 09:02:02, apavlov wrote:
> Can it happen under normal conditions? Should it be handled gracefully
in the
> Eclipse plugin code?

supportsValueModification method should be called before. So this
shouldn't happen with a correct client.

https://codereview.chromium.org/12300043/diff/2001/plugins/org.chromium.sdk.wipbackend.dev/src/org/chromium/sdk/internal/wip/WipValueBuilder.java
File
plugins/org.chromium.sdk.wipbackend.dev/src/org/chromium/sdk/internal/wip/WipValueBuilder.java
(right):

https://codereview.chromium.org/12300043/diff/2001/plugins/org.chromium.sdk.wipbackend.dev/src/org/chromium/sdk/internal/wip/WipValueBuilder.java#newcode157
plugins/org.chromium.sdk.wipbackend.dev/src/org/chromium/sdk/internal/wip/WipValueBuilder.java:157:
JsValue jsValue = wrap(valueData);

On 2013/02/20 09:02:02, apavlov wrote:
> is it necessary?

Done.

https://codereview.chromium.org/12300043/diff/2001/plugins/org.chromium.sdk/src/org/chromium/sdk/JsDeclarativeVariable.java
File
plugins/org.chromium.sdk/src/org/chromium/sdk/JsDeclarativeVariable.java
(right):

https://codereview.chromium.org/12300043/diff/2001/plugins/org.chromium.sdk/src/org/chromium/sdk/JsDeclarativeVariable.java#newcode8
plugins/org.chromium.sdk/src/org/chromium/sdk/JsDeclarativeVariable.java:8:
* A variable from JavaScript declarative scope. This variable is backed
a property of any object,

On 2013/02/20 09:02:02, apavlov wrote:
> backed BY a property ?

Done.

https://codereview.chromium.org/12300043/

[peter...@gmail.com]

Committed manually as r1141 (presubmit successful).

https://codereview.chromium.org/12300043/