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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 173753002: Drop redundant 'ExceptionMessages::failedToXXX' calls. (Closed)

Created:
6 years, 10 months ago by Mike West
Modified:
6 years, 10 months ago
CC:
blink-reviews, Nils Barth (inactive), kojih, arv+blink, jsbell+bindings_chromium.org, feature-media-reviews_chromium.org, sof, gasubic, fs, eric.carlson_apple.com, abarth-chromium, marja+watch_chromium.org, dglazkov+blink, nessy, adamk+blink_chromium.org, Raymond Toy, Nate Chapin, vcarbune.chromium, philipj_slow, Inactive
Visibility:
Public.

Description

Drop redundant 'ExceptionMessages::failedToXXX' calls. Since ExceptionState takes care of these internally, calling them when throwing an exception leads to redundancy in the exception messages. As a drive-by, this CL fixes the range representation in ExceptionMessages::indexOutsideRange. BUG=270033 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=167758

Patch Set 1 #

Patch Set 2 : appengine. :( #

Patch Set 3 : Rebaseline. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+36 lines, -67 lines) Patch
M LayoutTests/media/track/track-cue-mutable-expected.txt View 1 2 3 chunks +4 lines, -4 lines 0 comments Download
M LayoutTests/media/track/vtt-cue-exceptions-expected.txt View 1 2 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/media/video-volume-expected.txt View 1 2 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/webaudio/dom-exceptions-expected.txt View 2 chunks +9 lines, -9 lines 0 comments Download
M Source/bindings/v8/Dictionary.cpp View 1 2 1 chunk +1 line, -6 lines 0 comments Download
M Source/bindings/v8/ExceptionMessages.h View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/html/MediaController.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/track/vtt/VTTCue.cpp View 3 chunks +3 lines, -3 lines 0 comments Download
M Source/modules/webaudio/AnalyserNode.cpp View 4 chunks +6 lines, -24 lines 2 comments Download
M Source/modules/webaudio/DefaultAudioDestinationNode.cpp View 1 chunk +1 line, -6 lines 0 comments Download
M Source/modules/webaudio/WaveShaperNode.cpp View 1 chunk +1 line, -4 lines 0 comments Download

Messages

