deps: chakrashim enable JavaScript debugging #126

agarwal-sandeep · 2016-09-28T21:49:49Z

Checklist

make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
commit message follows commit guidelines

Affected core subsystem(s)

chakrashim, node

Description of change

This change adds JavaScript debugging support in node-chakracore by using
JsRT debugging apis added in chakracore release 1.3. A shim convert v8
debugging protocol messages to chakracore debugging apis and visa-versa.

node-chakracore only expose the debugging protocol messages and doesn't
shim all the apis which v8 provides, so it is only debuggable using tools
which just relies on debugging protocol json and doesn't use internals
of v8 debugging implementations.

PR-URL: nodejs/node-chakracore#[To be filled]
Reviewed-By: [To be filled]

digitalinfinity

Did a cosmetic review since I'm assuming this was reviewed when the changes where made in the debugging branch- overall, LGTM

digitalinfinity · 2016-10-03T04:57:00Z

deps/chakrashim/lib/chakra_debug.js

+      if (!('text' in obj)) {
+        obj['text'] = obj['display'];
+      }
+      if (!('className' in obj)) {


Curious- why is this within the 'display' in obj case? Should this be in 'type' in obj? Also, curious, do you need to use in here or would hasOwnProperty suffice? #Resolved

If you look at JsrtDebugUtils::AddPropertyType, we always add "type" but className in only for few cases. This adding of className as type is to satisfy VSCode.
hasOwnProperty will do but it seems most of the place we use in to check for property.

In reply to: 81491092 [](ancestors = 81491092)

digitalinfinity · 2016-10-03T04:58:02Z

deps/chakrashim/lib/chakra_debug.js

+    }
+
+    var PROPERTY_ATTRIBUTE_HAVE_CHILDRENS = 0x1;
+    //var PROPERTY_ATTRIBUTE_READ_ONLY_VALUE = 0x2;


Nit: remove commented code? #WontFix

Currently we don't use the read only attribute, but I will keep it here for clarity.

In reply to: 81491126 [](ancestors = 81491126)

digitalinfinity · 2016-10-03T05:11:17Z

deps/chakrashim/lib/chakra_debug.js

+
+  ChakraDebugEventProcessor.prototype[ChakraDebugEvent.SourceCompile] =
+    function() {
+    // {"fileName":"d:\\test.js","lineCount":4,"sourceLength":113,"scriptId":2}


remove commented out line? #Resolved

digitalinfinity · 2016-10-03T05:25:05Z

deps/chakrashim/src/v8debug.cc

    }
+
+    Debug::messageQueue.debugEventProcessCount++;


Is this always called on the same thread? If not, should this be interlocked? #ByDesign

JsDiagDebugEventHandler is supposed to be per runtime, so it is only called from one runtime at a time. No need for interlock.

In reply to: 81492243 [](ancestors = 81492243)

digitalinfinity · 2016-10-03T05:28:28Z

deps/chakrashim/src/v8debug.cc

