Skip to content

Merge main branch changes into dev. #270

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

Merged
merged 279 commits into from
Feb 8, 2021
Merged

Conversation

jonsimantov
Copy link
Contributor

No description provided.

wilhuff and others added 30 commits July 21, 2020 19:16
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
Wrapping macro bodies in do { } while(false) makes them into a statement that
legitimately should be followed by a semicolon.

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

PiperOrigin-RevId: 320449430
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
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
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
PiperOrigin-RevId: 320688041
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
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
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
PiperOrigin-RevId: 321183165
The generated headers are currently missing a newline prior to @ifdef cpp_examples which is causing doc generation to fail.

PiperOrigin-RevId: 321272103
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
To manually trigger a workflow in a branch, a workflow with the same name needs to exist in master. This adds such a workflow.
Update and rename firebase-cpp-sdk-issue.md to issue.md
Add a new "General Question" template
Also make sure new feature request issues are marked with "new" label
Based on our offline discussion about reducing the number of overloads.

PiperOrigin-RevId: 322722526
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
This includes a Throwable wrapper for jthrowable, Env implementatinos of
JNIEnv::Throw, plus tools for writing exception resilient code.
PiperOrigin-RevId: 322836751
@google-cla
Copy link

google-cla bot commented Feb 8, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no and removed cla: yes labels Feb 8, 2021
@google-cla
Copy link

google-cla bot commented Feb 8, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

a-maurice
a-maurice previously approved these changes Feb 8, 2021
@google-cla
Copy link

google-cla bot commented Feb 8, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no and removed cla: yes labels Feb 8, 2021
DellaBitta
DellaBitta previously approved these changes Feb 8, 2021
Copy link
Contributor

@DellaBitta DellaBitta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with some nits, which may not be picky enough to warrant changes here.

@jonsimantov jonsimantov dismissed stale reviews from DellaBitta and a-maurice via 5e02409 February 8, 2021 20:52
@google-cla
Copy link

google-cla bot commented Feb 8, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no and removed cla: yes labels Feb 8, 2021
@jonsimantov jonsimantov merged commit 224a55e into dev Feb 8, 2021
@jonsimantov jonsimantov deleted the feature/js-merge-main-to-dev-jan branch February 24, 2021 19:19
@firebase firebase locked and limited conversation to collaborators Mar 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants