New issue
Advanced search Search tips

Issue 601936 link

Starred by 3 users
Status: Fixed
Owner:
fdoray@chromium.org
Closed: Jun 2016
Cc:
grt@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

WorkItem should have "best effort" and "no rollback" flags instead of a single "ignore failure" flag.

Project Member Reported by fdoray@chromium.org, Apr 8 2016 
Currently, when the "ignore failure" flag of a WorkItem, its Do() method should always return true and Rollback() should be a no-op. Sometimes, we want only one of these 2 behaviors.

We should have 2 separate flags:
- "best effort": A WorkItemList should continue its execution if Do() returns false and Do() can perform partial work.
- "no rollback" Rollback() is a no-op.

Note also that not all WorkItems handle the "ignore failure" flag correctly.

Comment 1 by grt@chromium.org, Apr 8 2016

Components: -Internals>Install Internals>Installer
Thanks for taking this on, François.
Project Member

Comment 3 by bugdroid1@chromium.org, May 13 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e63a3064da66ff91ca8cccbb0c279185b0b93321

commit e63a3064da66ff91ca8cccbb0c279185b0b93321
Author: fdoray <fdoray@chromium.org>
Date: Fri May 13 21:58:58 2016

Add best-effort/allow rollback flags on WorkItem.

When the best-effort flag is set to true, Do() always return true
(i.e. it will never cause failure of its parent WorkItemList).

When the allow rollback flag is set to false, Rollback() is a no-op.

BUG= 601936 

