Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2023643002:
Add ARC app icons to the Task Manager UI (Closed)
|
Created:
4 years, 6 months ago by Yusuke Sato Modified:
4 years, 6 months ago CC:
Aaron Boodman, abarth-chromium, ben+mojo_chromium.org, chromium-reviews, darin (slow to review), davemoore+watch_chromium.org, elijahtaylor+arcwatch_chromium.org, hidehiko+watch_chromium.org, lhchavez+watch_chromium.org, oshima+watch_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yusukes+watch_chromium.org, yzshen+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd ARC app icons to the Task Manager UI
* Add |packages| member variable to arc::ArcProcess.
* Modify ArcProcessTask::ArcProcessTask() so it fetches and
sets an icon based on a package name in |packages|.
BUG=616159
Committed: https://crrev.com/cd2c2981937eb5c34e8cbf04ad2b772bf366cd49
Cr-Commit-Position: refs/heads/master@{#397310}
Patch Set 1 : review #
Total comments: 10
Patch Set 2 : address comments #Patch Set 3 : fix linux_chromium_gn_chromeos_rel #Patch Set 4 : Fix tab_manager_delegate_chromeos_unittest.cc #
Total comments: 2
Patch Set 5 : address comment #
Total comments: 4
Patch Set 6 : address two more comments #
Total comments: 8
Patch Set 7 : address comments #Patch Set 8 : revert back to the original test code #Messages
Total messages: 46 (18 generated)
Description was changed from
==========
wip
BUG=None
==========
to
==========
Add ARC app icons to the Task Manager UI
BUG=616159
==========
Patchset #1 (id:1) has been deleted
Description was changed from
==========
Add ARC app icons to the Task Manager UI
BUG=616159
==========
to
==========
Add ARC app icons to the Task Manager UI
* Add |packages| member variable to arc::ArcProcess.
* Modify ArcProcessTask::ArcProcessTask() so it fetches and
sets an icon based on a package name in |packages|.
BUG=616159
==========
yusukes@chromium.org changed reviewers: + dcheng@chromium.org, lhchavez@chromium.org
Please take a look
https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/chromeos... File chrome/browser/chromeos/arc/arc_process.h (left): https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/chromeos... chrome/browser/chromeos/arc/arc_process.h:14: struct ArcProcess { As discussed offline, this looks like it needs to be a legit class. https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/task_man... File chrome/browser/task_management/providers/arc/arc_process_task.cc (right): https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/task_man... chrome/browser/task_management/providers/arc/arc_process_task.cc:55: activity.emplace_back(packages.at(0), "" /* don't care */); nit: kEmptyActivityName? https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/task_man... chrome/browser/task_management/providers/arc/arc_process_task.cc:55: activity.emplace_back(packages.at(0), "" /* don't care */); Maybe add a comment explaining why you only choose the first package entry. https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/task_man... chrome/browser/task_management/providers/arc/arc_process_task.cc:114: set_icon(*icon_.ToImageSkia()); // pass a weak pointer of the |icon_|. Given that you only use one package name (and expect one icon back), break after this? (Adding a comment about it). https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/task_man... File chrome/browser/task_management/providers/arc/arc_process_task.h (right): https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/task_man... chrome/browser/task_management/providers/arc/arc_process_task.h:28: const std::vector<std::string>& activities); packages?
Thanks, PTAL https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/chromeos... File chrome/browser/chromeos/arc/arc_process.h (left): https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/chromeos... chrome/browser/chromeos/arc/arc_process.h:14: struct ArcProcess { On 2016/05/31 22:10:53, Luis Héctor Chávez wrote: > As discussed offline, this looks like it needs to be a legit class. Done. Changed to class and added DISALLOW_COPY_AND_ASSIGN following your suggestion offline. https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/task_man... File chrome/browser/task_management/providers/arc/arc_process_task.cc (right): https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/task_man... chrome/browser/task_management/providers/arc/arc_process_task.cc:55: activity.emplace_back(packages.at(0), "" /* don't care */); On 2016/05/31 22:10:53, Luis Héctor Chávez wrote: > nit: kEmptyActivityName? Done. https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/task_man... chrome/browser/task_management/providers/arc/arc_process_task.cc:55: activity.emplace_back(packages.at(0), "" /* don't care */); On 2016/05/31 22:10:53, Luis Héctor Chávez wrote: > Maybe add a comment explaining why you only choose the first package entry. Good idea. Done. https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/task_man... chrome/browser/task_management/providers/arc/arc_process_task.cc:114: set_icon(*icon_.ToImageSkia()); // pass a weak pointer of the |icon_|. On 2016/05/31 22:10:53, Luis Héctor Chávez wrote: > Given that you only use one package name (and expect one icon back), break after > this? (Adding a comment about it). Done. https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/task_man... File chrome/browser/task_management/providers/arc/arc_process_task.h (right): https://codereview.chromium.org/2023643002/diff/20001/chrome/browser/task_man... chrome/browser/task_management/providers/arc/arc_process_task.h:28: const std::vector<std::string>& activities); On 2016/05/31 22:10:53, Luis Héctor Chávez wrote: > packages? Done.
lgtm
yusukes@chromium.org changed reviewers: - dcheng@chromium.org
yusukes@chromium.org changed reviewers: + afakhry@chromium.org, dcheng@chromium.org, georgesak@chromium.org
Hi Ahmed, Georges, could you do an OWNER review? This is entirely for ARC (android runtime for Chrome OS). Georges: I modified chrome/browser/memory/tab_manager_delegate_chromeos*.cc because ArcProcess in chrome/browser/chromeos/arc/arc_process.h was changed from struct to class.
lgtm
https://codereview.chromium.org/2023643002/diff/80001/chrome/browser/task_man... File chrome/browser/task_management/providers/arc/arc_process_task.cc (right): https://codereview.chromium.org/2023643002/diff/80001/chrome/browser/task_man... chrome/browser/task_management/providers/arc/arc_process_task.cc:121: icon_ = icon; Is there a reason to store gfx::Image as a member here while the parent class stores gfx::ImageSkia? Now we have two members named |icon_| (Task::icon_ and ArcProcessTask::icon_)! If not needed, please remove that extra |icon_| member here, and just do set_icon(*icon_.ToImageSkia()); I think the ImageSkia& operator=(const ImageSkia& other); (used in set_icon()) will AddRef() the ImageSkiaStorage member of the ImageSkia object, and it should work fine.
Thanks, fixed. Please take another look. https://codereview.chromium.org/2023643002/diff/80001/chrome/browser/task_man... File chrome/browser/task_management/providers/arc/arc_process_task.cc (right): https://codereview.chromium.org/2023643002/diff/80001/chrome/browser/task_man... chrome/browser/task_management/providers/arc/arc_process_task.cc:121: icon_ = icon; On 2016/06/01 17:07:14, afakhry wrote: > Is there a reason to store gfx::Image as a member here while the parent class > stores gfx::ImageSkia? Now we have two members named |icon_| (Task::icon_ and > ArcProcessTask::icon_)! > > If not needed, please remove that extra |icon_| member here, and just do > set_icon(*icon_.ToImageSkia()); > I think the ImageSkia& operator=(const ImageSkia& other); (used in set_icon()) > will AddRef() the ImageSkiaStorage member of the ImageSkia object, and it should > work fine. I read the code again, and you're right. Removed the icon_ member from this class. Done.
lgtm with a couple of nits. Thanks. https://codereview.chromium.org/2023643002/diff/100001/chrome/browser/task_ma... File chrome/browser/task_management/providers/arc/arc_process_task.cc (right): https://codereview.chromium.org/2023643002/diff/100001/chrome/browser/task_ma... chrome/browser/task_management/providers/arc/arc_process_task.cc:41: const char kEmptyActivityName[] = ""; Nit: constexpr is the new preferred declaration of constants. See http://chromium-cpp.appspot.com/#core-whitelist https://codereview.chromium.org/2023643002/diff/100001/chrome/browser/task_ma... File chrome/browser/task_management/providers/arc/arc_process_task.h (right): https://codereview.chromium.org/2023643002/diff/100001/chrome/browser/task_ma... chrome/browser/task_management/providers/arc/arc_process_task.h:51: base::WeakPtrFactory<ArcProcessTask> weak_factory_; Nit: |weak_factory_| sounds weird. Please rename to |weak_ptr_factory_|.
https://codereview.chromium.org/2023643002/diff/100001/chrome/browser/task_ma... File chrome/browser/task_management/providers/arc/arc_process_task.cc (right): https://codereview.chromium.org/2023643002/diff/100001/chrome/browser/task_ma... chrome/browser/task_management/providers/arc/arc_process_task.cc:41: const char kEmptyActivityName[] = ""; On 2016/06/01 19:31:04, afakhry wrote: > Nit: constexpr is the new preferred declaration of constants. > See http://chromium-cpp.appspot.com/#core-whitelist Done. https://codereview.chromium.org/2023643002/diff/100001/chrome/browser/task_ma... File chrome/browser/task_management/providers/arc/arc_process_task.h (right): https://codereview.chromium.org/2023643002/diff/100001/chrome/browser/task_ma... chrome/browser/task_management/providers/arc/arc_process_task.h:51: base::WeakPtrFactory<ArcProcessTask> weak_factory_; On 2016/06/01 19:31:04, afakhry wrote: > Nit: |weak_factory_| sounds weird. Please rename to |weak_ptr_factory_|. Done.
Daniel, could you have a look at the mojom change?
https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/chromeo... File chrome/browser/chromeos/arc/arc_process.h (right): https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/chromeo... chrome/browser/chromeos/arc/arc_process.h:29: std::string process_name() const { return process_name_; } Out of curiosity, why does this return by value while other getters for complex types return by reference? https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/memory/... File chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc (right): https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/memory/... chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc:75: arc_processes.emplace_back(3, 30, "service", This should still use initializer list syntax. Ditto elsewhere.
Please take another look https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/chromeo... File chrome/browser/chromeos/arc/arc_process.h (right): https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/chromeo... chrome/browser/chromeos/arc/arc_process.h:29: std::string process_name() const { return process_name_; } On 2016/06/01 23:55:01, dcheng wrote: > Out of curiosity, why does this return by value while other getters for complex > types return by reference? Oops, this is just a typo. Fixed, thanks. https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/memory/... File chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc (right): https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/memory/... chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc:75: arc_processes.emplace_back(3, 30, "service", On 2016/06/01 23:55:01, dcheng wrote: > This should still use initializer list syntax. Ditto elsewhere. Done (and please let me know if I misunderstood your comment..)
https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/memory/... File chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc (right): https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/memory/... chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc:75: arc_processes.emplace_back(3, 30, "service", On 2016/06/02 at 00:19:26, Yusuke Sato wrote: > On 2016/06/01 23:55:01, dcheng wrote: > > This should still use initializer list syntax. Ditto elsewhere. > > Done (and please let me know if I misunderstood your comment..) I'm not sure why this had to change at all in this patch: I think the old code should have worked?
https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/memory/... File chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc (right): https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/memory/... chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc:75: arc_processes.emplace_back(3, 30, "service", On 2016/06/02 00:37:47, dcheng wrote: > On 2016/06/02 at 00:19:26, Yusuke Sato wrote: > > On 2016/06/01 23:55:01, dcheng wrote: > > > This should still use initializer list syntax. Ditto elsewhere. > > > > Done (and please let me know if I misunderstood your comment..) > > I'm not sure why this had to change at all in this patch: I think the old code > should have worked? Ah, I see. With the original code, the compiler tried to call the deleted copy constructor and failed. Do you know a way to work around this? In file included from ../../chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc:5: In file included from ../../chrome/browser/memory/tab_manager_delegate_chromeos.h:8: In file included from /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/memory:64: /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_construct.h:75:38: error: call to deleted constructor of 'arc::ArcProcess' { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); } ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_uninitialized.h:75:8: note: in instantiation of function template specialization 'std::_Construct<arc::ArcProcess, const arc::ArcProcess &>' requested here std::_Construct(std::__addressof(*__cur), *__first); ^ /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_uninitialized.h:117:2: note: in instantiation of function template specialization 'std::__uninitialized_copy<false>::__uninit_copy<const arc::ArcProcess *, arc::ArcProcess *>' requested here __uninit_copy(__first, __last, __result); ^ /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_uninitialized.h:258:19: note: in instantiation of function template specialization 'std::uninitialized_copy<const arc::ArcProcess *, arc::ArcProcess *>' requested here { return std::uninitialized_copy(__first, __last, __result); } ^ /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_vector.h:1204:11: note: in instantiation of function template specialization 'std::__uninitialized_copy_a<const arc::ArcProcess *, arc::ArcProcess *, arc::ArcProcess>' requested here std::__uninitialized_copy_a(__first, __last, ^ /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_vector.h:368:2: note: in instantiation of function template specialization 'std::vector<arc::ArcProcess, std::allocator<arc::ArcProcess> >::_M_range_initialize<const arc::ArcProcess *>' requested here _M_range_initialize(__l.begin(), __l.end(), ^ ../../chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc:71:48: note: in instantiation of member function 'std::vector<arc::ArcProcess, std::allocator<arc::ArcProcess> >::vector' requested here std::vector<arc::ArcProcess> arc_processes = { ^ ../../chrome/browser/chromeos/arc/arc_process.h:41:28: note: 'ArcProcess' has been explicitly marked deleted here DISALLOW_COPY_AND_ASSIGN(ArcProcess);
lgtm https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/memory/... File chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc (right): https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/memory/... chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc:75: arc_processes.emplace_back(3, 30, "service", On 2016/06/02 at 00:42:43, Yusuke Sato wrote: > On 2016/06/02 00:37:47, dcheng wrote: > > On 2016/06/02 at 00:19:26, Yusuke Sato wrote: > > > On 2016/06/01 23:55:01, dcheng wrote: > > > > This should still use initializer list syntax. Ditto elsewhere. > > > > > > Done (and please let me know if I misunderstood your comment..) > > > > I'm not sure why this had to change at all in this patch: I think the old code > > should have worked? > > Ah, I see. With the original code, the compiler tried to call the deleted copy constructor and failed. Do you know a way to work around this? > > In file included from ../../chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc:5: > In file included from ../../chrome/browser/memory/tab_manager_delegate_chromeos.h:8: > In file included from /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/memory:64: > /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_construct.h:75:38: error: call to deleted constructor of 'arc::ArcProcess' > { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); } > ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_uninitialized.h:75:8: note: in instantiation of function template specialization 'std::_Construct<arc::ArcProcess, const arc::ArcProcess &>' requested here > std::_Construct(std::__addressof(*__cur), *__first); > ^ > /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_uninitialized.h:117:2: note: in instantiation of function template specialization 'std::__uninitialized_copy<false>::__uninit_copy<const arc::ArcProcess *, arc::ArcProcess *>' requested here > __uninit_copy(__first, __last, __result); > ^ > /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_uninitialized.h:258:19: note: in instantiation of function template specialization 'std::uninitialized_copy<const arc::ArcProcess *, arc::ArcProcess *>' requested here > { return std::uninitialized_copy(__first, __last, __result); } > ^ > /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_vector.h:1204:11: note: in instantiation of function template specialization 'std::__uninitialized_copy_a<const arc::ArcProcess *, arc::ArcProcess *, arc::ArcProcess>' requested here > std::__uninitialized_copy_a(__first, __last, > ^ > /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_vector.h:368:2: note: in instantiation of function template specialization 'std::vector<arc::ArcProcess, std::allocator<arc::ArcProcess> >::_M_range_initialize<const arc::ArcProcess *>' requested here > _M_range_initialize(__l.begin(), __l.end(), > ^ > ../../chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc:71:48: note: in instantiation of member function 'std::vector<arc::ArcProcess, std::allocator<arc::ArcProcess> >::vector' requested here > std::vector<arc::ArcProcess> arc_processes = { > ^ > ../../chrome/browser/chromeos/arc/arc_process.h:41:28: note: 'ArcProcess' has been explicitly marked deleted here > DISALLOW_COPY_AND_ASSIGN(ArcProcess); Oh I see. Initializer list doesn't work with move-only types. OK, let's just revert this to your original code then: arc_processes.emplace_back(1, 10, "top", ...); arc_processes.emplace_back(2, 20, "foreground", ...); etc
https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/memory/... File chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc (right): https://codereview.chromium.org/2023643002/diff/120001/chrome/browser/memory/... chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc:75: arc_processes.emplace_back(3, 30, "service", On 2016/06/02 01:00:56, dcheng wrote: > On 2016/06/02 at 00:42:43, Yusuke Sato wrote: > > On 2016/06/02 00:37:47, dcheng wrote: > > > On 2016/06/02 at 00:19:26, Yusuke Sato wrote: > > > > On 2016/06/01 23:55:01, dcheng wrote: > > > > > This should still use initializer list syntax. Ditto elsewhere. > > > > > > > > Done (and please let me know if I misunderstood your comment..) > > > > > > I'm not sure why this had to change at all in this patch: I think the old > code > > > should have worked? > > > > Ah, I see. With the original code, the compiler tried to call the deleted copy > constructor and failed. Do you know a way to work around this? > > > > In file included from > ../../chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc:5: > > In file included from > ../../chrome/browser/memory/tab_manager_delegate_chromeos.h:8: > > In file included from > /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/memory:64: > > > /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_construct.h:75:38: > error: call to deleted constructor of 'arc::ArcProcess' > > { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); } > > ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_uninitialized.h:75:8: > note: in instantiation of function template specialization > 'std::_Construct<arc::ArcProcess, const arc::ArcProcess &>' requested here > > std::_Construct(std::__addressof(*__cur), *__first); > > ^ > > > /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_uninitialized.h:117:2: > note: in instantiation of function template specialization > 'std::__uninitialized_copy<false>::__uninit_copy<const arc::ArcProcess *, > arc::ArcProcess *>' requested here > > __uninit_copy(__first, __last, __result); > > ^ > > > /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_uninitialized.h:258:19: > note: in instantiation of function template specialization > 'std::uninitialized_copy<const arc::ArcProcess *, arc::ArcProcess *>' requested > here > > { return std::uninitialized_copy(__first, __last, __result); } > > ^ > > > /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_vector.h:1204:11: > note: in instantiation of function template specialization > 'std::__uninitialized_copy_a<const arc::ArcProcess *, arc::ArcProcess *, > arc::ArcProcess>' requested here > > std::__uninitialized_copy_a(__first, __last, > > ^ > > > /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_vector.h:368:2: > note: in instantiation of function template specialization > 'std::vector<arc::ArcProcess, std::allocator<arc::ArcProcess> > >::_M_range_initialize<const arc::ArcProcess *>' requested here > > _M_range_initialize(__l.begin(), __l.end(), > > ^ > > ../../chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc:71:48: > note: in instantiation of member function 'std::vector<arc::ArcProcess, > std::allocator<arc::ArcProcess> >::vector' requested here > > std::vector<arc::ArcProcess> arc_processes = { > > ^ > > ../../chrome/browser/chromeos/arc/arc_process.h:41:28: note: 'ArcProcess' has > been explicitly marked deleted here > > DISALLOW_COPY_AND_ASSIGN(ArcProcess); > > Oh I see. Initializer list doesn't work with move-only types. > > OK, let's just revert this to your original code then: > arc_processes.emplace_back(1, 10, "top", ...); > arc_processes.emplace_back(2, 20, "foreground", ...); > > etc Done.
The CQ bit was checked by yusukes@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from lhchavez@chromium.org, georgesak@chromium.org, afakhry@chromium.org, dcheng@chromium.org Link to the patchset: https://codereview.chromium.org/2023643002/#ps160001 (title: "revert back to the original test code")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2023643002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2023643002/160001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by yusukes@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2023643002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2023643002/160001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by yusukes@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2023643002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2023643002/160001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by yusukes@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2023643002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2023643002/160001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by yusukes@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2023643002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2023643002/160001
Message was sent while issue was closed.
Description was changed from ========== Add ARC app icons to the Task Manager UI * Add |packages| member variable to arc::ArcProcess. * Modify ArcProcessTask::ArcProcessTask() so it fetches and sets an icon based on a package name in |packages|. BUG=616159 ========== to ========== Add ARC app icons to the Task Manager UI * Add |packages| member variable to arc::ArcProcess. * Modify ArcProcessTask::ArcProcessTask() so it fetches and sets an icon based on a package name in |packages|. BUG=616159 ==========
Message was sent while issue was closed.
Committed patchset #8 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== Add ARC app icons to the Task Manager UI * Add |packages| member variable to arc::ArcProcess. * Modify ArcProcessTask::ArcProcessTask() so it fetches and sets an icon based on a package name in |packages|. BUG=616159 ========== to ========== Add ARC app icons to the Task Manager UI * Add |packages| member variable to arc::ArcProcess. * Modify ArcProcessTask::ArcProcessTask() so it fetches and sets an icon based on a package name in |packages|. BUG=616159 Committed: https://crrev.com/cd2c2981937eb5c34e8cbf04ad2b772bf366cd49 Cr-Commit-Position: refs/heads/master@{#397310} ==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/cd2c2981937eb5c34e8cbf04ad2b772bf366cd49 Cr-Commit-Position: refs/heads/master@{#397310} |
