Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2500363002:
Makes FunctionTemplate::HasInstance return true for a global proxy. (Closed)
|
Created:
4 years, 1 month ago by Yuki Modified:
4 years, 1 month ago Reviewers:
Toon Verwaest CC:
v8-reviews_googlegroups.com Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionMakes FunctionTemplate::HasInstance return true for a global proxy.
Makes FunctionTemplate::HasInstance follow the hidden prototype chain
for a global proxy object and return true if the global object passes
the test.
BUG=
Committed: https://crrev.com/96ad91a657fe83b124f9016b4f9c52512ed83831
Cr-Commit-Position: refs/heads/master@{#40989}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Changed the style. #Patch Set 3 : Changed the style from Chromium to V8. #Messages
Total messages: 28 (18 generated)
The CQ bit was checked by yukishiino@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: This issue passed the CQ dry run.
The CQ bit was checked by yukishiino@chromium.org
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
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. Committers are members of the group "project-v8-committers". Note that this has nothing to do with OWNERS files.
The CQ bit was unchecked by yukishiino@chromium.org
yukishiino@chromium.org changed reviewers: + verwaest@chromium.org
Hi Toon, could you review this CL? I'm working on http://crrev.com/2441593002 , where I need to check if the given global proxy object is an instance of Window (or Worker) or not. Jochen added support of internal fields in the global proxy object, but it turned out that it was not enough to handle the global proxy object. We need to determine the type of the global proxy object.
https://codereview.chromium.org/2500363002/diff/1/src/api.cc File src/api.cc (right): https://codereview.chromium.org/2500363002/diff/1/src/api.cc#newcode6397 src/api.cc:6397: } style nit, what about something like: if (obj->IsJSGlobalProxy()) { PrototypeIterator iter(i::JSObject::cast(*obj)); if (iter.IsAtEnd()) return false; return self->IsTemplateFor(iter->GetCurrent<i::JSGlobalObject>()); }
The CQ bit was checked by yukishiino@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...
https://codereview.chromium.org/2500363002/diff/1/src/api.cc File src/api.cc (right): https://codereview.chromium.org/2500363002/diff/1/src/api.cc#newcode6397 src/api.cc:6397: } On 2016/11/15 12:13:43, Toon Verwaest wrote: > style nit, what about something like: > > if (obj->IsJSGlobalProxy()) { > PrototypeIterator iter(i::JSObject::cast(*obj)); > if (iter.IsAtEnd()) return false; > return self->IsTemplateFor(iter->GetCurrent<i::JSGlobalObject>()); > } Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_presubmit on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/28534)
The CQ bit was checked by yukishiino@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...
Lgtm. (Do you really need the ->map()?)
On 2016/11/15 12:45:53, Toon Verwaest wrote: > Lgtm. (Do you really need the ->map()?) It was a compile error when I didn't write "->map()". So, I added it.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by yukishiino@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Makes FunctionTemplate::HasInstance return true for a global proxy. Makes FunctionTemplate::HasInstance follow the hidden prototype chain for a global proxy object and return true if the global object passes the test. BUG= ========== to ========== Makes FunctionTemplate::HasInstance return true for a global proxy. Makes FunctionTemplate::HasInstance follow the hidden prototype chain for a global proxy object and return true if the global object passes the test. BUG= Committed: https://crrev.com/96ad91a657fe83b124f9016b4f9c52512ed83831 Cr-Commit-Position: refs/heads/master@{#40989} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/96ad91a657fe83b124f9016b4f9c52512ed83831 Cr-Commit-Position: refs/heads/master@{#40989} |
