[Panels refactor] Track Panels in TaskManager now that they are not under the tab contents umbrella.

[Panels refactor] Track Panels in TaskManager now that they are not under the tab contents umbrella.

BUG=127323
TEST=TaskManagerBrowserTest.NoticePanelChanges


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=147564

[jennb, 2012-07-18 22:30:47]

jcampan, yoshiki - are one of you TaskManager experts that could review the task
manager changes in this patch?

[Dmitry Titov, 2012-07-18 22:52:14]

http://codereview.chromium.org/10790062/diff/8001/chrome/browser/task_manager...
File chrome/browser/task_manager/task_manager_resource_providers.cc (right):

http://codereview.chromium.org/10790062/diff/8001/chrome/browser/task_manager...
chrome/browser/task_manager/task_manager_resource_providers.cc:564: string16
TaskManagerPanelResource::GetProfileName() const {
this seems mostly duplicate the TaskManagerTabContentsResource::GetProfileName.
Maybe change the virtual to be GetProfile() and move the cache/name
functionality into a non-virtual method on the base class?

http://codereview.chromium.org/10790062/diff/8001/chrome/browser/ui/panels/pa...
File chrome/browser/ui/panels/panel.cc (right):

http://codereview.chromium.org/10790062/diff/8001/chrome/browser/ui/panels/pa...
chrome/browser/ui/panels/panel.cc:130: Panel** panel =
property_accessor()->GetProperty(contents->GetPropertyBag());
Seems a loop through PanelManager will be easier to read, smaller and not that
slow since we always have a very limited number of panels.

http://codereview.chromium.org/10790062/diff/8001/chrome/browser/ui/panels/pa...
chrome/browser/ui/panels/panel.cc:166: if (contents) {
can it be NULL? When?

[jennb, 2012-07-19 20:48:14]

On 2012/07/18 22:52:14, Dmitry Titov wrote:
>
http://codereview.chromium.org/10790062/diff/8001/chrome/browser/task_manager...
> File chrome/browser/task_manager/task_manager_resource_providers.cc (right):
> 
>
http://codereview.chromium.org/10790062/diff/8001/chrome/browser/task_manager...
> chrome/browser/task_manager/task_manager_resource_providers.cc:564: string16
> TaskManagerPanelResource::GetProfileName() const {
> this seems mostly duplicate the
TaskManagerTabContentsResource::GetProfileName.
> Maybe change the virtual to be GetProfile() and move the cache/name
> functionality into a non-virtual method on the base class?
> 

Consolidated duplicated code into a helper func.

>
http://codereview.chromium.org/10790062/diff/8001/chrome/browser/ui/panels/pa...
> File chrome/browser/ui/panels/panel.cc (right):
> 
>
http://codereview.chromium.org/10790062/diff/8001/chrome/browser/ui/panels/pa...
> chrome/browser/ui/panels/panel.cc:130: Panel** panel =
> property_accessor()->GetProperty(contents->GetPropertyBag());
> Seems a loop through PanelManager will be easier to read, smaller and not that
> slow since we always have a very limited number of panels.

Changed.

> 
>
http://codereview.chromium.org/10790062/diff/8001/chrome/browser/ui/panels/pa...
> chrome/browser/ui/panels/panel.cc:166: if (contents) {
> can it be NULL? When?

Change is no longer needed, but to answer your question, tests can create panels
without webcontents by not supplying any url.

[Dmitry Titov, 2012-07-19 21:12:14]

lgtm

[commit-bot: I haz the power, 2012-07-19 21:56:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jennb@chromium.org/10790062/12001

[commit-bot: I haz the power, 2012-07-19 23:17:11]

Change committed as 147564