Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(145)

Issue 2706813002: CHECK correct context usage in V8ScriptRunner::callFunction (Closed)

Created:
3 years, 10 months ago by jochen (gone - plz use gerrit)
Modified:
3 years, 10 months ago
Reviewers:
haraken, dcheng
CC:
blink-reviews, blink-reviews-bindings_chromium.org, chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

CHECK correct context usage in V8ScriptRunner::callFunction Not tests, since it shouldn't be possible to reach that. BUG=693695 R=dcheng@chromium.org,haraken@chromium.org Review-Url: https://codereview.chromium.org/2706813002 Cr-Commit-Position: refs/heads/master@{#451622} Committed: https://chromium.googlesource.com/chromium/src/+/5cde67543e5c89f1f9da84e1ae860c30283dee3c

Patch Set 1 #

Total comments: 2

Patch Set 2 : updates #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -2 lines) Patch
M third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp View 1 4 chunks +10 lines, -2 lines 0 comments Download

Messages

Total messages: 20 (12 generated)
jochen (gone - plz use gerrit)
3 years, 10 months ago (2017-02-20 09:52:01 UTC) #1
haraken
LGTM https://codereview.chromium.org/2706813002/diff/1/third_party/WebKit/Source/bindings/core/v8/V8EventListener.cpp File third_party/WebKit/Source/bindings/core/v8/V8EventListener.cpp (right): https://codereview.chromium.org/2706813002/diff/1/third_party/WebKit/Source/bindings/core/v8/V8EventListener.cpp#newcode114 third_party/WebKit/Source/bindings/core/v8/V8EventListener.cpp:114: frame, BindingSecurity::ErrorReportOption::DoNotReport)); Another idea would be to move ...
3 years, 10 months ago (2017-02-20 10:01:55 UTC) #4
dcheng
LGTM https://codereview.chromium.org/2706813002/diff/1/third_party/WebKit/Source/bindings/core/v8/V8ErrorHandler.cpp File third_party/WebKit/Source/bindings/core/v8/V8ErrorHandler.cpp (right): https://codereview.chromium.org/2706813002/diff/1/third_party/WebKit/Source/bindings/core/v8/V8ErrorHandler.cpp#newcode92 third_party/WebKit/Source/bindings/core/v8/V8ErrorHandler.cpp:92: toDOMWindow(callFunction->CreationContext())->toLocalDOMWindow(), Hmm... I guess I should fix this ...
3 years, 10 months ago (2017-02-20 10:07:42 UTC) #5
jochen (gone - plz use gerrit)
new approach (will update the CL description), ptal
3 years, 10 months ago (2017-02-20 12:23:11 UTC) #9
haraken
LGTM
3 years, 10 months ago (2017-02-20 12:23:56 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2706813002/20001
3 years, 10 months ago (2017-02-20 13:44:01 UTC) #16
commit-bot: I haz the power
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/5cde67543e5c89f1f9da84e1ae860c30283dee3c
3 years, 10 months ago (2017-02-20 14:25:13 UTC) #19
esprehn
3 years, 10 months ago (2017-02-21 23:37:12 UTC) #20
Message was sent while issue was closed.
This is adding even more overhead to custom element callbacks and other callback
APIs, can we make this a DCHECK? This alone is adding a dozen branches and many
nested function calls to every call into JS from blink.

Powered by Google App Engine
This is Rietveld 408576698