Review-Url: https://codereview.chromium.org/1882923003
Cr-Commit-Position: refs/heads/master@{#393652}

[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/chrome_installer.gypi
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/setup/install.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/setup/install_worker.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/setup/install_worker.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/setup/install_worker_unittest.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/setup/setup_main.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/setup/uninstall.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/setup/update_active_setup_version_work_item.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/setup/update_active_setup_version_work_item.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/BUILD.gn
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/callback_work_item.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/callback_work_item.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/conditional_work_item_list.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/conditional_work_item_list.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/conditional_work_item_list_unittest.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/copy_tree_work_item.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/copy_tree_work_item.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/create_dir_work_item.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/create_dir_work_item.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/create_reg_key_work_item.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/create_reg_key_work_item.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/delete_reg_key_work_item.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/delete_reg_key_work_item.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/delete_reg_value_work_item.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/delete_reg_value_work_item.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/delete_tree_work_item.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/delete_tree_work_item.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/delete_tree_work_item_unittest.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/install_util.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/install_util.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/installer_state.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/move_tree_work_item.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/move_tree_work_item.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/self_reg_work_item.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/self_reg_work_item.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/set_reg_value_work_item.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/set_reg_value_work_item.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/work_item.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/work_item.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/work_item_list.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/work_item_list.h
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/work_item_list_unittest.cc
[modify] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/work_item_mocks.h
[add] https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321/chrome/installer/util/work_item_unittest.cc
Project Member

Comment 4 by bugdroid1@chromium.org, May 14 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3fa7e2581d1da516a99020c1a4a8ef6d414313ad

commit 3fa7e2581d1da516a99020c1a4a8ef6d414313ad
Author: hans <hans@chromium.org>
Date: Sat May 14 19:57:30 2016

Revert of Add best-effort/allow rollback flags on WorkItem. (patchset #11 id:200001 of https://codereview.chromium.org/1882923003/ )

Reason for revert:
This broke the build:

..\..\chrome\installer\setup\app_launcher_installer.cc(52,9):  error: no member
named 'set_ignore_failure' in 'WorkItem'
      ->set_ignore_failure(true);
        ^

Original issue's description:
> Add best-effort/allow rollback flags on WorkItem.
>
> When the best-effort flag is set to true, Do() always return true
> (i.e. it will never cause failure of its parent WorkItemList).
>
> When the allow rollback flag is set to false, Rollback() is a no-op.
>
> BUG= 601936 
>
> Committed: https://crrev.com/e63a3064da66ff91ca8cccbb0c279185b0b93321
> Cr-Commit-Position: refs/heads/master@{#393652}

TBR=grt@chromium.org,stgao@chromium.org,fdoray@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 601936 

Review-Url: https://codereview.chromium.org/1976443005
Cr-Commit-Position: refs/heads/master@{#393736}

[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/chrome_installer.gypi
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/setup/install.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/setup/install_worker.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/setup/install_worker.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/setup/install_worker_unittest.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/setup/setup_main.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/setup/uninstall.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/setup/update_active_setup_version_work_item.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/setup/update_active_setup_version_work_item.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/BUILD.gn
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/callback_work_item.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/callback_work_item.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/conditional_work_item_list.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/conditional_work_item_list.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/conditional_work_item_list_unittest.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/copy_tree_work_item.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/copy_tree_work_item.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/create_dir_work_item.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/create_dir_work_item.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/create_reg_key_work_item.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/create_reg_key_work_item.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/delete_reg_key_work_item.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/delete_reg_key_work_item.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/delete_reg_value_work_item.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/delete_reg_value_work_item.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/delete_tree_work_item.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/delete_tree_work_item.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/delete_tree_work_item_unittest.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/install_util.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/install_util.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/installer_state.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/move_tree_work_item.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/move_tree_work_item.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/self_reg_work_item.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/self_reg_work_item.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/set_reg_value_work_item.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/set_reg_value_work_item.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/work_item.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/work_item.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/work_item_list.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/work_item_list.h
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/work_item_list_unittest.cc
[modify] https://crrev.com/3fa7e2581d1da516a99020c1a4a8ef6d414313ad/chrome/installer/util/work_item_mocks.h
[delete] https://crrev.com/001bec1f7849bfd71a9ec3fb9e90c6ed77ab171e/chrome/installer/util/work_item_unittest.cc
Project Member

Comment 5 by bugdroid1@chromium.org, May 18 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a5f09ba5c1e42487724bf62b9f25186b3e76c69d

commit a5f09ba5c1e42487724bf62b9f25186b3e76c69d
Author: fdoray <fdoray@chromium.org>
Date: Wed May 18 13:08:35 2016

Add best-effort/allow rollback flags on WorkItem.

When the best-effort flag is set to true, Do() always return true
(i.e. it will never cause failure of its parent WorkItemList).

When the enable rollback flag is set to false, Rollback() is a
no-op.

This CL is identical to the reverted CL
https://codereview.chromium.org/1882923003/ modulo a fix not to
break official builds.

BUG= 601936 

Review-Url: https://codereview.chromium.org/1982653002
Cr-Commit-Position: refs/heads/master@{#394396}

[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/chrome_installer.gypi
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/setup/app_launcher_installer.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/setup/install.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/setup/install_worker.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/setup/install_worker.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/setup/install_worker_unittest.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/setup/setup_main.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/setup/uninstall.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/setup/update_active_setup_version_work_item.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/setup/update_active_setup_version_work_item.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/BUILD.gn
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/callback_work_item.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/callback_work_item.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/conditional_work_item_list.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/conditional_work_item_list.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/conditional_work_item_list_unittest.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/copy_tree_work_item.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/copy_tree_work_item.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/create_dir_work_item.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/create_dir_work_item.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/create_reg_key_work_item.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/create_reg_key_work_item.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/delete_reg_key_work_item.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/delete_reg_key_work_item.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/delete_reg_value_work_item.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/delete_reg_value_work_item.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/delete_tree_work_item.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/delete_tree_work_item.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/delete_tree_work_item_unittest.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/install_util.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/install_util.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/installer_state.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/move_tree_work_item.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/move_tree_work_item.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/self_reg_work_item.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/self_reg_work_item.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/set_reg_value_work_item.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/set_reg_value_work_item.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/work_item.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/work_item.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/work_item_list.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/work_item_list.h
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/work_item_list_unittest.cc
[modify] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/work_item_mocks.h
[add] https://crrev.com/a5f09ba5c1e42487724bf62b9f25186b3e76c69d/chrome/installer/util/work_item_unittest.cc

Comment 6 by fdoray@chromium.org, Jun 3 2016

Status: Fixed (was: Started)
►

Sign in to add a comment