TableUpdate is a structure that carries the info to update the table. Even though the update instruction is essentially different between v2 and v4, it's still convenient to make TableUpdateV2 and TableUpdateV4 extend from the same base class. Besides, since RTTI is not allowed, we use a tag to implement a lightweight downcasting.
Created attachment 8768649 [details] [diff] [review] 0001-Bug-1285103-Refactor-TableUpdate-to-support-v2-and-v.patch
Comment on attachment 8768649 [details] [diff] [review] 0001-Bug-1285103-Refactor-TableUpdate-to-support-v2-and-v.patch Hi Francois, The main purpose of this patch is to make TableUpdate as abstract as possible so that it can be passed around functions for updating. To be honest, it's a little over-designed since there's few common behavior for TableUpdate v2 and v4. However, this tiny change makes the least intermediary code change. (For example, TableUpdate will be stored in somewhere and be passed to many other objects. If we don't have this abstract TableUpdate class, we might have to duplicate every single function for propagating TableUpdate.) The actual TableUpdateV4 specific interface is not fully designed yet. But with this patch, I can start working on "generating" TableUpdateV4 and Dimi can start working on "applying TableUpdateV4". Both of us can gradually check in code based on this patch with no dependencies. I am very glad to discuss this bug with you on irc if I haven't made everything clear enough. Thanks!
Comment on attachment 8768649 [details] [diff] [review] 0001-Bug-1285103-Refactor-TableUpdate-to-support-v2-and-v.patch Review of attachment 8768649 [details] [diff] [review]: ----------------------------------------------------------------- Should this be moved into and reviewed alongside another bug?
The component is self-explained. Not necessary to identify SafeBrowsing in the title.
Summary: [SafeBrowsing] Refactor TableUpdate to support V2 and V4 → Refactor TableUpdate to support V2 and V4
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1284204
You need to log in before you can comment on or make changes to this bug.