Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 2827323002: gpu:: Remove GpuDataManagerObserver::OnVideoMemoryUsageStatsUpdate(). (Closed)

Created:
7 months, 1 week ago by sadrul
Modified:
7 months ago
Reviewers:
jam, jbauman
CC:
chromium-reviews, jam, darin-cc_chromium.org, piman+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

gpu:: Remove GpuDataManagerObserver::OnVideoMemoryUsageStatsUpdate(). TaskManagerImpl is the only observer which requires this callback. In order to get the video memory usage, TaskManagerImpl installs itself as an observer, makes the request through GpuDataManager, and waits for the observer callback to be called. Instead, change this so the request to GpuDataManager::RequestVideoMemoryUsageStatsUpdate() includes the callback that needs to run when the stats become available. BUG=709332 Review-Url: https://codereview.chromium.org/2827323002 Cr-Commit-Position: refs/heads/master@{#466341} Committed: https://chromium.googlesource.com/chromium/src/+/f2c71b4cd910cd4d3d4d3fe7091c992bb881e3f9

Patch Set 1 #

Patch Set 2 : comment #

Total comments: 2

Patch Set 3 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+53 lines, -101 lines) Patch
M chrome/browser/task_manager/sampling/task_manager_impl.h View 6 chunks +7 lines, -9 lines 0 comments Download
M chrome/browser/task_manager/sampling/task_manager_impl.cc View 4 chunks +12 lines, -13 lines 0 comments Download
M content/browser/gpu/gpu_data_manager_impl.h View 2 chunks +3 lines, -4 lines 0 comments Download
M content/browser/gpu/gpu_data_manager_impl.cc View 2 chunks +4 lines, -8 lines 0 comments Download
M content/browser/gpu/gpu_data_manager_impl_private.h View 2 chunks +3 lines, -4 lines 0 comments Download
M content/browser/gpu/gpu_data_manager_impl_private.cc View 1 2 3 chunks +17 lines, -16 lines 0 comments Download
M content/browser/gpu/gpu_data_manager_impl_private_unittest.cc View 1 2 2 chunks +2 lines, -36 lines 0 comments Download
M content/public/browser/gpu_data_manager.h View 1 2 chunks +5 lines, -3 lines 0 comments Download
M content/public/browser/gpu_data_manager_observer.h View 2 chunks +0 lines, -8 lines 0 comments Download

Messages

Total messages: 23 (15 generated)
sadrul
7 months, 1 week ago (2017-04-20 16:32:18 UTC) #5
jbauman
https://codereview.chromium.org/2827323002/diff/20001/chrome/browser/task_manager/sampling/task_manager_impl.cc File chrome/browser/task_manager/sampling/task_manager_impl.cc (right): https://codereview.chromium.org/2827323002/diff/20001/chrome/browser/task_manager/sampling/task_manager_impl.cc#newcode477 chrome/browser/task_manager/sampling/task_manager_impl.cc:477: DCHECK_CURRENTLY_ON(content::BrowserThread::UI); Won't this be called on the IO thread ...
7 months, 1 week ago (2017-04-20 21:35:02 UTC) #9
sadrul
https://codereview.chromium.org/2827323002/diff/20001/chrome/browser/task_manager/sampling/task_manager_impl.cc File chrome/browser/task_manager/sampling/task_manager_impl.cc (right): https://codereview.chromium.org/2827323002/diff/20001/chrome/browser/task_manager/sampling/task_manager_impl.cc#newcode477 chrome/browser/task_manager/sampling/task_manager_impl.cc:477: DCHECK_CURRENTLY_ON(content::BrowserThread::UI); On 2017/04/20 21:35:02, jbauman wrote: > Won't this ...
7 months, 1 week ago (2017-04-21 03:41:01 UTC) #12
jbauman
lgtm
7 months, 1 week ago (2017-04-21 04:18:48 UTC) #13
sadrul
+jam@ for content/public and chrome/ changes.
7 months, 1 week ago (2017-04-21 04:32:00 UTC) #17
jam
lgtm
7 months, 1 week ago (2017-04-21 14:59:49 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2827323002/40001
7 months ago (2017-04-21 15:21:33 UTC) #20
commit-bot: I haz the power
7 months ago (2017-04-21 15:26:18 UTC) #23
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/f2c71b4cd910cd4d3d4d3fe7091c...

Powered by Google App Engine
This is Rietveld efc10ee0f