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

Issue 2272613003: binding: Retires ExceptionState::throwIfNeeded(). (Closed)

Created:
4 years, 4 months ago by Yuki
Modified:
4 years, 3 months ago
Reviewers:
haraken
CC:
chromium-reviews, caseq+blink_chromium.org, blink-reviews-style_chromium.org, blink-reviews-dom_chromium.org, sof, eae+blinkwatch, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, dglazkov+blink, apavlov+blink_chromium.org, devtools-reviews_chromium.org, blink-reviews, blink-reviews-bindings_chromium.org, rwlbuis, kozyatinskiy+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

binding: Retires ExceptionState::throwIfNeeded(). It's more convenient for call sites to let ExceptionState throw by default than to explicitly call to throwIfNeeded(). This prevents a mistake that a call site forgets to throw. ExceptionState now looks a kind of exception scope. BUG= Committed: https://crrev.com/652e2642c9a869235108602caf350d88c7ccf7ae Cr-Commit-Position: refs/heads/master@{#415139}

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : . #

Total comments: 2

Patch Set 4 : Synced. #

Patch Set 5 : Fixed more. Removing isolate(), etc. #

Patch Set 6 : More fix. #

Patch Set 7 : More fix. #

Patch Set 8 : Synced. #

Patch Set 9 : Synced. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+491 lines, -837 lines) Patch
M third_party/WebKit/Source/bindings/core/v8/ExceptionState.h View 1 2 3 4 5 6 4 chunks +47 lines, -32 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/ExceptionState.cpp View 1 2 3 4 5 6 2 chunks +81 lines, -95 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/PrivateScriptRunner.cpp View 2 chunks +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/V0CustomElementConstructorBuilder.cpp View 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/V8DOMWrapper.cpp View 3 chunks +0 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp View 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/custom/V8CSSStyleDeclarationCustom.cpp View 1 chunk +0 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/custom/V8CustomEventCustom.cpp View 2 chunks +1 line, -3 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/custom/V8DevToolsHostCustom.cpp View 2 chunks +2 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/custom/V8DocumentCustom.cpp View 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/custom/V8IntersectionObserverCustom.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/custom/V8MessageEventCustom.cpp View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/custom/V8MutationObserverCustom.cpp View 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp View 7 chunks +1 line, -9 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp View 2 chunks +0 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/bindings/modules/v8/V8ServiceWorkerMessageEventInternal.h View 2 chunks +1 line, -3 lines 0 comments Download
M third_party/WebKit/Source/bindings/modules/v8/custom/V8DeviceMotionEventCustom.cpp View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/bindings/modules/v8/custom/V8IDBObserverCustom.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/scripts/v8_methods.py View 2 chunks +1 line, -6 lines 0 comments Download
M third_party/WebKit/Source/bindings/scripts/v8_types.py View 1 chunk +1 line, -4 lines 0 comments Download
M third_party/WebKit/Source/bindings/templates/attributes.cpp View 1 2 3 7 chunks +6 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/bindings/templates/interface.cpp View 12 chunks +8 lines, -14 lines 0 comments Download
M third_party/WebKit/Source/bindings/templates/methods.cpp View 1 2 3 5 chunks +3 lines, -19 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestException.cpp View 1 1 chunk +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexed.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexedGlobal.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexedPrimaryGlobal.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp View 1 2 3 52 chunks +35 lines, -56 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface2.cpp View 1 27 chunks +11 lines, -37 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface3.cpp View 1 7 chunks +0 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceCheckSecurity.cpp View 1 7 chunks +3 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceConstructor.cpp View 1 11 chunks +6 lines, -21 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceConstructor2.cpp View 1 5 chunks +4 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceConstructor4.cpp View 1 2 chunks +1 line, -3 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceEventInitConstructor.cpp View 1 2 chunks +1 line, -3 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceGarbageCollected.cpp View 1 15 chunks +0 lines, -15 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceNamedConstructor.cpp View 1 4 chunks +2 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceNode.cpp View 1 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceOriginTrialEnabled.cpp View 1 6 chunks +5 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceSecureContext.cpp View 1 6 chunks +0 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestNode.cpp View 1 2 chunks +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp View 1 2 3 288 chunks +222 lines, -369 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestSpecialOperations.cpp View 1 3 chunks +2 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestTypedefs.cpp View 1 8 chunks +7 lines, -11 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/modules/V8TestInterface5.cpp View 1 23 chunks +15 lines, -27 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/modules/V8TestInterfacePartial.cpp View 1 8 chunks +2 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/core/dom/CompositorProxy.cpp View 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp View 1 2 3 4 5 6 7 8 1 chunk +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp View 1 2 3 4 5 6 7 8 4 chunks +4 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp View 1 2 3 4 1 chunk +5 lines, -3 lines 0 comments Download

