Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(482)

Side by Side Diff: android_webview/java/src/org/chromium/android_webview/crash/MinidumpUploaderImpl.java

Issue 2682913006: [Android WebView] Always schedule new upload job after copying minidump. (Closed)
Patch Set: Added inline comments explaining the new (non-)cancellation behaviour. Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright 2016 The Chromium Authors. All rights reserved. 1 // Copyright 2016 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 package org.chromium.android_webview.crash; 5 package org.chromium.android_webview.crash;
6 6
7 import android.content.Context; 7 import android.content.Context;
8 import android.net.ConnectivityManager; 8 import android.net.ConnectivityManager;
9 import android.net.NetworkInfo; 9 import android.net.NetworkInfo;
10 import android.webkit.ValueCallback; 10 import android.webkit.ValueCallback;
(...skipping 143 matching lines...) Expand 10 before | Expand all | Expand 10 after
154 public void run() { 154 public void run() {
155 File[] minidumps = mFileManager.getAllMinidumpFiles(MAX_UPLOAD_TRIES _ALLOWED); 155 File[] minidumps = mFileManager.getAllMinidumpFiles(MAX_UPLOAD_TRIES _ALLOWED);
156 for (File minidump : minidumps) { 156 for (File minidump : minidumps) {
157 // Store the name of the current minidump so that we can mark it as a failure from 157 // Store the name of the current minidump so that we can mark it as a failure from
158 // the main thread if JobScheduler tells us to stop. 158 // the main thread if JobScheduler tells us to stop.
159 MinidumpUploadCallable uploadCallable = createMinidumpUploadCall able( 159 MinidumpUploadCallable uploadCallable = createMinidumpUploadCall able(
160 minidump, mFileManager.getCrashUploadLogFile()); 160 minidump, mFileManager.getCrashUploadLogFile());
161 int uploadResult = uploadCallable.call(); 161 int uploadResult = uploadCallable.call();
162 162
163 // Job was canceled -> early out: any clean-up will be performed in cancelUploads(). 163 // Job was canceled -> early out: any clean-up will be performed in cancelUploads().
164 // Note that we check whether we are canceled AFTER trying to up load a minidump -
165 // this is to allow the uploading of at least one minidump per j ob (to deal with
166 // cases where we reschedule jobs over and over again and would never upload any
167 // minidumps because old jobs are canceled when scheduling new j obs).
164 if (getCancelUpload()) return; 168 if (getCancelUpload()) return;
165 169
166 if (uploadResult == MinidumpUploadCallable.UPLOAD_FAILURE) { 170 if (uploadResult == MinidumpUploadCallable.UPLOAD_FAILURE) {
167 String newName = CrashFileManager.tryIncrementAttemptNumber( minidump); 171 String newName = CrashFileManager.tryIncrementAttemptNumber( minidump);
168 if (newName == null) { 172 if (newName == null) {
169 Log.w(TAG, "Failed to increment attempt number of " + mi nidump); 173 Log.w(TAG, "Failed to increment attempt number of " + mi nidump);
170 } 174 }
171 } 175 }
172 } 176 }
173 177
(...skipping 21 matching lines...) Expand all
195 mWorkerThread = new Thread(new UploadRunnable(uploadsFinishedCallback), "mWorkerThread"); 199 mWorkerThread = new Thread(new UploadRunnable(uploadsFinishedCallback), "mWorkerThread");
196 setCancelUpload(false); 200 setCancelUpload(false);
197 201
198 createPlatformServiceBridge().queryMetricsSetting(new ValueCallback<Bool ean>() { 202 createPlatformServiceBridge().queryMetricsSetting(new ValueCallback<Bool ean>() {
199 public void onReceiveValue(Boolean enabled) { 203 public void onReceiveValue(Boolean enabled) {
200 // This callback should be received on the main thread (e.g. mWo rkerThread 204 // This callback should be received on the main thread (e.g. mWo rkerThread
201 // is not thread-safe). 205 // is not thread-safe).
202 ThreadUtils.assertOnUiThread(); 206 ThreadUtils.assertOnUiThread();
203 207
204 mPermittedByUser = enabled; 208 mPermittedByUser = enabled;
205 // Our job might have been cancelled by now - make sure we honou r this. 209 // Note that our job might have been cancelled by this time - ho wever, we do start
206 if (!getCancelUpload()) { 210 // our worker thread anyway to try to make some progress towards uploading
207 mWorkerThread.start(); 211 // minidumps.
208 } 212 // This is to ensure that in the case where an app is crashing o ver and over again
213 // - and we are rescheduling jobs over and over again - we are s till uploading at
214 // least one minidump per job, as long as that job starts before it is canceled by
215 // the next job.
216 // For cases where the job is cancelled because the network conn ection is lost, or
217 // because we switch over to a metered connection, we won't try to upload any
218 // minidumps anyway since we check the network connection just b efore the upload of
219 // each minidump.
220 mWorkerThread.start();
209 } 221 }
210 }); 222 });
211 } 223 }
212 224
225 @VisibleForTesting
226 public void joinWorkerThreadForTesting() throws InterruptedException {
227 mWorkerThread.join();
228 }
229
213 /** 230 /**
214 * @return whether to reschedule the uploads. 231 * @return whether to reschedule the uploads.
215 */ 232 */
216 @Override 233 @Override
217 public boolean cancelUploads() { 234 public boolean cancelUploads() {
218 setCancelUpload(true); 235 setCancelUpload(true);
219 236
220 // Reschedule if there are still minidumps to upload. 237 // Reschedule if there are still minidumps to upload.
221 return mFileManager.getAllMinidumpFiles(MAX_UPLOAD_TRIES_ALLOWED).length > 0; 238 return mFileManager.getAllMinidumpFiles(MAX_UPLOAD_TRIES_ALLOWED).length > 0;
222 } 239 }
223 } 240 }
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698