Avoid a crash in ProfileManager::GetLastUsedProfileAllowedByPolicy()

chrome/browser/media/tab_desktop_media_list.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/media/tab_desktop_media_list.h"
 
 #include "base/hash.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser.h"
@@ skipping 55 matching lines @@
   thumbnail_task_runner_ =
       worker_pool->GetSequencedTaskRunner(worker_pool->GetSequenceToken());
 }
 
 TabDesktopMediaList::~TabDesktopMediaList() {}
 
 void TabDesktopMediaList::Refresh() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   Profile* profile = ProfileManager::GetLastUsedProfileAllowedByPolicy();
-  std::vector<Browser*> browsers;
-  for (auto* browser : *BrowserList::GetInstance()) {
-    if (browser->profile()->GetOriginalProfile() ==
-        profile->GetOriginalProfile()) {
-      browsers.push_back(browser);
+  if (profile) {

[tommi (sloooow) - chröme, 2016/08/17 11:38:00]

nit: Change to an early return? 

[afakhry, 2016/08/17 15:33:00]

Do we need to execute the ScheduleNextRefresh at the end of the function in this
case?

[Sergey Ulanov, 2016/08/18 05:42:26]

If profile can change then the answer is yes. Maybe ,ove the code that posts
task for ScheduleNextRefresh() before this line?

[afakhry, 2016/08/18 16:36:51]

It seems from the comment above the PostTaskAndRepy() at the end, that it should
happen after UpdateSourceThumbnail() has been called for all favicons.

Looks like this should be just an early return. PTAL.

+    std::vector<Browser*> browsers;
+    for (auto* browser : *BrowserList::GetInstance()) {
+      if (browser->profile()->GetOriginalProfile() ==
+          profile->GetOriginalProfile()) {
+        browsers.push_back(browser);
+      }
+    }
+
+    ImageHashesMap new_favicon_hashes;
+    std::vector<SourceDescription> sources;
+    std::map<base::TimeTicks, SourceDescription> tab_map;
+    std::vector<std::pair<DesktopMediaID, gfx::ImageSkia>> favicon_pairs;
+
+    // Enumerate all tabs with their titles and favicons for a user profile.
+    for (auto* browser : browsers) {
+      const TabStripModel* tab_strip_model = browser->tab_strip_model();
+      DCHECK(tab_strip_model);
+
+      for (int i = 0; i < tab_strip_model->count(); i++) {
+        // Create id for tab.
+        content::WebContents* contents = tab_strip_model->GetWebContentsAt(i);
+        DCHECK(contents);
+        content::RenderFrameHost* main_frame = contents->GetMainFrame();
+        DCHECK(main_frame);
+        DesktopMediaID media_id(
+            DesktopMediaID::TYPE_WEB_CONTENTS, DesktopMediaID::kNullId,
+            content::WebContentsMediaCaptureId(
+                main_frame->GetProcess()->GetID(), main_frame->GetRoutingID()));
+
+        // Get tab's last active time stamp.
+        const base::TimeTicks t = contents->GetLastActiveTime();
+        tab_map.insert(std::make_pair(
+            t, SourceDescription(media_id, contents->GetTitle())));
+
+        // Get favicon for tab.
+        favicon::FaviconDriver* favicon_driver =
+            favicon::ContentFaviconDriver::FromWebContents(contents);
+        if (!favicon_driver)
+          continue;
+
+        gfx::Image favicon = favicon_driver->GetFavicon();
+        if (favicon.IsEmpty())
+          continue;
+
+        // Only new or changed favicon need update.
+        new_favicon_hashes[media_id] = GetImageHash(favicon);
+        if (!favicon_hashes_.count(media_id) ||
+            (favicon_hashes_[media_id] != new_favicon_hashes[media_id])) {
+          gfx::ImageSkia image = favicon.AsImageSkia();
+          image.MakeThreadSafe();
+          favicon_pairs.push_back(std::make_pair(media_id, image));
+        }
+      }
+    }
+    favicon_hashes_ = new_favicon_hashes;
+
+    // Sort tab sources by time. Most recent one first. Then update sources
+    // list.
+    for (auto it = tab_map.rbegin(); it != tab_map.rend(); ++it) {
+      sources.push_back(it->second);
+    }
+    UpdateSourcesList(sources);
+
+    for (const auto& it : favicon_pairs) {
+      // Create a thumbail in a different thread and update the thumbnail in
+      // current thread.
+      base::PostTaskAndReplyWithResult(
+          thumbnail_task_runner_.get(), FROM_HERE,
+          base::Bind(&CreateEnclosedFaviconImage, thumbnail_size_, it.second),
+          base::Bind(&TabDesktopMediaList::UpdateSourceThumbnail,
+                     weak_factory_.GetWeakPtr(), it.first));
     }
   }
-
-  ImageHashesMap new_favicon_hashes;
-  std::vector<SourceDescription> sources;
-  std::map<base::TimeTicks, SourceDescription> tab_map;
-  std::vector<std::pair<DesktopMediaID, gfx::ImageSkia>> favicon_pairs;
-
-  // Enumerate all tabs with their titles and favicons for a user profile.
-  for (auto* browser : browsers) {
-    const TabStripModel* tab_strip_model = browser->tab_strip_model();
-    DCHECK(tab_strip_model);
-
-    for (int i = 0; i < tab_strip_model->count(); i++) {
-      // Create id for tab.
-      content::WebContents* contents = tab_strip_model->GetWebContentsAt(i);
-      DCHECK(contents);
-      content::RenderFrameHost* main_frame = contents->GetMainFrame();
-      DCHECK(main_frame);
-      DesktopMediaID media_id(
-          DesktopMediaID::TYPE_WEB_CONTENTS, DesktopMediaID::kNullId,
-          content::WebContentsMediaCaptureId(main_frame->GetProcess()->GetID(),
-                                             main_frame->GetRoutingID()));
-
-      // Get tab's last active time stamp.
-      const base::TimeTicks t = contents->GetLastActiveTime();
-      tab_map.insert(
-          std::make_pair(t, SourceDescription(media_id, contents->GetTitle())));
-
-      // Get favicon for tab.
-      favicon::FaviconDriver* favicon_driver =
-          favicon::ContentFaviconDriver::FromWebContents(contents);
-      if (!favicon_driver)
-        continue;
-
-      gfx::Image favicon = favicon_driver->GetFavicon();
-      if (favicon.IsEmpty())
-        continue;
-
-      // Only new or changed favicon need update.
-      new_favicon_hashes[media_id] = GetImageHash(favicon);
-      if (!favicon_hashes_.count(media_id) ||
-          (favicon_hashes_[media_id] != new_favicon_hashes[media_id])) {
-        gfx::ImageSkia image = favicon.AsImageSkia();
-        image.MakeThreadSafe();
-        favicon_pairs.push_back(std::make_pair(media_id, image));
-      }
-    }
-  }
-  favicon_hashes_ = new_favicon_hashes;
-
-  // Sort tab sources by time. Most recent one first. Then update sources list.
-  for (auto it = tab_map.rbegin(); it != tab_map.rend(); ++it) {
-    sources.push_back(it->second);
-  }
-  UpdateSourcesList(sources);
-
-  for (const auto& it : favicon_pairs) {
-    // Create a thumbail in a different thread and update the thumbnail in
-    // current thread.
-    base::PostTaskAndReplyWithResult(
-        thumbnail_task_runner_.get(), FROM_HERE,
-        base::Bind(&CreateEnclosedFaviconImage, thumbnail_size_, it.second),
-        base::Bind(&TabDesktopMediaList::UpdateSourceThumbnail,
-                   weak_factory_.GetWeakPtr(), it.first));
-  }
-
   // ScheduleNextRefresh() needs to be called after all calls for
   // UpdateSourceThumbnail() have done. Therefore, a DoNothing task is posted
   // to the same sequenced task runner that CreateEnlargedFaviconImag()
   // is posted.
   thumbnail_task_runner_.get()->PostTaskAndReply(
       FROM_HERE, base::Bind(&base::DoNothing),
       base::Bind(&TabDesktopMediaList::ScheduleNextRefresh,
                  weak_factory_.GetWeakPtr()));
 }