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

Issue 2285933003: Remove more usage of the base::ListValue::Append(Value*) overload. (Closed)

Created:
4 years, 3 months ago by dcheng
Modified:
4 years, 3 months ago
CC:
chromium-reviews, tzik, posciak+watch_chromium.org, dmazzoni+watch_chromium.org, kinuko+watch, extensions-reviews_chromium.org, aboxhall+watch_chromium.org, samuong+watch_chromium.org, chromoting-reviews_chromium.org, jam, je_julie, darin-cc_chromium.org, devtools-reviews_chromium.org, chromium-apps-reviews_chromium.org, blink-worker-reviews_chromium.org, mlamouri+watch-content_chromium.org, jsbell+serviceworker_chromium.org, yuzo+watch_chromium.org, feature-media-reviews_chromium.org, nhiroki, piman+watch_chromium.org, michaeln, serviceworker-reviews, nektar+watch_chromium.org, kinuko+serviceworker, dtseng+watch_chromium.org, horo+watch_chromium.org, pfeldman
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove more usage of the base::ListValue::Append(Value*) overload. This overload is deprecated: prefer the std::unique_ptr<Value> version, which helps the compiler enforce that ownership transfer occurs. BUG=581865 R=danakj@chromium.org TBR=brettw@chromium.org Committed: https://crrev.com/031a8f86b4679f1fb077516c978ea12f76036d42 Cr-Commit-Position: refs/heads/master@{#417396}

Patch Set 1 #

Patch Set 2 : Small fixes #

Total comments: 8

Patch Set 3 : . #

Total comments: 3

Patch Set 4 : fix webrtc #

Patch Set 5 : Fix more tests #

Patch Set 6 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+170 lines, -163 lines) Patch
M base/json/json_parser.h View 1 2 3 2 chunks +10 lines, -11 lines 0 comments Download
M base/json/json_parser.cc View 13 chunks +20 lines, -20 lines 0 comments Download
M base/json/json_parser_unittest.cc View 7 chunks +7 lines, -7 lines 0 comments Download
M base/test/gtest_util.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/base/javascript_browser_test.cc View 1 2 3 1 chunk +2 lines, -5 lines 0 comments Download
M chrome/test/chromedriver/chrome/automation_extension.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/chromedriver/chrome/web_view_impl.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/test/chromedriver/commands_unittest.cc View 5 chunks +9 lines, -9 lines 0 comments Download
M chrome/test/chromedriver/element_commands.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/chromedriver/element_util.h View 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/test/chromedriver/element_util.cc View 2 chunks +7 lines, -6 lines 0 comments Download
M chrome/test/chromedriver/logging.cc View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/test/chromedriver/performance_logger_unittest.cc View 1 2 3 4 2 chunks +7 lines, -4 lines 0 comments Download
M chrome/test/chromedriver/window_commands.cc View 4 chunks +6 lines, -5 lines 0 comments Download
M content/browser/accessibility/accessibility_tree_formatter.cc View 2 chunks +6 lines, -3 lines 0 comments Download
M content/browser/accessibility/accessibility_ui.cc View 4 chunks +7 lines, -3 lines 0 comments Download
M content/browser/devtools/protocol/devtools_protocol_handler_generator.py View 4 chunks +16 lines, -15 lines 0 comments Download
M content/browser/gpu/gpu_internals_ui.cc View 2 chunks +7 lines, -6 lines 0 comments Download
M content/browser/service_worker/service_worker_internals_ui.cc View 2 chunks +4 lines, -5 lines 0 comments Download
M content/browser/webrtc/webrtc_internals.cc View 1 2 3 5 chunks +16 lines, -11 lines 0 comments Download
M content/renderer/devtools/devtools_agent.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M content/renderer/media/peer_connection_tracker.cc View 1 4 chunks +9 lines, -4 lines 0 comments Download
M device/geolocation/network_location_request.cc View 1 chunk +10 lines, -7 lines 0 comments Download
M google_apis/drive/drive_api_requests.cc View 2 chunks +3 lines, -2 lines 0 comments Download
M printing/print_settings_conversion.cc View 2 chunks +4 lines, -2 lines 0 comments Download
M remoting/client/plugin/chromoting_instance.cc View 1 chunk +1 line, -1 line 0 comments Download
M remoting/host/gcd_rest_client.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M skia/ext/benchmarking_canvas.cc View 1 2 23 chunks +5 lines, -26 lines 0 comments Download

