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

Issue 23477036: file_manager: getFileTasks() to return Drive apps iff all files are on Drive (Closed)

Created:
7 years, 3 months ago by satorux1
Modified:
7 years, 3 months ago
Reviewers:
mtomasz, benwells, kinaba
CC:
chromium-reviews, extensions-reviews_chromium.org, nkostylev+watch_chromium.org, rginda+watch_chromium.org, arv+watch_chromium.org, oshima+watch_chromium.org, chromium-apps-reviews_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Visibility:
Public.

Description

file_manager: getFileTasks() to return Drive apps iff all files are on Drive Previously, fileBrowserPrivate.getFileTasks() returned Drive app tasks regardless of whether files were on Drive or not, and the results were filtered in JS side with ".driveApp" field. It's much simpler for getFileTasks() to return Drive app tasks only if all files are on Drive. This way, we can get rid of the filtering logic in JS, as well as the ".driveApp" field. BUG=284242 TEST=Install Pixlr Editor from webstore; Launch Files.app; Confirm that Pixlr Editor is an option for a JPG file on Drive, but not an option for a JPG file on the local disk. TBR=benwells@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221230

Patch Set 1 #

Patch Set 2 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+29 lines, -25 lines) Patch
M chrome/browser/chromeos/extensions/file_manager/file_tasks.h View 3 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/extensions/file_manager/file_tasks.cc View 3 chunks +5 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/extensions/file_manager/file_tasks_unittest.cc View 6 chunks +21 lines, -11 lines 0 comments Download
M chrome/browser/resources/file_manager/js/file_tasks.js View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/common/extensions/api/file_browser_private.json View 1 chunk +0 lines, -4 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
satorux1
kinaba@ for everything mtomasz@ for JS changes benwells@ for chrome/common/extensions/api/file_browser_private.json
7 years, 3 months ago (2013-09-04 02:00:00 UTC) #1
mtomasz
js lgtm
7 years, 3 months ago (2013-09-04 02:05:38 UTC) #2
kinaba
lgtm
7 years, 3 months ago (2013-09-04 03:46:47 UTC) #3
satorux1
TBRing benwlles, as the change in file_browser_private.json is fairly minor.
7 years, 3 months ago (2013-09-04 08:00:32 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/satorux@chromium.org/23477036/5001
7 years, 3 months ago (2013-09-04 16:33:04 UTC) #5
commit-bot: I haz the power
7 years, 3 months ago (2013-09-04 18:32:18 UTC) #6
Message was sent while issue was closed.
Change committed as 221230

Powered by Google App Engine
This is Rietveld 408576698