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

Issue 715583008: Remove Dictionary::getOwnPropertyNames() (Closed)

Created:
6 years, 1 month ago by bashi
Modified:
6 years, 1 month ago
Reviewers:
haraken
CC:
blink-reviews, michaeln, jsbell+serviceworker_chromium.org, arv+blink, tzik, serviceworker-reviews, nhiroki, falken, tommyw+watchlist_chromium.org, blink-reviews-bindings_chromium.org, horo+watch_chromium.org, kinuko+serviceworker
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Remove Dictionary::getOwnPropertyNames() This is a follow-up CL of https://codereview.chromium.org/661453004/. I don't see why the call sites of getOwnPropertyNames() can't use getPropertyNames() instead. The reason why they need property names is that we don't support OpenEndedDictionary and IDL dictionary which contains an union type at this point. In both cases we should be able to use getPropertyNames(). Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185098

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -29 lines) Patch
M Source/bindings/core/v8/Dictionary.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/bindings/core/v8/Dictionary.cpp View 1 chunk +0 lines, -23 lines 0 comments Download
M Source/modules/mediastream/MediaConstraintsImpl.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/modules/mediastream/RTCPeerConnection.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/modules/serviceworkers/Headers.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 6 (2 generated)
bashi
PTAL? (I'm asking many reviews today, but none of them are urgent, so no hurry:)
6 years, 1 month ago (2014-11-11 02:50:20 UTC) #2
haraken
LGTM
6 years, 1 month ago (2014-11-11 03:17:37 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/715583008/1
6 years, 1 month ago (2014-11-11 03:21:53 UTC) #5
commit-bot: I haz the power
6 years, 1 month ago (2014-11-11 04:03:06 UTC) #6
Message was sent while issue was closed.
Committed patchset #1 (id:1) as 185098

Powered by Google App Engine
This is Rietveld 408576698