Add best-effort/allow rollback flags on WorkItem.

chrome/installer/util/work_item_list.cc

 // Copyright (c) 2011 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 "chrome/installer/util/work_item_list.h"
 
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "chrome/installer/util/callback_work_item.h"
 #include "chrome/installer/util/copy_tree_work_item.h"
@@ skipping 14 matching lines @@
   for (WorkItemIterator itr = executed_list_.begin();
        itr != executed_list_.end(); ++itr) {
     delete (*itr);
   }
 }
 
 WorkItemList::WorkItemList()
     : status_(ADD_ITEM) {
 }
 
-bool WorkItemList::Do() {
-  if (status_ != ADD_ITEM)
-    return false;
+bool WorkItemList::DoImpl() {
+  DCHECK_EQ(ADD_ITEM, status_);
 
   bool result = true;
   while (!list_.empty()) {
     WorkItem* work_item = list_.front();
     list_.pop_front();
     executed_list_.push_front(work_item);
     if (!work_item->Do()) {
       LOG(ERROR) << "item execution failed " << work_item->log_message();
       result = false;
       break;
     }
   }
 
   if (result)
     VLOG(1) << "list execution succeeded";
 
   status_ = LIST_EXECUTED;
   return result;
 }
 
-void WorkItemList::Rollback() {
+void WorkItemList::RollbackImpl() {
   if (status_ != LIST_EXECUTED)
     return;
 
   for (WorkItemIterator itr = executed_list_.begin();
        itr != executed_list_.end(); ++itr) {
     (*itr)->Rollback();
   }
 
   status_ = LIST_ROLLED_BACK;
   return;
@@ skipping 160 matching lines @@
   WorkItem* item = WorkItem::CreateSelfRegWorkItem(dll_path, do_register,
                                                    user_level_registration);
   AddWorkItem(item);
   return item;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 NoRollbackWorkItemList::~NoRollbackWorkItemList() {
 }
 
-bool NoRollbackWorkItemList::Do() {
-  if (status_ != ADD_ITEM)
-    return false;
+bool NoRollbackWorkItemList::DoImpl() {

[grt (UTC plus 2), 2016/04/27 17:29:17]

do we need this class? can WIL::DoImpl check its own allow_rollback and apply it
to the items in the list? same q for best_effort? this fn is a hybrid of
best_effort on |this| WorkItem + no rollback, but the best_effortness isn't
documented.

[fdoray, 2016/05/02 20:10:00]

Applying the WIL's allow_rollback/best_effort flag to the WorkItems in the list
makes sense.

NoRollbackWorkItem and WorkItemList+disable_rollback+best_effort are identical
except that WIL+disable_rollback+best_effort doesn't return false if one of its
WorkItems fails. Would it matter if we weren't able to log these errors [1] [2]?

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/installer/s...
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/installer/s...

[grt (UTC plus 2), 2016/05/06 18:39:13]

No. It'd be nice to have something like:

  VLOG(1) << "Beginning execution of work item list " << log_message();

at the top of WorkItemList::Do() so that we can easily see in verbose logs what
is about to happen, and then for each WIL creator to set a log_message on the
list.

+  DCHECK_EQ(ADD_ITEM, status_);
 
   bool result = true;
   while (!list_.empty()) {
     WorkItem* work_item = list_.front();
     list_.pop_front();
     executed_list_.push_front(work_item);
-    work_item->set_ignore_failure(true);
+    work_item->set_allow_rollback(false);
     if (!work_item->Do()) {
       LOG(ERROR) << "NoRollbackWorkItemList: item execution failed "
                  << work_item->log_message();
       result = false;
     }
   }
 
   if (result)
     VLOG(1) << "NoRollbackWorkItemList: list execution succeeded";
 
   status_ = LIST_EXECUTED;
   return result;
 }
 
-void NoRollbackWorkItemList::Rollback() {
+void NoRollbackWorkItemList::RollbackImpl() {
   // Ignore rollback.
 }