Skip to content

Commit

Permalink
arc: Add 'OPEN SETTINGS' button to the storage notification
Browse files Browse the repository at this point in the history
When a USB memory is plugged, we currently show "Removable
device detected" notification with one button, "OPEN FILES
APP". This CL adds one more button "OPEN SETTINGS" to the
notification when ARC is enabled for the user to allow the
user to control ARC apps' USB storage access settings.

UI mock: go/filesapp-arc-extdriveaccess (page 10)

BUG=936814
TEST=browser_tests --gtest_filter='FileManagerJsTest.DeviceHandlerTest'

Change-Id: I027d70a80e678ab86de4c192556f7890dd336d71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1585382
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659651}
  • Loading branch information
yusukes authored and Commit Bot committed May 14, 2019
1 parent 133d4ee commit bb2d5c5
Show file tree
Hide file tree
Showing 12 changed files with 216 additions and 11 deletions.
9 changes: 9 additions & 0 deletions chrome/app/chromeos_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -1727,12 +1727,21 @@
<message name="IDS_REMOVABLE_DEVICE_NAVIGATION_MESSAGE" desc="Text of notification message to navigate users to the Files app. If user clicks a button in the notification, the Files app opens.">
Explore the device's content in the Files app.
</message>
<message name="IDS_REMOVABLE_DEVICE_ALLOW_PLAY_STORE_ACCESS_MESSAGE" desc="Text of notification message to navigate users to the ARC settings page. If user clicks a button in the notification, the page opens.">
Allow Play Store applications to access this device via Settings.
</message>
<message name="IDS_REMOVABLE_DEVICE_PLAY_STORE_APPS_HAVE_ACCESS_MESSAGE" desc="Text of notification message to navigate users to the ARC settings page. If user clicks a button in the notification, the page opens.">
Play Store applications have access to this device.
</message>
<message name="IDS_REMOVABLE_DEVICE_NAVIGATION_MESSAGE_READONLY_POLICY" desc="Text of notification message to navigate users to the Files app when attaching a removable media, but the administrator's poilcy forbids writing data to them. If user clicks a button in the notification, the Files app opens.">
Explore the device's content in the Files app. The content is restricted by an admin and can’t be modified.
</message>
<message name="IDS_REMOVABLE_DEVICE_NAVIGATION_BUTTON_LABEL" desc="Text of a button label in a notification to navigate users to the Files app. If user clicks the button, the Files app opens.">
Open Files app
</message>
<message name="IDS_REMOVABLE_DEVICE_OPEN_SETTTINGS_BUTTON_LABEL" desc="Text of a button label in a notification to navigate users to the ARC settings page. If user clicks the button, the page opens.">
Open settings
</message>
<message name="IDS_REMOVABLE_DEVICE_IMPORT_MESSAGE" desc="Text of notification message to start the media import flow. If user clicks a button in the notification, the import flow begins.">
Back up media from the device using the Files app.
</message>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "chromeos/dbus/power/power_manager_client.h"
#include "chromeos/disks/disk.h"
#include "chromeos/login/login_state/login_state.h"
#include "components/arc/arc_prefs.h"
#include "components/arc/intent_helper/arc_intent_helper_bridge.h"
#include "components/drive/chromeos/file_system_interface.h"
#include "components/drive/drive_pref_names.h"
Expand Down Expand Up @@ -576,6 +577,9 @@ void EventRouter::ObserveEvents() {
crostini::prefs::kCrostiniEnabled,
base::BindRepeating(&EventRouter::OnCrostiniEnabledChanged,
weak_factory_.GetWeakPtr()));
pref_change_registrar_->Add(arc::prefs::kArcEnabled, callback);
pref_change_registrar_->Add(arc::prefs::kArcHasAccessToRemovableMedia,
callback);

chromeos::system::TimezoneSettings::GetInstance()->AddObserver(this);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
#include "chromeos/constants/chromeos_features.h"
#include "chromeos/settings/timezone_settings.h"
#include "components/account_id/account_id.h"
#include "components/arc/arc_prefs.h"
#include "components/drive/drive_pref_names.h"
#include "components/drive/event_logger.h"
#include "components/prefs/pref_service.h"
Expand Down Expand Up @@ -232,6 +233,9 @@ FileManagerPrivateGetPreferencesFunction::Run() {
result.timezone =
base::UTF16ToUTF8(chromeos::system::TimezoneSettings::GetInstance()
->GetCurrentTimezoneID());
result.arc_enabled = service->GetBoolean(arc::prefs::kArcEnabled);
result.arc_removable_media_access_enabled =
service->GetBoolean(arc::prefs::kArcHasAccessToRemovableMedia);

return RespondNow(OneArgument(result.ToValue()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -699,12 +699,18 @@ std::unique_ptr<base::DictionaryValue> GetFileManagerStrings() {
SET_STRING("REFRESH_BUTTON_LABEL", IDS_FILE_BROWSER_REFRESH_BUTTON_LABEL);
SET_STRING("REMOVABLE_DEVICE_DETECTION_TITLE",
IDS_REMOVABLE_DEVICE_DETECTION_TITLE);
SET_STRING("REMOVABLE_DEVICE_ALLOW_PLAY_STORE_ACCESS_MESSAGE",
IDS_REMOVABLE_DEVICE_ALLOW_PLAY_STORE_ACCESS_MESSAGE);
SET_STRING("REMOVABLE_DEVICE_PLAY_STORE_APPS_HAVE_ACCESS_MESSAGE",
IDS_REMOVABLE_DEVICE_PLAY_STORE_APPS_HAVE_ACCESS_MESSAGE);
SET_STRING("REMOVABLE_DEVICE_IMPORT_BUTTON_LABEL",
IDS_REMOVABLE_DEVICE_IMPORT_BUTTON_LABEL);
SET_STRING("REMOVABLE_DEVICE_IMPORT_MESSAGE",
IDS_REMOVABLE_DEVICE_IMPORT_MESSAGE);
SET_STRING("REMOVABLE_DEVICE_NAVIGATION_BUTTON_LABEL",
IDS_REMOVABLE_DEVICE_NAVIGATION_BUTTON_LABEL);
SET_STRING("REMOVABLE_DEVICE_OPEN_SETTTINGS_BUTTON_LABEL",
IDS_REMOVABLE_DEVICE_OPEN_SETTTINGS_BUTTON_LABEL);
SET_STRING("REMOVABLE_DEVICE_NAVIGATION_MESSAGE",
IDS_REMOVABLE_DEVICE_NAVIGATION_MESSAGE);
SET_STRING("REMOVABLE_DEVICE_NAVIGATION_MESSAGE_READONLY_POLICY",
Expand Down
2 changes: 2 additions & 0 deletions chrome/common/extensions/api/file_manager_private.idl
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,8 @@ dictionary Preferences {
boolean searchSuggestEnabled;
boolean use24hourClock;
DOMString timezone;
boolean arcEnabled;
boolean arcRemovableMediaAccessEnabled;
};

dictionary PreferencesChange {
Expand Down
4 changes: 3 additions & 1 deletion third_party/closure_compiler/externs/file_manager_private.js
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,9 @@ chrome.fileManagerPrivate.FileWatchEvent;
* cellularDisabled: boolean,
* searchSuggestEnabled: boolean,
* use24hourClock: boolean,
* timezone: string
* timezone: string,
* arcEnabled: boolean,
* arcRemovableMediaAccessEnabled: boolean
* }}
*/
chrome.fileManagerPrivate.Preferences;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bd8487d0c71df15d2c301449065d140a1da9e69d
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
79c37a54e384cabb2ccea2697453b364bd76cdd7
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
79c37a54e384cabb2ccea2697453b364bd76cdd7
110 changes: 101 additions & 9 deletions ui/file_manager/file_manager/background/js/device_handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class DeviceHandler extends cr.EventTarget {
chrome.notifications.onClicked.addListener(
this.onNotificationClicked_.bind(this));
chrome.notifications.onButtonClicked.addListener(
this.onNotificationClicked_.bind(this));
this.onNotificationButtonClicked_.bind(this));
}

/**
Expand Down Expand Up @@ -253,8 +253,20 @@ class DeviceHandler extends cr.EventTarget {
DeviceHandler.Notification.DEVICE_NAVIGATION_READONLY_POLICY.show(
/** @type {string} */ (metadata.devicePath));
} else {
DeviceHandler.Notification.DEVICE_NAVIGATION.show(
/** @type {string} */ (metadata.devicePath));
chrome.fileManagerPrivate.getPreferences(pref => {
if (!pref.arcEnabled) {
DeviceHandler.Notification.DEVICE_NAVIGATION.show(
/** @type {string} */ (metadata.devicePath));
} else if (pref.arcRemovableMediaAccessEnabled) {
DeviceHandler.Notification.DEVICE_NAVIGATION_APPS_HAVE_ACCESS
.show(
/** @type {string} */ (metadata.devicePath));
} else {
DeviceHandler.Notification.DEVICE_NAVIGATION_ALLOW_APP_ACCESS
.show(
/** @type {string} */ (metadata.devicePath));
}
});
}
}
});
Expand All @@ -266,31 +278,58 @@ class DeviceHandler extends cr.EventTarget {
}

/**
* Handles notification body or button click.
* Handles notification body click.
* @param {string} id ID of the notification.
* @private
*/
onNotificationClicked_(id) {
util.doIfPrimaryContext(() => {
this.onNotificationClickedInternal_(id);
this.onNotificationClickedInternal_(id, -1 /* index */);
});
}

/**
* Handles notification button click.
* @param {string} id ID of the notification.
* @param {number} index index of the button.
* @private
*/
onNotificationButtonClicked_(id, index) {
util.doIfPrimaryContext(() => {
this.onNotificationClickedInternal_(id, index);
});
}

