Skip to content

Uncaught IllegalStateException #121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
berinhardt opened this issue Aug 31, 2020 · 4 comments
Closed

Uncaught IllegalStateException #121

berinhardt opened this issue Aug 31, 2020 · 4 comments

Comments

@berinhardt
Copy link

Please fill in the following fields:

Pre-built SDK from the website
Firebase C++ SDK version: 6.15.1
Firebase plugins in use (Auth, Database, etc.): admob
Additional SDKs you are using (Facebook, AdMob, etc.):

  • 'com.google.firebase:firebase-bom:25.8.0'
  • 'com.google.ads.mediation:facebook:5.11.0.0'
  • 'com.google.android.ads:mediation-test-suite:1.4.0'
  • 'com.google.gms:google-services:4.3.3'

Platform you are using the C++ SDK on (Mac, Windows, or Linux): Windows
Platform you are targeting (iOS, Android, and/or desktop): Android

Please describe the issue here:

There are some cases that interstitial->LoadAd(request); throws an IllegalStateException that is not caught crashing the app instead of failing the Future. I've put together a quick pr to fix it in this repo.

Please answer the following, if applicable:

What's the issue repro rate? it almost never happens, but when it happens it always happen until you reinstall the app

@berinhardt berinhardt added the new New issue. label Aug 31, 2020
@morganchen12 morganchen12 added api: ads and removed new New issue. labels Aug 31, 2020
berinhardt added a commit to berinhardt/firebase-cpp-sdk that referenced this issue Aug 31, 2020
@chkuang-g
Copy link
Contributor

chkuang-g commented Sep 2, 2020

Hi @berinhardt

Thank you for reporting this, and even better, with a pull request!

It looks like a legit fix. However, current we cannot accept pull request on this repo until we fully migrate our build process to Github, which still needs some time.

Are you OK if we fix this from our side and publish the change in the next release?

Shawn

@chkuang-g chkuang-g added needs-info Need information for the developer type: question and removed type: question labels Sep 2, 2020
@berinhardt
Copy link
Author

Of course, please do that!

@vimanyu vimanyu added type: bug and removed needs-info Need information for the developer labels Sep 9, 2020
@vimanyu vimanyu added this to the 6.16.0 milestone Sep 9, 2020
@vimanyu
Copy link
Contributor

vimanyu commented Sep 9, 2020

Thanks for the response and the fix! We will make sure to get it in our next release. As @chkuang-g mentioned, this might take some time as we are in the middle of migrating our build process right now.

a-maurice pushed a commit that referenced this issue Sep 30, 2020
…l ads.

