Add an experimental public API for value serialization.

Add an experimental public API for value serialization.

Suitably scary warnings attached, as this will yet evolve (notably to handle
host objects, which are not currently handled).

Unit tests adjusted to use the public version of ValueSerializer, eliminating
any need they have to access v8::internal.

With this, Blink can begin using this code experimentally behind a flag as it
develops.

BUG=chromium:148757
Committed: https://crrev.com/58cac6501f6b17d9cc8fb84ce65e7c89416dd9af
Cr-Commit-Position: refs/heads/master@{#38915}

[jbroman, 2016-08-23 19:15:05]

The CQ bit was checked by jbroman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-23 19:15:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-23 19:24:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-23 19:24:29]

Dry run: Try jobs failed on following builders:
  v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)

[jbroman, 2016-08-23 19:59:13]

The CQ bit was checked by jbroman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-23 19:59:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-23 20:25:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-23 20:25:52]

Dry run: This issue passed the CQ dry run.

[jbroman, 2016-08-23 20:37:14]

jbroman@chromium.org changed reviewers:
+ jkummerow@chromium.org

[jbroman, 2016-08-23 20:37:15]

PS1 used unique_ptr for the private data (i::ValueSerializer, etc.), but unlike
Chromium, V8 doesn't suppress C4251, and so some bots object to the use of
unique_ptr even when all functions which use it are out-of-line and exported.

void* pointers are used elsewhere in include.h for the same thing; this approach
is mildly safer than that, but still requires a little bit of manual memory
management code, which is unfortunate.

[Jakob Kummerow, 2016-08-24 17:41:04]

Looks good, but I see a chicken-and-egg problem around
private_->supports_legacy_wire_format.

https://codereview.chromium.org/2274693002/diff/20001/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/2274693002/diff/20001/src/api.cc#newcode2851
src/api.cc:2851: CHECK(private_);
What does this guard against? A future constructor that forgets to initialize
|private_|? Maybe make it a DCHECK then.

https://codereview.chromium.org/2274693002/diff/20001/src/api.cc#newcode2878
src/api.cc:2878: bool supports_legacy_wire_format;
How about initializing this?

https://codereview.chromium.org/2274693002/diff/20001/src/api.cc#newcode2887
src/api.cc:2887: }
how about: else { private_ = nullptr; } ?

https://codereview.chromium.org/2274693002/diff/20001/src/api.cc#newcode2907
src/api.cc:2907: CHECK(private_);
Presumably this needs to be called before ReadHeader (which checks for
private_->supports_legacy_wire_format), but it doesn't allow the embedder to
check for successful initialization without crashing.

[jbroman, 2016-08-24 21:26:00]

The CQ bit was checked by jbroman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-24 21:26:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jbroman, 2016-08-24 21:26:36]

PTAL.

https://codereview.chromium.org/2274693002/diff/20001/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/2274693002/diff/20001/src/api.cc#newcode2851
src/api.cc:2851: CHECK(private_);
On 2016/08/24 at 17:41:04, Jakob wrote:
> What does this guard against? A future constructor that forgets to initialize
|private_|? Maybe make it a DCHECK then.

Fair enough, it's probably premature since there's no actual way for this to
fail as yet. Removed.

I considered having failure of WriteHeader/WriteValue delete private_ to make it
failing to respect the error a more aggressive failure, but didn't end up doing
that.

https://codereview.chromium.org/2274693002/diff/20001/src/api.cc#newcode2878
src/api.cc:2878: bool supports_legacy_wire_format;
On 2016/08/24 at 17:41:03, Jakob wrote:
> How about initializing this?

Done.

https://codereview.chromium.org/2274693002/diff/20001/src/api.cc#newcode2887
src/api.cc:2887: }
On 2016/08/24 at 17:41:04, Jakob wrote:
> how about: else { private_ = nullptr; } ?

Not one, but two uninitialized variables in one small CL.

/me hangs his head in shame.

Done.

https://codereview.chromium.org/2274693002/diff/20001/src/api.cc#newcode2907
src/api.cc:2907: CHECK(private_);
On 2016/08/24 at 17:41:04, Jakob wrote:
> Presumably this needs to be called before ReadHeader (which checks for
private_->supports_legacy_wire_format), but it doesn't allow the embedder to
check for successful initialization without crashing.

Added a comment to clarify usage, and made it just return if private_ is null
(since the embedder will notice the failure shortly when they try to
ReadHeader). Seemed nicer than adding two-phase initialization or an
easy-to-forget "is it OK to proceed" getter.

[commit-bot: I haz the power, 2016-08-24 21:50:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-24 21:50:20]

Dry run: This issue passed the CQ dry run.

[Jakob Kummerow, 2016-08-25 08:29:17]

lgtm

[jbroman, 2016-08-25 14:11:20]

jbroman@chromium.org changed reviewers:
+ jochen@chromium.org

[jbroman, 2016-08-25 14:11:21]

+jochen for include/

[jochen (gone - plz use gerrit), 2016-08-25 14:42:43]

lgtm

[jbroman, 2016-08-25 15:56:01]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2016-08-25 15:56:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-25 15:59:54]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-25 16:00:36]

Description was changed from

==========
Add an experimental public API for value serialization.

Suitably scary warnings attached, as this will yet evolve (notably to handle
host objects, which are not currently handled).

Unit tests adjusted to use the public version of ValueSerializer, eliminating
any need they have to access v8::internal.

With this, Blink can begin using this code experimentally behind a flag as it
develops.

BUG=chromium:148757
==========

to

==========
Add an experimental public API for value serialization.

Suitably scary warnings attached, as this will yet evolve (notably to handle
host objects, which are not currently handled).

Unit tests adjusted to use the public version of ValueSerializer, eliminating
any need they have to access v8::internal.

With this, Blink can begin using this code experimentally behind a flag as it
develops.

BUG=chromium:148757

Committed: https://crrev.com/58cac6501f6b17d9cc8fb84ce65e7c89416dd9af
Cr-Commit-Position: refs/heads/master@{#38915}
==========

[commit-bot: I haz the power, 2016-08-25 16:00:38]

Patchset 3 (id:??) landed as
https://crrev.com/58cac6501f6b17d9cc8fb84ce65e7c89416dd9af
Cr-Commit-Position: refs/heads/master@{#38915}