/**
* @param {string} id ID of the notification.
* @param {number} index index of the button.
* @private
*/
onNotificationClickedInternal_(id) {
onNotificationClickedInternal_(id, index) {
const pos = id.indexOf(':');
const type = id.substr(0, pos);
const devicePath = id.substr(pos + 1);
if (type === 'deviceNavigation' || type === 'deviceFail') {
chrome.notifications.clear(id, () => {});
this.openMediaDirectory_(null, devicePath, null);
} else if (type === 'deviceImport') {
return;
}
if (type === 'deviceImport') {
chrome.notifications.clear(id, () => {});
this.openMediaDirectory_(null, devicePath, 'DCIM');
return;
}
if (type !== 'deviceNavigationAppAccess') {
return;
}
chrome.notifications.clear(id, () => {});
const secondButtonIndex = 1;
if (index === secondButtonIndex) {
chrome.fileManagerPrivate.openSettingsSubpage(
'storage/externalStoragePreferences');
return;
}
this.openMediaDirectory_(null, devicePath, null);
}

/**
Expand Down Expand Up @@ -350,8 +389,20 @@ DeviceHandler.Notification = class {
* @param {string=} opt_buttonLabel String ID of the button label.
* @param {boolean=} opt_isClickable True if the notification body is
* clickable.
* @param {string=} opt_additionalMessage String ID of additional message.
* @param {string=} opt_secondButtonLabel String ID of the second button
* label.
*/
constructor(prefix, title, message, opt_buttonLabel, opt_isClickable) {
constructor(
prefix, title, message, opt_buttonLabel, opt_isClickable,
opt_additionalMessage, opt_secondButtonLabel) {
// Check that second button is used with primary button, because
// notifications API is based in button index, so the second button index
// is always 1.
if (opt_secondButtonLabel) {
console.assert(opt_buttonLabel !== undefined);
}

/**
* Prefix of notification ID.
* @type {string}
Expand Down Expand Up @@ -382,6 +433,18 @@ DeviceHandler.Notification = class {
*/
this.isClickable = opt_isClickable || false;

/**
* String ID of additional message.
* @type {?string}
*/
this.additionalMessage = opt_additionalMessage || null;

/**
* String ID of second button label.
* @type {?string}
*/
this.secondButtonLabel = opt_secondButtonLabel || null;

/**
* Queue of API call.
* @type {AsyncUtil.Queue}
Expand Down Expand Up @@ -433,11 +496,16 @@ DeviceHandler.Notification = class {
showInternal_(notificationId, message, callback) {
const buttons =
this.buttonLabel ? [{title: str(this.buttonLabel)}] : undefined;
if (this.secondButtonLabel) {
buttons.push({title: str(this.secondButtonLabel)});
}
const additionalMessage =
this.additionalMessage ? (' ' + str(this.additionalMessage)) : '';
chrome.notifications.create(
notificationId, {
type: 'basic',
title: str(this.title),
message: message || str(this.message),
message: message || (str(this.message) + additionalMessage),
iconUrl: chrome.runtime.getURL('/common/images/icon96.png'),
buttons: buttons,
isClickable: this.isClickable
Expand Down Expand Up @@ -466,6 +534,30 @@ DeviceHandler.Notification = class {
}
};

/**
* @type {DeviceHandler.Notification}
* @const
*/
DeviceHandler.Notification.DEVICE_NAVIGATION_ALLOW_APP_ACCESS =
new DeviceHandler.Notification(
'deviceNavigationAppAccess', 'REMOVABLE_DEVICE_DETECTION_TITLE',
'REMOVABLE_DEVICE_NAVIGATION_MESSAGE',
'REMOVABLE_DEVICE_NAVIGATION_BUTTON_LABEL', true,
'REMOVABLE_DEVICE_ALLOW_PLAY_STORE_ACCESS_MESSAGE',
'REMOVABLE_DEVICE_OPEN_SETTTINGS_BUTTON_LABEL');

/**
* @type {DeviceHandler.Notification}
* @const
*/
DeviceHandler.Notification.DEVICE_NAVIGATION_APPS_HAVE_ACCESS =
new DeviceHandler.Notification(
'deviceNavigationAppAccess', 'REMOVABLE_DEVICE_DETECTION_TITLE',
'REMOVABLE_DEVICE_NAVIGATION_MESSAGE',
'REMOVABLE_DEVICE_NAVIGATION_BUTTON_LABEL', true,
'REMOVABLE_DEVICE_PLAY_STORE_APPS_HAVE_ACCESS_MESSAGE',
'REMOVABLE_DEVICE_OPEN_SETTTINGS_BUTTON_LABEL');

/**
* @type {DeviceHandler.Notification}
* @const
Expand Down
Loading

0 comments on commit bb2d5c5

Please sign in to comment.