Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2788513003:
[ES6 modules] Make ModuleMap TraceWrapperBase (Closed)
|
Created:
3 years, 8 months ago by kouhei (in TOK) Modified:
3 years, 8 months ago CC:
chromium-reviews, sof, eae+blinkwatch, blink-reviews-dom_chromium.org, dglazkov+blink, blink-reviews, rwlbuis Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[ES6 modules] Make ModuleMap TraceWrapperBase
This CL makes ModuleMap a TraceWrapperBase so that it can trace into ModuleScript,
which will eventually keep on to its m_instantiationError via TraceWrapperV8Reference.
This CL is required for https://codereview.chromium.org/2782403002/
BUG=594639
Review-Url: https://codereview.chromium.org/2788513003
Cr-Commit-Position: refs/heads/master@{#462047}
Committed: https://chromium.googlesource.com/chromium/src/+/b146b4e388d8a0132a0a51cda5b8ebd8237b732e
Patch Set 1 #
Total comments: 4
Patch Set 2 : dep #Patch Set 3 : yhirano #
Total comments: 2
Patch Set 4 : tzik #
Depends on Patchset: Dependent Patchsets: Messages
Total messages: 38 (31 generated)
kouhei@chromium.org changed reviewers: + hiroshige@chromium.org, yhirano@chromium.org
The CQ bit was checked by yhirano@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by kouhei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by kouhei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by kouhei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
lgtm https://codereview.chromium.org/2788513003/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/dom/ModuleMap.cpp (right): https://codereview.chromium.org/2788513003/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/dom/ModuleMap.cpp:28: DECLARE_VIRTUAL_TRACE_WRAPPERS(); ditto https://codereview.chromium.org/2788513003/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/dom/ModuleMap.h (right): https://codereview.chromium.org/2788513003/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/dom/ModuleMap.h:36: DECLARE_VIRTUAL_TRACE_WRAPPERS(); No _VIRTUAL_ is needed?
https://codereview.chromium.org/2788513003/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/dom/ModuleMap.cpp (right): https://codereview.chromium.org/2788513003/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/dom/ModuleMap.cpp:28: DECLARE_VIRTUAL_TRACE_WRAPPERS(); On 2017/04/05 05:56:46, yhirano wrote: > ditto Done. https://codereview.chromium.org/2788513003/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/dom/ModuleMap.h (right): https://codereview.chromium.org/2788513003/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/dom/ModuleMap.h:36: DECLARE_VIRTUAL_TRACE_WRAPPERS(); On 2017/04/05 05:56:46, yhirano wrote: > No _VIRTUAL_ is needed? Done.
Patchset #4 (id:60001) has been deleted
The CQ bit was checked by kouhei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
tzik@chromium.org changed reviewers: + tzik@chromium.org
https://codereview.chromium.org/2788513003/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/ModuleMap.cpp (right): https://codereview.chromium.org/2788513003/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/ModuleMap.cpp:134: MapImpl::AddResult result = m_map.insert(request.url(), nullptr); The value part of MapImpl is a TraceWrapperMember<Entry>, right? IIUC, it doesn't have a constructor that takes single pointer. Do you have an idea what's happening here?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2788513003/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/ModuleMap.cpp (right): https://codereview.chromium.org/2788513003/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/ModuleMap.cpp:134: MapImpl::AddResult result = m_map.insert(request.url(), nullptr); On 2017/04/05 09:05:23, tzik wrote: > The value part of MapImpl is a TraceWrapperMember<Entry>, right? > IIUC, it doesn't have a constructor that takes single pointer. Do you have an > idea what's happening here? Done. Updated per discussed offline.
The CQ bit was checked by kouhei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
The CQ bit was checked by kouhei@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yhirano@chromium.org Link to the patchset: https://codereview.chromium.org/2788513003/#ps80001 (title: "tzik")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1491396919229460,
"parent_rev": "4a61ca6aca8b876c5a62f561db98a6ea8b482754", "commit_rev":
"b146b4e388d8a0132a0a51cda5b8ebd8237b732e"}
Message was sent while issue was closed.
Description was changed from ========== [ES6 modules] Make ModuleMap TraceWrapperBase This CL makes ModuleMap a TraceWrapperBase so that it can trace into ModuleScript, which will eventually keep on to its m_instantiationError via TraceWrapperV8Reference. This CL is required for https://codereview.chromium.org/2782403002/ BUG=594639 ========== to ========== [ES6 modules] Make ModuleMap TraceWrapperBase This CL makes ModuleMap a TraceWrapperBase so that it can trace into ModuleScript, which will eventually keep on to its m_instantiationError via TraceWrapperV8Reference. This CL is required for https://codereview.chromium.org/2782403002/ BUG=594639 Review-Url: https://codereview.chromium.org/2788513003 Cr-Commit-Position: refs/heads/master@{#462047} Committed: https://chromium.googlesource.com/chromium/src/+/b146b4e388d8a0132a0a51cda5b8... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:80001) as https://chromium.googlesource.com/chromium/src/+/b146b4e388d8a0132a0a51cda5b8... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
