|
|
Created:
4 years ago by Andra Paraschiv Modified:
4 years ago CC:
chromium-reviews, extensions-reviews_chromium.org, sadrul, Matt Giuca, tfarina, kalyank, chromium-apps-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionEnhance chrome.app.window API for shelf integration with restore support
Enhance the chrome.app.window API for shelf integration with the possibility of
restoring showInShelf=true pinned windows.
Based on https://codereview.chromium.org/2341643002
Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>;
BUG=610299
TEST = https://codereview.chromium.org/2297633002
Committed: https://crrev.com/4e4fb8bbebc3d8f3e38130021daec21b760c5840
Cr-Commit-Position: refs/heads/master@{#438803}
Patch Set 1 #
Total comments: 12
Patch Set 2 : Review v2 #
Total comments: 6
Patch Set 3 : Review v3 #Patch Set 4 : Review v4 #
Total comments: 10
Patch Set 5 : Review v5 #Messages
Total messages: 30 (8 generated)
Description was changed from ========== Enhance chrome.app.window API for shelf integration with restore support Enhance the chrome.app.window API for shelf integration with the possibility of restoring showInShelf=true pinned windows. Based on https://codereview.chromium.org/2341643002 Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>; BUG=610299 TEST = https://codereview.chromium.org/2297633002 ========== to ========== Enhance chrome.app.window API for shelf integration with restore support Enhance the chrome.app.window API for shelf integration with the possibility of restoring showInShelf=true pinned windows. Based on https://codereview.chromium.org/2341643002 Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>; BUG=610299 TEST = https://codereview.chromium.org/2297633002 ==========
andra.paraschiv@intel.com changed reviewers: + skuhne@chromium.org, stevenjb@chromium.org
Could you please take a look? Thank you.
Sorry for the delay. Please see comments! https://codereview.chromium.org/2523053004/diff/1/apps/launcher.h File apps/launcher.h (right): https://codereview.chromium.org/2523053004/diff/1/apps/launcher.h#newcode39 apps/launcher.h:39: // |launch_id| is an id that can be passed to an app when launched in order to Could you please order the parameters in the proper order? https://codereview.chromium.org/2523053004/diff/1/chrome/browser/ui/ash/launc... File chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_browsertest.cc (right): https://codereview.chromium.org/2523053004/diff/1/chrome/browser/ui/ash/launc... chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_browsertest.cc:2426: } Could you add a unit test to test that you get in deed multiple items? And also how they get sorted? https://codereview.chromium.org/2523053004/diff/1/chrome/browser/ui/extension... File chrome/browser/ui/extensions/app_launch_params.h (right): https://codereview.chromium.org/2523053004/diff/1/chrome/browser/ui/extension... chrome/browser/ui/extensions/app_launch_params.h:45: std::string launch_id; Can you elaborate what kind of id's these are? If they are "only stringified numbers" why not using numbers? And if they can be in fact real strings - how would they be sorted (so which one would go first)? https://codereview.chromium.org/2523053004/diff/1/extensions/browser/api/app_... File extensions/browser/api/app_runtime/app_runtime_api.cc (left): https://codereview.chromium.org/2523053004/diff/1/extensions/browser/api/app_... extensions/browser/api/app_runtime/app_runtime_api.cc:57: Since nothing has changed in this file - could you re-insert the line?
https://codereview.chromium.org/2523053004/diff/1/apps/launcher.h File apps/launcher.h (right): https://codereview.chromium.org/2523053004/diff/1/apps/launcher.h#newcode50 apps/launcher.h:50: PLAY_STORE_STATUS_UNKNOWN); Given that we already have a bunch of these, I would rather add one more (e.g. LaunchPlatformAppWithCommandLineAndLaunchId) than add "" to a bunch of existing calls. Otherwise, we also need to include /* launch_id */ after "" in all of those calls. https://codereview.chromium.org/2523053004/diff/1/chrome/browser/ui/ash/launc... File chrome/browser/ui/ash/launcher/chrome_launcher_controller.h (right): https://codereview.chromium.org/2523053004/diff/1/chrome/browser/ui/ash/launc... chrome/browser/ui/ash/launcher/chrome_launcher_controller.h:148: int event_flags); Same here, we seem to have a lot of existing calls to this.
andra.paraschiv@intel.com changed reviewers: + benwells@chromium.org
Thank you all for feedback, I uploaded a new patch set. https://codereview.chromium.org/2523053004/diff/1/apps/launcher.h File apps/launcher.h (right): https://codereview.chromium.org/2523053004/diff/1/apps/launcher.h#newcode39 apps/launcher.h:39: // |launch_id| is an id that can be passed to an app when launched in order to On 2016/12/03 01:55:27, Mr4D wrote: > Could you please order the parameters in the proper order? Done. https://codereview.chromium.org/2523053004/diff/1/apps/launcher.h#newcode50 apps/launcher.h:50: PLAY_STORE_STATUS_UNKNOWN); On 2016/12/05 16:47:31, stevenjb wrote: > Given that we already have a bunch of these, I would rather add one more (e.g. > LaunchPlatformAppWithCommandLineAndLaunchId) than add "" to a bunch of existing > calls. Otherwise, we also need to include /* launch_id */ after "" in all of > those calls. Thank you, I added a new one. https://codereview.chromium.org/2523053004/diff/1/chrome/browser/ui/ash/launc... File chrome/browser/ui/ash/launcher/chrome_launcher_controller.h (right): https://codereview.chromium.org/2523053004/diff/1/chrome/browser/ui/ash/launc... chrome/browser/ui/ash/launcher/chrome_launcher_controller.h:148: int event_flags); On 2016/12/05 16:47:31, stevenjb wrote: > Same here, we seem to have a lot of existing calls to this. Done. https://codereview.chromium.org/2523053004/diff/1/chrome/browser/ui/ash/launc... File chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_browsertest.cc (right): https://codereview.chromium.org/2523053004/diff/1/chrome/browser/ui/ash/launc... chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_browsertest.cc:2426: } On 2016/12/03 01:55:27, Mr4D wrote: > Could you add a unit test to test that you get in deed multiple items? And also > how they get sorted? Yes, I can add another unit test for the restoring after pinning part in this CL - https://codereview.chromium.org/2297633002/. There is already an unit test that open and check multiple shelf windows per app - ShowInShelfWindowsWithWindowKeySet. Also, I added in the CL mentioned before unit tests for pinning support for items shown in shelf (e.g. PinRunningShowInShelfWindow). https://codereview.chromium.org/2523053004/diff/1/chrome/browser/ui/extension... File chrome/browser/ui/extensions/app_launch_params.h (right): https://codereview.chromium.org/2523053004/diff/1/chrome/browser/ui/extension... chrome/browser/ui/extensions/app_launch_params.h:45: std::string launch_id; On 2016/12/03 01:55:27, Mr4D wrote: > Can you elaborate what kind of id's these are? If they are "only stringified > numbers" why not using numbers? And if they can be in fact real strings - how > would they be sorted (so which one would go first)? This id is a real string and is set for a window when it is created - https://developer.chrome.com/apps/app_window#type-CreateWindowOptions. A previously pinned shelf item can be opened by the app using this id passed through launchData when handling the onLaunched event - https://developer.chrome.com/apps/app_runtime#event-onLaunched. https://codereview.chromium.org/2523053004/diff/1/extensions/browser/api/app_... File extensions/browser/api/app_runtime/app_runtime_api.cc (left): https://codereview.chromium.org/2523053004/diff/1/extensions/browser/api/app_... extensions/browser/api/app_runtime/app_runtime_api.cc:57: On 2016/12/03 01:55:27, Mr4D wrote: > Since nothing has changed in this file - could you re-insert the line? Done.
Thank you all for feedback, I uploaded a new patch set.
https://codereview.chromium.org/2523053004/diff/20001/apps/app_load_service.cc File apps/app_load_service.cc (right): https://codereview.chromium.org/2523053004/diff/20001/apps/app_load_service.c... apps/app_load_service.cc:120: extensions::SOURCE_LOAD_AND_LAUNCH); Please remove any formatting-only changes like this. https://codereview.chromium.org/2523053004/diff/20001/apps/launcher.cc File apps/launcher.cc (right): https://codereview.chromium.org/2523053004/diff/20001/apps/launcher.cc#newcod... apps/launcher.cc:434: } Don't duplucate all of this code, have LaunchPlatformAppWithCommandLine call this with launch_id = "" (or use helper functions). https://codereview.chromium.org/2523053004/diff/20001/apps/launcher.h File apps/launcher.h (right): https://codereview.chromium.org/2523053004/diff/20001/apps/launcher.h#newcode55 apps/launcher.h:55: // trace how the app is launched. Rather than repeating the above, the comment should be something like "As above but includes |launch_id| which will be passed to the app when launched."
Is there something in particular you want me to look at?
On 2016/12/06 23:09:08, benwells wrote: > Is there something in particular you want me to look at? Could you please take a look on the following files? apps/launcher.cc apps/launcher.h chrome/browser/ui/extensions/app_launch_params.h chrome/browser/ui/extensions/application_launch.cc When I uploaded the patch set, you were one of the suggested owner reviewers for these files. Feedback from you is needed before landing this.
Thank you Steven for review, I updated the CL. https://codereview.chromium.org/2523053004/diff/20001/apps/app_load_service.cc File apps/app_load_service.cc (right): https://codereview.chromium.org/2523053004/diff/20001/apps/app_load_service.c... apps/app_load_service.cc:120: extensions::SOURCE_LOAD_AND_LAUNCH); On 2016/12/06 16:42:07, stevenjb wrote: > Please remove any formatting-only changes like this. Done. https://codereview.chromium.org/2523053004/diff/20001/apps/launcher.cc File apps/launcher.cc (right): https://codereview.chromium.org/2523053004/diff/20001/apps/launcher.cc#newcod... apps/launcher.cc:434: } On 2016/12/06 16:42:07, stevenjb wrote: > Don't duplucate all of this code, have LaunchPlatformAppWithCommandLine call > this with launch_id = "" (or use helper functions). Yes, sorry for this, I fixed it now. https://codereview.chromium.org/2523053004/diff/20001/apps/launcher.h File apps/launcher.h (right): https://codereview.chromium.org/2523053004/diff/20001/apps/launcher.h#newcode55 apps/launcher.h:55: // trace how the app is launched. On 2016/12/06 16:42:07, stevenjb wrote: > Rather than repeating the above, the comment should be something like "As above > but includes |launch_id| which will be passed to the app when launched." Thank you, I updated the comment.
On 2016/12/07 08:13:55, Andra Paraschiv wrote: > On 2016/12/06 23:09:08, benwells wrote: > > Is there something in particular you want me to look at? > > Could you please take a look on the following files? > > apps/launcher.cc > apps/launcher.h > chrome/browser/ui/extensions/app_launch_params.h > chrome/browser/ui/extensions/application_launch.cc > > When I uploaded the patch set, you were one of the suggested owner reviewers for > these files. Feedback from you is needed before landing this. OK, great. When adding multiple reviewers it's customary to let them know what to review :) Taking a look now!
the files you asked me to look at lgtm
On 2016/12/09 06:12:38, benwells wrote: > On 2016/12/07 08:13:55, Andra Paraschiv wrote: > > On 2016/12/06 23:09:08, benwells wrote: > > > Is there something in particular you want me to look at? > > > > Could you please take a look on the following files? > > > > apps/launcher.cc > > apps/launcher.h > > chrome/browser/ui/extensions/app_launch_params.h > > chrome/browser/ui/extensions/application_launch.cc > > > > When I uploaded the patch set, you were one of the suggested owner reviewers > for > > these files. Feedback from you is needed before landing this. > > OK, great. When adding multiple reviewers it's customary to let them know what > to review :) > > Taking a look now! Thank you benwells for review.
lgtm
https://codereview.chromium.org/2523053004/diff/60001/apps/launcher.cc File apps/launcher.cc (right): https://codereview.chromium.org/2523053004/diff/60001/apps/launcher.cc#newcod... apps/launcher.cc:413: launch_data->id.reset(new std::string(launch_id)); Rather than potentially allocating an empty string here, wrap this wit if(!launch_id.empty()) https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... File chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc (right): https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc:113: event_flags); Why use WithLaunchId with an empty launch_id() ? https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... File chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.cc (right): https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.cc:84: ui::EF_NONE); LaunchApp() ?
Thank you for feedback, Steven. I added the fixes. https://codereview.chromium.org/2523053004/diff/60001/apps/launcher.cc File apps/launcher.cc (right): https://codereview.chromium.org/2523053004/diff/60001/apps/launcher.cc#newcod... apps/launcher.cc:413: launch_data->id.reset(new std::string(launch_id)); On 2016/12/09 17:00:56, stevenjb wrote: > Rather than potentially allocating an empty string here, wrap this wit > if(!launch_id.empty()) Done. https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... File chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc (right): https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc:113: event_flags); On 2016/12/09 17:00:57, stevenjb wrote: > Why use WithLaunchId with an empty launch_id() ? Launch id should be non-empty if the window was created with showInShelf=true and the id in CreateWindowOptions was set. When the window is pinned, then closed, the launch_id is stored in the AppShorcutLauncherItem (https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/launcher/chrome_la...) https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... File chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.cc (right): https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.cc:84: ui::EF_NONE); On 2016/12/09 17:00:57, stevenjb wrote: > LaunchApp() ? Done.
https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... File chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc (right): https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc:113: event_flags); On 2016/12/12 14:28:22, Andra Paraschiv wrote: > On 2016/12/09 17:00:57, stevenjb wrote: > > Why use WithLaunchId with an empty launch_id() ? > > Launch id should be non-empty if the window was created with showInShelf=true > and the id in CreateWindowOptions was set. When the window is pinned, then > closed, the launch_id is stored in the AppShorcutLauncherItem > (https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/launcher/chrome_la...) > Let me be more clear: Why not use launcher_controller()->LaunchApp()?
https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... File chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc (right): https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc:113: event_flags); On 2016/12/13 19:31:14, stevenjb wrote: > On 2016/12/12 14:28:22, Andra Paraschiv wrote: > > On 2016/12/09 17:00:57, stevenjb wrote: > > > Why use WithLaunchId with an empty launch_id() ? > > > > Launch id should be non-empty if the window was created with showInShelf=true > > and the id in CreateWindowOptions was set. When the window is pinned, then > > closed, the launch_id is stored in the AppShorcutLauncherItem > > > (https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/launcher/chrome_la...) > > > > Let me be more clear: > > Why not use launcher_controller()->LaunchApp()? In this case, if we use LaunchApp instead of LaunchAppWithLaunchId, the launch_id parameter that will be passed down the following call list will be empty: AppShorcutLauncherItemController::Launch ChromeLauncherController::LaunchApp(app_id, ...) LauncherControllerHelper::LaunchApp(app_id, ...) LauncherControllerHelper::LaunchAppWithLaunchId(app_id, ""/*launch_id*/, ...) <- in this method, the launch_id value will be used to set the launch_id application parameter (params.launch_id). This parameter value will be used to set the launchData.id. In order to restore a pinned window that was closed before, we should pass the launch_id parameter value (that can be non-empty for showInShelf=true windows) down through the call list to the onLaunched event launchData.id - https://developer.chrome.com/apps/app_runtime#event-onLaunched. This way, if the id is not empty, only the current pinned window will be restored and not the whole set of windows that have the same app_id. Thank you.
lgtm https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... File chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc (right): https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc:113: event_flags); On 2016/12/14 14:59:38, Andra Paraschiv wrote: > On 2016/12/13 19:31:14, stevenjb wrote: > > On 2016/12/12 14:28:22, Andra Paraschiv wrote: > > > On 2016/12/09 17:00:57, stevenjb wrote: > > > > Why use WithLaunchId with an empty launch_id() ? > > > > > > Launch id should be non-empty if the window was created with > showInShelf=true > > > and the id in CreateWindowOptions was set. When the window is pinned, then > > > closed, the launch_id is stored in the AppShorcutLauncherItem > > > > > > (https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/launcher/chrome_la...) > > > > > > > Let me be more clear: > > > > Why not use launcher_controller()->LaunchApp()? > > In this case, if we use LaunchApp instead of LaunchAppWithLaunchId, the > launch_id parameter that will be passed down the following call list will be > empty: > > AppShorcutLauncherItemController::Launch > ChromeLauncherController::LaunchApp(app_id, ...) > LauncherControllerHelper::LaunchApp(app_id, ...) > LauncherControllerHelper::LaunchAppWithLaunchId(app_id, ""/*launch_id*/, ...) <- > in this method, the launch_id value will be used to set the launch_id > application parameter (params.launch_id). This parameter value will be used to > set the launchData.id. > > In order to restore a pinned window that was closed before, we should pass the > launch_id parameter value (that can be non-empty for showInShelf=true windows) > down through the call list to the onLaunched event launchData.id - > https://developer.chrome.com/apps/app_runtime#event-onLaunched. This way, if the > id is not empty, only the current pinned window will be restored and not the > whole set of windows that have the same app_id. > > Thank you. Ugh. My apologies. I was somehow reading that as LaunchId() not launch_id(). Sorry about that, too many code reviews.
https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... File chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc (right): https://codereview.chromium.org/2523053004/diff/60001/chrome/browser/ui/ash/l... chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc:113: event_flags); On 2016/12/14 18:40:21, stevenjb wrote: > On 2016/12/14 14:59:38, Andra Paraschiv wrote: > > On 2016/12/13 19:31:14, stevenjb wrote: > > > On 2016/12/12 14:28:22, Andra Paraschiv wrote: > > > > On 2016/12/09 17:00:57, stevenjb wrote: > > > > > Why use WithLaunchId with an empty launch_id() ? > > > > > > > > Launch id should be non-empty if the window was created with > > showInShelf=true > > > > and the id in CreateWindowOptions was set. When the window is pinned, then > > > > closed, the launch_id is stored in the AppShorcutLauncherItem > > > > > > > > > > (https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/launcher/chrome_la...) > > > > > > > > > > Let me be more clear: > > > > > > Why not use launcher_controller()->LaunchApp()? > > > > In this case, if we use LaunchApp instead of LaunchAppWithLaunchId, the > > launch_id parameter that will be passed down the following call list will be > > empty: > > > > AppShorcutLauncherItemController::Launch > > ChromeLauncherController::LaunchApp(app_id, ...) > > LauncherControllerHelper::LaunchApp(app_id, ...) > > LauncherControllerHelper::LaunchAppWithLaunchId(app_id, ""/*launch_id*/, ...) > <- > > in this method, the launch_id value will be used to set the launch_id > > application parameter (params.launch_id). This parameter value will be used to > > set the launchData.id. > > > > In order to restore a pinned window that was closed before, we should pass the > > launch_id parameter value (that can be non-empty for showInShelf=true windows) > > down through the call list to the onLaunched event launchData.id - > > https://developer.chrome.com/apps/app_runtime#event-onLaunched. This way, if > the > > id is not empty, only the current pinned window will be restored and not the > > whole set of windows that have the same app_id. > > > > Thank you. > > Ugh. My apologies. I was somehow reading that as LaunchId() not launch_id(). > Sorry about that, too many code reviews. There's no problem, Steven, it's good we have this discussions based on the review in order to understand which it's the right way to include the launch_id in the codebase. Thanks, and I also added a unit test for the restore part in this CL - https://codereview.chromium.org/2297633002/ .
The CQ bit was checked by andra.paraschiv@intel.com
The patchset sent to the CQ was uploaded after l-g-t-m from benwells@chromium.org, skuhne@chromium.org Link to the patchset: https://codereview.chromium.org/2523053004/#ps80001 (title: "Review v5")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481797646579520, "parent_rev": "0bf32cebd9d69d878528ff244fae6b14ff1f810f", "commit_rev": "e55dcae75ad15eda02d610d2628b9ef15b6cc29e"}
Message was sent while issue was closed.
Description was changed from ========== Enhance chrome.app.window API for shelf integration with restore support Enhance the chrome.app.window API for shelf integration with the possibility of restoring showInShelf=true pinned windows. Based on https://codereview.chromium.org/2341643002 Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>; BUG=610299 TEST = https://codereview.chromium.org/2297633002 ========== to ========== Enhance chrome.app.window API for shelf integration with restore support Enhance the chrome.app.window API for shelf integration with the possibility of restoring showInShelf=true pinned windows. Based on https://codereview.chromium.org/2341643002 Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>; BUG=610299 TEST = https://codereview.chromium.org/2297633002 Review-Url: https://codereview.chromium.org/2523053004 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Enhance chrome.app.window API for shelf integration with restore support Enhance the chrome.app.window API for shelf integration with the possibility of restoring showInShelf=true pinned windows. Based on https://codereview.chromium.org/2341643002 Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>; BUG=610299 TEST = https://codereview.chromium.org/2297633002 Review-Url: https://codereview.chromium.org/2523053004 ========== to ========== Enhance chrome.app.window API for shelf integration with restore support Enhance the chrome.app.window API for shelf integration with the possibility of restoring showInShelf=true pinned windows. Based on https://codereview.chromium.org/2341643002 Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>; BUG=610299 TEST = https://codereview.chromium.org/2297633002 Committed: https://crrev.com/4e4fb8bbebc3d8f3e38130021daec21b760c5840 Cr-Commit-Position: refs/heads/master@{#438803} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/4e4fb8bbebc3d8f3e38130021daec21b760c5840 Cr-Commit-Position: refs/heads/master@{#438803} |