add ES6 feature: ArrayBuffer #1467

jiangzidong · 2016-12-01T15:34:51Z

I am trying to add some useful ES6 builtin object into JerryScript. What are your opinion/plan about ES6 enhancement?

I plan to support ArrayBuffer, TypedArray and DataView first, because they both provide the raw data buffer, and should be useful in IoT/Embedded/Wearable situations.In this PR, I added ArrayBuffer, which is the foundation of TypedArray and DataView.

ArrayBuffer object's ext_object_p->u.class_prop.value is a pointer to arraybuffer struct, which contains the buffer length and buffer pointer. Both arraybuffer struct and buffer itself are stored in jerry heap.
add macro ACCESSOR_VALUE to declare the named accessor properties of builtin object.
The feature is always on , because no one defines CONFIG_DISABLE_ARRAYBUFFER_BUILTIN for now. What about extend the profile from [minimal, full] to [minimal, full, es6]? Then we can manage the es6 feature macros in the es6 profile

Any comments/feedback are welcome :)

jiangzidong · 2016-12-01T15:42:15Z

one simple case

var a = new ArrayBuffer(123);
print(a.byteLength);//123
a.byteLength = 456;
print(a.byteLength);//123
var b = a.slice(5,10)
print(b.byteLength)//5

it will show

123
123
5

LaszloLango · 2016-12-02T07:42:06Z

@jiangzidong, IMHO non of the es6 features should be enabled by default. Full profile means full es5.1 compatibility. Adding es6 as a new profile is a good idea. I'd prefer [minimal, full-es5.1, (es6 or part-es6)]. At some point the es6 profile will not be an extension of es5, but it will has a different core behavior.

jiangzidong · 2016-12-02T08:07:55Z

@LaszloLango, I prefer [minimal, full-es5.1, part-es6]
Because enabling all es6 feature will be a long-term task, and before it is totally done, it shoule be 'part-es6' to avoid misunderstanding.

And what about [minimal, full, part-es6], the full is exact full-es5.1, so that old users will not noticed any changes. part-es6 is more like a WIP profile. When it is finished, then we can rename it [minimal, es5.1, es6] or others.

@LaszloLango, could you share us what is upsteam's plan on es6-feature enhancement? Thanks

jiangzidong · 2016-12-02T08:11:09Z

BTW, there may be some problems in travis-ci... It run for a long time, and seems hung.
the PR passed the tests in my local machine.

akosthekiss · 2016-12-02T08:18:31Z

@jiangzidong Just checked, travis-ci hasn't started one of the OSX jobs yet. Their status page is reporting build delays on OSX; according to updates, their infra provider is doing some cleanup/maintenance/restart. I guess that we will have to wait a bit until everything gets resolved. (Travis-CI service status can be monitored here: https://www.traviscistatus.com )

LaszloLango · 2016-12-02T08:57:35Z

@jiangzidong, related issue: #205

LaszloLango

I just ran through the patch, but looks good in general.

LaszloLango · 2016-12-02T09:36:50Z

jerry-core/ecma/base/ecma-helpers-value.c

@@ -73,12 +73,14 @@ ecma_get_value_type_field (ecma_value_t value) /**< ecma value */
  return value & ECMA_VALUE_TYPE_MASK;
 } /* ecma_get_value_type_field */

+
+


remove these empty lines

LaszloLango · 2016-12-02T09:44:49Z

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer.c

