Skip to content

Commit

Permalink
Reduce the size of AutofillTable.
Browse files Browse the repository at this point in the history
- Declares string constants for table and column names.
  They are grouped by table on top of autofill_table.cc. As some
  column names (such as "guid") occur multiple times, and to prevent
  unnecessarily long variable names, they are only declared once.
  Comments indicate the names of the variables in this case.
- Defines helper functions to construct SQL statements from these
  constants.
- Applies them throughout most of the file to remove many queries
  specified as string literals.
- Simplifies the code by combining common operations into helper
  functions, such as checking if a table exists and creating it if not.
- Extracts common code between SetServerProfiles() and
  SetServerAddressData() into a separate function.
This reduces the binary size by > 10 kb.

String constants for table and column names have the additional
advantage of preventing typos.

While adapting the code, it was discovered that the:
- full_name_status is not described in the schema.
- The column of the payments_upi_vpa table is called vpa instead of
  vpa_id.
This was adapted in the header file.

Some few queries were not refactored.
- Complicated select queries. Even though they could be adapted in
  a similar fashing, there are few of them and it becomes increasingly
  hard to read.
- UPDATE statements, once again because there is few of them.
- Combined INSERT/SELECT statements in the migration logic. As we
  currently discuss removing some of the migration logic, this was not
  adapted to prevent unnecessary work.

Moreover, the size could be further reduced by defining e.g. constants
for type names (e.g. "VARCHAR"). This is not done due to readability
concerns.

Because of inconsistent whitespacing in the current statements, the
changes did cause differences in the golden files. They amounted to
differences like "INSERT INTO x VALUES(" vs "INSERT INTO x VALUES (".
For this reason sanitization logic was added to this unit test.
This has the side effect of fixing the golden file difference in
schema version 52.

Lastly, two unnecessary Statement::Reset calls at the end of a
Statement's lifetime were removed.

Bug: 927429
Change-Id: Idf4880e0783e9ddaa58a4d47e165652545bf4b78
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3681640
Reviewed-by: Christoph Schwering <schwering@google.com>
Commit-Queue: Florian Leimgruber <fleimgruber@google.com>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1016790}
  • Loading branch information
florianleimgruber authored and Chromium LUCI CQ committed Jun 22, 2022
1 parent 85f66d2 commit 39be17d
Show file tree
Hide file tree
Showing 3 changed files with 1,553 additions and 1,718 deletions.
Loading

0 comments on commit 39be17d

Please sign in to comment.