Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2710233002:
Tidy ScriptRunner pending script handling. (Closed)
DescriptionTidy ScriptRunner pending script handling.
R=hiroshige
BUG=686281
Review-Url: https://codereview.chromium.org/2710233002
Cr-Commit-Position: refs/heads/master@{#452619}
Committed: https://chromium.googlesource.com/chromium/src/+/641c52c72d465295087d9a937dab9dea7333aecb
Patch Set 1 #Patch Set 2 : add xref comment #
Total comments: 2
Patch Set 3 : use std::find() #Messages
Total messages: 18 (12 generated)
The CQ bit was checked by sigbjornf@opera.com 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...
sigbjornf@opera.com changed reviewers: + hiroshige@chromium.org, kouhei@chromium.org
please take a look. seeing as you're busy improving the quality of this code atm, some minor ScriptRunner issues :)
Description was changed from ========== Simplify ScriptRunner pending script handling. R= BUG=686281 ========== to ========== Tidy ScriptRunner pending script handling. R= BUG=686281 ==========
https://codereview.chromium.org/2710233002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/ScriptRunner.cpp (right): https://codereview.chromium.org/2710233002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/ScriptRunner.cpp:140: auto it = std::find_if( Lack of (Heap)Deque::find() looks causing similar code, e.g. https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/core/f... It would be nice if we can provide a more convenient way, e.g. std::find() rather than using anonymous comparator, or defining Deque::find(), and then replace similar code patterns (I could find only one above though).
The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run
https://codereview.chromium.org/2710233002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/ScriptRunner.cpp (right): https://codereview.chromium.org/2710233002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/ScriptRunner.cpp:140: auto it = std::find_if( On 2017/02/23 17:07:19, hiroshige wrote: > Lack of (Heap)Deque::find() looks causing similar code, e.g. > https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/core/f... > > It would be nice if we can provide a more convenient way, e.g. std::find() > rather than using anonymous comparator, or defining Deque::find(), and then > replace similar code patterns (I could find only one above though). Simplified to std::find(), Member<T> does have an implicit conversion to T*, as wanted.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
Description was changed from ========== Tidy ScriptRunner pending script handling. R= BUG=686281 ========== to ========== Tidy ScriptRunner pending script handling. R=hiroshige BUG=686281 ==========
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 sigbjornf@opera.com
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": 40001, "attempt_start_ts": 1487882703286710,
"parent_rev": "27aabb44f4320bcd7e6b01d33cc1649e2f955194", "commit_rev":
"641c52c72d465295087d9a937dab9dea7333aecb"}
Message was sent while issue was closed.
Description was changed from ========== Tidy ScriptRunner pending script handling. R=hiroshige BUG=686281 ========== to ========== Tidy ScriptRunner pending script handling. R=hiroshige BUG=686281 Review-Url: https://codereview.chromium.org/2710233002 Cr-Commit-Position: refs/heads/master@{#452619} Committed: https://chromium.googlesource.com/chromium/src/+/641c52c72d465295087d9a937dab... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/641c52c72d465295087d9a937dab... |