+  }
+
+  void MessageQueue::SaveMessage(const uint16_t* command, int length) {
+    uint16_t* msg = new uint16_t[length + 1];


Where is this msg deleted? Might be good to add a comment here on the lifetime semantics of this allocation. #Resolved

agarwal-sandeep · 2016-10-03T19:54:51Z

@digitalinfinity This changes were not reviewed previously

obastemur · 2016-10-05T10:35:20Z

deps/chakrashim/lib/chakra_debug.js

+    function Response(request) {
+      this.seq = _nextResponseSeq++;
+      this.type = 'response';
+      this.request_seq = request.seq;


this.request_seq unused property ? #ByDesign

This is part of debugging protocol https://github.com/v8/v8/wiki/Debugging-Protocol

In reply to: 81944574 [](ancestors = 81944574)

obastemur · 2016-10-05T10:41:30Z

deps/chakrashim/lib/chakra_debug.js

+          if (filter == element.GetFileName()) {
+            found = true;
+          }
+        }


from 652 to 655 can be simplified into one line

found = filter == element.GetFileName(); #Resolved

obastemur · 2016-10-05T10:52:26Z

deps/chakrashim/src/jsrtcontextshim.cc

+  buffer[_countof(chakra_debug_native)] = L'\0';
+
+  JsValueRef getInitFunction;
+  if (JsParseScriptWithAttributes(buffer,


I'm not sure if this works on xplat. Reminder: [unix wchar_t 4 bytes] #Resolved

Debugging support is not xplat ready yet, once we have xplat string APIs we need to use that and make some changes. Take a look at https://github.com/agarwal-sandeep/node-chakracore/blob/debugging-vscode-xplat-jc/deps/chakrashim/src/v8debug.cc which uses the prototype xplat string APIs. #Resolved

obastemur · 2016-10-05T10:57:16Z

deps/chakrashim/src/v8debug.cc

+    this->msgQueue = nullptr;
+  }
+
+  void MessageQueue::SaveMessage(const uint16_t* command, int length) {


uint16_t and wchar_t ? (2 bytes / 4 bytes) #Resolved

On windows uint16_t and wchar_t are both 2 bytes and debugging support is currently only for windows. I have a static_assert to make sure we fix this for xplat #Resolved

Is there reason to use 2 different types? (Why not avoid wchar_t and only use uint16_t and use v8::String where string is needed.)

In reply to: 82074191 [](ancestors = 82074191)

Thanks! removed wchar_t usage and instead used v8::String::NewFromTwoByte

In reply to: 82251507 [](ancestors = 82251507,82074191)

obastemur · 2016-10-05T10:58:56Z

deps/chakrashim/src/v8debug.cc

+    JsValueRef args[] = { jsrt::GetUndefined(), debugEventRef };
+    JsValueRef result;
+    errorCode = JsCallFunction(
+      this->processShouldContinueFn, args, _countof(args), &result);


Do we have a definition for _countof non-pal ? #Resolved

We probably have one defined in xplat branch.

In reply to: 81947919 [](ancestors = 81947919)

kunalspathak · 2016-10-05T20:31:13Z

src/node.cc

@@ -4250,6 +4245,13 @@ void Init(int* argc,
    exit(9);
  }

+  // CHAKRA-TODO : fix this to not do it here


HAKRA-TODO : fix this to not do it here [](start = 6, length = 39)

What is the problem here? #ByDesign

No problem, but I would like to find a way to not make any NODE_ENGINE_CHAKRACORE specific change to node.cc and somehow determine this in shim itself #ByDesign

kunalspathak · 2016-10-05T20:39:18Z

deps/chakrashim/src/jsrtisolateshim.cc

@@ -59,12 +63,6 @@ IsolateShim::~IsolateShim() {
 }

 /* static */ v8::Isolate * IsolateShim::New() {
-  // CHAKRA-TODO: Disable multiple isolate for now until it is fully implemented


CHAKRA-TODO: Disable multiple isolate for now until it is fully implemented [](start = 5, length = 75)

Has this changed now? Why is this removed? #ByDesign

Because now we need atleast 2 isolate (one main isolate and another for debugger agent). #Resolved

kunalspathak · 2016-10-05T20:42:47Z

deps/chakrashim/src/jsrtcontextshim.cc

+
+  CHAKRA_VERIFY_NOERROR(errorCode);
+
+  return errorCode == JsNoError;


rrorCode == JsNoError; [](start = 10, length = 22)

nit: won't this always be true given that you have CHAKRA_VERIFY_NOERROR(errorCode); above? #Resolved

kunalspathak · 2016-10-05T20:43:31Z

deps/chakrashim/src/jsrtcontextshim.cc

@@ -447,6 +451,46 @@ bool ContextShim::ExecuteChakraShimJS() {
                        &result) == JsNoError;
 }

+bool ContextShim::ExecuteChakraDebugShimJS(JsValueRef * chakraDebugObject) {


ExecuteChakraDebugShimJS [](start = 18, length = 24)

suggestion : can we extract out common code of ExecuteChakraDebugShimJS and ExecuteChakraShimJS because in future we might have additional shim.js files that we will execute? #WontFix

I though of that but that will make things a bit complicated as we need to allocate buffer using new and then in all failure cases make sure it is deleted, Also there not not much command code. We call different APIs in both functions. For now I think we used keep it as it is and if we add more shim then we should definitely refactor this.

In reply to: 82066039 [](ancestors = 82066039)

kunalspathak · 2016-10-05T20:46:12Z

deps/chakrashim/src/v8context.cc

+
+  if (v8::Debug::IsDebugEnabled() && isoShim->debugContext == nullptr) {
+    // If JavaScript debugging APIs need to be exposed then make sure
+    // debugContext is avaliable and chakra_debug.js is compiled. Inject


avaliable [](start = 23, length = 9)

nit: spelling #Resolved

kunalspathak · 2016-10-05T21:05:25Z

deps/chakrashim/src/v8debug.cc

-error:
-#else
-  hr = E_FAIL;  // ChakraCore does not support JsStartDebugging
+  static JsValueRef CALLBACK Log(


Log [](start = 29, length = 3)

If i understand correctly is this for debugging the debugger code and will never called from user scenario? #ByDesign

Yes #Resolved

kunalspathak · 2016-10-05T21:11:15Z

deps/chakrashim/src/v8debug.cc

+    return returnRef;
+  }
+
+  static JsValueRef CALLBACK JsSetBreakpoint(


JsSetBreakpoint [](start = 29, length = 15)

Can you different nomenclature for these methods? Usually Js*() methods are Jsrt APIs. Would be good to reflect that these are shim methods. #WontFix

all jsrt debugging APIs are named JsDiag* so I choose this name, would keep it this way. #Closed

kunalspathak · 2016-10-05T21:20:06Z

deps/chakrashim/lib/chakra_debug.js

+          }
+        };
+        var properties = [];
+        for (var propsLen = 0; propsLen < propertiesArray.length; ++propsLen) {


for (va [](start = 8, length = 7)

suggestion : some of the for-loops and foreach can be converted to .map() #Resolved

kunalspathak · 2016-10-05T21:23:39Z

deps/chakrashim/lib/chakra_debug.js

+  ExecutionState.prototype.GetObjectEvalHandle = function() {
+    if (!this.objectEval) {
+      var objEval = CallHostFunction(chakraDebug.JsDiagEvaluate, 'Object', 0);
+      this.objectEval = objEval[1];


bjEval[1] [](start = 25, length = 9)

If i understand correctly, objEval[0] holds if call succeed or not. You never check that? #Resolved

Should never fail as we are trying to get the global 'Object' #Resolved

Ok. Can we have some an if-check saying if it fails then something bad is going on? #Resolved

kunalspathak · 2016-10-05T21:26:49Z

deps/chakrashim/lib/chakra_debug.js

+        success = true;
+        DebugManager.BreakpointManager.AddBreakpoint(v8Breakpoint);
+      }
+      // ToDo: Add to list


/ ToDo: Add to list [](start = 7, length = 19)

Is this still a TODO? #Resolved

No, seems a left over. Removed

In reply to: 82074541 [](ancestors = 82074541)

kunalspathak · 2016-10-05T21:30:32Z

deps/chakrashim/src/v8v8.cc

    } else if (remove_flags &&
               (startsWith(
                 arg, "--debug")  // Ignore some flags to reduce unit test noise
                || startsWith(arg, "--harmony")
-                || startsWith(arg, "--stack-size="))) {
+                || startsWith(arg, "--stack-size=")
+                || startsWith(arg, "--nolazy"))) {


"--nolazy") [](start = 35, length = 11)

why this change? #ByDesign

Its like disabling deferred parsing in V8, VSCode by defaults add this flag so we need to ignore it. #ByDesign

jianchun · 2016-10-06T17:24:02Z

deps/chakrashim/include/v8-debug.h

 #include <v8.h>
+#include "uv.h"
+#include <queue>


nit: cpplint demands <...> before "..." includes. #Resolved

strange I ran cpplint and didn't see any warning/error. I will run again. #Resolved

That's from my memory...

In reply to: 82239941 [](ancestors = 82239941)

Made the change but cpplint.py is not showing at as a error/warning

In reply to: 82240850 [](ancestors = 82240850,82239941)

jianchun · 2016-10-06T17:24:58Z

deps/chakrashim/include/v8-debug.h


 namespace v8 {
+class V8_EXPORT MessageQueue {
+ public:
+    MessageQueue();


nit: cpplint / consistency prefers these to be 2-space indented. #Pending

Strange, cpplint doesn't show any error/warning with current spacing but if I make it 2 space cpplint is giving error.

In reply to: 82239807 [](ancestors = 82239807)

jianchun · 2016-10-06T17:29:17Z

deps/chakrashim/include/v8-debug.h

  static MaybeLocal<Value> GetMirror(Local<Context> context,
                                     Handle<Value> obj) {
    return MaybeLocal<Value>();
  }
+  static void StartDebugging(JsRuntimeHandle runtime);


I recall v8::Debug is a public class in v8 headers. These look like deviate from v8 signature. Do we need to put them here, or can we move them to private headers?

Same question to other new classes in this file. #Resolved

moved most of new code to jsrtdebug.cc/jsrtdebug.h

In reply to: 82240689 [](ancestors = 82240689)

jianchun · 2016-10-06T17:35:39Z

deps/chakrashim/lib/chakra_debug.js

+    enabledLogType[logTypes.API] = false; // For development purpose
+    enabledLogType[logTypes.Output] = traceDebugJson === true;
+    enabledLogType[logTypes.Input] = traceDebugJson === true;
+    enabledLogType[logTypes.Error] = false; // For development purpose


nit nit pick: overkill, 4 flag variables suffice #Resolved

Maybe you can also support turning these on through environment variables, so we don't need to recompile.

In reply to: 82242030 [](ancestors = 82242030)

Now we only have LogAPI, DebugJson and Error. DebugJson is enabled using --trace_debug_json and Error is enabled by default.

Seems like when we execute chakra_debug process object is not yet set so can't use that to get the environment variable. Will see if I can do something for dynamically enable/disable after the PR as a separate change.

In reply to: 82242030 [](ancestors = 82242030)

jianchun · 2016-10-06T17:45:21Z

deps/chakrashim/lib/chakra_debug.js

+    ScriptId: 0,
+    ScriptName: 1,
+    ScriptRegExp: 2
+  };


Can these be messed up by user script? Should they be hidden/readonly somehow? #ByDesign

Yes if you use vm.runInDebugContext and that's by design and same as v8

In reply to: 82244119 [](ancestors = 82244119)

jianchun · 2016-10-06T17:56:01Z

deps/chakrashim/lib/chakra_debug.js

+    this.objectPrototypeEval = undefined;
+  }
+
+  ExecutionState.prototype.ChangeToBreak = function() {


Why not use cool ES6 class for these #ByDesign

Most of these is trying to mimic how its done in V8 (debug.js) and I would like to keep it as close as possible in terms of structure.

In reply to: 82246479 [](ancestors = 82246479)

jianchun · 2016-10-06T17:57:44Z

deps/chakrashim/lib/chakra_debug.js

+
+        if (found == true) {
+          var scriptObj = element.GetProtocolObject();
+          if (includeSource == true) {


nit: if (found) ... if (includeSource) ... #Resolved

Sorry I didn't understood what you mean here? found is used to track if a particular scriptid is requested and includeSource is tracking if additionally we need to include completed source of the script or not.

In reply to: 82246914 [](ancestors = 82246914)

I think what @jianchun meant is instead of having if(found == true) you could simply do if(found) and vice versa for includeSource. #Resolved

That would make sense, thanks

In reply to: 82484091 [](ancestors = 82484091)

jianchun · 2016-10-06T18:15:00Z

deps/chakrashim/src/v8v8.cc

@@ -140,23 +147,14 @@ bool V8::Initialize() {
  if (g_disposed) {
    return false;  // Can no longer Initialize if Disposed
  }
-#ifndef NODE_ENGINE_CHAKRACORE


What's "node-chakra" / "node-uwp" debugging story after this change? Can this be merged to those branches? (Would this break VS debugging?) #Pending

Disucssed with @digitalinfinity @aruneshchandra and for now we will go ahead with this break and understand why node-chakra/node-uwp needs to take changes from node-chakracore. If we need to support PDM based debugging will bring back in a separate change

In reply to: 82250358 [](ancestors = 82250358)

digitalinfinity · 2016-10-16T19:37:02Z

deps/chakrashim/lib/chakra_debug.js

+    return result;
+  }
+
+  function CallHostFunction(fn) {


Nit: This looks very very similar to the previous function- can you combine? #WontFix

CallHostFunction dumps the API output/result whereas CallHostFunctionNoLog doesn't, will keep it separate

In reply to: 83558470 [](ancestors = 83558470)

nit: you could still extract the common code while keeping the functionality separate.

digitalinfinity · 2016-10-16T19:38:51Z

deps/chakrashim/lib/chakra_debug.js

+
+  if (!globalObject.Debug) {
+    Object.defineProperty(globalObject, 'Debug', {
+      value: {}, enumerable: false, configurable: false, writable: false


The property descriptor keys are set to their default values- why not just do globalObject.Debug = {} #Resolved

digitalinfinity · 2016-10-16T19:48:24Z

deps/chakrashim/lib/chakra_debug.js

+          var bpId = element.breakpointId;
+          activeBreakpointList[bpId] = _breakpoints[bpId];
+        });
+        _breakpoints = activeBreakpointList;


Nit: can be a little more succinct doing _breakpoints = breakpoints.map(function(element, index, array) { return element.breakpointId; }) #WontFix

Changing this is breaking from bp removal logic, will keep it this way to not break anything.

In reply to: 83558695 [](ancestors = 83558695)

digitalinfinity · 2016-10-16T19:54:02Z

deps/chakrashim/src/jsrtdebug.h

+  static void SetChakraDebugObject(JsValueRef chakraDebugObject);
+ private:
+  static void InstallHostCallback(JsValueRef chakraDebugObject,
+  const wchar_t *name,


Nit: indent function parameters #Resolved

good catch, cpplint didn't catch this.

In reply to: 83558816 [](ancestors = 83558816)

digitalinfinity · 2016-10-16T19:58:03Z

Left a bunch of nitpicky comments but you can fix after merging

This change adds JavaScript debugging support in node-chakracore by using JsRT debugging apis added in chakracore release 1.3. A shim convert v8 debugging protocol messages to chakracore debugging apis and visa-versa. node-chakracore only expose the debugging protocol messages and doesn't shim all the apis which v8 provides, so it is only debuggable using tools which just relies on debugging protocol json and doesn't use internals of v8 debugging implementations. PR-URL: nodejs#126 Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com> Reviewed-By: Jianchun Xu <Jianchun.Xu@microsoft.com> Reviewed-By: Kunal Pathak <Kunal.Pathak@microsoft.com> Reviewed-By: Oguz Bastemur <ogbastem@microsoft.com>

digitalinfinity · 2016-10-18T21:58:51Z

Yay! 🎉 🍾 🍰

digitalinfinity approved these changes Oct 3, 2016

View reviewed changes

obastemur reviewed Oct 5, 2016

View reviewed changes

kunalspathak reviewed Oct 5, 2016

View reviewed changes

jianchun reviewed Oct 6, 2016

View reviewed changes

digitalinfinity reviewed Oct 16, 2016

View reviewed changes

agarwal-sandeep force-pushed the debugging-vscode-pr branch from e3d2a58 to 99d6392 Compare October 18, 2016 18:59

agarwal-sandeep merged commit 99d6392 into nodejs:chakracore-master Oct 18, 2016


		CHAKRA_VERIFY_NOERROR(errorCode);

		return errorCode == JsNoError;

deps: chakrashim enable JavaScript debugging #126

deps: chakrashim enable JavaScript debugging #126

Conversation

agarwal-sandeep commented Sep 28, 2016

Checklist

Affected core subsystem(s)

Description of change

digitalinfinity left a comment

Choose a reason for hiding this comment

digitalinfinity Oct 3, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

digitalinfinity Oct 3, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

digitalinfinity Oct 3, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

digitalinfinity Oct 3, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

digitalinfinity Oct 3, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

agarwal-sandeep commented Oct 3, 2016

obastemur Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

obastemur Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

obastemur Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

agarwal-sandeep Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

obastemur Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

agarwal-sandeep Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

obastemur Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kunalspathak Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

agarwal-sandeep Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

kunalspathak Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

agarwal-sandeep Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

kunalspathak Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

kunalspathak Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kunalspathak Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

kunalspathak Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

agarwal-sandeep Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

kunalspathak Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

agarwal-sandeep Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

kunalspathak Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

kunalspathak Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

agarwal-sandeep Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

kunalspathak Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

kunalspathak Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kunalspathak Oct 5, 2016 • edited by agarwal-sandeep Loading

Choose a reason for hiding this comment

agarwal-sandeep Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

jianchun Oct 6, 2016 • edited by agarwal-sandeep Loading

digitalinfinity Oct 3, 2016 •

edited by agarwal-sandeep

Loading

digitalinfinity Oct 3, 2016 •

edited by agarwal-sandeep

Loading

digitalinfinity Oct 3, 2016 •

edited by agarwal-sandeep

Loading

digitalinfinity Oct 3, 2016 •

edited by agarwal-sandeep

Loading

digitalinfinity Oct 3, 2016 •

edited by agarwal-sandeep

Loading

obastemur Oct 5, 2016 •

edited by agarwal-sandeep

Loading

obastemur Oct 5, 2016 •

edited by agarwal-sandeep

Loading

obastemur Oct 5, 2016 •

edited by agarwal-sandeep

Loading

agarwal-sandeep Oct 5, 2016 •

edited

Loading

obastemur Oct 5, 2016 •

edited by agarwal-sandeep

Loading

agarwal-sandeep Oct 5, 2016 •

edited

Loading

obastemur Oct 5, 2016 •

edited by agarwal-sandeep

Loading

kunalspathak Oct 5, 2016 •

edited by agarwal-sandeep

Loading

agarwal-sandeep Oct 5, 2016 •

edited

Loading

kunalspathak Oct 5, 2016 •

edited by agarwal-sandeep

Loading

agarwal-sandeep Oct 5, 2016 •

edited

Loading

kunalspathak Oct 5, 2016 •

edited by agarwal-sandeep

Loading

kunalspathak Oct 5, 2016 •

edited by agarwal-sandeep

Loading

kunalspathak Oct 5, 2016 •

edited by agarwal-sandeep

Loading

kunalspathak Oct 5, 2016 •

edited by agarwal-sandeep

Loading

agarwal-sandeep Oct 5, 2016 •

edited

Loading

kunalspathak Oct 5, 2016 •

edited by agarwal-sandeep

Loading

agarwal-sandeep Oct 5, 2016 •

edited

Loading

kunalspathak Oct 5, 2016 •

edited by agarwal-sandeep

Loading

kunalspathak Oct 5, 2016 •

edited by agarwal-sandeep

Loading

agarwal-sandeep Oct 5, 2016 •

edited

Loading

kunalspathak Oct 5, 2016 •

edited by agarwal-sandeep

Loading

kunalspathak Oct 5, 2016 •

edited by agarwal-sandeep

Loading

kunalspathak Oct 5, 2016 •

edited by agarwal-sandeep

Loading

agarwal-sandeep Oct 5, 2016 •

edited

Loading

jianchun Oct 6, 2016 •

edited by agarwal-sandeep

Loading

agarwal-sandeep Oct 6, 2016 •

edited

Loading

jianchun Oct 6, 2016 •

edited by agarwal-sandeep

Loading

jianchun Oct 6, 2016 •

edited by agarwal-sandeep

Loading

jianchun Oct 6, 2016 •

edited by agarwal-sandeep

Loading

jianchun Oct 6, 2016 •

edited by agarwal-sandeep

Loading

jianchun Oct 6, 2016 •

edited by agarwal-sandeep

Loading

jianchun Oct 6, 2016 •

edited by agarwal-sandeep

Loading

kunalspathak Oct 7, 2016 •

edited by agarwal-sandeep

Loading

jianchun Oct 6, 2016 •

edited by agarwal-sandeep

Loading

digitalinfinity Oct 16, 2016 •

edited by agarwal-sandeep

Loading

digitalinfinity Oct 16, 2016 •

edited by agarwal-sandeep

Loading

digitalinfinity Oct 16, 2016 •

edited by agarwal-sandeep

Loading

digitalinfinity Oct 16, 2016 •

edited by agarwal-sandeep

Loading