[Offline Pages] Moves Coordinator to using MarkAttemptStarted/MarkAttemptCompleted API.

components/offline_pages/background/save_page_request.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/offline_pages/background/save_page_request.h"
 
 namespace offline_pages {
 
 SavePageRequest::SavePageRequest(int64_t request_id,
                                  const GURL& url,
@@ skipping 60 matching lines @@
   // check here to ensure we only start tasks in status pending, and bail out in
   // other cases.
   last_attempt_time_ = start_time;
   ++attempt_count_;
 }
 
 void SavePageRequest::MarkAttemptCompleted() {
   last_attempt_time_ = base::Time();
 }
 
+void SavePageRequest::MarkAttemptAborted() {
+  DCHECK_GT(attempt_count_, 0);
+  last_attempt_time_ = base::Time();
+  // TODO(dougarnett): Would be safer if we had two persisted counters
+  // (attempts_started and attempts_completed) rather just one with decrement.
+  --attempt_count_;

[fgorski, 2016/08/04 06:01:34]

based on discussion we had today with pete, dimich and justin, we should
probably be incrementing attempt count upon unsuccessful completion that is
retriable, rather than before making an attempt. That prevents us from
decrementing on the abort.

[dougarnett, 2016/08/04 16:24:38]

So change MarkAttemptStarted and MarkAttemptCompleted then - in this CL?

Btw, I'm concerned that we have some way to stop processing requests that never
complete - that is, after they start, we always get swapped out or chrome
crashes. Did you guys discuss this type of situation? I had raised it with Pete
on Tuesday as a flaw with how he was currently doing the increment after attempt
completed.

[dougarnett, 2016/08/04 16:45:28]

Pondering a bit more, I think we should keep this as it is for this iteration. I
think it is better approach prior to add state variable or doing more work to
uses last attempt time to imply the in-progress state. I could add more text to
the above TODO but doesn't really seem necessary as another iteration planned in
near term.

+}
+
 }  // namespace offline_pages