Commit 67c9378b authored by Azeem Arshad's avatar Azeem Arshad Committed by Chromium LUCI CQ

[NearbyShare] Do not repeat onboarding from sharesheet action.

This CL makes sure that when sharing is initiated from the
sharesheet action, the feature is silently enabled and the user
is taken to the discovery page if they've already completed
the onboarding flow once.

Fixed this by exposing the onBoardingCompletePref value in
nearby share settings mojo interface and checking this value
in the send flow.

Fixed: 1157142
Change-Id: Ie65aa9ce22d8e8a38ce06e299b446865837ca60d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2586099Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarJosh Nohle <nohle@chromium.org>
Commit-Queue: Azeem Arshad <azeemarshad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836898}
parent aacf6c1f
......@@ -77,6 +77,11 @@ const std::vector<std::string> NearbyShareSettings::GetAllowedContacts() const {
return allowed_contacts;
}
bool NearbyShareSettings::IsOnboardingComplete() const {
return pref_service_->GetBoolean(
prefs::kNearbySharingOnboardingCompletePrefName);
}
bool NearbyShareSettings::IsDisabledByPolicy() const {
return !GetEnabled() && pref_service_->IsManagedPreference(
prefs::kNearbySharingEnabledPrefName);
......@@ -102,6 +107,11 @@ void NearbyShareSettings::SetEnabled(bool enabled) {
}
}
void NearbyShareSettings::IsOnboardingComplete(
base::OnceCallback<void(bool)> callback) {
std::move(callback).Run(IsOnboardingComplete());
}
void NearbyShareSettings::GetDeviceName(
base::OnceCallback<void(const std::string&)> callback) {
std::move(callback).Run(GetDeviceName());
......
......@@ -52,6 +52,7 @@ class NearbyShareSettings : public nearby_share::mojom::NearbyShareSettings,
nearby_share::mojom::DataUsage GetDataUsage() const;
nearby_share::mojom::Visibility GetVisibility() const;
const std::vector<std::string> GetAllowedContacts() const;
bool IsOnboardingComplete() const;
// Returns true if the feature is disabled by policy.
bool IsDisabledByPolicy() const;
......@@ -62,6 +63,7 @@ class NearbyShareSettings : public nearby_share::mojom::NearbyShareSettings,
observer) override;
void GetEnabled(base::OnceCallback<void(bool)> callback) override;
void SetEnabled(bool enabled) override;
void IsOnboardingComplete(base::OnceCallback<void(bool)> callback) override;
void GetDeviceName(
base::OnceCallback<void(const std::string&)> callback) override;
void ValidateDeviceName(
......
......@@ -94,10 +94,17 @@ Polymer({
},
/**
* Called when all settings values have been retrieved.
* Called when component is attached and all settings values have been
* retrieved.
*/
onSettingsRetrieved() {
if (this.settings.enabled) {
if (this.settings.isOnboardingComplete) {
if (!this.settings.enabled) {
// When a new share is triggered, if the user has completed onboarding
// previously, then silently enable the feature and continue to
// discovery page directly.
this.set('settings.enabled', true);
}
this.getViewManager_().switchView(Page.DISCOVERY);
} else {
this.getViewManager_().switchView(Page.ONBOARDING);
......
......@@ -17,7 +17,8 @@ cr.define('nearby_share', function() {
* deviceName:string,
* dataUsage:nearbyShare.mojom.DataUsage,
* visibility:nearbyShare.mojom.Visibility,
* allowedContacts:Array<string>
* allowedContacts:Array<string>,
* isOnboardingComplete:boolean,
* }}
*/
/* #export */ let NearbySettings;
......@@ -55,6 +56,7 @@ cr.define('nearby_share', function() {
this.nearbyShareSettings_.getDataUsage(),
this.nearbyShareSettings_.getVisibility(),
this.nearbyShareSettings_.getAllowedContacts(),
this.nearbyShareSettings_.isOnboardingComplete(),
])
.then((results) => {
this.set('settings.enabled', results[0].enabled);
......@@ -62,6 +64,7 @@ cr.define('nearby_share', function() {
this.set('settings.dataUsage', results[2].dataUsage);
this.set('settings.visibility', results[3].visibility);
this.set('settings.allowedContacts', results[4].allowedContacts);
this.set('settings.isOnboardingComplete', results[5].completed);
this.onSettingsRetrieved();
});
},
......
......@@ -70,6 +70,8 @@ interface NearbyShareSettings {
GetEnabled() => (bool enabled);
// Set the enabled state for the Nearby Share feature.
SetEnabled(bool enabled);
// Get the onboarding completion state for the Nearby Share feature.
IsOnboardingComplete() => (bool completed);
// Get the device name shown to a sender when this device is available as
// a share target. |device_name| is the new device name.
......
......@@ -18,6 +18,8 @@ import {FakeNearbyShareSettings} from './shared/fake_nearby_share_settings.m.js'
suite('ShareAppTest', function() {
/** @type {!NearbyShareAppElement} */
let shareAppElement;
/** @type {!nearbyShare.mojom.NearbyShareSettingsInterface} */
let fakeSettings;
/** @param {!string} page Page to check if it is active. */
function isPageActive(page) {
......@@ -29,10 +31,12 @@ suite('ShareAppTest', function() {
* This allows both sub-suites to share the same setup logic but with a
* different enabled state which changes the routing of the first view.
* @param {boolean} enabled The value of the enabled setting.
* @param {boolean=} isOnboardingComplete The value of the onboarding
* completion state.
*/
function sharedSetup(enabled) {
/** @type {!nearbyShare.mojom.NearbyShareSettingsInterface} */
let fakeSettings = new FakeNearbyShareSettings();
function sharedSetup(enabled, isOnboardingComplete) {
fakeSettings = new FakeNearbyShareSettings();
fakeSettings.setIsOnboardingCompleteForTest(!!isOnboardingComplete);
fakeSettings.setEnabled(enabled);
setNearbyShareSettingsForTesting(fakeSettings);
......@@ -68,13 +72,24 @@ suite('ShareAppTest', function() {
});
suite('DisabledTests', function() {
setup(function() {
sharedSetup(false);
});
teardown(sharedTeardown);
test(
'enables feature and opens discovery if onboarding is complete',
async function() {
sharedSetup(false, true);
assertEquals('NEARBY-SHARE-APP', shareAppElement.tagName);
assertEquals(null, shareAppElement.$$('.active'));
// We have to wait for settings to return from the mojo after which
// the app will route to the correct page.
await waitAfterNextRender(shareAppElement);
const enabledResponse = await fakeSettings.getEnabled();
assertTrue(enabledResponse && enabledResponse.enabled);
assertTrue(isPageActive('discovery'));
});
test('renders onboarding page when disabled', async function() {
sharedSetup(false);
assertEquals('NEARBY-SHARE-APP', shareAppElement.tagName);
assertEquals(null, shareAppElement.$$('.active'));
// We have to wait for settings to return from the mojo after which
......@@ -84,6 +99,7 @@ suite('ShareAppTest', function() {
});
test('changes page on event', async function() {
sharedSetup(false);
assertEquals('NEARBY-SHARE-APP', shareAppElement.tagName);
assertEquals(null, shareAppElement.$$('.active'));
// We have to wait for settings to return from the mojo after which
......
......@@ -28,6 +28,8 @@ cr.define('nearby_share', function() {
/** @private {!nearbyShare.mojom.DeviceNameValidationResult} */
this.nextDeviceNameResult_ =
nearbyShare.mojom.DeviceNameValidationResult.kValid;
/** @private {!boolean} */
this.isOnboardingComplete_ = false;
/** @private {Object} */
this.$ = {
close() {},
......@@ -64,11 +66,21 @@ cr.define('nearby_share', function() {
*/
setEnabled(enabled) {
this.enabled_ = enabled;
if (this.enabled_) {
this.isOnboardingComplete_ = true;
}
if (this.observer_) {
this.observer_.onEnabledChanged(enabled);
}
}
/**
* @return {!Promise<{completed: !boolean}>}
*/
async isOnboardingComplete() {
return {completed: this.isOnboardingComplete_};
}
/**
* @return {!Promise<{deviceName: !string}>}
*/
......@@ -157,6 +169,13 @@ cr.define('nearby_share', function() {
this.observer_.onAllowedContactsChanged(this.allowedContacts_);
}
}
/**
* @param { !boolean } completed
*/
setIsOnboardingCompleteForTest(completed) {
this.isOnboardingComplete_ = completed;
}
}
// #cr_define_end
return {FakeNearbyShareSettings: FakeNearbyShareSettings};
......
......@@ -32,6 +32,7 @@ suite('nearby-contact-visibility', () => {
deviceName: 'deviceName',
dataUsage: nearbyShare.mojom.DataUsage.kOnline,
visibility: nearbyShare.mojom.Visibility.kUnknown,
isOnboardingComplete: false,
allowedContacts: [],
};
......
......@@ -35,6 +35,7 @@ suite('nearby-onboarding-page', function() {
deviceName: deviceName,
dataUsage: nearbyShare.mojom.DataUsage.kOnline,
visibility: nearbyShare.mojom.Visibility.kAllContacts,
isOnboardingComplete: false,
allowedContacts: [],
};
document.body.appendChild(element);
......
......@@ -24,6 +24,7 @@ suite('nearby-visibility-page', function() {
deviceName: 'deviceName',
dataUsage: nearbyShare.mojom.DataUsage.kOnline,
visibility: nearbyShare.mojom.Visibility.kAllContacts,
isOnboardingComplete: false,
allowedContacts: [],
};
document.body.appendChild(visibility_page);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment