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

Issue 2285183005: Use StringView in Dictionary and DictionaryHelper. (Closed)

Created:
4 years, 3 months ago by esprehn
Modified:
4 years, 3 months ago
Reviewers:
haraken, peria
CC:
blink-reviews, blink-reviews-bindings_chromium.org, chromium-reviews, bashi
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Use StringView in Dictionary and DictionaryHelper. This lets us avoid all of the string copies and externalization costs for the key names when getting properties. BUG=615174 Committed: https://crrev.com/f2c9273fe51951b3cb68cc7fe634cc91265cce41 Cr-Commit-Position: refs/heads/master@{#415549}

Patch Set 1 #

Patch Set 2 : rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -53 lines) Patch
M third_party/WebKit/Source/bindings/core/v8/Dictionary.h View 1 4 chunks +10 lines, -15 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/Dictionary.cpp View 1 4 chunks +6 lines, -12 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/DictionaryHelperForBindings.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/DictionaryHelperForCore.cpp View 15 chunks +23 lines, -23 lines 0 comments Download
M third_party/WebKit/Source/bindings/modules/v8/DictionaryHelperForModules.cpp View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 30 (17 generated)
esprehn
4 years, 3 months ago (2016-08-30 06:40:51 UTC) #7
haraken
LGTM For some historical reasons, we have mixed 'String' and 'const String &' in the ...
4 years, 3 months ago (2016-08-30 09:21:20 UTC) #9
esprehn
On 2016/08/30 at 09:21:20, haraken wrote: > LGTM > > For some historical reasons, we ...
4 years, 3 months ago (2016-08-30 09:57:03 UTC) #10
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/2285183005/1
4 years, 3 months ago (2016-08-30 09:57:35 UTC) #12
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/249064)
4 years, 3 months ago (2016-08-30 10:03:18 UTC) #14
haraken
On 2016/08/30 09:57:03, esprehn wrote: > On 2016/08/30 at 09:21:20, haraken wrote: > > LGTM ...
4 years, 3 months ago (2016-08-30 10:12:30 UTC) #15
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/2285183005/1
4 years, 3 months ago (2016-08-30 16:10:18 UTC) #17
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/249262)
4 years, 3 months ago (2016-08-30 16:16:40 UTC) #19
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/2285183005/1
4 years, 3 months ago (2016-08-31 00:14:47 UTC) #21
commit-bot: I haz the power
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds/61172) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, ...
4 years, 3 months ago (2016-08-31 00:18:15 UTC) #23
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/2285183005/20001
4 years, 3 months ago (2016-08-31 00:21:20 UTC) #26
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 3 months ago (2016-08-31 03:04:03 UTC) #28
commit-bot: I haz the power
4 years, 3 months ago (2016-08-31 03:05:39 UTC) #30
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/f2c9273fe51951b3cb68cc7fe634cc91265cce41
Cr-Commit-Position: refs/heads/master@{#415549}

Powered by Google App Engine
This is Rietveld 408576698