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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months, 1 week ago by sadrul
Modified:
3 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
Commit queue not available (can’t edit this change).

Messages

Total messages: 23 (15 generated)
sadrul
3 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 ...
3 months 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 ...
3 months ago (2017-04-21 03:41:01 UTC) #12
jbauman
lgtm
3 months ago (2017-04-21 04:18:48 UTC) #13
sadrul
+jam@ for content/public and chrome/ changes.
3 months ago (2017-04-21 04:32:00 UTC) #17
jam
lgtm
3 months 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
3 months ago (2017-04-21 15:21:33 UTC) #20
commit-bot: I haz the power
3 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 25c286973