New issue
Advanced search Search tips

Issue 877281 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac
Pri: 3
Type: Task


Show other hotlists

Hotlists containing this issue:
Autofill-Fixit


Sign in to add a comment

Refactor DictionaryValue to Value in payments_client

Project Member Reported by sujiezhu@google.com, Aug 23

Issue description

The DictionaryValue is used in GetRequestContent(), ParseResponse() and some helper functions for GetUploadDetailsRequest, UnmaskCardRequest and UploadCardRequest.

https://cs.chromium.org/chromium/src/components/autofill/core/browser/payments/payments_client.cc?l=372-435&rcl=6bc32445df16acf82f40643689e693ec8515de45

For more information, see comment thread: https://chromium-review.googlesource.com/c/chromium/src/+/1178571/4/components/autofill/core/browser/payments/payments_client.cc#199
 

Comment 1 Deleted

Comment 2 Deleted

Owner: seblalancette@chromium.org
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c186a64d5b1ac81e5a73429811bdbe68268c0f49

commit c186a64d5b1ac81e5a73429811bdbe68268c0f49
Author: Sebastien Lalancette <seblalancette@chromium.org>
Date: Fri Dec 21 22:12:31 2018

Refactored DictionaryValue/ListValue to Value to base::Value in payments_client.

Some context from base/values.h:

base::Value is currently in the process of being refactored. Design doc:
https://docs.google.com/document/d/1uDLu5uTRlCWePxQUEHc8yNQdEoE1BDISYdpggWEABnw

Previously (which is how most code that currently exists is written), Value
used derived types to implement the individual data types, and base::Value
was just a base class to refer to them. This required everything be heap
allocated.

OLD WAY:

  std::unique_ptr<base::Value> GetFoo() {
    std::unique_ptr<DictionaryValue> dict;
    dict->SetString("mykey", foo);
    return dict;
  }

The new design makes base::Value a variant type that holds everything in
a union. It is now recommended to pass by value with std::move rather than
use heap allocated values. The DictionaryValue and ListValue subclasses
exist only as a compatibility shim that we're in the process of removing.

NEW WAY:

  base::Value GetFoo() {
    base::Value dict(base::Value::Type::DICTIONARY);
    dict.SetKey("mykey", base::Value(foo));
    return dict;
  }

Bug:  877281 
Change-Id: Ia16fb806a3de551dc427ed6e9a8600f0f8e5acb7
Reviewed-on: https://chromium-review.googlesource.com/c/1384608
Reviewed-by: Jared Saul <jsaul@google.com>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Sebastien Lalancette <seblalancette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618626}
[modify] https://crrev.com/c186a64d5b1ac81e5a73429811bdbe68268c0f49/components/autofill/core/browser/credit_card_save_manager.cc
[modify] https://crrev.com/c186a64d5b1ac81e5a73429811bdbe68268c0f49/components/autofill/core/browser/credit_card_save_manager.h
[modify] https://crrev.com/c186a64d5b1ac81e5a73429811bdbe68268c0f49/components/autofill/core/browser/local_card_migration_manager.cc
[modify] https://crrev.com/c186a64d5b1ac81e5a73429811bdbe68268c0f49/components/autofill/core/browser/local_card_migration_manager.h
[modify] https://crrev.com/c186a64d5b1ac81e5a73429811bdbe68268c0f49/components/autofill/core/browser/payments/payments_client.cc
[modify] https://crrev.com/c186a64d5b1ac81e5a73429811bdbe68268c0f49/components/autofill/core/browser/payments/payments_client.h
[modify] https://crrev.com/c186a64d5b1ac81e5a73429811bdbe68268c0f49/components/autofill/core/browser/payments/payments_client_unittest.cc
[modify] https://crrev.com/c186a64d5b1ac81e5a73429811bdbe68268c0f49/components/autofill/core/browser/payments/payments_request.h
[modify] https://crrev.com/c186a64d5b1ac81e5a73429811bdbe68268c0f49/components/autofill/core/browser/payments/test_payments_client.cc
[modify] https://crrev.com/c186a64d5b1ac81e5a73429811bdbe68268c0f49/components/autofill/core/browser/payments/test_payments_client.h
[modify] https://crrev.com/c186a64d5b1ac81e5a73429811bdbe68268c0f49/components/autofill/core/browser/test_local_card_migration_manager.cc
[modify] https://crrev.com/c186a64d5b1ac81e5a73429811bdbe68268c0f49/components/autofill/core/browser/test_local_card_migration_manager.h

Status: Fixed (was: Started)
Refactoring was checked-in.

Sign in to add a comment