Bump Dexter dependency to 5.0.0

[ggalmazor]

Closes #2715

What has been done to verify that this works as intended?

Here's a capture of LeakCanary before updating Dexter:

Here's a capture after updating it:

I've tried to do similar runs with the All Widgets form (fill the same fields, use the geo & photo widgets, etc.)

Why is this the best possible solution? Were any other approaches considered?

This is a straightforward change to update a dependency.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Users should not expect any visible change, other than stability improvements (I guess).

Do we need any specific form for testing your changes? If so, please attach one.

Nope.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No.

Before submitting this PR, please make sure you have:

• run ./gradlew checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
• verified that any code or assets from external sources are properly credited in comments and/or in the about file.
• verified that any new UI elements use theme colors. UI Components Style guidelines

[ggalmazor]

@shobhitagarwal1612, could you say if the procedure I've followed is correct?

[shobhitagarwal1612]

Yes, that's what I would also do.

It appears from the second attached screenshot that leak canary was able to detect another leak. Can you please verify that detected leak isn't caused by the Dexter library? You can do that by clicking on that.

[ggalmazor]

@shobhitagarwal1612, maybe you can help me interpret what I see :)

[ggalmazor]

Could it be that there's a TimerTask keeping a reference to a GeoPointActivity reference?

There's one being created at GeoPointActivity.onResume()

In any case, it doesn't look like Dexter is involved, right?

[shobhitagarwal1612]

Yes, you're right. Thanks for working on this

[yanokwa]

@shobhitagarwal1612 You added the needs testing label, but then merged the PR. Is this something that needs to go through QA?

[shobhitagarwal1612]

The change was small and even the release notes of Dexter library suggested that they haven't modified any other behaviour. I verified the PR myself but thought it would be even better if this is verified by the QAs as well. I've always been amazed by their results. The reason I merged this was because this change was necessary for the unit tests PR I submitted and it was also causing build failures for other's PRs

[yanokwa]

Can you help guide the testing? What exactly should QA verify hasn’t been broken?

[shobhitagarwal1612]

@yanokwa Sure

Things to verify:

• Permission dialogs appear when the permission is not granted
• No crash whenever trying to grant/deny permission in the permission dialog

[kkrawczyk123]

Verified those cases on Androids 6.0, 7.0 and 8.1.

@opendatakit-bot unlabel "needs testing"
@opendatakit-bot label "behavior verified"