New issue
Advanced search Search tips

Issue 881076 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Migrate to using JSON protocol instead of XML for the component updater

Project Member Reported by sorin@chromium.org, Sep 5

Issue description

We'd like to remove the dependency on the libxml and use a lighter text protocol in the component updater.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 7

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

commit c303bf437bfdf68e82caa5da28cb8e090af994b0
Author: Sorin Jianu <sorin@chromium.org>
Date: Fri Sep 07 16:19:33 2018

Make Configurator::ExtraRequestParams return a base::flat_map instead of std::string.

This is a mechanical change.

The rationale for this change is that the value returned from this function is
formated inside a component updater request. How this value is formated depends
on the protocol encoding.
Therefore, to abstract out the protocol encoding (XML or JSON), the function
must return its data as a map of key-values instead of joined strings.

Also, as a small change, replace std::map with base::flat_map for the
extra request headers.

BUG: 881076
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: Ic39e94e8de0e3e27128a80cd24c055bdd434c45f
Reviewed-on: https://chromium-review.googlesource.com/1208134
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Minh Nguyen <mxnguyen@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589536}
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/chrome/browser/component_updater/chrome_component_updater_configurator.cc
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/chrome/browser/extensions/updater/chrome_update_client_config.cc
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/chrome/browser/extensions/updater/chrome_update_client_config.h
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/component_updater/configurator_impl.cc
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/component_updater/configurator_impl.h
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/configurator.h
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/protocol_builder.cc
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/protocol_builder.h
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/protocol_builder_unittest.cc
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/request_sender.cc
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/request_sender.h
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/request_sender_unittest.cc
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/test_configurator.cc
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/test_configurator.h
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/update_checker.cc
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/update_checker.h
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/update_checker_unittest.cc
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/update_client_unittest.cc
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/utils.cc
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/components/update_client/utils.h
[modify] https://crrev.com/c303bf437bfdf68e82caa5da28cb8e090af994b0/ios/chrome/browser/component_updater/ios_component_updater_configurator.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 10

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

commit b82ab3fa976bf43a6099d728e239f42ab292ed68
Author: Sorin Jianu <sorin@chromium.org>
Date: Wed Oct 10 23:18:26 2018

Allow using RE2 to match expectations in update_client unit tests.

This is a mechanical change.

As we are working toward migrating the update protocol from XML to JSON,
we are expecting to write more precise unit tests, where matching
of requests or responses matters, especially when they contain random
values such as ids.

This change introduces a dependency on RE2 and shows an example of
matching using regex.

Bug: 881076
Change-Id: I3941ac47deda2de825ff9da7435e35b4d053245a
Reviewed-on: https://chromium-review.googlesource.com/c/1274002
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598560}
[modify] https://crrev.com/b82ab3fa976bf43a6099d728e239f42ab292ed68/components/update_client/BUILD.gn
[modify] https://crrev.com/b82ab3fa976bf43a6099d728e239f42ab292ed68/components/update_client/DEPS
[modify] https://crrev.com/b82ab3fa976bf43a6099d728e239f42ab292ed68/components/update_client/ping_manager_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 11

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

commit 3615223f2b387309539d8bbc287b4c52a9c5f3fa
Author: Sorin Jianu <sorin@chromium.org>
Date: Thu Oct 11 17:06:00 2018

Fix test break on iOS.

In this case, arch="iPhone10,3" was not matched by \w+.

Also, fixed escaping . in several places inside regexes.


Bug: 881076
Change-Id: I2814eedce66b1b3c0a8fa8fff0eb6490dc4906cb
Reviewed-on: https://chromium-review.googlesource.com/c/1276889
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598808}
[modify] https://crrev.com/3615223f2b387309539d8bbc287b4c52a9c5f3fa/components/update_client/ping_manager_unittest.cc

Cc: sorin@chromium.org
 Issue 848505  has been merged into this issue.
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 19

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

commit 47a4dd8c660ea5da120f387e91086c91e68eb351
Author: Sorin Jianu <sorin@chromium.org>
Date: Fri Oct 19 16:34:54 2018

Allow injection of protocol factory in update_client through Configurator.

This is a mechanical change. Its purpose is allowing different wire
protocol parsers and serializers to be injected in the UpdateChecker and
PingManager instances in the UpdateClient using the Configuration instance.

The current code hardcodes an instance of ProtocolHandlerXm. There is work
in progress to support JSON.

