Feature/cancel background task when disable #302
Conversation
|
Hi @jaelim-ms, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
TTYL, MSBOT; |
Current coverage is 100% (diff: 100%)@@ develop #302 diff @@
=====================================
Files 60 60
Lines 2704 2711 +7
Methods 0 0
Messages 0 0
Branches 493 494 +1
=====================================
+ Hits 2704 2711 +7
Misses 0 0
Partials 0 0
|
| Crashes.getInstance().onChannelReady(sContext, channel); | ||
| assertNotNull(Crashes.getLastSessionCrashReport()); | ||
|
|
||
| /* Try to get last session crash after Crashes service completed processing. */ | ||
| waitForCrashesHandlerTasksToComplete(); | ||
| // waitForCrashesHandlerTasksToComplete(); |
There was a problem hiding this comment.
Good catch. Forgot to remove the line after testing.
|
|
||
| @Override | ||
| public void run() { | ||
| /* Call callbacks for getInstanceLastSessionCrashReport(ResultCallback) . */ |
There was a problem hiding this comment.
new line should go before comment
| errorLogReport.log.setErrorAttachment(attachment); | ||
| mChannel.enqueue(errorLogReport.log, ERROR_GROUP); | ||
|
|
||
| /* Clean up an error log file and map entry. */ |
There was a problem hiding this comment.
tab instead of whitespaces apparently
There was a problem hiding this comment.
This is very weird. My AS configuration should use spaces instead of tabs. It is not possible!!
|
|
||
| /* Processed crash report for the last session. */ | ||
| if (isInstanceEnabled()) | ||
| mHandler.getLooper().quit(); |
There was a problem hiding this comment.
This can't work if the SDK is re-enabled later, I don't see code that re-creates a new HandlerThread the next time the SDK is enabled. The code that creates the HandlerThread should be moved to the init while enabled method I guess.
There was a problem hiding this comment.
Nope. Basically all file related operations should be just one time job so it doesn't have any scenarios that require this handler by disable/enable.
There was a problem hiding this comment.
If the SDK is disabled while processing pending reports, it must resume when enabled again, that was the case before.
There was a problem hiding this comment.
We are deleting all logs when the service gets disabled. There is nothing we need to take care of.
There was a problem hiding this comment.
But we still try to post runnables to Handler no if we reenable?
There was a problem hiding this comment.
No. It goes through initialization steps but it won't post any runnables to that handler because the service already deleted all pending files when it disabled.
There was a problem hiding this comment.
Ok, but later when we move listFiles and delete calls to worker thread too we will need to do this modification as well.
No description provided.