Display Window Icon In Picker UI

chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc

Index: chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc
diff --git a/chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc b/chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc
index ef1b7644b09028168643bc7f842db7ecd5d84138..380577e16aecc241607b2eff521c206c0ac0c265 100644
--- a/chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc
+++ b/chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc
@@ -7,11 +7,17 @@
 #include "base/command_line.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/media/desktop_media_list.h"
+#include "chrome/browser/media/window_icon_util.h"
 #include "chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.h"
 #include "chrome/browser/ui/views/desktop_capture/desktop_media_source_view.h"
+#include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/browser/browser_thread.h"
+#include "extensions/grit/extensions_browser_resources.h"
+#include "grit/theme_resources.h"
 #include "ui/accessibility/ax_view_state.h"
+#include "ui/aura/window.h"
+#include "ui/base/resource/resource_bundle.h"
 #include "ui/views/focus/focus_manager.h"
 
 using content::DesktopMediaID;
@@ -20,6 +26,27 @@ namespace {
 
 const int kDesktopMediaSourceViewGroupId = 1;
 
+#if defined(OS_CHROMEOS)

[sky, 2016/08/26 16:40:35]

I think you want USE_ASH here and below.

[qiangchen, 2016/08/26 17:30:01]

Done.

+// As aura framework uses empty icon to represent default icon, it relies on

[sky, 2016/08/26 16:40:35]

aura here is misleading because aura is used on linux and windows too.

[qiangchen, 2016/08/26 17:30:01]

Moved part of the comment below. Here we just need to explain what this function
does, no need to explain why call this function.

+// the caller to handle the case.
+// Here we are going to display default app icon for app windows without an
+// icon, and display product logo for chrome browser windows.
+gfx::ImageSkia LoadDefaultIcon(aura::Window* window) {
+  BrowserView* browser_view =
+      BrowserView::GetBrowserViewForNativeWindow(window);
+  Browser* browser = browser_view ? browser_view->browser() : nullptr;
+
+  // Apps could be launched in a view other than BrowserView, so we count those
+  // windows without Browser association as apps.
+  // Technically dev tool is actually a special app, but we would like to
+  // display product logo for it, because intuitively it is internal to browser.
+  bool is_app = browser ? browser->is_app() && !browser->is_devtools() : true;

[sky, 2016/08/26 16:40:35]

I don't think you need the ternary here, it just makes it harder to read:

is_app = browser && browser->is_app() && !browser->is_devtoops();

[qiangchen, 2016/08/26 17:30:01]

Done.

But it should be !browser || (....)

+  int idr = is_app ? IDR_APP_DEFAULT_ICON : IDR_PRODUCT_LOGO_32;
+
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  return *rb.GetImageSkiaNamed(idr);
+}
+#endif
 }  // namespace
 
 DesktopMediaListView::DesktopMediaListView(
@@ -151,6 +178,17 @@ void DesktopMediaListView::OnSourceAdded(DesktopMediaList* list, int index) {
 
   source_view->SetName(source.name);
   source_view->SetGroup(kDesktopMediaSourceViewGroupId);
+  if (source.id.type == DesktopMediaID::TYPE_WINDOW) {
+    gfx::ImageSkia icon_image = GetWindowIcon(source.id);
+#if defined(OS_CHROMEOS)
+    if (icon_image.isNull()) {
+      aura::Window* window = DesktopMediaID::GetAuraWindowById(source.id);
+      if (window)
+        icon_image = LoadDefaultIcon(window);
+    }
+#endif
+    source_view->SetIcon(icon_image);
+  }
   AddChildViewAt(source_view, index);
 
   if ((child_count() - 1) % active_style_->columns == 0)