Commit 3bdb9041 authored by Mark Schillaci's avatar Mark Schillaci Committed by Chromium LUCI CQ

Accessibility Image Descriptions Port, Part VII - Incognito

This CL is the seventh part of an effort to port the existing Desktop
feature to Android. This feature enables a user to send an image to
Google to process to generate a descriptive alt text if a website
does not provide one.

Design Doc: go/2020-q1-android-image-descriptions
Slide Deck: go/clank-imageDescriptions
Launch Bug: 1057168

Original Desktop Design Doc for reference:
go/chrome-a11y-annotations-design

This is a conservative approach, we use a separate set of profile
Prefs for this feature rather than syncing with the Desktop Prefs.
This can be updated in time as needed.

----------

This CL adds the following:

- Syncs Regular and Incognito behavior as per new reqs
- Corrects unit tests for above
- Adds an AXMode observer to watch for screen readers being enabled

----------


AX-Relnotes: Android accessibility image descriptions behavior in incognito tabs will now match the regular tabs behavior for that profile.
Bug: 1057169
Change-Id: I27b6cb1c3da4cae441a6519815f7835049827452
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2585587Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarMark Schillaci <mschillaci@google.com>
Commit-Queue: Mark Schillaci <mschillaci@google.com>
Cr-Commit-Position: refs/heads/master@{#836174}
parent dc584e77
...@@ -482,7 +482,7 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate ...@@ -482,7 +482,7 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate
menu.findItem(R.id.get_image_descriptions_id).setVisible(true); menu.findItem(R.id.get_image_descriptions_id).setVisible(true);
int titleId = R.string.menu_stop_image_descriptions; int titleId = R.string.menu_stop_image_descriptions;
Profile profile = Profile.fromWebContents(currentTab.getWebContents()); Profile profile = Profile.getLastUsedRegularProfile();
// If image descriptions are not enabled, then we want the menu item to be "Get". // If image descriptions are not enabled, then we want the menu item to be "Get".
if (!ImageDescriptionsController.getInstance().imageDescriptionsEnabled(profile)) { if (!ImageDescriptionsController.getInstance().imageDescriptionsEnabled(profile)) {
titleId = R.string.menu_get_image_descriptions; titleId = R.string.menu_get_image_descriptions;
......
...@@ -50,7 +50,6 @@ import org.chromium.chrome.browser.multiwindow.MultiWindowModeStateDispatcher; ...@@ -50,7 +50,6 @@ import org.chromium.chrome.browser.multiwindow.MultiWindowModeStateDispatcher;
import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper; import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper;
import org.chromium.chrome.browser.preferences.Pref; import org.chromium.chrome.browser.preferences.Pref;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.profiles.ProfileJni;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.TabModel; import org.chromium.chrome.browser.tabmodel.TabModel;
import org.chromium.chrome.browser.tabmodel.TabModelSelector; import org.chromium.chrome.browser.tabmodel.TabModelSelector;
...@@ -112,8 +111,6 @@ public class AppMenuPropertiesDelegateUnitTest { ...@@ -112,8 +111,6 @@ public class AppMenuPropertiesDelegateUnitTest {
@Mock @Mock
private UserPrefs.Natives mUserPrefsJniMock; private UserPrefs.Natives mUserPrefsJniMock;
@Mock @Mock
private Profile.Natives mProfileJniMock;
@Mock
private Profile mProfile; private Profile mProfile;
@Mock @Mock
private PrefService mPrefService; private PrefService mPrefService;
...@@ -149,10 +146,8 @@ public class AppMenuPropertiesDelegateUnitTest { ...@@ -149,10 +146,8 @@ public class AppMenuPropertiesDelegateUnitTest {
mJniMocker.mock(ContentFeatureListImplJni.TEST_HOOKS, mContentFeatureListJniMock); mJniMocker.mock(ContentFeatureListImplJni.TEST_HOOKS, mContentFeatureListJniMock);
mJniMocker.mock(UserPrefsJni.TEST_HOOKS, mUserPrefsJniMock); mJniMocker.mock(UserPrefsJni.TEST_HOOKS, mUserPrefsJniMock);
mJniMocker.mock(ProfileJni.TEST_HOOKS, mProfileJniMock);
Profile.setLastUsedProfileForTesting(mProfile); Profile.setLastUsedProfileForTesting(mProfile);
Mockito.when(mUserPrefsJniMock.get(mProfile)).thenReturn(mPrefService); Mockito.when(mUserPrefsJniMock.get(mProfile)).thenReturn(mPrefService);
Mockito.when(mProfileJniMock.fromWebContents(any())).thenReturn(mProfile);
FeatureList.setTestCanUseDefaultsForTesting(); FeatureList.setTestCanUseDefaultsForTesting();
mAppMenuPropertiesDelegate = Mockito.spy(new AppMenuPropertiesDelegateImpl( mAppMenuPropertiesDelegate = Mockito.spy(new AppMenuPropertiesDelegateImpl(
......
...@@ -37,6 +37,7 @@ ...@@ -37,6 +37,7 @@
#include "base/android/jni_android.h" #include "base/android/jni_android.h"
#include "chrome/browser/image_descriptions/jni_headers/ImageDescriptionsController_jni.h" #include "chrome/browser/image_descriptions/jni_headers/ImageDescriptionsController_jni.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "ui/accessibility/platform/ax_platform_node.h"
#endif #endif
using LanguageInfo = language::UrlLanguageHistogram::LanguageInfo; using LanguageInfo = language::UrlLanguageHistogram::LanguageInfo;
...@@ -125,10 +126,21 @@ class ImageAnnotatorClient : public image_annotation::Annotator::Client { ...@@ -125,10 +126,21 @@ class ImageAnnotatorClient : public image_annotation::Annotator::Client {
} // namespace } // namespace
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
AccessibilityLabelsService::AccessibilityLabelsService(Profile* profile)
: profile_(profile) {}
AccessibilityLabelsService::~AccessibilityLabelsService() = default; AccessibilityLabelsService::~AccessibilityLabelsService() = default;
#else #else
// On Android we must add/remove a NetworkChangeObserver during construction/
// destruction to provide the "Only on Wi-Fi" functionality.
// We also add/remove an AXModeObserver to track users enabling a screenreader.
AccessibilityLabelsService::AccessibilityLabelsService(Profile* profile)
: profile_(profile) {
net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
ui::AXPlatformNode::AddAXModeObserver(this);
}
AccessibilityLabelsService::~AccessibilityLabelsService() { AccessibilityLabelsService::~AccessibilityLabelsService() {
net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this); net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
ui::AXPlatformNode::RemoveAXModeObserver(this);
} }
#endif #endif
...@@ -159,12 +171,6 @@ void AccessibilityLabelsService::InitOffTheRecordPrefs( ...@@ -159,12 +171,6 @@ void AccessibilityLabelsService::InitOffTheRecordPrefs(
prefs::kAccessibilityImageLabelsEnabled, false); prefs::kAccessibilityImageLabelsEnabled, false);
off_the_record_profile->GetPrefs()->SetBoolean( off_the_record_profile->GetPrefs()->SetBoolean(
prefs::kAccessibilityImageLabelsOptInAccepted, false); prefs::kAccessibilityImageLabelsOptInAccepted, false);
#if defined(OS_ANDROID)
off_the_record_profile->GetPrefs()->SetBoolean(
prefs::kAccessibilityImageLabelsEnabledAndroid, false);
off_the_record_profile->GetPrefs()->SetBoolean(
prefs::kAccessibilityImageLabelsOnlyOnWifi, true);
#endif
} }
void AccessibilityLabelsService::Init() { void AccessibilityLabelsService::Init() {
...@@ -191,13 +197,6 @@ void AccessibilityLabelsService::Init() { ...@@ -191,13 +197,6 @@ void AccessibilityLabelsService::Init() {
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr()));
} }
AccessibilityLabelsService::AccessibilityLabelsService(Profile* profile)
: profile_(profile) {
#if defined(OS_ANDROID)
net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
#endif
}
ui::AXMode AccessibilityLabelsService::GetAXMode() { ui::AXMode AccessibilityLabelsService::GetAXMode() {
ui::AXMode ax_mode = ui::AXMode ax_mode =
content::BrowserAccessibilityState::GetInstance()->GetAccessibilityMode(); content::BrowserAccessibilityState::GetInstance()->GetAccessibilityMode();
...@@ -304,6 +303,13 @@ void AccessibilityLabelsService::OnNetworkChanged( ...@@ -304,6 +303,13 @@ void AccessibilityLabelsService::OnNetworkChanged(
->SetImageLabelsModeForProfile(GetAndroidEnabledStatus(), profile_); ->SetImageLabelsModeForProfile(GetAndroidEnabledStatus(), profile_);
} }
void AccessibilityLabelsService::OnAXModeAdded(ui::AXMode mode) {
// When the AXMode changes (e.g. user turned on a screenreader), we want to
// (potentially) update the AXMode of all web contents for current profile.
content::BrowserAccessibilityState::GetInstance()
->SetImageLabelsModeForProfile(GetAndroidEnabledStatus(), profile_);
}
bool AccessibilityLabelsService::GetAndroidEnabledStatus() { bool AccessibilityLabelsService::GetAndroidEnabledStatus() {
// On Android, user has an option to toggle "only on wifi", so also check // On Android, user has an option to toggle "only on wifi", so also check
// the current connection type if necessary. // the current connection type if necessary.
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
#include "net/base/network_change_notifier.h" #include "net/base/network_change_notifier.h"
#include "ui/accessibility/ax_mode_observer.h"
#endif #endif
class Profile; class Profile;
...@@ -35,9 +36,11 @@ class PrefRegistrySyncable; ...@@ -35,9 +36,11 @@ class PrefRegistrySyncable;
class AccessibilityLabelsService class AccessibilityLabelsService
: public KeyedService : public KeyedService
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
// On Android, implement NetworkChangeObserver for "only on wifi" option. // On Android, implement NetworkChangeObserver for "only on wifi" option,
// and an AXModeObserver for detecting when a screen reader is enabled.
, ,
public net::NetworkChangeNotifier::NetworkChangeObserver public net::NetworkChangeNotifier::NetworkChangeObserver,
public ui::AXModeObserver
#endif #endif
{ {
...@@ -71,6 +74,9 @@ class AccessibilityLabelsService ...@@ -71,6 +74,9 @@ class AccessibilityLabelsService
void OnNetworkChanged( void OnNetworkChanged(
net::NetworkChangeNotifier::ConnectionType type) override; net::NetworkChangeNotifier::ConnectionType type) override;
// ui::AXModeObserver
void OnAXModeAdded(ui::AXMode mode) override;
bool GetAndroidEnabledStatus(); bool GetAndroidEnabledStatus();
#endif #endif
......
...@@ -116,7 +116,7 @@ public class ImageDescriptionsController { ...@@ -116,7 +116,7 @@ public class ImageDescriptionsController {
*/ */
public void onImageDescriptionsMenuItemSelected( public void onImageDescriptionsMenuItemSelected(
Context context, ModalDialogManager modalDialogManager, WebContents webContents) { Context context, ModalDialogManager modalDialogManager, WebContents webContents) {
Profile profile = Profile.fromWebContents(webContents); Profile profile = Profile.getLastUsedRegularProfile();
boolean enabledBeforeMenuItemSelected = imageDescriptionsEnabled(profile); boolean enabledBeforeMenuItemSelected = imageDescriptionsEnabled(profile);
if (enabledBeforeMenuItemSelected) { if (enabledBeforeMenuItemSelected) {
......
...@@ -41,7 +41,6 @@ import org.chromium.chrome.browser.preferences.ChromePreferenceKeys; ...@@ -41,7 +41,6 @@ import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
import org.chromium.chrome.browser.preferences.Pref; import org.chromium.chrome.browser.preferences.Pref;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager; import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.profiles.ProfileJni;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner; import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.components.prefs.PrefService; import org.chromium.components.prefs.PrefService;
import org.chromium.components.user_prefs.UserPrefs; import org.chromium.components.user_prefs.UserPrefs;
...@@ -67,9 +66,6 @@ public class ImageDescriptionsControllerTest extends DummyUiActivityTestCase { ...@@ -67,9 +66,6 @@ public class ImageDescriptionsControllerTest extends DummyUiActivityTestCase {
@Mock @Mock
private UserPrefs.Natives mUserPrefsJniMock; private UserPrefs.Natives mUserPrefsJniMock;
@Mock
private Profile.Natives mProfileJniMock;
@Mock @Mock
private Profile mProfile; private Profile mProfile;
...@@ -95,9 +91,6 @@ public class ImageDescriptionsControllerTest extends DummyUiActivityTestCase { ...@@ -95,9 +91,6 @@ public class ImageDescriptionsControllerTest extends DummyUiActivityTestCase {
Profile.setLastUsedProfileForTesting(mProfile); Profile.setLastUsedProfileForTesting(mProfile);
when(mUserPrefsJniMock.get(mProfile)).thenReturn(mPrefService); when(mUserPrefsJniMock.get(mProfile)).thenReturn(mPrefService);
mJniMocker.mock(ProfileJni.TEST_HOOKS, mProfileJniMock);
when(mProfileJniMock.fromWebContents(any())).thenReturn(mProfile);
mJniMocker.mock(ImageDescriptionsControllerJni.TEST_HOOKS, mControllerJniMock); mJniMocker.mock(ImageDescriptionsControllerJni.TEST_HOOKS, mControllerJniMock);
resetSharedPreferences(); resetSharedPreferences();
......
...@@ -52,7 +52,7 @@ public class ImageDescriptionsDialog ...@@ -52,7 +52,7 @@ public class ImageDescriptionsDialog
mModalDialogManager = modalDialogManager; mModalDialogManager = modalDialogManager;
mControllerDelegate = delegate; mControllerDelegate = delegate;
mWebContents = webContents; mWebContents = webContents;
mProfile = Profile.fromWebContents(mWebContents); mProfile = Profile.getLastUsedRegularProfile();
mContext = context; mContext = context;
// Set initial state. // Set initial state.
......
...@@ -44,7 +44,6 @@ import org.chromium.chrome.browser.preferences.ChromePreferenceKeys; ...@@ -44,7 +44,6 @@ import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
import org.chromium.chrome.browser.preferences.Pref; import org.chromium.chrome.browser.preferences.Pref;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager; import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.profiles.ProfileJni;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner; import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.components.browser_ui.modaldialog.AppModalPresenter; import org.chromium.components.browser_ui.modaldialog.AppModalPresenter;
import org.chromium.components.browser_ui.widget.RadioButtonWithDescription; import org.chromium.components.browser_ui.widget.RadioButtonWithDescription;
...@@ -72,9 +71,6 @@ public class ImageDescriptionsDialogTest extends DummyUiActivityTestCase { ...@@ -72,9 +71,6 @@ public class ImageDescriptionsDialogTest extends DummyUiActivityTestCase {
@Mock @Mock
private UserPrefs.Natives mUserPrefsJniMock; private UserPrefs.Natives mUserPrefsJniMock;
@Mock
private Profile.Natives mProfileJniMock;
@Mock @Mock
private Profile mProfile; private Profile mProfile;
...@@ -99,9 +95,6 @@ public class ImageDescriptionsDialogTest extends DummyUiActivityTestCase { ...@@ -99,9 +95,6 @@ public class ImageDescriptionsDialogTest extends DummyUiActivityTestCase {
Profile.setLastUsedProfileForTesting(mProfile); Profile.setLastUsedProfileForTesting(mProfile);
when(mUserPrefsJniMock.get(mProfile)).thenReturn(mPrefService); when(mUserPrefsJniMock.get(mProfile)).thenReturn(mPrefService);
mJniMocker.mock(ProfileJni.TEST_HOOKS, mProfileJniMock);
when(mProfileJniMock.fromWebContents(any())).thenReturn(mProfile);
mAppModalPresenter = new AppModalPresenter(getActivity()); mAppModalPresenter = new AppModalPresenter(getActivity());
mModalDialogManager = mModalDialogManager =
new ModalDialogManager(mAppModalPresenter, ModalDialogManager.ModalDialogType.APP); new ModalDialogManager(mAppModalPresenter, ModalDialogManager.ModalDialogType.APP);
......
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