Messages

Total messages: 72 (49 generated)
Yuki
Could you review this CL?
4 years, 4 months ago (2016-08-24 11:43:54 UTC) #9
haraken
Thanks for taking over my CL! https://codereview.chromium.org/2272613003/diff/40001/third_party/WebKit/Source/bindings/core/v8/custom/V8DocumentCustom.cpp File third_party/WebKit/Source/bindings/core/v8/custom/V8DocumentCustom.cpp (left): https://codereview.chromium.org/2272613003/diff/40001/third_party/WebKit/Source/bindings/core/v8/custom/V8DocumentCustom.cpp#oldcode90 third_party/WebKit/Source/bindings/core/v8/custom/V8DocumentCustom.cpp:90: if (exceptionState.throwIfNeeded()) Don't ...
4 years, 3 months ago (2016-08-24 13:33:02 UTC) #14
Yuki
https://codereview.chromium.org/2272613003/diff/40001/third_party/WebKit/Source/bindings/core/v8/custom/V8DocumentCustom.cpp File third_party/WebKit/Source/bindings/core/v8/custom/V8DocumentCustom.cpp (left): https://codereview.chromium.org/2272613003/diff/40001/third_party/WebKit/Source/bindings/core/v8/custom/V8DocumentCustom.cpp#oldcode90 third_party/WebKit/Source/bindings/core/v8/custom/V8DocumentCustom.cpp:90: if (exceptionState.throwIfNeeded()) On 2016/08/24 13:33:02, haraken wrote: > > ...
4 years, 3 months ago (2016-08-24 13:36:20 UTC) #15
haraken
LGTM
4 years, 3 months ago (2016-08-24 13:45:08 UTC) #16
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/2272613003/40001
4 years, 3 months ago (2016-08-25 06:12:14 UTC) #20
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/129666)
4 years, 3 months ago (2016-08-25 07:10:02 UTC) #22
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/2272613003/80001
4 years, 3 months ago (2016-08-25 10:28:27 UTC) #29
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/286427)
4 years, 3 months ago (2016-08-25 11:58:37 UTC) #31
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/2272613003/100001
4 years, 3 months ago (2016-08-25 12:51:05 UTC) #34
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/284669)
4 years, 3 months ago (2016-08-25 14:06:36 UTC) #36
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/2272613003/140001
4 years, 3 months ago (2016-08-29 03:05:32 UTC) #47
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/131609)
4 years, 3 months ago (2016-08-29 04:34:02 UTC) #49
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/2272613003/140001
4 years, 3 months ago (2016-08-29 05:06:03 UTC) #51
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/131656)
4 years, 3 months ago (2016-08-29 05:56:29 UTC) #53
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/2272613003/140001
4 years, 3 months ago (2016-08-29 05:59:39 UTC) #55
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/131700)
4 years, 3 months ago (2016-08-29 06:53:03 UTC) #57
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/2272613003/160001
4 years, 3 months ago (2016-08-29 06:57:15 UTC) #60
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/131736)
4 years, 3 months ago (2016-08-29 08:09:28 UTC) #62
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/2272613003/160001
4 years, 3 months ago (2016-08-29 09:30:09 UTC) #64
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/131879)
4 years, 3 months ago (2016-08-29 10:57:03 UTC) #66
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/2272613003/160001
4 years, 3 months ago (2016-08-30 02:45:02 UTC) #68
commit-bot: I haz the power
Committed patchset #9 (id:160001)
4 years, 3 months ago (2016-08-30 05:11:52 UTC) #70
commit-bot: I haz the power
4 years, 3 months ago (2016-08-30 05:14:28 UTC) #72
Message was sent while issue was closed.
Patchset 9 (id:??) landed as
https://crrev.com/652e2642c9a869235108602caf350d88c7ccf7ae
Cr-Commit-Position: refs/heads/master@{#415139}

Powered by Google App Engine
This is Rietveld 408576698