Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(89)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 269573010: Download and cache kiosk app extension when it is added via kiosk mamangement ui. (Closed)

Created:
6 years, 7 months ago by jennyz
Modified:
6 years, 7 months ago
Reviewers:
xiyuan
CC:
chromium-reviews, extensions-reviews_chromium.org, nkostylev+watch_chromium.org, oshima+watch_chromium.org, chromium-apps-reviews_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Visibility:
Public.

Description

Download and cache kiosk app extension when it is added via kiosk mamangement ui. BUG=369550 R=xiyuan@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=267988

Patch Set 1 #

Total comments: 16

Patch Set 2 : Add code review comments. #

Total comments: 4

Patch Set 3 : Reset external_cache_ is clean up, change crx file cache dir, etc. #

Total comments: 2

Patch Set 4 : Move GetKioskAppCrxCacheDir out of KioskAppDataDelegate. #

Total comments: 2

Patch Set 5 : Remove the change from chromeos_path.cc/h. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+112 lines, -7 lines) Patch
M chrome/browser/chromeos/app_mode/kiosk_app_manager.h View 1 2 3 7 chunks +19 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/app_mode/kiosk_app_manager.cc View 1 2 3 11 chunks +65 lines, -5 lines 0 comments Download
M chrome/browser/chromeos/extensions/external_cache.h View 2 chunks +10 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/extensions/external_cache.cc View 1 2 3 chunks +18 lines, -0 lines 0 comments Download

Messages

Total messages: 19 (0 generated)
jennyz
6 years, 7 months ago (2014-05-02 00:44:40 UTC) #1
xiyuan
https://codereview.chromium.org/269573010/diff/1/chrome/browser/chromeos/app_mode/kiosk_app_data.h File chrome/browser/chromeos/app_mode/kiosk_app_data.h (right): https://codereview.chromium.org/269573010/diff/1/chrome/browser/chromeos/app_mode/kiosk_app_data.h#newcode71 chrome/browser/chromeos/app_mode/kiosk_app_data.h:71: bool IsCrxCacheLoading() const; This probably should better be part ...
6 years, 7 months ago (2014-05-02 03:22:11 UTC) #2
jennyz
https://codereview.chromium.org/269573010/diff/1/chrome/browser/chromeos/app_mode/kiosk_app_data.h File chrome/browser/chromeos/app_mode/kiosk_app_data.h (right): https://codereview.chromium.org/269573010/diff/1/chrome/browser/chromeos/app_mode/kiosk_app_data.h#newcode71 chrome/browser/chromeos/app_mode/kiosk_app_data.h:71: bool IsCrxCacheLoading() const; On 2014/05/02 03:22:12, xiyuan wrote: > ...
6 years, 7 months ago (2014-05-02 17:46:54 UTC) #3
xiyuan
https://codereview.chromium.org/269573010/diff/20001/chrome/browser/chromeos/app_mode/kiosk_app_manager.h File chrome/browser/chromeos/app_mode/kiosk_app_manager.h (right): https://codereview.chromium.org/269573010/diff/20001/chrome/browser/chromeos/app_mode/kiosk_app_manager.h#newcode223 chrome/browser/chromeos/app_mode/kiosk_app_manager.h:223: base::FilePath cache_dir_; nit: no need to keep as a ...
6 years, 7 months ago (2014-05-02 18:33:50 UTC) #4
jennyz
Working on another pack to make tests happy. https://codereview.chromium.org/269573010/diff/20001/chrome/browser/chromeos/app_mode/kiosk_app_manager.h File chrome/browser/chromeos/app_mode/kiosk_app_manager.h (right): https://codereview.chromium.org/269573010/diff/20001/chrome/browser/chromeos/app_mode/kiosk_app_manager.h#newcode223 chrome/browser/chromeos/app_mode/kiosk_app_manager.h:223: base::FilePath ...
6 years, 7 months ago (2014-05-02 21:05:02 UTC) #5
xiyuan
Mostly good. https://codereview.chromium.org/269573010/diff/40001/chrome/browser/chromeos/app_mode/kiosk_app_data_delegate.h File chrome/browser/chromeos/app_mode/kiosk_app_data_delegate.h (right): https://codereview.chromium.org/269573010/diff/40001/chrome/browser/chromeos/app_mode/kiosk_app_data_delegate.h#newcode22 chrome/browser/chromeos/app_mode/kiosk_app_data_delegate.h:22: virtual void GetKioskAppCrxCacheDir(base::FilePath* cache_dir) = 0; This ...
6 years, 7 months ago (2014-05-02 22:19:44 UTC) #6
jennyz
https://codereview.chromium.org/269573010/diff/40001/chrome/browser/chromeos/app_mode/kiosk_app_data_delegate.h File chrome/browser/chromeos/app_mode/kiosk_app_data_delegate.h (right): https://codereview.chromium.org/269573010/diff/40001/chrome/browser/chromeos/app_mode/kiosk_app_data_delegate.h#newcode22 chrome/browser/chromeos/app_mode/kiosk_app_data_delegate.h:22: virtual void GetKioskAppCrxCacheDir(base::FilePath* cache_dir) = 0; On 2014/05/02 22:19:45, ...
6 years, 7 months ago (2014-05-02 22:31:01 UTC) #7
xiyuan
LGTM. Thank you.
6 years, 7 months ago (2014-05-02 22:39:23 UTC) #8
jennyz
The CQ bit was checked by jennyz@chromium.org
6 years, 7 months ago (2014-05-02 22:42:12 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jennyz@chromium.org/269573010/60001
6 years, 7 months ago (2014-05-02 22:42:32 UTC) #10
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-02 22:52:55 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium
6 years, 7 months ago (2014-05-02 22:52:56 UTC) #12
xiyuan
https://codereview.chromium.org/269573010/diff/60001/chromeos/chromeos_paths.cc File chromeos/chromeos_paths.cc (right): https://codereview.chromium.org/269573010/diff/60001/chromeos/chromeos_paths.cc#newcode71 chromeos/chromeos_paths.cc:71: case DIR_KIOSK_EXTENSIONS_CACHE: This is no longer needed, right?
6 years, 7 months ago (2014-05-02 22:59:28 UTC) #13
jennyz
https://codereview.chromium.org/269573010/diff/60001/chromeos/chromeos_paths.cc File chromeos/chromeos_paths.cc (right): https://codereview.chromium.org/269573010/diff/60001/chromeos/chromeos_paths.cc#newcode71 chromeos/chromeos_paths.cc:71: case DIR_KIOSK_EXTENSIONS_CACHE: On 2014/05/02 22:59:28, xiyuan wrote: > This ...
6 years, 7 months ago (2014-05-02 23:09:04 UTC) #14
jennyz
The CQ bit was checked by jennyz@chromium.org
6 years, 7 months ago (2014-05-02 23:09:56 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jennyz@chromium.org/269573010/80001
6 years, 7 months ago (2014-05-02 23:10:23 UTC) #16
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-03 00:17:18 UTC) #17
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel on tryserver.chromium
6 years, 7 months ago (2014-05-03 00:17:19 UTC) #18
jennyz
6 years, 7 months ago (2014-05-03 00:26:01 UTC) #19
Message was sent while issue was closed. 
Committed patchset #5 manually as r267988 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698