+  JERRY_UNUSED (this_arg);
+  if (ecma_is_value_object (arg))
+  {
+    //TODO: if arg has [[ViewArrayBuffer]], return true


/* TODO: if arg has [[ViewArrayBuffer]], return true */

LaszloLango · 2016-12-02T09:46:23Z

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer.c

+{
+  JERRY_ASSERT (arguments_list_len == 0 || arguments_list_p != NULL);
+  return ecma_raise_type_error (ECMA_ERR_MSG ("Constructor ArrayBuffer requires 'new'"));
+


Move the empty line upper. :)

LaszloLango · 2016-12-02T09:47:59Z

jerry-core/ecma/builtin-objects/ecma-builtin-internal-routines-template.inc.h

@@ -55,6 +62,13 @@ enum
  PASTE (ECMA_ROUTINE_START_, BUILTIN_UNDERSCORED_ID) = ECMA_BUILTIN_ID__COUNT - 1,
 #define ROUTINE(name, c_function_name, args_number, length_prop_value) \
  ECMA_ROUTINE_ ## name ## c_function_name,
+#define ACCESSOR_VALUE(name, c_getter_func_name, c_setter_func_name, prop_attributes) \


I think these should be undefined somewhere later.

like ROUTINE, they all be undefined in specific xxx.inc.h (BUILTIN_INC_HEADER_NAME)

LaszloLango · 2016-12-02T09:49:13Z

jerry-core/ecma/builtin-objects/ecma-builtins.c

+  if (is_accessor)
+  {
+    ecma_create_named_accessor_property (object_p,
+                                        string_p,


wrong indentation (one missing space)

kenchris · 2016-12-02T10:02:41Z

You should probably call it es2015 instead of es6 as that is the official name, and because es2016 is already out

zherczeg · 2016-12-02T11:10:15Z

Thank you very much for working on this useful feature. This is a great contribution.

I have been thinking about these typed array before. I agree that these buffers should be part ECMA_OBJECT_TYPE_CLASS, and store the length in the custom value part. Since the buffer cannot be changed, we can allocate everything in one allocaton. Just change the second argument of ecma_create_object to sizeof (ecma_extended_object_t) + byte_size. Thus ecma_arraybuffer_t could go away.

I would not add 3 new macros for accessor types. I would add a special constant when read or write is not allowed.

About the naming, I would prefer [minimal, es5.1-full, es2015-part].

zherczeg · 2016-12-02T11:15:54Z

I also saw several coding style errors. Please check our guidelines #1459

jiangzidong · 2016-12-05T02:10:21Z

@kenchris , you are right, and I will change all 'es6' in comments to 'es2015', thanks

jiangzidong · 2016-12-05T02:32:34Z

@zherczeg Thanks very much for your comments.

about directly alloc buffer in the tail of ecma_extended_object_t
I can change to your style, and modify the related gc-sweep code

about macros for accessor type
I don't quite understand what you mean. You won't add any new macros for accessors or you just add one common macro for accessors?

about profile name
I will take that.

jiangzidong · 2016-12-05T04:57:25Z

@zherczeg one quiz about code style:
what if A a = func (x , y, z) exceeds 120, and has to be written in multiple line, e.g

  ecma_arraybuffer_t *arraybuffer_p =
  (ecma_arraybuffer_t *) ecma_get_pointer_from_ecma_value (ext_object_p->u.class_prop.value);

is the style above ok?

jiangzidong · 2016-12-05T06:17:43Z

I updated the PR
1, modify some coding style error
2, es6 -> ECMA2015
3, abandon ecma arraybuffer_t, directly alloc buffer in the tail of object

Now I will keep all the intermediate commits, and merge into one after passing the reviews

LaszloLango · 2016-12-05T07:36:32Z

@jiangzidong,

ecma_arraybuffer_t *arraybuffer_p;
arraybuffer_p = (ecma_arraybuffer_t *) ecma_get_pointer_from_ecma_value (ext_object_p->u.class_prop.value);

jiangzidong · 2016-12-05T07:41:14Z

@LaszloLango but if the

var_name = (typename) function_name (arg_name);

exceeds 120, how to split it?

zherczeg · 2016-12-05T07:50:33Z

Just be creative. You can introduce macros if the expression is frequent, or introduce another local variable. There is always a solution.

zherczeg · 2016-12-05T07:54:03Z

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer-prototype.c

+
+  ECMA_OP_TO_NUMBER_FINALIZE (start_num);
+
+  JERRY_ASSERT (start <= len && end <= len);


Complie time assert? Shouldn't an error (perhaps rangeerror) be thrown?

"start <= len && end <= len" should be ensured by ecma_builtin_helper_array_index_normalize

zherczeg · 2016-12-05T07:54:41Z

jerry-core/ecma/base/ecma-gc.c

+      else
+      {
+        size = (ecma_get_object_is_builtin (object_p) ? sizeof (ecma_extended_built_in_object_t)
+                                                             : sizeof (ecma_extended_object_t));


Misaligned colon.

zherczeg · 2016-12-05T07:56:55Z

jerry-core/ecma/base/ecma-gc.c

+          arraybuffer_length = ecma_get_uint32_from_value (ext_object_p->u.class_prop.value);
+          is_contain_arraybuffer = true;
+          ecma_free_value (ext_object_p->u.class_prop.value);
+          break;


Is there a built-in array buffer? If not, we can just free the object here. Furthermore we need guards, since array-buffer might be disabled at compile time.

zherczeg · 2016-12-05T07:57:35Z

jerry-core/ecma/base/ecma-helpers.h

@@ -281,7 +281,8 @@ extern ecma_value_t *ecma_get_internal_property (ecma_object_t *, ecma_internal_
 extern ecma_property_value_t *
 ecma_create_named_data_property (ecma_object_t *, ecma_string_t *, uint8_t, ecma_property_t **);
 extern ecma_property_value_t *
-ecma_create_named_accessor_property (ecma_object_t *, ecma_string_t *, ecma_object_t *, ecma_object_t *, uint8_t);
+ecma_create_named_accessor_property (ecma_object_t *, ecma_string_t *, ecma_object_t *,
+                                    ecma_object_t *, uint8_t, ecma_property_t **);


another space is required.

zherczeg · 2016-12-05T07:58:16Z

jerry-core/ecma/builtin-objects/ecma-builtin-array.inc.h

-
+#undef ACCESSOR_VALUE
+#undef ACCESSOR_RO_VALUE
+#undef ACCESSOR_WO_VALUE


Please remove ACCESSOR_RO_VALUE and ACCESSOR_WO_VALUE. Only keep ACCESSOR_VALUE.

zherczeg · 2016-12-05T07:59:16Z

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer-prototype.c

+#include "jrt-libc-includes.h"
+
+
+


Remove two newlines.

zherczeg · 2016-12-05T08:01:23Z

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer-prototype.inc.h

+/* readonly accessor properties */
+ACCESSOR_RO_VALUE (LIT_MAGIC_STRING_BYTE_LENGTH_UL,
+                  ecma_builtin_arraybuffer_prototype_bytelength_getter,
+                  ECMA_PROPERTY_FIXED)


Please also fix the indentation here.

zherczeg · 2016-12-05T08:02:03Z

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer.c

+                                  ecma_value_t arg) /**< argument 1 */
+{
+  JERRY_UNUSED (this_arg);
+  if (ecma_is_value_object (arg))


Newline after JERRY_UNUSED

zherczeg · 2016-12-05T08:03:22Z

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer.c

+ecma_builtin_arraybuffer_dispatch_call (const ecma_value_t *arguments_list_p, /**< arguments list */
+                                   ecma_length_t arguments_list_len) /**< number of arguments */
+{
+  JERRY_ASSERT (arguments_list_len == 0 || arguments_list_p != NULL);


Why do you add this assert here? Nothing prevents a JS coder to pass arguments here. We should just throw an error regardless of the passed arguments.

Oh no, this check the opposite. Disregad this comment.

I refered to other builtin_xxx_dispatch_call. xxx_dispatch_call is called by ecma_builtin_dispatch_call, and should be satisfy (arguments_list_len == 0 || arguments_list_p != NULL)
if not, it is engine's faults, not JS coders'

I got it. That is why I said that disregard this comment :)

sorry I missed your update :)

zherczeg · 2016-12-05T08:06:22Z

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer.c

+ */
+
+#endif /* !CONFIG_DISABLE_ARRAYBUFFER_BUILTIN
+ */


Do not split into two lines.

zherczeg · 2016-12-06T08:14:12Z

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer-prototype.c

+  if (ecma_is_value_object (this_arg))
+  {
+    ecma_object_t *object_p = ecma_get_object_from_value (this_arg);
+    if (ecma_object_class_is (object_p, LIT_MAGIC_STRING_ARRAY_BUFFER_UL))


Perhaps you could add a newline before this if. Not mandatory id you don't want.

zherczeg · 2016-12-06T08:20:50Z

jerry-core/ecma/builtin-objects/ecma-builtins.c

+    if (getter_p)
+    {
+      ecma_deref_object (getter_p);
+    }


The newline after this line is not needed.

zherczeg · 2016-12-06T08:22:31Z

jerry-core/ecma/operations/ecma-arraybuffer-object.c

+ */
+ecma_value_t
+ecma_op_create_arraybuffer_object (const ecma_value_t *arguments_list_p, /**< list of arguments that
+                                                                         are passed to String constructor */


Asterisk * is missing and alignment is wrong.

zherczeg · 2016-12-06T08:23:35Z

jerry-core/ecma/operations/ecma-arraybuffer-object.c

+ecma_arraybuffer_get_length (ecma_object_t *object_p) /**< pointer to the ArrayBuffer object */
+{
+  JERRY_ASSERT (ecma_object_class_is (object_p, LIT_MAGIC_STRING_ARRAY_BUFFER_UL));
+  ecma_extended_object_t *ext_object_p = (ecma_extended_object_t *) object_p;


I would add a newline after the ASSERT here and below.

zherczeg · 2016-12-06T08:24:26Z

Apart from a few changes this patch looks very good for me. Thank you for the hard work.

jiangzidong · 2016-12-06T08:52:38Z

@zherczeg Thanks for your review and guidance

zherczeg · 2016-12-06T09:06:05Z

LGTM from my side

LaszloLango · 2016-12-06T14:54:40Z

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer-prototype.c

+#include "ecma-globals.h"
+#include "ecma-helpers.h"
+#include "ecma-objects.h"
+#include "ecma-arraybuffer-object.h"


Move this include to the top

LaszloLango · 2016-12-06T14:56:48Z

LGTM after the style fix.

tilmannOSG · 2016-12-06T15:38:21Z

jerry-core/CMakeLists.txt

@@ -18,7 +18,7 @@ set(JERRY_CORE_NAME jerry-core)
 project (${JERRY_CORE_NAME} C)

 # Optional features
-set(FEATURE_PROFILE         "full" CACHE STRING "Profile types: full, minimal")
+set(FEATURE_PROFILE         "es5.1-full" CACHE STRING "Profile types: es5.1-full, minimal, es2015-part")


Sorry, I'm a bit late to the party but why not drop the "-full" from es5.1? I think it is implied that it covers all of ES5.1.
The "-part" seems a bit confusing as well, can't we just call "es2015-wip" or "es2015-experimental" to make clear that the work on the profile is ongoing?

ok, I will use [minimal es5.1 es2015-wip]

IMHO, 'wip' and 'experimental' have a different meaning. They imply the profile is unstable and means "use it if you are know what you are doing". 'Part' or 'partial' means some part of the standard is implemented and can be used without any risk.

what about [minimal es5.1 es2015-part] ? @LaszloLango @tilmannOSG

@jiangzidong I discussed this further with @LaszloLango and we came to the conclusion that using "es2015-subset" would be the best way to indicate that the profile is stable but incomplete. So [minimal, es5.1, es2015-subset] it is :)

tilmannOSG

Thanks for working on this! @zherczeg and @LaszloLango have already covered the technical details very well. Mostly minor review feedback from my side.
Only one major item: Can you please include some regression tests for the new feature?

tilmannOSG · 2016-12-06T15:44:04Z

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer-prototype.c

@@ -0,0 +1,141 @@
+/* Copyright 2016 Intel Corporation


Please use the generic copyright notice for the project instead, we are phasing out individual copyright notices:
"Copyright JS Foundation and other contributors, http://js.foundation" (without the quotes)
There's more information about that in #1473.

Do you mean it should be Copyright JS Foundation and other contributors, http://js.foundation as the only copyright notice for all files in jerryscript?

will any other copyright notices (samsung, intel or szeged univ) be removed?

Yes, that is correct, all code contributed directly to the project should only carry a "Copyright JS Foundation and other contributors, http://js.foundation" (without the quotes) copyright notice. The only exception is third-party code where copyright notices need to be preserved according to the license (e.g. the math library used for jerry-libm).
Yes, all other copyright notices for code contributed directly to the project will be changed as well. You can see the whole list of changes already in #1473.

tilmannOSG · 2016-12-06T15:45:16Z

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer-prototype.inc.h

@@ -0,0 +1,66 @@
+/* Copyright 2016 Intel Corporation


Please use the generic copyright notice for the project instead, we are phasing out individual copyright notices:
"Copyright JS Foundation and other contributors, http://js.foundation" (without the quotes)
There's more information about that in #1473.

tilmannOSG · 2016-12-06T15:45:39Z

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer.c

@@ -0,0 +1,103 @@
+/* Copyright 2016 Intel Corporation


Please use the generic copyright notice for the project instead, we are phasing out individual copyright notices:
"Copyright JS Foundation and other contributors, http://js.foundation" (without the quotes)
There's more information about that in #1473.

tilmannOSG · 2016-12-06T15:46:01Z

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer.inc.h

@@ -0,0 +1,66 @@
+/* Copyright 2016 Intel Corporation


Please use the generic copyright notice for the project instead, we are phasing out individual copyright notices:
"Copyright JS Foundation and other contributors, http://js.foundation" (without the quotes)
There's more information about that in #1473.

tilmannOSG · 2016-12-06T15:47:02Z

jerry-core/ecma/operations/ecma-arraybuffer-object.c

@@ -0,0 +1,123 @@
+/* Copyright 2016 Intel Corporation


Please use the generic copyright notice for the project instead, we are phasing out individual copyright notices:
"Copyright JS Foundation and other contributors, http://js.foundation" (without the quotes)
There's more information about that in #1473.

tilmannOSG · 2016-12-06T15:47:28Z

jerry-core/ecma/operations/ecma-arraybuffer-object.h

@@ -0,0 +1,46 @@
+/* Copyright 2016 Intel Corporation


Please use the generic copyright notice for the project instead, we are phasing out individual copyright notices:
"Copyright JS Foundation and other contributors, http://js.foundation" (without the quotes)
There's more information about that in #1473.

jiangzidong · 2016-12-07T00:53:51Z

@tilmannOSG I don't quite understand what is "regression test" here, could you please explain a little more?

LaszloLango · 2016-12-07T08:05:02Z

@jiangzidong, @tilmannOSG, testing the ES6 features is not trivial. The testing scripts must be updated too. We can do it in a followup patch.

What we need is to add a new list (or two) to tests/jerry-test-suite (like the existing minimal-profile-list) and update the run-tests.py to use them.

tilmannOSG · 2016-12-07T09:08:50Z

@jiangzidong At this point it's sufficient to add a couple of tests for ArrayBuffer to tests/jerry-test-suite to have some basic coverage of the code that you are adding in this patch. Since this is a feature that's only available in the ES2015 profile you need to add a new list of tests specific to that profile (as @LaszloLango mentioned already). Feel free to reach out to @LaszloLango directly for any questions about the current testing infrastructure :)
We can add Travis CI testing of the ES2015 profile and running test262 in ES2015 mode in a separate patch.

jiangzidong · 2016-12-07T11:11:01Z

@tilmannOSG May I add the test-related code in next Patch?

tilmannOSG · 2016-12-07T11:42:42Z

@jiangzidong While we prefer to have a test included with any bigger change, I think in this case given the size of the patch and based on your previous contributions it would be ok to add the test in an additional patch :)
Do you think you could get the tests done in the next couple of days?

jiangzidong · 2016-12-07T11:45:58Z

@tilmannOSG Yes, I will. I just need to figure out how to prevent es5.1 profile from running the es2015 test case

tilmannOSG · 2016-12-07T11:51:31Z

@jiangzidong Great, thank you!

tilmannOSG · 2016-12-07T11:58:34Z

@jiangzidong So looks like the only pending items are:

Changing the copyright notices
Changing the profiles to [minimal, es5.1, es2015-subset]
Squashing the patches into a single commit

After those changes the PR is ready to land :)

JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]

Add a set of tests to ensure basic coverage of the ArrayBuffer built-in. Support for ArrayBuffer was added in #1467. Closes #1475. JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]

jiangzidong force-pushed the arraybuffer branch from beff8d1 to d74c5b4 Compare December 2, 2016 00:52

LaszloLango added development Feature implementation ecma builtins Related to ECMA built-in routines labels Dec 2, 2016

LaszloLango requested changes Dec 2, 2016

View reviewed changes

zherczeg reviewed Dec 5, 2016

View reviewed changes

jerry-core/ecma/builtin-objects/ecma-builtin-arraybuffer-prototype.c

#include "jrt-libc-includes.h"

Copy link

Member

zherczeg Dec 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove two newlines.

zherczeg reviewed Dec 5, 2016

View reviewed changes

jiangzidong force-pushed the arraybuffer branch from 18d3f23 to bdde0e5 Compare December 6, 2016 06:20

zherczeg reviewed Dec 6, 2016

View reviewed changes

jiangzidong force-pushed the arraybuffer branch from bdde0e5 to 2d35a5d Compare December 6, 2016 08:48

LaszloLango requested changes Dec 6, 2016

View reviewed changes

tilmannOSG reviewed Dec 6, 2016

View reviewed changes

tilmannOSG requested changes Dec 6, 2016

View reviewed changes

tilmannOSG approved these changes Dec 7, 2016

View reviewed changes

jiangzidong force-pushed the arraybuffer branch from c9d02e6 to 440ffc1 Compare December 7, 2016 12:09

add ES6 feature: ArrayBuffer

440ffc1

JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]

LaszloLango approved these changes Dec 7, 2016

View reviewed changes

zherczeg merged commit 29d058c into jerryscript-project:master Dec 7, 2016

tilmannOSG mentioned this pull request Dec 7, 2016

ArrayBuffer needs regression tests #1475

Closed

jiangzidong mentioned this pull request Dec 8, 2016

add arraybuffer test file #1477

Merged

zherczeg mentioned this pull request Jan 19, 2017

Nominating Zidong Jiang (@jiangzidong) to become a JerryScript committer #1535

Closed

		@@ -73,12 +73,14 @@ ecma_get_value_type_field (ecma_value_t value) /*< ecma value /
		return value & ECMA_VALUE_TYPE_MASK;
		} /* ecma_get_value_type_field */


		ECMA_OP_TO_NUMBER_FINALIZE (start_num);

		JERRY_ASSERT (start <= len && end <= len);

add ES6 feature: ArrayBuffer #1467

add ES6 feature: ArrayBuffer #1467

Uh oh!

Conversation

jiangzidong commented Dec 1, 2016

Uh oh!

jiangzidong commented Dec 1, 2016

Uh oh!

LaszloLango commented Dec 2, 2016

Uh oh!

jiangzidong commented Dec 2, 2016

Uh oh!

jiangzidong commented Dec 2, 2016

Uh oh!

akosthekiss commented Dec 2, 2016

Uh oh!

LaszloLango commented Dec 2, 2016

Uh oh!

LaszloLango left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kenchris commented Dec 2, 2016

Uh oh!

zherczeg commented Dec 2, 2016

Uh oh!

zherczeg commented Dec 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jiangzidong commented Dec 5, 2016

Uh oh!

jiangzidong commented Dec 5, 2016

Uh oh!

jiangzidong commented Dec 5, 2016

Uh oh!

jiangzidong commented Dec 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LaszloLango commented Dec 5, 2016

Uh oh!

jiangzidong commented Dec 5, 2016

Uh oh!

zherczeg commented Dec 5, 2016

Uh oh!

zherczeg Dec 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zherczeg Dec 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

zherczeg commented Dec 2, 2016 •

edited

Loading

jiangzidong commented Dec 5, 2016 •

edited

Loading

zherczeg Dec 5, 2016 •

edited

Loading

zherczeg Dec 5, 2016 •

edited

Loading

zherczeg Dec 5, 2016 •

edited

Loading

zherczeg Dec 6, 2016 •

edited

Loading