Messages

Total messages: 37 (24 generated)
dcheng
https://codereview.chromium.org/2285933003/diff/20001/chrome/test/chromedriver/logging.cc File chrome/test/chromedriver/logging.cc (right): https://codereview.chromium.org/2285933003/diff/20001/chrome/test/chromedriver/logging.cc#newcode11 chrome/test/chromedriver/logging.cc:11: #include <memory> Not sure what happened here. I guess ...
4 years, 3 months ago (2016-08-26 23:24:41 UTC) #3
danakj
LGTM but.. https://codereview.chromium.org/2285933003/diff/20001/base/json/json_parser.h File base/json/json_parser.h (right): https://codereview.chromium.org/2285933003/diff/20001/base/json/json_parser.h#newcode176 base/json/json_parser.h:176: // Liststd::unique_ptr<Value>. broken comment https://codereview.chromium.org/2285933003/diff/20001/chrome/test/base/javascript_browser_test.cc File chrome/test/base/javascript_browser_test.cc ...
4 years, 3 months ago (2016-08-27 00:07:14 UTC) #8
dcheng
The WebRTC changes are technically behavior changes, but the actual observer just queues some javascript ...
4 years, 3 months ago (2016-08-30 06:47:48 UTC) #11
danakj
https://codereview.chromium.org/2285933003/diff/20001/content/browser/accessibility/accessibility_tree_formatter.cc File content/browser/accessibility/accessibility_tree_formatter.cc (right): https://codereview.chromium.org/2285933003/diff/20001/content/browser/accessibility/accessibility_tree_formatter.cc#newcode68 content/browser/accessibility/accessibility_tree_formatter.cc:68: RecursiveBuildAccessibilityTree(*child_node, child_dict.get()); On 2016/08/30 06:47:48, dcheng wrote: > On ...
4 years, 3 months ago (2016-09-06 21:55:39 UTC) #22
dcheng
+tommi, mind taking a look at the webrtc changes? https://codereview.chromium.org/2285933003/diff/40001/content/browser/webrtc/webrtc_internals.cc File content/browser/webrtc/webrtc_internals.cc (right): https://codereview.chromium.org/2285933003/diff/40001/content/browser/webrtc/webrtc_internals.cc#newcode134 content/browser/webrtc/webrtc_internals.cc:134: ...
4 years, 3 months ago (2016-09-06 22:01:18 UTC) #24
danakj
On Tue, Sep 6, 2016 at 3:01 PM, <dcheng@chromium.org> wrote: > +tommi, mind taking a ...
4 years, 3 months ago (2016-09-06 22:11:15 UTC) #25
dcheng
On 2016/09/06 22:11:15, danakj wrote: > On Tue, Sep 6, 2016 at 3:01 PM, <mailto:dcheng@chromium.org> ...
4 years, 3 months ago (2016-09-06 22:23:45 UTC) #26
tommi (sloooow) - chröme
lgtm for webrtc
4 years, 3 months ago (2016-09-07 17:44:22 UTC) #27
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/2285933003/100001
4 years, 3 months ago (2016-09-08 05:01:52 UTC) #30
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/289420)
4 years, 3 months ago (2016-09-08 05:50:22 UTC) #32
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/2285933003/100001
4 years, 3 months ago (2016-09-08 16:39:38 UTC) #34
commit-bot: I haz the power
Committed patchset #6 (id:100001)
4 years, 3 months ago (2016-09-08 21:04:51 UTC) #35
commit-bot: I haz the power
4 years, 3 months ago (2016-09-08 21:08:28 UTC) #37
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/031a8f86b4679f1fb077516c978ea12f76036d42
Cr-Commit-Position: refs/heads/master@{#417396}

Powered by Google App Engine
This is Rietveld 408576698