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

Issue 6912024: Support for component extensions as apps on the new tab page. Added filebrowser. (Closed)

Created:
9 years, 7 months ago by dgozman%chromium.org
Modified:
9 years, 7 months ago
CC:
chromium-reviews, John Grabowski, achuith+watch_chromium.org, rginda+watch_chromium.org, arv (Not doing code reviews), dyu1, Paweł Hajdan Jr., estade+watch_chromium.org, anantha
Visibility:
Public.

Description

Support for component extensions as apps on the new tab page. Added filebrowser. BUG=chromium-os:14543 TEST=File Manager should be on NTP. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86621

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 5

Patch Set 3 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -10 lines) Patch
M chrome/browser/resources/file_manager/manifest.json View 1 2 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/resources/ntp/apps.js View 1 2 3 chunks +15 lines, -7 lines 0 comments Download
M chrome/browser/ui/webui/ntp/app_launcher_handler.cc View 1 2 4 chunks +9 lines, -3 lines 0 comments Download
M chrome/test/functional/ntp.py View 1 2 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
dgozman
Hello, I'm not sure whom to ask for review, so included both Jon and Erik. ...
9 years, 7 months ago (2011-05-03 15:33:28 UTC) #1
Evan Stade
lgtm (but I am not an owner) http://codereview.chromium.org/6912024/diff/1005/chrome/browser/resources/ntp/apps.js File chrome/browser/resources/ntp/apps.js (right): http://codereview.chromium.org/6912024/diff/1005/chrome/browser/resources/ntp/apps.js#newcode708 chrome/browser/resources/ntp/apps.js:708: if (app['is_component']) ...
9 years, 7 months ago (2011-05-03 17:02:55 UTC) #2
arv (Not doing code reviews)
LGTM
9 years, 7 months ago (2011-05-03 21:51:57 UTC) #3
Nirnimesh
http://codereview.chromium.org/6912024/diff/1005/chrome/test/functional/ntp.py File chrome/test/functional/ntp.py (right): http://codereview.chromium.org/6912024/diff/1005/chrome/test/functional/ntp.py#newcode22 chrome/test/functional/ntp.py:22: _EXPECTED_DEFAULT_APPS.append({u'name': u'File Manager'}) If you don't mind could you ...
9 years, 7 months ago (2011-05-03 22:06:50 UTC) #4
dgozman
http://codereview.chromium.org/6912024/diff/1005/chrome/browser/resources/ntp/apps.js File chrome/browser/resources/ntp/apps.js (right): http://codereview.chromium.org/6912024/diff/1005/chrome/browser/resources/ntp/apps.js#newcode708 chrome/browser/resources/ntp/apps.js:708: if (app['is_component']) { On 2011/05/03 17:02:55, Evan Stade wrote: ...
9 years, 7 months ago (2011-05-24 12:55:20 UTC) #5
Nirnimesh
http://codereview.chromium.org/6912024/diff/1005/chrome/test/functional/ntp.py File chrome/test/functional/ntp.py (right): http://codereview.chromium.org/6912024/diff/1005/chrome/test/functional/ntp.py#newcode22 chrome/test/functional/ntp.py:22: _EXPECTED_DEFAULT_APPS.append({u'name': u'File Manager'}) On 2011/05/24 12:55:21, dgozman wrote: > ...
9 years, 7 months ago (2011-05-24 17:29:42 UTC) #6
dgozman
9 years, 7 months ago (2011-05-25 11:07:25 UTC) #7
On 2011/05/24 17:29:42, Nirnimesh wrote:
> http://codereview.chromium.org/6912024/diff/1005/chrome/test/functional/ntp.py
> File chrome/test/functional/ntp.py (right):
> 
>
http://codereview.chromium.org/6912024/diff/1005/chrome/test/functional/ntp.p...
> chrome/test/functional/ntp.py:22: _EXPECTED_DEFAULT_APPS.append({u'name':
u'File
> Manager'})
> On 2011/05/24 12:55:21, dgozman wrote:
> > On 2011/05/03 22:06:50, Nirnimesh wrote:
> > > If you don't mind could you run these tests to confirm that everything
> passes
> > > please?
> > > 
> > > run_remote_tests.sh --target IP desktopui_PyAutoFunctionalTests
> > 
> > Ok, I run these tests. But they succeed, if I comment 'Get Started', so
there
> > are only 'Web Store' and 'File Manager'.
> > Do you know, is it ok, or it just works strange on my device?
> 
> Did you emerge chromeos-chrome after commenting 'Get started'?
> Anyway, commit it, and I'll let you know if something breaks. I don't expect
it
> to.

Yes, I did emerge. Thanks, I'll commit now.

Powered by Google App Engine
This is Rietveld 408576698