Fix: whereField creates a memory leak

[milaGGL]

fix: #12613
revert the thread safe memoizer introduced in this pr: #11781

@cherylEnkidu :
This PR intends to fix the data race and memory leak issue.

When using memorized field, it leads to data race. However, the solution for fixing data race issue introduces memory leak.

So this PR construct a flatten filters for composite filter during initialization.

The reason for not providing GetFlattenFilter() in base filter class is creating flatten filter in field filter constructor will cause infinity loop.

Since GetFlattenFilter() is widely used in query calls, init flatten filters vector in composite filter during construction won't hurt efficiency a lot comparing to construct flatten filters vector upon request.

[google-oss-bot]

Coverage Report ¹

Affected Products

• FirebaseFirestore-iOS-FirebaseFirestoreInternal.framework

  Overall coverage changed from 88.21% (eca84fd) to 88.19% (9b34ba4) by -0.02%.

  Filename	Base (eca84fd)	Merge (9b34ba4)	Diff
  composite_filter.cc	90.10%	89.25%	-0.85%
  field_filter.cc	95.00%	94.78%	-0.22%
  filter.cc	70.00%	62.50%	-7.50%
  grpc_stream.cc	99.01%	98.02%	-0.99%
  leveldb_key.cc	98.63%	98.43%	-0.20%
  query.cc	98.43%	98.33%	-0.11%
  query_core.cc	95.80%	95.81%	+0.01%
  target.cc	96.23%	96.24%	+0.01%
  task.cc	94.78%	96.52%	+1.74%

Test Logs

• Base (eca84fd): https://github.com/firebase/firebase-ios-sdk/actions/runs/9768771643
• Merge (9b34ba4): https://github.com/firebase/firebase-ios-sdk/actions/runs/9770673602

1. https://storage.googleapis.com/firebase-sdk-metric-reports/nbwANxnpho.html

[cherylEnkidu]

Hi @ehsannas , could you please review this fix?

[ehsannas]

The reason for not providing GetFlattenFilter() in base filter class is creating flatten filter in field filter constructor will cause infinity loop.

why? maybe you shouldn't call GetFlattenedFilters() in the constructor, but should still have GetFlattenedFilters() defined for field filters [which just returns *this].

[cherylEnkidu]

The reason for not providing GetFlattenFilter() in base filter class is creating flatten filter in field filter constructor will cause infinity loop.

why? maybe you shouldn't call GetFlattenedFilters() in the constructor, but should still have GetFlattenedFilters() defined for field filters [which just returns *this].

@ehsannas I put back GetFlattenedFilters() in the Filter class and return a shared pointer to vector.

[ehsannas]

  // Don't deference filter.GetFlattenedFilters() directly. if the filter is a
  // field filter, calling for (const FieldFilter& subFilter :
  // *filter.GetFlattenedFilters()) will deference a temporary object.

This is the result of bad design and devs can't be expected to "do the right thing" by reading the comment.

why can't field filters follow the same pattern as composite filters to avoid this problem?

please reach out offline if you think more discussion is needed. Thanks!