deps: cherry-pick dbfe4a49d8 from upstream V8

[jkrems]

Original commit message:

Introduce ScriptOrModule and HostDefinedOptions

This patch introduces a new container type ScriptOrModule which
provides the name and the host defined options of the script/module.

This patch also introduces a new PrimitivesArray that can hold
Primitive values, which the embedder can use to store metadata.

The HostDefinedOptions is passed to V8 through the ScriptOrigin, and
passed back to the embedder through HostImportModuleDynamically for
module loading.

Bug: v8:5785, v8:6658, v8:6683
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I56c26fc9a680b273ac0a6691e5ad75f15b8dc80a
Reviewed-on: https://chromium-review.googlesource.com/622158
Reviewed-by: Adam Klein <[email protected]>
Reviewed-by: Georg Neis <[email protected]>
Commit-Queue: Sathya Gunasekaran <[email protected]>
Cr-Commit-Position: refs/heads/master@{#47724}

Refs: v8/v8@dbfe4a4
Refs: #15713

/cc @nodejs/v8

Checklist

• make -j4 test-only passes - something's weird about the doc & lint test setup on master
• commit message follows commit guidelines

Affected core subsystem(s)

V8

[jkrems]

The patch did apply cleanly to everything but deps/v8/tools/v8heapconst.py and I wasn't sure how to regenerate that file..? I can build node and run all tests but I have no idea if that file matters at runtime or somewhere in our build process.

[hashseed]

I don't think Node.js tests v8heapconst.py. Just for the record though: build for x64 release build and run mkgrokdump.

[jkrems]

The "build" part is where I failed. I'm sure I'm just missing something basic here. I tried building the way I would in a V8 checkout and don't get past the initial step:

# Running inside deps/v8
> gn gen out.gn
gn.py: Could not find checkout in any parent of the current path.
This must be run inside a checkout.

The node build itself doesn't seem to include mkgrokdump (find ./out | grep mkgrokdump).

[fhinkel]

Thanks! Can you update the v8_embedder_string number in common.gypi, please. See backporting guide.

[jkrems]

Ah, I saw it in the guide but then forgot about it for some reason. v8_embedder_string is updated now.

[targos]

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/1039/

[fhinkel]

Did you run git commit --amend --reset-author? Usually we set the author of cherry-picks to the person back-porting, not the original author. Though I'm sure @gsathya doesn't mind.

[hashseed]

You would need to run gclient sync to build with gn.

[jkrems]

@fhinkel Thanks. I should really work on my reading comprehension. :D Did a reset of the author so it's consistent with past backports.

[fhinkel]

No worries. That document got really long and it's hard to spot and remember all the steps.

[targos]

it's hard to spot and remember all the steps.

That's why I made an automated tool ;)

[jkrems]

Heh, I think I got confused by its name. Next time!

[bmeck]

Looks fine to me on first glance, will try and get to full check tonight

[fhinkel]

@targos Life changing! Thanks 🙏

[BridgeAR]

Seems like this needs a rebase and is otherwise fine?

[BridgeAR]

@nodejs/v8 PTAL

[bnoordhuis]

This patch is already in 6.2, isn't it? We just need to upgrade to >= 6.2.436.

[jkrems]

@bnoordhuis That was my initial assumption but @targos suggested that we wouldn't get 6.2.436 because the branch was cut at an earlier version of 6.2:

Then you're waiting for 6.3 because the 6.2 branch was cut at an earlier minor version.
If that commit alone is enough, an option is to cherry-pick it.

See #15713 (comment)

If we're fine pulling in a version of 6.2 after the stable branch was cut(*), then this cherry-pick isn't necessary. My motivation here is to have a path towards dynamic import in node 8 (experimental) so we can continue fleshing out the module story.

P.S.: Punting on the rebase until there's agreement. So far it only affects the embedder string, so it's pretty straightforward to resolve when the time comes.

(*) This is based on my understanding that there's one specific 6.2.x that is considered the stable branch for 6.2 and a couple of 6.2.(x+n)s that were released later on but are pretty much "dead".

[bnoordhuis]

You're right. LGTM.

[targos]

IMHO this is good to go after running the regular pull request CI

[jkrems]

CI: https://ci.nodejs.org/job/node-test-pull-request/11709/

[jkrems]

Landed as 3a4fe77

[targos]

Should this land on v9.x? I'm afraid it might be ABI incompatible.

[jkrems]

@targos It would be great if we could land it on 9.x but I'm not sure how to verify that it's ABI compatible. I would assume that the change of the dynamic import callback signature wouldn't count since it only affects an unshipped feature..?

[targos]

The signature of ScriptOrigin's constructor changed. I don't know if it's something that might be used.

[bnoordhuis]

It can't land in any of the stable branches for the reason @targos mentions; ScriptOrigin is used by some add-ons.

[jkrems]

Gotcha, thanks! Given that this only enables a feature that's still flagged anyhow in all stable versions, I assume even if we could create a patch to make it ABI compatible, it wouldn't be worth it.