Total messages: 34 (0 generated)
Mike West
Mind taking a look, Jochen? WebAudio and a few other places redundantly call 'ExceptionMessages::failedToXXX'. I ...
6 years, 10 months ago (2014-02-20 13:21:55 UTC) #1
haraken
Would you reupload? The side-by-side diff is broken.
6 years, 10 months ago (2014-02-20 13:28:53 UTC) #2
Mike West
On 2014/02/20 13:28:53, haraken wrote: > Would you reupload? The side-by-side diff is broken. Ah, ...
6 years, 10 months ago (2014-02-20 14:05:33 UTC) #3
haraken
LGTM
6 years, 10 months ago (2014-02-20 15:11:37 UTC) #4
Mike West
The CQ bit was checked by mkwst@chromium.org
6 years, 10 months ago (2014-02-20 15:37:47 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mkwst@chromium.org/173753002/170001
6 years, 10 months ago (2014-02-20 15:38:04 UTC) #6
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-20 19:23:45 UTC) #7
commit-bot: I haz the power
Retried try job too often on win_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_rel&number=28174
6 years, 10 months ago (2014-02-20 19:23:45 UTC) #8
Mike West
The CQ bit was checked by mkwst@chromium.org
6 years, 10 months ago (2014-02-21 05:45:58 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mkwst@chromium.org/173753002/170001
6 years, 10 months ago (2014-02-21 05:46:12 UTC) #10
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-21 07:17:22 UTC) #11
commit-bot: I haz the power
Retried try job too often on win_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_rel&number=28261
6 years, 10 months ago (2014-02-21 07:17:23 UTC) #12
Mike West
The CQ bit was checked by mkwst@chromium.org
6 years, 10 months ago (2014-02-21 16:01:15 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mkwst@chromium.org/173753002/170001
6 years, 10 months ago (2014-02-21 16:01:26 UTC) #14
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-21 17:22:33 UTC) #15
commit-bot: I haz the power
Retried try job too often on win_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_rel&number=28334
6 years, 10 months ago (2014-02-21 17:22:34 UTC) #16
Mike West
The CQ bit was checked by mkwst@chromium.org
6 years, 10 months ago (2014-02-24 08:10:52 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mkwst@chromium.org/173753002/170001
6 years, 10 months ago (2014-02-24 08:10:58 UTC) #18
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-24 09:34:24 UTC) #19
commit-bot: I haz the power
Retried try job too often on win_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_rel&number=28555
6 years, 10 months ago (2014-02-24 09:34:25 UTC) #20
Mike West
The CQ bit was checked by mkwst@chromium.org
6 years, 10 months ago (2014-02-24 10:02:13 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mkwst@chromium.org/173753002/170001
6 years, 10 months ago (2014-02-24 10:02:21 UTC) #22
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-24 11:28:54 UTC) #23
commit-bot: I haz the power
Retried try job too often on win_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_rel&number=28577
6 years, 10 months ago (2014-02-24 11:28:54 UTC) #24
Mike West
The CQ bit was checked by mkwst@chromium.org
6 years, 10 months ago (2014-02-24 12:45:55 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mkwst@chromium.org/173753002/170001
6 years, 10 months ago (2014-02-24 12:46:01 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mkwst@chromium.org/173753002/170001
6 years, 10 months ago (2014-02-24 13:34:32 UTC) #27
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-24 14:55:17 UTC) #28
commit-bot: I haz the power
Retried try job too often on win_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_rel&number=28632
6 years, 10 months ago (2014-02-24 14:55:17 UTC) #29
Mike West
The CQ bit was checked by mkwst@chromium.org
6 years, 10 months ago (2014-02-25 07:38:21 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mkwst@chromium.org/173753002/170001
6 years, 10 months ago (2014-02-25 07:40:05 UTC) #31
commit-bot: I haz the power
Change committed as 167758
6 years, 10 months ago (2014-02-25 09:05:01 UTC) #32
aandrey
FYI https://codereview.chromium.org/173753002/diff/170001/Source/modules/webaudio/AnalyserNode.cpp File Source/modules/webaudio/AnalyserNode.cpp (right): https://codereview.chromium.org/173753002/diff/170001/Source/modules/webaudio/AnalyserNode.cpp#newcode78 Source/modules/webaudio/AnalyserNode.cpp:78: ExceptionMessages::indexOutsideRange("FTT size", size, RealtimeAnalyser::MinFFTSize, ExceptionMessages::InclusiveBound, RealtimeAnalyser::MaxFFTSize, ExceptionMessages::InclusiveBound) FTT ...
6 years, 10 months ago (2014-02-25 10:14:50 UTC) #33
Mike West
6 years, 10 months ago (2014-02-25 10:26:30 UTC) #34
Message was sent while issue was closed. 
On 2014/02/25 10:14:50, aandrey wrote:
> FYI
> 
>
https://codereview.chromium.org/173753002/diff/170001/Source/modules/webaudio...
> File Source/modules/webaudio/AnalyserNode.cpp (right):
> 
>
https://codereview.chromium.org/173753002/diff/170001/Source/modules/webaudio...
> Source/modules/webaudio/AnalyserNode.cpp:78:
> ExceptionMessages::indexOutsideRange("FTT size", size,
> RealtimeAnalyser::MinFFTSize, ExceptionMessages::InclusiveBound,
> RealtimeAnalyser::MaxFFTSize, ExceptionMessages::InclusiveBound)
> FTT -> FFT
> 
>
https://codereview.chromium.org/173753002/diff/170001/Source/modules/webaudio...
> Source/modules/webaudio/AnalyserNode.cpp:79: : ("The value provided (" +
> String::number(size) + " is not a power of two."));
> " is -> ") is

Good eyes! Sorry about those typos: https://codereview.chromium.org/179643002/

Powered by Google App Engine
This is Rietveld 408576698