Revert of Fix stability and data racing issues, coalesce more updates for JumpList (patchset #6 id:420001 of https://codereview.chromium.org/2931573003/ )
Reason for revert:
Caused crash as recorded in crbug/733319 and crbug/733098
Original issue's description:
> Fix stability and data racing issues, coalesce more updates for JumpList
>
> This CL contains the following five main changes plus lots of updates
> on variable/method names and comments.
>
> 1. Fix the JumpList data-racing issue by separating different thread
> tasks.
>
> There are two threads involved in updating the JumpList. The UI thread
> lives the module to collect JumpList data for update, and a non-UI
> thread lives the RunUpdateJumpList module which updates the JumpList
> and notifies the shell. Previously, the two modules were interleaved
> to some extent, which required the JumpList data to be shared between
> the two threads. This caused the data racing issue and was solved by
> introducing a lock. The interleaving of these two modules complicates
> the code and makes it very difficult to add unit tests. This CL
> separates these two modules. In more details, the JumpList data is now
> only accessible to the UI thread. Local copies of the data are made
> and sent to the non-UI thread to run the update task. Once that update
> task finishes on the non-UI thread, the update results are transferred
> to the UI thread via PostTaskAndReply. The UI thread then decides how
> to update the JumpList data according to the update results.
>
> 2. Fix the JumpList stability issue by making the RunUpdateJumpList
> API static so it outlives the JumpList instance.
>
> When a JumpList::RunUpdateJumpList task is posted on a non-UI thread
> and about to run, the JumpList instance may have already been
> destructed. This caused stability issues as the RunUpdateJumpList API
> was non-static. This CL fixes it by making this API and other APIs it
> calls static.
>
> 3. Coalesce JumpList updates from two services by keeping only one
> timer.
>
> Previously, there were two timers to coalesce updates from the
> TopSites service and the TabRestore service, respectively. When
> notifications from both services came, even at the same time, multiple
> RunUpdateJumpList tasks were posted where they should be coalesced
> into one. This caused a waste of many shell notification runs. Note
> that there was no wasted work on icon operations as different JumpList
> categories were processed on demand. This CL trims out this waste by
> keeping only one timer to coalesce updates triggered by different
> services.
>
> 4. Cancel fetching most-visited URLs when Jumplist is destroyed.
>
> This CL makes it possible to cancel the tasks of fetching most-visited
> URLs when JumpList is destroyed, which otherwise is wasted. This also
> avoids unnecessary favicon loading triggered upon the completion of
> the URL fetch.
>
> 5. Make JumpList not refcounted.
>
> JumpList is owned by BrowserView. It is RefCounted but there doesn't
> appear to be a reason for that. This CL changes JumpList to be not
> refcounted by making it a regular keyed service, rather than a
> refcounted keyed service as it was.
>
> BUG=40407, 179576, 498987, 717236, 720028, 732586
>
> Review-Url: https://codereview.chromium.org/2931573003
> Cr-Commit-Position: refs/heads/master@{#479095}
> Committed: https://chromium.googlesource.com/chromium/src/+/4a5397999a0f493c47632e1a8bd93686de54adbf
TBR=grt@chromium.org,pkasting@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=40407, 179576, 498987, 717236, 720028, 732586
Review-Url: https://codereview.chromium.org/2937993002
Cr-Commit-Position: refs/heads/master@{#479434}
6 files changed