Bug: 881076
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I40cb1605f134a5d3f168a511094b2b265bc50f25
Reviewed-on: https://chromium-review.googlesource.com/c/1289510
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Minh Nguyen <mxnguyen@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601179}
[modify] https://crrev.com/47a4dd8c660ea5da120f387e91086c91e68eb351/chrome/browser/component_updater/chrome_component_updater_configurator.cc
[modify] https://crrev.com/47a4dd8c660ea5da120f387e91086c91e68eb351/chrome/browser/extensions/updater/chrome_update_client_config.cc
[modify] https://crrev.com/47a4dd8c660ea5da120f387e91086c91e68eb351/chrome/browser/extensions/updater/chrome_update_client_config.h
[modify] https://crrev.com/47a4dd8c660ea5da120f387e91086c91e68eb351/components/component_updater/configurator_impl.cc
[modify] https://crrev.com/47a4dd8c660ea5da120f387e91086c91e68eb351/components/component_updater/configurator_impl.h
[modify] https://crrev.com/47a4dd8c660ea5da120f387e91086c91e68eb351/components/update_client/BUILD.gn
[modify] https://crrev.com/47a4dd8c660ea5da120f387e91086c91e68eb351/components/update_client/configurator.h
[modify] https://crrev.com/47a4dd8c660ea5da120f387e91086c91e68eb351/components/update_client/ping_manager.cc
[add] https://crrev.com/47a4dd8c660ea5da120f387e91086c91e68eb351/components/update_client/protocol_handler.cc
[add] https://crrev.com/47a4dd8c660ea5da120f387e91086c91e68eb351/components/update_client/protocol_handler.h
[modify] https://crrev.com/47a4dd8c660ea5da120f387e91086c91e68eb351/components/update_client/test_configurator.cc
[modify] https://crrev.com/47a4dd8c660ea5da120f387e91086c91e68eb351/components/update_client/test_configurator.h
[modify] https://crrev.com/47a4dd8c660ea5da120f387e91086c91e68eb351/components/update_client/update_checker.cc
[modify] https://crrev.com/47a4dd8c660ea5da120f387e91086c91e68eb351/ios/chrome/browser/component_updater/ios_component_updater_configurator.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 10

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

commit c05d46514a79d4f4abf653163119823fb811e4bd
Author: Sorin Jianu <sorin@chromium.org>
Date: Sat Nov 10 01:12:57 2018

Extension updater test: remove "required" from the canned XML response.

The "required" attribute name in the update manifest is ignored by the
update checker, and it can be removed to eliminate confusion and for
simplicity reasons.

This has been discovered when implementing the updater JSON protocol.

Bug: 881076
Change-Id: Ie8ad8ad816c380375d85b6402f1270f21d7b3b20
Reviewed-on: https://chromium-review.googlesource.com/c/1330700
Reviewed-by: Minh Nguyen <mxnguyen@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607068}
[modify] https://crrev.com/c05d46514a79d4f4abf653163119823fb811e4bd/chrome/test/data/extensions/updater/updatecheck_reply_update_1.xml

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 14

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

commit 55587d34b652d245cf899ace77448a9dc2300471
Author: Sorin Jianu <sorin@chromium.org>
Date: Wed Nov 14 21:43:27 2018

Implement support for JSON in update_client and the component updater.

Bug: 881076
Change-Id: I41d7f87d46225ebc919473e8bd165268d699e45e
Reviewed-on: https://chromium-review.googlesource.com/c/1334411
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Minh Nguyen <mxnguyen@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608132}
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/chrome/browser/extensions/updater/extension_update_client_base_browsertest.cc
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/chrome/browser/extensions/updater/extension_update_client_base_browsertest.h
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/chrome/browser/extensions/updater/update_service_browsertest.cc
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/chrome/browser/policy/policy_browsertest.cc
[add] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/chrome/test/data/extensions/updater/ping_reply_1.json
[add] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/chrome/test/data/extensions/updater/updatecheck_reply_noupdate_1.json
[add] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/chrome/test/data/extensions/updater/updatecheck_reply_update_1.json
[delete] https://crrev.com/af023b4c246ad7080b51f6bccf6c26c2a54cdb21/components/test/data/update_client/updatecheck_diff_reply_1.xml
[delete] https://crrev.com/af023b4c246ad7080b51f6bccf6c26c2a54cdb21/components/test/data/update_client/updatecheck_diff_reply_2.xml
[delete] https://crrev.com/af023b4c246ad7080b51f6bccf6c26c2a54cdb21/components/test/data/update_client/updatecheck_diff_reply_3.xml
[add] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/test/data/update_client/updatecheck_reply_1.json
[delete] https://crrev.com/af023b4c246ad7080b51f6bccf6c26c2a54cdb21/components/test/data/update_client/updatecheck_reply_2.xml
[delete] https://crrev.com/af023b4c246ad7080b51f6bccf6c26c2a54cdb21/components/test/data/update_client/updatecheck_reply_3.xml
[add] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/test/data/update_client/updatecheck_reply_4.json
[delete] https://crrev.com/af023b4c246ad7080b51f6bccf6c26c2a54cdb21/components/test/data/update_client/updatecheck_reply_empty
[add] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/test/data/update_client/updatecheck_reply_noupdate.json
[add] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/test/data/update_client/updatecheck_reply_parse_error.json
[add] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/test/data/update_client/updatecheck_reply_unknownapp.json
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/BUILD.gn
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/component.cc
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/ping_manager_unittest.cc
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/protocol_definition.h
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/protocol_handler.cc
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/protocol_handler.h
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/protocol_parser.h
[add] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/protocol_parser_json.cc
[add] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/protocol_parser_json.h
[add] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/protocol_parser_json_unittest.cc
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/protocol_serializer.cc
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/protocol_serializer.h
[add] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/protocol_serializer_json.cc
[add] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/protocol_serializer_json.h
[add] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/protocol_serializer_json_unittest.cc
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/protocol_serializer_xml.cc
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/protocol_serializer_xml_unittest.cc
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/test_configurator.cc
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/test_configurator.h
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/update_checker_unittest.cc
[modify] https://crrev.com/55587d34b652d245cf899ace77448a9dc2300471/components/update_client/update_client_unittest.cc

Sign in to add a comment