Commit 3ffda819 authored by Sami Kyöstilä's avatar Sami Kyöstilä Committed by Chromium LUCI CQ

Revert "Accessibility Image Descriptions Port, Part VII - Incognito"

This reverts commit 3bdb9041.

Reason for revert: Seems to be causing crashes on Pixel 2: https://ci.chromium.org/ui/p/chrome/builders/ci/android-pixel2-perf/13030/overview

Original change's description:
> 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/+/2585587
> Reviewed-by: Theresa  <twellington@chromium.org>
> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
> Reviewed-by: Mark Schillaci <mschillaci@google.com>
> Commit-Queue: Mark Schillaci <mschillaci@google.com>
> Cr-Commit-Position: refs/heads/master@{#836174}

TBR=dmazzoni@chromium.org,twellington@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com,mschillaci@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1057169
Change-Id: Ib28af4808090c432219137456451aa1b9ce6c230
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2589933Reviewed-by: default avatarSami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836625}
parent 1d55bbad
...@@ -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.getLastUsedRegularProfile(); Profile profile = Profile.fromWebContents(currentTab.getWebContents());
// 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,6 +50,7 @@ import org.chromium.chrome.browser.multiwindow.MultiWindowModeStateDispatcher; ...@@ -50,6 +50,7 @@ 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;
...@@ -111,6 +112,8 @@ public class AppMenuPropertiesDelegateUnitTest { ...@@ -111,6 +112,8 @@ 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;
...@@ -146,8 +149,10 @@ public class AppMenuPropertiesDelegateUnitTest { ...@@ -146,8 +149,10 @@ 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,7 +37,6 @@ ...@@ -37,7 +37,6 @@
#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;
...@@ -126,21 +125,10 @@ class ImageAnnotatorClient : public image_annotation::Annotator::Client { ...@@ -126,21 +125,10 @@ 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
...@@ -171,6 +159,12 @@ void AccessibilityLabelsService::InitOffTheRecordPrefs( ...@@ -171,6 +159,12 @@ 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() {
...@@ -197,6 +191,13 @@ void AccessibilityLabelsService::Init() { ...@@ -197,6 +191,13 @@ 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();
...@@ -303,13 +304,6 @@ void AccessibilityLabelsService::OnNetworkChanged( ...@@ -303,13 +304,6 @@ 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,7 +16,6 @@ ...@@ -16,7 +16,6 @@
#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;
...@@ -36,11 +35,9 @@ class PrefRegistrySyncable; ...@@ -36,11 +35,9 @@ 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
{ {
...@@ -74,9 +71,6 @@ class AccessibilityLabelsService ...@@ -74,9 +71,6 @@ 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.getLastUsedRegularProfile(); Profile profile = Profile.fromWebContents(webContents);
boolean enabledBeforeMenuItemSelected = imageDescriptionsEnabled(profile); boolean enabledBeforeMenuItemSelected = imageDescriptionsEnabled(profile);
if (enabledBeforeMenuItemSelected) { if (enabledBeforeMenuItemSelected) {
......
...@@ -41,6 +41,7 @@ import org.chromium.chrome.browser.preferences.ChromePreferenceKeys; ...@@ -41,6 +41,7 @@ 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;
...@@ -66,6 +67,9 @@ public class ImageDescriptionsControllerTest extends DummyUiActivityTestCase { ...@@ -66,6 +67,9 @@ 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;
...@@ -91,6 +95,9 @@ public class ImageDescriptionsControllerTest extends DummyUiActivityTestCase { ...@@ -91,6 +95,9 @@ 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.getLastUsedRegularProfile(); mProfile = Profile.fromWebContents(mWebContents);
mContext = context; mContext = context;
// Set initial state. // Set initial state.
......
...@@ -44,6 +44,7 @@ import org.chromium.chrome.browser.preferences.ChromePreferenceKeys; ...@@ -44,6 +44,7 @@ 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;
...@@ -71,6 +72,9 @@ public class ImageDescriptionsDialogTest extends DummyUiActivityTestCase { ...@@ -71,6 +72,9 @@ 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;
...@@ -95,6 +99,9 @@ public class ImageDescriptionsDialogTest extends DummyUiActivityTestCase { ...@@ -95,6 +99,9 @@ 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