(reported in #121

New error code and message for the default fallback case in switch statements- ConstantsHelper.CALLBACK_ERROR_UNKOWN and ConstantsHelper.CALLBACK_ERROR_MESSAGE_UNKNOWN.
Corresponding new error code added to C++ side (admob/types.h)

Removed a redundant overridden method as it was just calling its base class method directly.

Updated release notes with this bugfix

PiperOrigin-RevId: 333125360
DellaBitta added a commit that referenced this issue Oct 1, 2020
* Use Executor::CreateSerial

PiperOrigin-RevId: 316809187

* Fix future crash that cause unity editor to crash second time enter play mode.

FutureProxyManager still grabs FutureHandle that supposed to be cleaned up and released. Force release them during proxy manager's destructor.

PiperOrigin-RevId: 316809477

* Fix integration test caused by curl http2 default setting

PiperOrigin-RevId: 317136423

* Pin the Firestore pod used by the open source repo to the version from 6.26.0

PiperOrigin-RevId: 317141369

* Project import generated by Copybara.

PiperOrigin-RevId: 317141369

* Add unit tests for the Firebase libraries

PiperOrigin-RevId: 317141369

* Add empty workflow to manually trigger.

* Add a user callback executor for android.

Also fixes a flaky test because assertion is some times done before expected remote event arrives.

PiperOrigin-RevId: 317154095

* Migrating deprecated aliases for absl::StrSplit, and the corresponding predicates.

See b/158478280 and go/absl-cleanup-lsc for more details.

This change was produced using rename_function with the spec:
rename {
  rename_spec {
    new_header: "third_party/absl/strings/str_split.h"
    old_name: "strings::Split"
    new_name: "absl::StrSplit"
  }
}

Additionally, a global find/replace was done on:
strings::AllowEmpty -> absl::AllowEmpty
strings::SkipEmpty -> absl::SkipEmpty
strings::SkipWhitespace -> absl::SkipWhitespace

Tested:
    TAP sample presubmit queue
    http://test/OCL:317011231:BASE:317041434:1592460837225:fc71f604
PiperOrigin-RevId: 317399788

* Incremented version numbers to 6.15.1

PiperOrigin-RevId: 317402209

* Automated g4 rollback of changelist 317154095.

*** Reason for rollback ***

Not ready for release yet

*** Original change description ***

Add a user callback executor for android.

Also fixes a flaky test because assertion is some times done before expected remote event arrives.

***

PiperOrigin-RevId: 317736475

* Automated g4 rollback of changelist 314233757.

*** Reason for rollback ***

Not ready for release yet

*** Original change description ***

[C++] Provide a default executor with settings on iOS

***

PiperOrigin-RevId: 317791137

* Fix a missing call to `ClearListeners` in ~FirestoreInternal.

Also fix a memory leak during listener unregistration

PiperOrigin-RevId: 318538053

* Enable full stack traces for crashes in unit tests

Fix leaks in firestore_test that the HeapChecker found.

Test with `blaze test -c dbg` to get line numbers in stack frames.

PiperOrigin-RevId: 318644593

* Hooked up Persistence into Realtime Database.

PiperOrigin-RevId: 318885157

* Automated g4 rollback of changelist 317736475.

*** Reason for rollback ***

re-roll cl/317154095

SKIP_FIRESTORE_KOKORO_BUILD_TEST=true

*** Original change description ***

Automated g4 rollback of changelist 317154095.

*** Reason for rollback ***

Not ready for release yet

*** Original change description ***

Add a user callback executor for android.

Also fixes a flaky test because assertion is some times done before expected remote event arrives.

***

***

PiperOrigin-RevId: 318891491

* Removed errant include.

PiperOrigin-RevId: 318899934

* Automated g4 rollback of changelist 317791137.

*** Reason for rollback ***

Re-roll cl/314233757

SKIP_FIRESTORE_KOKORO_BUILD_TEST=true

*** Original change description ***

Automated g4 rollback of changelist 314233757.

*** Reason for rollback ***

Not ready for release yet

*** Original change description ***

[C++] Provide a default executor with settings on iOS

***

***

PiperOrigin-RevId: 318938740

* Fix issues with event accumulation in Firestore C++ integration tests

There are several related flaws fixed here:

  * The mutex in TestEventListener was not applied consistently, leading to
    data races that showed up as strangely failing tests on forge.
  * The result of FirestoreIntegrationTest::Await was implicitly trusted to
    produce some results, but this isn't the case when it times out. In this
    error case the caller would read uninitialized memory almost immediately
    after, leading to crashes before the log message about the timeout had
    necessarily been writen.
  * The result of FirestoreIntegrationTest::Await was implicitly trusted not to
    produce more results than requested, but this didn't always happen either.
    This would cause failures where a test would request `n` events, and use
    the last `n` that arrived, skipping extra events. Now the EventAccumulator
    ensures that it consumes events in sequence.

PiperOrigin-RevId: 318943435

* Fix test warm-up

Certain tests include a warm-up step to ensure that the backend was actually
available, but only waited for any event. Unfortunately this doesn't work,
because when the server is unavailable, the SDK will serve a from-cache event
indicating the document doesn't exist.

Change all these to wait for a from-cache: false event, guaranteeing that the
server is actually available and confirming the document doesn't exist.

PiperOrigin-RevId: 318948824

* Implement type map for public to internal types

This simplifies the specification of promises, cleanup functions, and
converters, since they now only need to specify the public type.

PiperOrigin-RevId: 318960642

* Split PromiseFactory out of WrapperFuture

This makes promise creation more closely match iOS and paves the way for
removing WrapperFuture altogether.

Use PromiseFactory in all internal type implementations instead of extending
WrapperFuture and then dDelete WrapperFuture.

PiperOrigin-RevId: 319025783

* Remove LastResult implementations

PiperOrigin-RevId: 319034882

* Fix enabling debug logging from Unity in iOS.

Previously, setting FirebaseFirestore.LogLevel to LogLevel.Debug would get bumped back to LogLevel.Info as an unexpected side effect of creating of a new firebase::App object. The apparent effect to users was that enabling debug logging had no effect. This made it challenging to debug issues with customers because they were unable to collect valuable debug logs when using iOS as their platform.

PiperOrigin-RevId: 319056991

* Cleanup listener callbacks on `DocumentReference` and `Query`.

This CL is the last in a series described in cl/312713181, cl/317397413, and resolves b/156024690.

In this CL the new pattern for managing listener callbacks implemented in `FirebaseFirestore.ListenForSnapshotsInSync()` is being applied to all other places where we expose listeners (i.e. `Query` and `DocumentReference`).

PiperOrigin-RevId: 319089024

* Migrate const std::string& parameters to absl::string_view.

THIS CHANGE IS BELIEVED SAFE

Templated asynchronous code can change the lifetime of string data as a result
of this change; however, the most common uses of these (lambdas and callbacks)
are excluded from this change.
Further, your TAP tests pass.

go/string-ref-to-string-view-lsc

Tested:
    TAP --sample ran all affected tests and none failed
    http://test/OCL:319219698:BASE:319213711:1593616691507:a583f044
PiperOrigin-RevId: 319286243

* Avoid double deletion of internal objects during cleanup.

This has been observed during cleanup when a DocumentReferenceInternal is
destroyed, its Future API can end up deleting orphaned Future APIs that contain
Futures holding the containing `DocumentReference`.

PiperOrigin-RevId: 320260380

* Avoid empty statement warnings in Firebase assertions

Wrapping macro bodies in do { } while(false) makes them into a statement that
legitimately should be followed by a semicolon.

PiperOrigin-RevId: 320272779

* Miscellaneous test-only fixes

  * Avoid dereferencing awaited pointers after Await has failed, preventing
    crashes after test timeouts.
  * Await `CollectionReference::Add`, preventing nondeterminism in tests.

PiperOrigin-RevId: 320449430

* Delete C++ Firestore objects when C# has no more references to them.

This was achieved by using the CppInstanceManager, which provides this functionality elsewhere in the Unity SDK. When running UIHandlerAutomated, this eliminates all 11 leaks from Firestore::GetFirestore().

PiperOrigin-RevId: 320609048

* Remove the unconditional calls to Firebase::Terminate() in test cleanup.

These calls were present as a workaround for bugs in the Firestore destructor; however, since those bugs have been fixed the calls to Terminate() are now superfluous and, worse, can hide bugs. As a result, they are being removed.

PiperOrigin-RevId: 320627333

* Change std:unique_ptr to UniquePtr

This is causing Rapid build fail with error message like "error: no template named 'make_unique' in namespace 'std'; did you mean 'MakeUnique'?"
Since we have our own implementation, we should use it consistently.

PiperOrigin-RevId: 320681249

* Update Android deps to latest

PiperOrigin-RevId: 320688041

* Make test failures in numeric transforms tests easier to read

Previously, if the value was of the wrong type, you'd get a message like:

    Value of: snap.Get("sum").is_integer()
      Actual: false
    Expected: true

Which would give no indication of what the actual type or value was. Now tests
will fail like this:

   Expected equality of these values:
     snap.Get("sum")
       Which is: 1337 (Type::kDouble)
     FieldValue::Integer(value)
       Which is: 1337 (Type::kInteger)

As an added bonus this also simplifies the calling code because now we can just
assert that a value in a snapshot is equal to some expected value and
GoogleTest will do the heavy lifting of printing the differences.

One unsolved problem with this approach is how to handle equality within
epsilon for floating point values. This turns out to be non-trivial without
writing custom matchers, which is beyond the scope of what I wanted to tackle
here. Instead of solving this I've changed the tests to use values that have an
exact representation as doubles. This is easier to do for integral values than
for fractional ones so the tests now use integer-valued doubles to achieve the
same effect of cumulative addition as before.

PiperOrigin-RevId: 321022746

* Add JNI Object wrapper and type conversion traits

This is the first in a series of commits that aims to convert our JNI usage to
a more modern approach while still remaining STLPort compatible.

PiperOrigin-RevId: 321059370

* Add JNI ownership types

These generate local and global reference subtypes of a given JNI wrapper that
make it possible to automatically emit DeleteLocalRef and DeleteGlobalRef
calls in the course of regular usage.

PiperOrigin-RevId: 321175591

* Add FIREventScreenView and params

PiperOrigin-RevId: 321183165

* Add another line in the generate constant header script

The generated headers are currently missing a newline prior to @ifdef cpp_examples which is causing doc generation to fail.

PiperOrigin-RevId: 321272103

* Add initial Env with minimal support for Strings

PiperOrigin-RevId: 321475293

* Rework ToJni conversions to enable pass-through of JNI types

Previously, all types needed to be in the JniTypeMap, but this was never
intended to be the end state because it would require an entry for all subtypes
of Object which couldn't scale.

The new implementation based on ranked choice overload selection also resolves
ambiguities. For example: unsigned char could be a uint8_t (which converts to
jbyte) or the underlying type for jboolean. Absent the ranking, an argument of
type unsigned char would resolve to multiple overloads and would be ambiguous.

PiperOrigin-RevId: 321835773

* Add support for calling methods and getting fields.

PiperOrigin-RevId: 322398533

* Create dummy workflow for integration tests (#106)

To manually trigger a workflow in a branch, a workflow with the same name needs to exist in master. This adds such a workflow.

* Create feature-request.md

* Update and rename firebase-cpp-sdk-issue.md to issue.md

Update issue template

* Update issue.md

* Update issue templates

Add a new "General Question" template
Also make sure new feature request issues are marked with "new" label

* Update feature-request.md

* Revise string methods to be wrapper-only

Based on our offline discussion about reducing the number of overloads.

PiperOrigin-RevId: 322722526

* Fix deletion of the pointers returned from CreateFirestore().

The pointers returned from CreateFirestore() should be explicitly deleted, as well as the underlying app pointer, as is done in the ~FirestoreIntegrationTest() destructor for Firestore pointers returned from firestore() and CachedFirestore(). Several test cases in firestore_test.cc were failing to perform this deletion on the pointers returned from CreateFirestore(), resulting in memory leaks.

To facilitate consistency in this deletion logic, I exposed the Release() method so that they could be called by tests that use CreateFirestore().

I then updated the code in firestore_test.cc to call Release() on every pointer returned from CreateFirestore(). This fixes the memory leaks of the objects returned from CreateFirestore(), at least in the case that the tests pass. A future changelist will migrate usages of CreateFirestore() to CachedFirestore() to ensure that the Firestore pointers are deleted whether or not the test passes.

PiperOrigin-RevId: 322814077

* Add exception support to Env

This includes a Throwable wrapper for jthrowable, Env implementatinos of
JNIEnv::Throw, plus tools for writing exception resilient code.
PiperOrigin-RevId: 322836751

* Add logic to ignore a specific file in Kokoro build logic

The chromium_certificate_verifier uses a cc file as an include file, which breaks the CMake logic with add_library.  By excluding that file from the add_library, but keeping it in the zip file, it will still be available for the build to work, but will no longer try to treat it as a source file, which caused the problem.

Also, made the local_test script use the regular_grpc flag, since it is now needed

PiperOrigin-RevId: 323644686

* Add support for cacheable global declarations of JNI types

This includes:

  * constructors
  * instance methods
  * static fields
  * static methods

PiperOrigin-RevId: 323697946

* Add support for embedded files and other odds and ends

PiperOrigin-RevId: 323710861

* Convert CollectionReference to new JNI framework

PiperOrigin-RevId: 323822884

* Add C++ proxies for Java standard library types

This includes proxies for commonly used elements of java.util:

  * ArrayList
  * Collection
  * HashMap
  * Iterator
  * List
  * Map
  * Set

Only commonly used methods are included. Additional methods can be added as
needed.

PiperOrigin-RevId: 324895315

* Fix assertion incorrectly checking a moved-from rather than the moved-to object.

This should fix #85.

Note that this _was_ covered by tests (checked manually). Presumably, because a moved-from `std::function` is in a valid but unspecified state, it might or might not be left empty, depending on the implementation.

PiperOrigin-RevId: 324919105

* Improve `Variant` error message when used with an unsupported type.

In the previous implementation, `set_value_t` was declared to be a template function but never defined (instead, a few specializations were defined). This could cause a linker error when a `Variant` is used with an unsupported type (using `firestore::FieldValue` as the example):
```
ld.lld: error: blaze-out/k8-fastbuild/bin/_solib_k8/libfirebase_Sfirestore_Sclient_Scpp_Slibfield_Uvalue_Utest.ifso: undefined reference to void firebase::Variant::set_value_t<firebase::firestore::FieldValue>(firebase::firestore::FieldValue) [--no-allow-shlib-undefined]
```
(see https://sponge.corp.google.com/invocation?id=0dbf93d2-8644-4058-9b98-0513ad032e08&searchFor=user%3Avarconst)

Deleting the regular template has the advantage of turning it into a compile-time error that contains a reference to the line in the source code that triggers it:

```
In file included from firebase/firestore/client/cpp/src/tests/field_value_test.cc:17:
./firebase/app/client/cpp/src/include/firebase/variant.h:134:5: error: call to deleted member function 'set_value_t'
    set_value_t(value);
    ^~~~~~~~~~~
firebase/firestore/client/cpp/src/tests/field_value_test.cc:295:10: note: in instantiation of function template specialization 'firebase::Variant::Variant<firebase::firestore::FieldValue>' requested here
  return FieldValue{};
         ^
./firebase/app/client/cpp/src/include/firebase/variant.h:1114:8: note: candidate function [with T = firebase::firestore::FieldValue] has been explicitly deleted
  void set_value_t(T value) = delete;
       ^
1 error generated.
```
(see https://sponge.corp.google.com/invocation?id=8122d244-50d5-4072-98fb-9c5c11dc42ad&searchFor=user%3Avarconst)

This should be a non-breaking change (the code that was compiling previously should be unaffected).

PiperOrigin-RevId: 324927992

* Add support for creating and manipulating Java arrays to the `JNIEnv` wrapper

PiperOrigin-RevId: 325087878

* Improve the tests for ClearPersistence() to use the default Firebase app instead of a custom one.

This fixes a concern raised by @pengzk in cl/309775646 that the Firestore* pointers were leaking. By using the default Firestore object retrieved via firestore() instead of a custom Firestore object returned from CreateFirestore() the Firestore* pointer will be properly cleaned up upon test completion.

PiperOrigin-RevId: 325229850

* Convert DocumentReference and supporting classes to the new JNI framework

This includes:
  * SourceInternal
  * MetadataChangesInternal
  * FieldPathConverter
  * SetOptionsInternal
  * DocumentReferenceInternal

This also includes some changes to other types (like QueryInternal) to shim in
usage of the changed types. In some cases both the new and old ways coincide
within a method leading to some temporary bloat. These will be cleaned up in a
subsequent CL.

PiperOrigin-RevId: 325352325

* Rework MapFieldValue and MapFieldPathValue calls

Also:

  * Centralize Java update methods' first-pair-separate-from-varargs handling
    in a new MakeUpdateFieldPathArgs method
  * Also begin work on new-style error handling in transactions by bridging
    Env::UnhandledExceptionHandler to TransactionInternal::PreserveException.

PiperOrigin-RevId: 325468252

* Automated g4 rollback of changelist 325229850.

*** Reason for rollback ***

firestore_test_android appears to have been broken by this change (b/163526671).

*** Original change description ***

Improve the tests for ClearPersistence() to use the default Firebase app instead of a custom one.

This fixes a concern raised by @pengzk in cl/309775646 that the Firestore* pointers were leaking. By using the default Firestore object retrieved via firestore() instead of a custom Firestore object returned from CreateFirestore() the Firestore* pointer will be properly cleaned up upon test completion.

***

PiperOrigin-RevId: 326267389

* Updated MessageForwardingService to be a JobIntentService for Android O.

As of Android O, background services can no longer spawn other background
services, they must instead create JobIntentServices, which can queue up work
for the OS to perform when it is ready. This change also requires that the
JobIntentService be declared in the manifest file.

PiperOrigin-RevId: 326749322

* Deprecating Send.

When Send is actually removed from the underlying iOS SDK, we should check in cr/322694231 which removes it from C++ entirely.

PiperOrigin-RevId: 326753736

* LSC: Change for loops to use const refs, per performance-for-range-copy
clang-tidy.

This CL optimizes C++11 range-based for loops where the variable is copied in
each iteration but it would suffice to obtain it by const reference.  This is
only applied to loop variables of types that are expensive to copy which means
they are not trivially copyable or have a non-trivial copy constructor or
destructor.

To ensure that it is safe to replace the copy with a const reference the
following heuristic is employed:
  The loop variable is const qualified.
  The loop variable is not const, but only const methods or operators are
  invoked on it, or it is used as const reference or value argument in
  constructors or function calls.

See go/lsc-performance-for-range-copy for details.

Applied ClangTidy fixes to google3 paths matching '[abcdefghijklmnoprs].*'.

The changelist was automatically generated using the apply_fixes.sh script:
  devtools/cymbal/clang_tidy/apply_fixes.sh --checks=-*,performance-for-range-copy --create_changelist --files_to_process=[abcdefghijklmnoprs].*

Track: canary
Findings source: /placer/prod/home/devtools-cxx-clang-tidy/canary/latest/clang_tidy-?????-of-?????
Subcategories: performance-for-range-copy

WARNING: Automatically generated changes should be carefully inspected. For most
ClangTidy warnings multiple alternative fixes are possible and only one of them
is suggested. There's no guarantee that it will be the best one (or even a right
fix). The amount of reviewers' attention to automated changes should be at least
the same as for manually written code.

Tested:
    tap_presubmit: http://test/OCL:323300886:BASE:324354295:1596315075612:bb83204a
    Some tests failed; test failures are believed to be unrelated to this CL
PiperOrigin-RevId: 326827164

* Update the Android dependencies for M77

PiperOrigin-RevId: 327556559

* Convert DocumentChange to the new JNI framework

PiperOrigin-RevId: 327653990

* Convert ServerTimetampBehavior to new JNI framework

PiperOrigin-RevId: 327717920

* Convert SnapshotMetadata to the new JNI framework

PiperOrigin-RevId: 327727645

* Make Unity send Cloud API headers indicating the language version and the Firestore version.

This only works for iOS and Android for now; Android is still a TODO.

Also bump the Firestore version to 1.16.4 (it hasn't been updated in a long time and was still 1.1.0).

From debug logging, the value sent via the header looks like:
```
gl-dotnet/4.0.30319.42000 fire/1.16.4 grpc/1.32.0-dev
```

PiperOrigin-RevId: 327871420

* Convert DocumentSnapshot to the new JNI framework

PiperOrigin-RevId: 327886189

* Improve the error message that is generated when a Future fails.

PiperOrigin-RevId: 328179796

* Convert Blob to the new JNI framework

Also fix redundant blob caching behavior in `FieldValueInternal`, consolidating
the code to populate `cached_blob_` in a single `EnsureCachedBlob` method.

PiperOrigin-RevId: 328849949

* Convert Direction to the new JNI framework

Note that this gets rid of the enum caching mechanism in favor of just making a
JNI call to get the associated field. This reduces complexity, removes static
state, and greatly reduces the amount of code, with a minor performance penalty
to code that's not remotely performance critical.

PiperOrigin-RevId: 328856579

* Convert Settings to the new JNI framework

PiperOrigin-RevId: 329734038

* Convert WriteBatch to the new JNI framework

PiperOrigin-RevId: 329805287

* Convert Timestamp to the new JNI framework

PiperOrigin-RevId: 329814232

* Change forward declaration for SignInResult to struct

SignInResult is declared as a struct in user.h but forward declared here as a class.
This did not cause error in our build but is causing compiler error for user.

Fixed #120

PiperOrigin-RevId: 329958702

* Convert QuerySnapshot to the new JNI framework

PiperOrigin-RevId: 330111277

* Update the Android dependencies for M78

PiperOrigin-RevId: 330559438

* Re-write `run_local_tests.sh` to run the tests in parallel.

When I profiled this change locally, the runtime of the script was reduced from about 30 minutes down to 12 minutes when the blaze commands were run in parallel. This makes sense because a lot of the time is spent just waiting for Kokoro to perform the build.

PiperOrigin-RevId: 330748434

* Plumb through error messages for Query and Document listens in C++ and Unity.

NOTE: This changelist is a breaking change in that it changes the public C++ Firestore API in a backwards-incompatible manner.
PiperOrigin-RevId: 330757239

* Check that calls to Terminate() and ClearPersistence() actually succeed.

I had incorrectly assumed that `Await()` would fail the test if the `Future` it was specified failed. This incorrect assumption hid the fact that `ClearPersistence()` failed on Android, resulting in unexplained downstream failures, resulting in a rollback: cl/326267389.

With this change, the failure of the `Future` objects returned from `ClearPersistence()` and `Terminate()` will be noted, highlighting the source of the test failures.

PiperOrigin-RevId: 330777066

* Add logic to detect the compiler version and the C++ language standard used.

This is largely adapted from the Cloud C++ APIs with the following changes:

* added C++20 to the list of possible standards;
* added "latest" as a possible C++ standard for MSVC;
* added the C++ standard library vendor to the compiler info string, primarily
  to track STLPort.

Here are some sample values I'm getting:
* on my Mac laptop: `AppleClang-11.0.3.11030032-ex-2011-libcpp`;
* google3 Android build: `Clang-9.0.8-ex-2017-libcpp`;
* google3 Windows build: `MSVC-19.22.27905-ex-2017-msvc`;
* google3 Linux build: `Clang-9999.0.0-noex-2017-libcpp` (unfortunately,
  Clang trunk in google3 simply sets the version to 9999:
  https://source.corp.google.com/piper///depot/google3/third_party/llvm/llvm-project/clang/BUILD;l=374-383;rcl=330443088)

Note that as long as the C++ SDKs are distributed in precompiled form, the compiler version reported will be from the compiler we used to compile the binaries, not the compiler used by our users. Unfortunately, there is no clear way to avoid this that wouldn't cause an ODR violation (if e.g. `CompilerId` were placed in a header as an inline function, it would end up having different definitions depending on whether it's included internally or by the user (indirectly)). However, this shouldn't be a big issue as the metrics will still allow tracking total C++ usage and, to an extent, usage by platform.

PiperOrigin-RevId: 331654266

* Automated g4 rollback of changelist 326267389.

*** Reason for rollback ***

Roll forward cl/325229850: Improve the tests for ClearPersistence() to use the default Firebase app instead of a custom one.

I've added a workaround for the broken test, a call to Terminate() in Android, and created b/168628900 to track the underlying bug.

*** Original change description ***

Automated g4 rollback of changelist 325229850.

*** Reason for rollback ***

firestore_test_android appears to have been broken by this change (b/163526671).

*** Original change description ***

Improve the tests for ClearPersistence() to use the default Firebase app instead of a custom one.

This fixes a concern raised by @pengzk in cl/309775646 that the Firestore* pointers were leaking. By using the default Firestore object retrieved via firestore() instead of a custom Firestore object return...

***

PiperOrigin-RevId: 331991549

* Temporarily add a platform token to the Cloud API headers.

Until b/135633112 is resolved and support for Firebase platform logging is
rolled out on the Firestore backend, there is no good way to estimate the
number of users of the Firestore C++ SDK by platform. As a workaround, add
a platform token to the Cloud headers. Proper support for platform tokens was
actually never added to Cloud headers, but the data processing pipeline
simply ignores unknown tokens so there is no harm in doing this addition.

The implementation reuses the notion of a client language in the Core API to
lump together two tokens (the language and the platform). This is deemed
acceptable only due to the temporary nature of this workaround.

Sample of the header values I'm seeing:
- running C++ tests on Linux in google3:
  `gl-cpp/Clang-9999.0.0-noex-2017-libcpp gl-linux/ fire/1.17.1 grpc/1.33.0-dev`
  (notice the `gl-linux/` token);
- in Unity: `gl-dotnet/4.0.30319 gl-macos/ fire/1.17.1 grpc/1.33.0-dev` (notice
  the `gl-mac/` token);

PiperOrigin-RevId: 332094253

* Add play-services-base to Android deps for analytics/admob/remote_config.

Due to a change in the transitive dependencies of analytics, play-services-base is no longer present. This is needed to check for the presence of Google Play Services. Admob and Remote Config both depend on analytics (through 'app'), which is how they previously got that dependency. Other APIs depend directly on play-services-base at the Android level.

PiperOrigin-RevId: 332267491

* Update the Build and Podfiles

PiperOrigin-RevId: 332327282

* Update the version numbers for 6.16.0

PiperOrigin-RevId: 332496615

* Convert GeoPoint to the new JNI framework

PiperOrigin-RevId: 333116446

* Handling `IllegalStateException` when loading or creating interstitial ads.
(reported in #121

New error code and message for the default fallback case in switch statements- ConstantsHelper.CALLBACK_ERROR_UNKOWN and ConstantsHelper.CALLBACK_ERROR_MESSAGE_UNKNOWN.
Corresponding new error code added to C++ side (admob/types.h)

Removed a redundant overridden method as it was just calling its base class method directly.

Updated release notes with this bugfix

PiperOrigin-RevId: 333125360

* Convert Query to the new JNI framework

PiperOrigin-RevId: 333157347

* Copybara Firestore

PiperOrigin-RevId: 333174406

* Added a test for C++ namespace renaming to Blastdoor tests, and added a fix to
rename symbols that are external references implemented in other files.

This fixes a Blastdoor issue where a symbol that needs to be renamed is passed
between multiple internal libraries -- it needs to be renamed regardless of whether its
implementation is present or if it is an external symbol.

PiperOrigin-RevId: 333175764

* Set the language in Cloud headers to `gl-cpp/` in the C++ SDK on Android.

This is possible now that the Copybara CL cr/333174406 is submitted. This CL requires on Android functionality available starting from M80 (Firestore 21.7.0).

PiperOrigin-RevId: 333356092

* Convert FirestoreInternal to the new JNI framework

This converts all method invocations but does not necessarily convert all uses
of older APIs because those are still needed to support classes that have not
yet converted.

PiperOrigin-RevId: 333429091

* Convert ListenerRegistration to the new JNI framework

PiperOrigin-RevId: 333539837

* Fix it so only external C++ symbols matching the list of namespaces to rename
are renamed. C symbols that are external will not be renamed.

Also adds a flag to turn off this new behavior, though it's on by default for
now.

PiperOrigin-RevId: 333613425

* Convert EventListener to the new JNI framework

PiperOrigin-RevId: 334212158

* Clean up exception naming

Rename FirebaseFirestoreExceptionInternal to ExceptionInternal.  Rename methods
in ExceptionInternal to better reflect their function.

PiperOrigin-RevId: 334219916

* Convert ExceptionInternal to the new JNI framework

Also partially convert TransactionInternal since these are so intertwined.

PiperOrigin-RevId: 334241429

* Make snapshot from TransactionGetResult return by value.

This ensure C# DocumentSnapshot will own the C++ DocumentSnapshot when it is read from within a transaction.

PiperOrigin-RevId: 334276809

* Add Ad Impression constants to public header

PiperOrigin-RevId: 334451358

* Finish conversion of TransactionInternal to the new JNI framework

PiperOrigin-RevId: 334459761

* Add boxed primitive types to the JNI library

PiperOrigin-RevId: 334472082

* Add example to ad unit in header file

PiperOrigin-RevId: 334482610

* Add note about value param in ad impression event

PiperOrigin-RevId: 334497833

* Add Equals to the JNI Object proxy.

Also add GetClass to some JNI proxies.

Remove Wrapper::EqualsJavaObject

PiperOrigin-RevId: 334501941

* Fix to messaging's LibraryManifest.xml

Embedded service into application.

Co-authored-by: wuandy <[email protected]>
Co-authored-by: cynthiajiang <[email protected]>
Co-authored-by: amaurice <[email protected]>
Co-authored-by: Googler <[email protected]>
Co-authored-by: Jon Simantov <[email protected]>
Co-authored-by: amablue <[email protected]>
Co-authored-by: mcg <[email protected]>
Co-authored-by: chkuang <[email protected]>
Co-authored-by: anonymous-akorn <[email protected]>
Co-authored-by: chkuang-g <[email protected]>
Co-authored-by: varconst <[email protected]>
@vimanyu
Copy link
Contributor

vimanyu commented Oct 5, 2020

Hi @berinhardt,
Thanks for reporting this and for the proposed fix. These changes went into the latest release. Am closing the ticket for now but please feel free to reopen if you are still noticing problems.

@vimanyu vimanyu closed this as completed Oct 5, 2020
@firebase firebase locked and limited conversation to collaborators Nov 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants