New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 754570 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

InstallableManager mixes management of pending "tasks" with business logic to process those tasks.

Project Member Reported by mcgreevy@chromium.org, Aug 11 2017

Issue description

This is a tracking bug for a series of CLs which will disentangle some of the InstallableManager code.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 16 2017

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

commit 3207643ec34580dd1c1081300adff01c4a5b919b
Author: Michael McGreevy <mcgreevy@chromium.org>
Date: Wed Aug 16 02:15:26 2017

Add TaskQueue helper class for InstallableManager.

This is a first step in separating the management of the task queue from the
business logic involved in processing each task.

Bug:  754570 
Change-Id: Ibfcaaabbe8c6579f763fc280014f4654eb39b4d5
Reviewed-on: https://chromium-review.googlesource.com/611842
Commit-Queue: Michael McGreevy <mcgreevy@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494669}
[modify] https://crrev.com/3207643ec34580dd1c1081300adff01c4a5b919b/chrome/browser/installable/installable_manager.cc
[modify] https://crrev.com/3207643ec34580dd1c1081300adff01c4a5b919b/chrome/browser/installable/installable_manager.h
[modify] https://crrev.com/3207643ec34580dd1c1081300adff01c4a5b919b/chrome/browser/installable/installable_manager_browsertest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 16 2017

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

commit 81c64b020dcb79b3cf9f5ffb6b8d45e835f3679a
Author: Michael McGreevy <mcgreevy@chromium.org>
Date: Wed Aug 16 04:10:51 2017

Extract is_active_ management from StartNextTask.

This moves management of is_active_ closer to call of TaskQueue methods, so that a
followup CL can move is_active_ management into TaskQueue.

Bug:  754570 
Change-Id: Icd47c59f40ac8f517c4620e411f2514adc845659
Reviewed-on: https://chromium-review.googlesource.com/616360
Commit-Queue: Michael McGreevy <mcgreevy@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494692}
[modify] https://crrev.com/81c64b020dcb79b3cf9f5ffb6b8d45e835f3679a/chrome/browser/installable/installable_manager.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 16 2017

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

commit 3d42b3ca4f8ceee331efdb3a2cc59c248c70dd48
Author: Michael McGreevy <mcgreevy@chromium.org>
Date: Wed Aug 16 08:38:07 2017

move is_active_ state into TaskQueue.

This is a transitional refactoring step.

Bug:  754570 
Change-Id: Ic66ee7e934ee0e10118e253726d1733c3cbd58a1
Reviewed-on: https://chromium-review.googlesource.com/616364
Commit-Queue: Michael McGreevy <mcgreevy@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494726}
[modify] https://crrev.com/3d42b3ca4f8ceee331efdb3a2cc59c248c70dd48/chrome/browser/installable/installable_manager.cc
[modify] https://crrev.com/3d42b3ca4f8ceee331efdb3a2cc59c248c70dd48/chrome/browser/installable/installable_manager.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 17 2017

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

commit 8c2079c0d62d6973f2a317986bbc5537c7321253
Author: Michael McGreevy <mcgreevy@chromium.org>
Date: Thu Aug 17 02:24:29 2017

Have TaskQueue::{Next,PauseCurrent} update is_active_.

This is a transitional refactoring step towards having all management of
is_active_ done from within TaskQueue, and then removing this explicit state
althogether.

Bug:  754570 
Change-Id: Ia0dc8ee2179eb97757a22038b0a62b0b3c768e1f
Reviewed-on: https://chromium-review.googlesource.com/616487
Commit-Queue: Michael McGreevy <mcgreevy@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495053}
[modify] https://crrev.com/8c2079c0d62d6973f2a317986bbc5537c7321253/chrome/browser/installable/installable_manager.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 17 2017

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

commit 1fdf3c0022234e2e17edb27dfe41224bd9e7df9f
Author: Michael McGreevy <mcgreevy@chromium.org>
Date: Thu Aug 17 04:54:26 2017

Have TaskQueue::Reset update is_active.

Bug:  754570 
Change-Id: Ia962bc418942b49e8bdc41c54478f65002a4317b
Reviewed-on: https://chromium-review.googlesource.com/618205
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Michael McGreevy <mcgreevy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495078}
[modify] https://crrev.com/1fdf3c0022234e2e17edb27dfe41224bd9e7df9f/chrome/browser/installable/installable_manager.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 17 2017

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

commit 0e5748b41fbd3833269ce2da9fee3fcc6df84dac
Author: Michael McGreevy <mcgreevy@chromium.org>
Date: Thu Aug 17 06:10:57 2017

Have TaskQueue::Insert update is_active_

Bug:  754570 
Change-Id: Ie5c91215243cf7496b74c72ff138a2ed0c12c2f3
Reviewed-on: https://chromium-review.googlesource.com/618206
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Michael McGreevy <mcgreevy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495090}
[modify] https://crrev.com/0e5748b41fbd3833269ce2da9fee3fcc6df84dac/chrome/browser/installable/installable_manager.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 17 2017

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

commit 9bb442b562aa8fa8d435ec7a77ddcaf8bf189e91
Author: Michael McGreevy <mcgreevy@chromium.org>
Date: Thu Aug 17 07:47:20 2017

remove TaskQueue::is_active_

is_active can now be implicitly defined in terms of whether TaskQueue::tasks_
contains any elements. It's now redundant, so we can remove it.

Bug:  754570 
Change-Id: I788c5441c974ca74df94e40b1cc10ed89b07db98
Reviewed-on: https://chromium-review.googlesource.com/618061
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Michael McGreevy <mcgreevy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495101}
[modify] https://crrev.com/9bb442b562aa8fa8d435ec7a77ddcaf8bf189e91/chrome/browser/installable/installable_manager.cc
[modify] https://crrev.com/9bb442b562aa8fa8d435ec7a77ddcaf8bf189e91/chrome/browser/installable/installable_manager.h

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 18 2017

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

commit 98e14cc4a76efcbcc4e54d818085629792c96d22
Author: Michael McGreevy <mcgreevy@chromium.org>
Date: Fri Aug 18 04:16:51 2017

Delete InstallableManager::StartNextTask.

Bug:  754570 
Change-Id: I412c10d11dcca2ead3941cbd77c903f063340fe7
Reviewed-on: https://chromium-review.googlesource.com/618408
Commit-Queue: Michael McGreevy <mcgreevy@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495457}
[modify] https://crrev.com/98e14cc4a76efcbcc4e54d818085629792c96d22/chrome/browser/installable/installable_manager.cc
[modify] https://crrev.com/98e14cc4a76efcbcc4e54d818085629792c96d22/chrome/browser/installable/installable_manager.h

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 22 2017

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

commit 9de29d97a143703c636d10945330e18480c6d574
Author: Michael McGreevy <mcgreevy@chromium.org>
Date: Tue Aug 22 03:08:27 2017

Pull TaskQueue out of InstallableManager.

This will make TaskQueue easier to test.

InstallableParams and InstallableData have also been moved into their own files
to avoid a cyclical dependency.

Bug:  754570 
Change-Id: If8ae6b942fd0b09b0fba11abcd28dddf75960f89
Reviewed-on: https://chromium-review.googlesource.com/623018
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Michael McGreevy <mcgreevy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496195}
[modify] https://crrev.com/9de29d97a143703c636d10945330e18480c6d574/chrome/browser/BUILD.gn
[add] https://crrev.com/9de29d97a143703c636d10945330e18480c6d574/chrome/browser/installable/installable_data.h
[modify] https://crrev.com/9de29d97a143703c636d10945330e18480c6d574/chrome/browser/installable/installable_manager.cc
[modify] https://crrev.com/9de29d97a143703c636d10945330e18480c6d574/chrome/browser/installable/installable_manager.h
[add] https://crrev.com/9de29d97a143703c636d10945330e18480c6d574/chrome/browser/installable/installable_params.h
[add] https://crrev.com/9de29d97a143703c636d10945330e18480c6d574/chrome/browser/installable/installable_task_queue.cc
[add] https://crrev.com/9de29d97a143703c636d10945330e18480c6d574/chrome/browser/installable/installable_task_queue.h

Components: UI>Browser>WebAppInstalls
Status: Fixed (was: Started)

Sign in to add a comment