diff --git a/base/memory/shared_memory.h b/base/memory/shared_memory.h index 47c33522..9a41598 100644 --- a/base/memory/shared_memory.h +++ b/base/memory/shared_memory.h
@@ -208,7 +208,7 @@ // that takes ownership of the handle. As such, it's not valid to pass the // sample handle to the IPC subsystem twice. Returns an invalid handle on // failure. - SharedMemoryHandle GetReadOnlyHandle(); + SharedMemoryHandle GetReadOnlyHandle() const; // Returns an ID for the mapped region. This is ID of the SharedMemoryHandle // that was mapped. The ID is valid even after the SharedMemoryHandle is
diff --git a/base/memory/shared_memory_fuchsia.cc b/base/memory/shared_memory_fuchsia.cc index 5262e16..15211d9 100644 --- a/base/memory/shared_memory_fuchsia.cc +++ b/base/memory/shared_memory_fuchsia.cc
@@ -149,7 +149,7 @@ return handle.Duplicate(); } -SharedMemoryHandle SharedMemory::GetReadOnlyHandle() { +SharedMemoryHandle SharedMemory::GetReadOnlyHandle() const { zx_handle_t duped_handle; const int kNoWriteOrExec = ZX_DEFAULT_VMO_RIGHTS &
diff --git a/base/memory/shared_memory_mac.cc b/base/memory/shared_memory_mac.cc index d2bb5ec5b..e2735f7 100644 --- a/base/memory/shared_memory_mac.cc +++ b/base/memory/shared_memory_mac.cc
@@ -244,7 +244,7 @@ } } -SharedMemoryHandle SharedMemory::GetReadOnlyHandle() { +SharedMemoryHandle SharedMemory::GetReadOnlyHandle() const { if (shm_.type_ == SharedMemoryHandle::POSIX) { // We could imagine re-opening the file from /dev/fd, but that can't make it // readonly on Mac: https://codereview.chromium.org/27265002/#msg10.
diff --git a/base/memory/shared_memory_nacl.cc b/base/memory/shared_memory_nacl.cc index 148e6b0..442c0360 100644 --- a/base/memory/shared_memory_nacl.cc +++ b/base/memory/shared_memory_nacl.cc
@@ -130,7 +130,7 @@ } } -SharedMemoryHandle SharedMemory::GetReadOnlyHandle() { +SharedMemoryHandle SharedMemory::GetReadOnlyHandle() const { // Untrusted code can't create descriptors or handles, which is needed to // drop permissions. return SharedMemoryHandle();
diff --git a/base/memory/shared_memory_posix.cc b/base/memory/shared_memory_posix.cc index c148d34..f0e4f95 100644 --- a/base/memory/shared_memory_posix.cc +++ b/base/memory/shared_memory_posix.cc
@@ -363,7 +363,7 @@ } #endif // !defined(OS_ANDROID) -SharedMemoryHandle SharedMemory::GetReadOnlyHandle() { +SharedMemoryHandle SharedMemory::GetReadOnlyHandle() const { CHECK(readonly_shm_.IsValid()); return readonly_shm_.Duplicate(); }
diff --git a/base/memory/shared_memory_win.cc b/base/memory/shared_memory_win.cc index b05be75..942d9ea8 100644 --- a/base/memory/shared_memory_win.cc +++ b/base/memory/shared_memory_win.cc
@@ -338,7 +338,7 @@ return true; } -SharedMemoryHandle SharedMemory::GetReadOnlyHandle() { +SharedMemoryHandle SharedMemory::GetReadOnlyHandle() const { HANDLE result; ProcessHandle process = GetCurrentProcess(); if (!::DuplicateHandle(process, shm_.GetHandle(), process, &result,
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ExportWarningDialogFragment.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ExportWarningDialogFragment.java new file mode 100644 index 0000000..52090b2 --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ExportWarningDialogFragment.java
@@ -0,0 +1,51 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.chrome.browser.preferences.password; + +import android.app.Dialog; +import android.app.DialogFragment; +import android.content.DialogInterface; +import android.os.Bundle; +import android.support.v7.app.AlertDialog; + +import org.chromium.chrome.R; + +/** + * Shows the dialog that gives the user some tips for how to treat the exported passwords securely. + */ +public class ExportWarningDialogFragment extends DialogFragment { + // This handler is used to answer the user actions on the dialog. + private DialogInterface.OnClickListener mHandler; + + public void setExportWarningHandler(DialogInterface.OnClickListener handler) { + mHandler = handler; + } + + /** + * Opens the dialog with the warning and sets the button listener to a fragment identified by ID + * passed in arguments. + */ + @Override + public Dialog onCreateDialog(Bundle savedInstanceState) { + return new AlertDialog.Builder(getActivity(), R.style.SimpleDialog) + .setPositiveButton(R.string.save_password_preferences_export_action_title, mHandler) + .setNegativeButton(R.string.cancel, mHandler) + .setMessage(getActivity().getResources().getString( + R.string.settings_passwords_export_description)) + .create(); + } + + @Override + public void onCreate(Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + // If there is savedInstanceState, then the dialog is being recreated by Android and will + // lack the necessary callbacks. Dismiss immediately as the settings page will need to be + // recreated anyway. + if (savedInstanceState != null) { + dismiss(); + return; + } + } +}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java index 44781481..48850618 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
@@ -5,7 +5,6 @@ package org.chromium.chrome.browser.preferences.password; import android.app.Fragment; -import android.app.KeyguardManager; import android.content.ClipData; import android.content.ClipboardManager; import android.content.Context; @@ -86,7 +85,6 @@ public static final String VIEW_PASSWORDS = "view-passwords"; private ClipboardManager mClipboard; - private KeyguardManager mKeyguardManager; private Bundle mExtras; private View mView; private boolean mViewButtonPressed; @@ -132,9 +130,6 @@ usernameView.findViewById(R.id.password_entry_editor_row_data); usernameDataView.setText(name); hookupCopyUsernameButton(usernameView); - mKeyguardManager = - (KeyguardManager) getActivity().getApplicationContext().getSystemService( - Context.KEYGUARD_SERVICE); if (ReauthenticationManager.isReauthenticationApiAvailable()) { hidePassword(); hookupPasswordButtons(); @@ -360,7 +355,8 @@ copyPasswordButton.setOnClickListener(new View.OnClickListener() { @Override public void onClick(View v) { - if (!mKeyguardManager.isKeyguardSecure()) { + if (!ReauthenticationManager.isScreenLockSetUp( + getActivity().getApplicationContext())) { Toast.makeText(getActivity().getApplicationContext(), R.string.password_entry_editor_set_lock_screen, Toast.LENGTH_LONG) .show(); @@ -379,7 +375,8 @@ public void onClick(View v) { TextView passwordView = (TextView) mView.findViewById(R.id.password_entry_editor_password); - if (!mKeyguardManager.isKeyguardSecure()) { + if (!ReauthenticationManager.isScreenLockSetUp( + getActivity().getApplicationContext())) { Toast.makeText(getActivity().getApplicationContext(), R.string.password_entry_editor_set_lock_screen, Toast.LENGTH_LONG) .show();
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordReauthenticationFragment.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordReauthenticationFragment.java index 5b26043..5643bda 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordReauthenticationFragment.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordReauthenticationFragment.java
@@ -14,6 +14,8 @@ import android.os.Build.VERSION_CODES; import android.os.Bundle; +import org.chromium.base.VisibleForTesting; + /** Show the lock screen confirmation and lock the screen. */ public class PasswordReauthenticationFragment extends Fragment { // The key for the description argument, which is used to retrieve an explanation of the @@ -49,6 +51,7 @@ /** * Prevent calling the {@link #lockDevice} method in {@link #onCreate}. */ + @VisibleForTesting public static void preventLockingForTesting() { sPreventLockDevice = true; }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ReauthenticationManager.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ReauthenticationManager.java index 4f00db0..f9a1859 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ReauthenticationManager.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/ReauthenticationManager.java
@@ -7,6 +7,8 @@ import android.app.Fragment; import android.app.FragmentManager; import android.app.FragmentTransaction; +import android.app.KeyguardManager; +import android.content.Context; import android.os.Build; import android.os.Bundle; import android.view.View; @@ -18,6 +20,9 @@ * settings UI. */ public final class ReauthenticationManager { + // Used for various ways to override checks provided by this class. + public enum OverrideState { NOT_OVERRIDDEN, AVAILABLE, UNAVAILABLE } + // Useful for retrieving the fragment in tests. @VisibleForTesting public static final String FRAGMENT_TAG = "reauthentication-manager-fragment"; @@ -29,6 +34,14 @@ // means there was no successful reauthentication yet. private static long sLastReauthTimeMillis; + // Used in tests to override the result of checking for screen lock set-up. This allows the + // tests to be independent of a particular device configuration. + private static OverrideState sScreenLockSetUpOverride = OverrideState.NOT_OVERRIDDEN; + + // Used in tests to override the result of checking for availability of the screen-locking API. + // This allows the tests to be independent of a particular device configuration. + private static OverrideState sApiOverride = OverrideState.NOT_OVERRIDDEN; + /** * Stores the timestamp of last reauthentication of the user. */ @@ -36,12 +49,37 @@ sLastReauthTimeMillis = value; } + @VisibleForTesting + public static void setScreenLockSetUpOverride(OverrideState screenLockSetUpOverride) { + sScreenLockSetUpOverride = screenLockSetUpOverride; + } + + @VisibleForTesting + public static void setApiOverride(OverrideState apiOverride) { + // Ensure that tests don't accidentally try to launch the OS-provided lock screen. + if (apiOverride == OverrideState.AVAILABLE) { + PasswordReauthenticationFragment.preventLockingForTesting(); + } + + sApiOverride = apiOverride; + } + /** * Checks whether reauthentication is available on the device at all. * @return The result of the check. */ public static boolean isReauthenticationApiAvailable() { - return Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP; + switch (sApiOverride) { + case NOT_OVERRIDDEN: + return Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP; + case AVAILABLE: + return true; + case UNAVAILABLE: + return false; + } + // This branch is not reachable. + assert false; + return false; } /** @@ -76,4 +114,24 @@ && (System.currentTimeMillis() - sLastReauthTimeMillis) < VALID_REAUTHENTICATION_TIME_INTERVAL_MILLIS; } + + /** + * Checks whether the user set up screen lock so that it can be used for reauthentication. Can + * be overridden in tests. + * @param context The context to retrieve the KeyguardManager to find out. + */ + public static boolean isScreenLockSetUp(Context context) { + switch (sScreenLockSetUpOverride) { + case NOT_OVERRIDDEN: + return ((KeyguardManager) context.getSystemService(Context.KEYGUARD_SERVICE)) + .isKeyguardSecure(); + case AVAILABLE: + return true; + case UNAVAILABLE: + return false; + } + // This branch is not reachable. + assert false; + return false; + } }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java index 4611618..e95d349 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
@@ -4,6 +4,7 @@ package org.chromium.chrome.browser.preferences.password; +import android.content.DialogInterface; import android.content.Intent; import android.net.Uri; import android.os.Bundle; @@ -12,6 +13,7 @@ import android.preference.PreferenceCategory; import android.preference.PreferenceFragment; import android.preference.PreferenceScreen; +import android.support.v7.app.AlertDialog; import android.text.SpannableString; import android.text.style.ForegroundColorSpan; import android.view.Menu; @@ -33,6 +35,7 @@ import org.chromium.chrome.browser.preferences.PreferencesLauncher; import org.chromium.chrome.browser.preferences.TextMessagePreference; import org.chromium.ui.text.SpanApplier; +import org.chromium.ui.widget.Toast; /** * The "Save passwords" screen in Settings, which allows the user to enable or disable password @@ -49,6 +52,9 @@ // Used to pass the password id into a new activity. public static final String PASSWORD_LIST_ID = "id"; + // The key for saving |mExportRequested| to instance bundle. + private static final String SAVED_STATE_EXPORT_REQUESTED = "saved-state-export-requested"; + public static final String PREF_SAVE_PASSWORDS_SWITCH = "save_passwords_switch"; public static final String PREF_AUTOSIGNIN_SWITCH = "autosignin_switch"; @@ -69,6 +75,9 @@ private boolean mNoPasswords; private boolean mNoPasswordExceptions; + // True if the user triggered the password export flow and this fragment is waiting for the + // result of the user's reauthentication. + private boolean mExportRequested; private Preference mLinkPref; private ChromeSwitchPreference mSavePasswordsSwitch; private ChromeBaseCheckBoxPreference mAutoSignInSwitch; @@ -80,9 +89,15 @@ getActivity().setTitle(R.string.prefs_saved_passwords); setPreferenceScreen(getPreferenceManager().createPreferenceScreen(getActivity())); PasswordManagerHandlerProvider.getInstance().addObserver(this); - if (ChromeFeatureList.isEnabled(EXPORT_PASSWORDS)) { + if (ChromeFeatureList.isEnabled(EXPORT_PASSWORDS) + && ReauthenticationManager.isReauthenticationApiAvailable()) { setHasOptionsMenu(true); } + if (savedInstanceState != null + && savedInstanceState.containsKey(SAVED_STATE_EXPORT_REQUESTED)) { + mExportRequested = + savedInstanceState.getBoolean(SAVED_STATE_EXPORT_REQUESTED, mExportRequested); + } } @Override @@ -95,12 +110,42 @@ public boolean onOptionsItemSelected(MenuItem item) { int id = item.getItemId(); if (id == R.id.export_passwords) { - // TODO(crbug.com/788701): Trigger the exporting dialogue here. + if (!ReauthenticationManager.isScreenLockSetUp(getActivity().getApplicationContext())) { + Toast.makeText(getActivity().getApplicationContext(), + R.string.password_export_set_lock_screen, Toast.LENGTH_LONG) + .show(); + } else if (ReauthenticationManager.authenticationStillValid()) { + exportAfterReauth(); + } else { + mExportRequested = true; + ReauthenticationManager.displayReauthenticationFragment( + R.string.lockscreen_description_export, getView().getId(), + getFragmentManager()); + } return true; } return super.onOptionsItemSelected(item); } + /** Continues with the password export flow after the user successfully reauthenticated. */ + private void exportAfterReauth() { + ExportWarningDialogFragment exportWarningDialogFragment = new ExportWarningDialogFragment(); + exportWarningDialogFragment.setExportWarningHandler(new DialogInterface.OnClickListener() { + /** On positive button response asks the parent to continue with the export flow. */ + @Override + public void onClick(DialogInterface dialog, int which) { + if (which == AlertDialog.BUTTON_POSITIVE) { + exportAfterWarning(); + } + } + }); + exportWarningDialogFragment.show(getFragmentManager(), null); + } + + private void exportAfterWarning() { + // TODO(crbug.com/788701): Start the export. + } + /** * Empty screen message when no passwords or exceptions are stored. */ @@ -225,10 +270,20 @@ @Override public void onResume() { super.onResume(); + if (mExportRequested) { + mExportRequested = false; + if (ReauthenticationManager.authenticationStillValid()) exportAfterReauth(); + } rebuildPasswordLists(); } @Override + public void onSaveInstanceState(Bundle outState) { + super.onSaveInstanceState(outState); + outState.putBoolean(SAVED_STATE_EXPORT_REQUESTED, mExportRequested); + } + + @Override public void onDestroy() { super.onDestroy(); PasswordManagerHandlerProvider.getInstance().removeObserver(this);
diff --git a/chrome/android/java/strings/android_chrome_strings.grd b/chrome/android/java/strings/android_chrome_strings.grd index f65de3b83..c4347f7 100644 --- a/chrome/android/java/strings/android_chrome_strings.grd +++ b/chrome/android/java/strings/android_chrome_strings.grd
@@ -473,6 +473,9 @@ <message name="IDS_PASSWORD_ENTRY_EDITOR_SET_LOCK_SCREEN" desc='Text prompting user to set device lock in order to view/copy passwords'> To view or copy your password here, set screen lock on this device. </message> + <message name="IDS_PASSWORD_EXPORT_SET_LOCK_SCREEN" desc="Text prompting user to set device lock in order to be able to export passwords"> + Turn on screen lock in Settings to export your passwords from this device + </message> <message name="IDS_PASSWORD_GENERATION_POPUP_CONTENT_DESCRIPTION" desc="The text announced by the screen reader when the password generation popup is shown."> Showing password generation popup </message> @@ -482,6 +485,9 @@ <message name="IDS_SAVE_PASSWORD_PREFERENCES_EXPORT_ACTION_DESCRIPTION" desc="The description of a menu item to trigger exporting passwords from the password settings."> Export passwords stored with Chrome </message> + <message name="IDS_SETTINGS_PASSWORDS_EXPORT_DESCRIPTION" desc="Text shown to the user who initiated exporting passwords, as a warning before any passwords have been exported."> + Your passwords will be visible to anyone who can see the exported passwords file. + </message> <!-- Lock Screen Fragment --> <message name="IDS_LOCKSCREEN_DESCRIPTION_COPY" desc="When a user attempts to copy a password for a particular website into clipboard in Chrome's settings, Chrome launches a lock screen to verify the user's identity and displays the following explanation."> @@ -490,6 +496,9 @@ <message name="IDS_LOCKSCREEN_DESCRIPTION_VIEW" desc="When a user attempts to view a password for a particular website in Chrome's settings, Chrome launches a lock screen to verify the user's identity and displays the following explanation."> Unlock to view your password </message> + <message name="IDS_LOCKSCREEN_DESCRIPTION_EXPORT" desc="When a user attempts to export saved passwords in Chrome's settings, Chrome launches a lock screen to verify the user's identity and displays the following explanation."> + Unlock to export your passwords + </message> <!-- Homepage preferences --> <message name="IDS_OPTIONS_HOMEPAGE_EDIT_TITLE" desc="The title of the screen that allows users to change the URL that opens when they tap on the home page button in the omnibox.">
diff --git a/chrome/android/java_sources.gni b/chrome/android/java_sources.gni index 123d912..cfca668c 100644 --- a/chrome/android/java_sources.gni +++ b/chrome/android/java_sources.gni
@@ -968,6 +968,7 @@ "java/src/org/chromium/chrome/browser/preferences/languages/LanguageItem.java", "java/src/org/chromium/chrome/browser/preferences/languages/LanguageListBaseAdapter.java", "java/src/org/chromium/chrome/browser/preferences/languages/LanguageListPreference.java", + "java/src/org/chromium/chrome/browser/preferences/password/ExportWarningDialogFragment.java", "java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java", "java/src/org/chromium/chrome/browser/preferences/password/PasswordManagerHandlerProvider.java", "java/src/org/chromium/chrome/browser/preferences/password/PasswordReauthenticationFragment.java",
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java index c7d1f24..776efb7d 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java
@@ -4,10 +4,18 @@ package org.chromium.chrome.browser.preferences.password; +import static android.support.test.espresso.action.ViewActions.click; +import static android.support.test.espresso.assertion.ViewAssertions.matches; +import static android.support.test.espresso.matcher.RootMatchers.withDecorView; +import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed; +import static android.support.test.espresso.matcher.ViewMatchers.isEnabled; +import static android.support.test.espresso.matcher.ViewMatchers.withContentDescription; +import static android.support.test.espresso.matcher.ViewMatchers.withText; + +import static org.hamcrest.Matchers.containsString; + import android.support.test.InstrumentationRegistry; import android.support.test.espresso.Espresso; -import android.support.test.espresso.action.ViewActions; -import android.support.test.espresso.matcher.ViewMatchers; import android.support.test.filters.SmallTest; import org.junit.Assert; @@ -20,6 +28,8 @@ import org.chromium.base.test.BaseJUnit4ClassRunner; import org.chromium.base.test.util.Feature; import org.chromium.chrome.R; +import org.chromium.chrome.browser.PasswordManagerHandler; +import org.chromium.chrome.browser.SavedPasswordEntry; import org.chromium.chrome.browser.preferences.ChromeBaseCheckBoxPreference; import org.chromium.chrome.browser.preferences.ChromeSwitchPreference; import org.chromium.chrome.browser.preferences.PrefServiceBridge; @@ -29,6 +39,8 @@ import org.chromium.chrome.test.util.browser.Features; import org.chromium.chrome.test.util.browser.Features.EnableFeatures; +import java.util.ArrayList; + /** * Tests for the "Save Passwords" settings screen. */ @@ -40,6 +52,60 @@ @Rule public TestRule mProcessor = new Features.InstrumentationProcessor(); + private static final class FakePasswordManagerHandler implements PasswordManagerHandler { + // This class has exactly one observer, set on construction and expected to last at least as + // long as this object (a good candidate is the owner of this object). + private final PasswordListObserver mObserver; + + // The faked contents of the password store to be displayed. + private ArrayList<SavedPasswordEntry> mSavedPasswords; + + public void setSavedPasswords(ArrayList<SavedPasswordEntry> savedPasswords) { + mSavedPasswords = savedPasswords; + } + + /** + * Constructor. + * @param PasswordListObserver The only observer. + */ + public FakePasswordManagerHandler(PasswordListObserver observer) { + mObserver = observer; + } + + // Pretends that the updated lists are |mSavedPasswords| for the saved passwords and an + // empty list for exceptions and immediately calls the observer. + @Override + public void updatePasswordLists() { + mObserver.passwordListAvailable(mSavedPasswords.size()); + } + + @Override + public SavedPasswordEntry getSavedPasswordEntry(int index) { + return mSavedPasswords.get(index); + } + + @Override + public String getSavedPasswordException(int index) { + // Define this method before starting to use it in tests. + assert false; + return null; + } + + @Override + public void removeSavedPasswordEntry(int index) { + // Define this method before starting to use it in tests. + assert false; + return; + } + + @Override + public void removeSavedPasswordException(int index) { + // Define this method before starting to use it in tests. + assert false; + return; + } + } + /** * Ensure that resetting of empty passwords list works. */ @@ -178,21 +244,135 @@ } /** - * Ensure that the export menu item is included and hidden behind the overflow menu. + * Check that the export menu item is included and hidden behind the overflow menu. Check that + * the menu displays the warning before letting the user export passwords. */ @Test @SmallTest @Feature({"Preferences"}) @EnableFeatures("password-export") public void testExportMenuItem() throws Exception { + ReauthenticationManager.setApiOverride(ReauthenticationManager.OverrideState.AVAILABLE); + ReauthenticationManager.setScreenLockSetUpOverride( + ReauthenticationManager.OverrideState.AVAILABLE); + final Preferences preferences = PreferencesTest.startPreferences(InstrumentationRegistry.getInstrumentation(), SavePasswordsPreferences.class.getName()); Espresso.openActionBarOverflowOrOptionsMenu( InstrumentationRegistry.getInstrumentation().getTargetContext()); - Espresso.onView(ViewMatchers.withText( - R.string.save_password_preferences_export_action_title)) - .perform(ViewActions.click()); + // Before tapping the menu item for export, pretend that the last successful + // reauthentication just happened. This will allow the export flow to continue. + ReauthenticationManager.setLastReauthTimeMillis(System.currentTimeMillis()); + Espresso.onView(withText(R.string.save_password_preferences_export_action_title)) + .perform(click()); + + Espresso.onView(withText(R.string.settings_passwords_export_description)) + .check(matches(isDisplayed())); + } + + /** + * Check whether the user is asked to set up a screen lock if attempting to export passwords. + */ + @Test + @SmallTest + @Feature({"Preferences"}) + @EnableFeatures("password-export") + public void testExportMenuItemNoLock() throws Exception { + ReauthenticationManager.setApiOverride(ReauthenticationManager.OverrideState.AVAILABLE); + ReauthenticationManager.setScreenLockSetUpOverride( + ReauthenticationManager.OverrideState.UNAVAILABLE); + + final Preferences preferences = + PreferencesTest.startPreferences(InstrumentationRegistry.getInstrumentation(), + SavePasswordsPreferences.class.getName()); + + Espresso.openActionBarOverflowOrOptionsMenu( + InstrumentationRegistry.getInstrumentation().getTargetContext()); + Espresso.onView(withText(R.string.save_password_preferences_export_action_title)) + .perform(click()); + Espresso.onView(withText(R.string.password_export_set_lock_screen)) + .inRoot(withDecorView(isEnabled())) + .check(matches(isDisplayed())); + } + + /** + * Check whether the user is asked to set up a screen lock if attempting to view passwords. + */ + @Test + @SmallTest + @Feature({"Preferences"}) + public void testViewPasswordNoLock() throws Exception { + FakePasswordManagerHandler handler = + new FakePasswordManagerHandler(PasswordManagerHandlerProvider.getInstance()); + ArrayList<SavedPasswordEntry> entries = new ArrayList<SavedPasswordEntry>(); + entries.add(new SavedPasswordEntry("https://example.com", "test user", "password")); + handler.setSavedPasswords(entries); + ThreadUtils.runOnUiThreadBlocking(new Runnable() { + @Override + public void run() { + PasswordManagerHandlerProvider.getInstance().setPasswordManagerHandlerForTest( + handler); + } + }); + + ReauthenticationManager.setApiOverride(ReauthenticationManager.OverrideState.AVAILABLE); + ReauthenticationManager.setScreenLockSetUpOverride( + ReauthenticationManager.OverrideState.UNAVAILABLE); + + final Preferences preferences = + PreferencesTest.startPreferences(InstrumentationRegistry.getInstrumentation(), + SavePasswordsPreferences.class.getName()); + + Espresso.onView(withText(containsString("test user"))).perform(click()); + + Espresso.onView(withContentDescription(R.string.password_entry_editor_copy_stored_password)) + .perform(click()); + Espresso.onView(withText(R.string.password_entry_editor_set_lock_screen)) + .inRoot(withDecorView(isEnabled())) + .check(matches(isDisplayed())); + + handler.setSavedPasswords(null); + } + + /** + * Check whether the user can view a saved password. + */ + @Test + @SmallTest + @Feature({"Preferences"}) + public void testViewPassword() throws Exception { + FakePasswordManagerHandler handler = + new FakePasswordManagerHandler(PasswordManagerHandlerProvider.getInstance()); + ArrayList<SavedPasswordEntry> entries = new ArrayList<SavedPasswordEntry>(); + entries.add(new SavedPasswordEntry("https://example.com", "test user", "test password")); + handler.setSavedPasswords(entries); + ThreadUtils.runOnUiThreadBlocking(new Runnable() { + @Override + public void run() { + PasswordManagerHandlerProvider.getInstance().setPasswordManagerHandlerForTest( + handler); + } + }); + + ReauthenticationManager.setApiOverride(ReauthenticationManager.OverrideState.AVAILABLE); + ReauthenticationManager.setScreenLockSetUpOverride( + ReauthenticationManager.OverrideState.AVAILABLE); + + final Preferences preferences = + PreferencesTest.startPreferences(InstrumentationRegistry.getInstrumentation(), + SavePasswordsPreferences.class.getName()); + + Espresso.onView(withText(containsString("test user"))).perform(click()); + + // Before tapping the view button, pretend that the last successful reauthentication just + // happened. This will allow the export flow to continue. + ReauthenticationManager.setLastReauthTimeMillis(System.currentTimeMillis()); + Espresso.onView(withContentDescription(R.string.password_entry_editor_view_stored_password)) + .perform(click()); + Espresso.onView(withText("test password")).check(matches(isDisplayed())); + + handler.setSavedPasswords(null); } }
diff --git a/content/browser/renderer_host/media/audio_input_sync_writer.cc b/content/browser/renderer_host/media/audio_input_sync_writer.cc index 18314cbc..681680b 100644 --- a/content/browser/renderer_host/media/audio_input_sync_writer.cc +++ b/content/browser/renderer_host/media/audio_input_sync_writer.cc
@@ -145,10 +145,16 @@ media::ComputeAudioInputBufferSizeChecked(params, shared_memory_segment_count); + if (!requested_memory_size.IsValid()) + return nullptr; + + // Make sure we can share the memory read-only with the client. + base::SharedMemoryCreateOptions shmem_options; + shmem_options.size = requested_memory_size.ValueOrDie(); + shmem_options.share_read_only = true; auto shared_memory = std::make_unique<base::SharedMemory>(); - if (!requested_memory_size.IsValid() || - !shared_memory->CreateAndMapAnonymous( - requested_memory_size.ValueOrDie())) { + if (!shared_memory->Create(shmem_options) || + !shared_memory->Map(shmem_options.size)) { return nullptr; }
diff --git a/content/renderer/media/mojo_audio_input_ipc_unittest.cc b/content/renderer/media/mojo_audio_input_ipc_unittest.cc index 4ff63b2d..1915654 100644 --- a/content/renderer/media/mojo_audio_input_ipc_unittest.cc +++ b/content/renderer/media/mojo_audio_input_ipc_unittest.cc
@@ -56,7 +56,7 @@ base::SyncSocket::Handle socket_handle, bool initially_muted) override { base::SharedMemory sh_mem( - mem_handle, /*read_only*/ false); // Releases the shared memory handle. + mem_handle, /*read_only*/ true); // Releases the shared memory handle. base::SyncSocket socket(socket_handle); // Releases the socket descriptor. GotOnStreamCreated(initially_muted); }
diff --git a/ios/chrome/browser/ui/toolbar/clean/toolbar_button.mm b/ios/chrome/browser/ui/toolbar/clean/toolbar_button.mm index 2f0eb44d..d38ebe90 100644 --- a/ios/chrome/browser/ui/toolbar/clean/toolbar_button.mm +++ b/ios/chrome/browser/ui/toolbar/clean/toolbar_button.mm
@@ -50,7 +50,8 @@ if (self.visibilityMask & ToolbarComponentVisibilityCompactWidthOnlyWhenEnabled) { newHiddenValue = !self.enabled; - } else if (self.visibilityMask & ToolbarComponentVisibilityCompactWidth) { + } else if (self.visibilityMask & ToolbarComponentVisibilityCompactWidth || + self.visibilityMask & ToolbarComponentVisibilityIPhoneOnly) { newHiddenValue = NO; } } else {
diff --git a/ios/chrome/browser/ui/toolbar/clean/toolbar_button_factory.mm b/ios/chrome/browser/ui/toolbar/clean/toolbar_button_factory.mm index f62dfdd..35aa4b2 100644 --- a/ios/chrome/browser/ui/toolbar/clean/toolbar_button_factory.mm +++ b/ios/chrome/browser/ui/toolbar/clean/toolbar_button_factory.mm
@@ -130,9 +130,7 @@ ? ToolbarControllerStyleLightMode : ToolbarControllerStyleIncognitoMode; ToolbarToolsMenuButton* toolsMenuButton = - [[ToolbarToolsMenuButton alloc] initWithFrame:CGRectZero - style:style - small:YES]; + [[ToolbarToolsMenuButton alloc] initWithFrame:CGRectZero style:style]; toolsMenuButton.accessibilityLabel = l10n_util::GetNSString(IDS_IOS_TOOLBAR_SETTINGS);
diff --git a/ios/chrome/browser/ui/toolbar/clean/toolbar_component_options.h b/ios/chrome/browser/ui/toolbar/clean/toolbar_component_options.h index 499470d..8f84539 100644 --- a/ios/chrome/browser/ui/toolbar/clean/toolbar_component_options.h +++ b/ios/chrome/browser/ui/toolbar/clean/toolbar_component_options.h
@@ -20,6 +20,8 @@ // Use this option when the component should be visible in CompactWidth only // if it's enabled. ToolbarComponentVisibilityCompactWidthOnlyWhenEnabled = 1 << 2, + // Use this option when the component should be always visible on iPhone only. + ToolbarComponentVisibilityIPhoneOnly = 1 << 3, }; #endif // IOS_CHROME_BROWSER_UI_TOOLBAR_CLEAN_TOOLBAR_COMPONENT_OPTIONS_H_
diff --git a/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.h b/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.h index 3f4f4c4..4854334 100644 --- a/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.h +++ b/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.h
@@ -17,10 +17,11 @@ extern const CGFloat kIncognitoToolbarBackgroundColor; // Stackview constraints. -extern const CGFloat kVerticalMargin; +extern const CGFloat kLocationBarVerticalMargin; +extern const CGFloat kButtonVerticalMargin; +extern const CGFloat kLeadingMarginIPad; extern const CGFloat kHorizontalMargin; extern const CGFloat kStackViewSpacing; -extern const CGFloat kTrailingMargin; // Location bar styling. extern const CGFloat kLocationBarBorderWidth; @@ -34,11 +35,14 @@ extern const CGFloat kProgressBarHeight; // Toolbar Buttons. +extern const CGFloat kToolsMenuButtonWidth; extern const CGFloat kToolbarButtonWidth; extern const CGFloat kToolbarButtonTitleNormalColor; extern const CGFloat kToolbarButtonTitleHighlightedColor; extern const CGFloat kIncognitoToolbarButtonTitleNormalColor; extern const CGFloat kIncognitoToolbarButtonTitleHighlightedColor; +extern const CGFloat kBackButtonImageInset; +extern const CGFloat kForwardButtonImageInset; // Maximum number of tabs displayed by the button containing the tab count. extern const NSInteger kShowTabStripButtonMaxTabCount;
diff --git a/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.mm b/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.mm index ad2dff32..20a0ddc 100644 --- a/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.mm +++ b/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.mm
@@ -11,10 +11,11 @@ const CGFloat kToolbarBackgroundColor = 0xF2F2F2; const CGFloat kIncognitoToolbarBackgroundColor = 0x505050; -const CGFloat kVerticalMargin = 7.0f; +const CGFloat kLocationBarVerticalMargin = 7.0f; +const CGFloat kButtonVerticalMargin = 4.0f; +const CGFloat kLeadingMarginIPad = 4.0f; const CGFloat kHorizontalMargin = 1.0f; -const CGFloat kStackViewSpacing = 2.0f; -const CGFloat kTrailingMargin = 10.0f; +const CGFloat kStackViewSpacing = -2.0f; const CGFloat kLocationBarBorderWidth = 1.0f; const CGFloat kLocationBarBorderColor = 0xD0D0D0; @@ -24,11 +25,14 @@ const CGFloat kProgressBarHeight = 2.0f; -const CGFloat kToolbarButtonWidth = 42.0f; +const CGFloat kToolsMenuButtonWidth = 44.0f; +const CGFloat kToolbarButtonWidth = 48.0f; const CGFloat kToolbarButtonTitleNormalColor = 0x555555; const CGFloat kIncognitoLocationBarBorderColor = 0x4C4C4C; const CGFloat kToolbarButtonTitleHighlightedColor = 0x4285F4; const CGFloat kIncognitoToolbarButtonTitleNormalColor = 0xEEEEEE; const CGFloat kIncognitoToolbarButtonTitleHighlightedColor = 0x888a8c; +const CGFloat kBackButtonImageInset = -9; +const CGFloat kForwardButtonImageInset = -7; const NSInteger kShowTabStripButtonMaxTabCount = 99;
diff --git a/ios/chrome/browser/ui/toolbar/clean/toolbar_tools_menu_button.h b/ios/chrome/browser/ui/toolbar/clean/toolbar_tools_menu_button.h index dc4ab5c2..ccda6ec 100644 --- a/ios/chrome/browser/ui/toolbar/clean/toolbar_tools_menu_button.h +++ b/ios/chrome/browser/ui/toolbar/clean/toolbar_tools_menu_button.h
@@ -20,7 +20,7 @@ // |frame| and the |style| of the toolbar it belongs to. - (instancetype)initWithFrame:(CGRect)frame style:(ToolbarControllerStyle)style - small:(BOOL)smallButton NS_DESIGNATED_INITIALIZER; + NS_DESIGNATED_INITIALIZER; - (instancetype)initWithFrame:(CGRect)frame NS_UNAVAILABLE;
diff --git a/ios/chrome/browser/ui/toolbar/clean/toolbar_tools_menu_button.mm b/ios/chrome/browser/ui/toolbar/clean/toolbar_tools_menu_button.mm index 8478475..fc62a54 100644 --- a/ios/chrome/browser/ui/toolbar/clean/toolbar_tools_menu_button.mm +++ b/ios/chrome/browser/ui/toolbar/clean/toolbar_tools_menu_button.mm
@@ -20,8 +20,6 @@ // Position of the topmost dot. const CGFloat kDotOffsetX = 22; const CGFloat kDotOffsetY = 18; -// Y offset of the topmost dot in case the button is the smaller version. -const CGFloat kSmallButtonOffset = 3; // Vertical space between dots. const CGFloat kVerticalSpaceBetweenDots = 6; // The duration of the animation, in seconds. @@ -60,9 +58,6 @@ BOOL animationOnGoing_; } -// Whether the button is the smaller version. -@property(nonatomic, assign, getter=isSmallButton) BOOL smallButton; - // Tints of the button. @property(nonatomic, strong) UIColor* normalStateTint; @property(nonatomic, strong) UIColor* highlightedStateTint; @@ -71,16 +66,13 @@ @implementation ToolbarToolsMenuButton -@synthesize smallButton = _smallButton; @synthesize normalStateTint = _normalStateTint; @synthesize highlightedStateTint = _highlightedStateTint; - (instancetype)initWithFrame:(CGRect)frame - style:(ToolbarControllerStyle)style - small:(BOOL)smallButton { + style:(ToolbarControllerStyle)style { if (self = [super initWithFrame:frame]) { style_ = style; - _smallButton = smallButton; pathLayers_ = [[NSMutableArray alloc] initWithCapacity:kNumberOfDots]; [self setTintColor:toolbar::NormalButtonTint(style_) @@ -158,11 +150,6 @@ self.tintColor = newTint; } -// Returns the Y offset of the topmost dot. -- (CGFloat)dotOffsetY { - return self.smallButton ? kDotOffsetY - kSmallButtonOffset : kDotOffsetY; -} - // Initializes the pathLayers. - (void)initializeShapeLayers { for (NSUInteger i = 0; i < pathLayers_.count; i++) { @@ -172,7 +159,7 @@ pathLayers_ = [[NSMutableArray alloc] initWithCapacity:kNumberOfDots]; for (NSUInteger i = 0; i < kNumberOfDots; i++) { const CGFloat x = kDotOffsetX; - const CGFloat y = [self dotOffsetY] + kVerticalSpaceBetweenDots * i; + const CGFloat y = kDotOffsetY + kVerticalSpaceBetweenDots * i; UIBezierPath* path = [UIBezierPath bezierPath]; [path moveToPoint:CGPointMake(x - kMaxWidthOfSegment * 0.5, y)];
diff --git a/ios/chrome/browser/ui/toolbar/clean/toolbar_view_controller.mm b/ios/chrome/browser/ui/toolbar/clean/toolbar_view_controller.mm index 694c191..44fcd944 100644 --- a/ios/chrome/browser/ui/toolbar/clean/toolbar_view_controller.mm +++ b/ios/chrome/browser/ui/toolbar/clean/toolbar_view_controller.mm
@@ -211,7 +211,6 @@ self.backButton, self.forwardButton, self.reloadButton, self.stopButton ]]; self.leadingStackView.translatesAutoresizingMaskIntoConstraints = NO; - self.leadingStackView.spacing = kStackViewSpacing; self.leadingStackView.distribution = UIStackViewDistributionFill; self.trailingStackView = [[UIStackView alloc] initWithArrangedSubviews:@[ @@ -275,15 +274,15 @@ ]]; // Stack views directly in view constraints. Main StackViews. - // Layout: |-[leadingStackView]-[locationBarContainer]-[trailingStackView]-|. + // Layout: |[leadingStackView]-[locationBarContainer]-[trailingStackView]|. + CGFloat leadingMargin = IsIPadIdiom() ? kLeadingMarginIPad : 0; UILayoutGuide* viewSafeAreaGuide = SafeAreaLayoutGuideForView(self.view); NSArray* stackViewRegularConstraints = @[ [self.leadingStackView.leadingAnchor constraintEqualToAnchor:viewSafeAreaGuide.leadingAnchor - constant:kHorizontalMargin], + constant:leadingMargin], [self.trailingStackView.trailingAnchor - constraintEqualToAnchor:viewSafeAreaGuide.trailingAnchor - constant:-kHorizontalMargin] + constraintEqualToAnchor:viewSafeAreaGuide.trailingAnchor] ]; [self.regularToolbarConstraints addObjectsFromArray:stackViewRegularConstraints]; @@ -300,17 +299,19 @@ @"trailingStack" : self.trailingStackView }, @{ - @"height" : @(kToolbarHeight - 2 * kVerticalMargin), - @"margin" : @(kVerticalMargin), - @"spacing" : @(kStackViewSpacing) + @"height" : @(kToolbarHeight - 2 * kButtonVerticalMargin), + @"margin" : @(kButtonVerticalMargin), + @"spacing" : @(kHorizontalMargin) }); // LocationBarContainer constraints. NSArray* locationBarRegularConstraints = @[ - [self.locationBarContainer.bottomAnchor - constraintEqualToAnchor:self.leadingStackView.bottomAnchor], - [self.locationBarContainer.topAnchor - constraintEqualToAnchor:self.leadingStackView.topAnchor] + [self.view.bottomAnchor + constraintEqualToAnchor:self.locationBarContainer.bottomAnchor + constant:kLocationBarVerticalMargin], + [self.locationBarContainer.heightAnchor + constraintEqualToConstant:kToolbarHeight - + 2 * kLocationBarVerticalMargin], ]; [self.regularToolbarConstraints addObjectsFromArray:locationBarRegularConstraints]; @@ -350,6 +351,10 @@ [buttonConstraints addObject:[self.backButton.widthAnchor constraintEqualToConstant:kToolbarButtonWidth]]; + if (!IsIPadIdiom()) { + self.backButton.imageEdgeInsets = + UIEdgeInsetsMakeDirected(0, 0, 0, kBackButtonImageInset); + } [self.backButton addTarget:self.dispatcher action:@selector(goBack) forControlEvents:UIControlEventTouchUpInside]; @@ -367,6 +372,10 @@ [buttonConstraints addObject:[self.forwardButton.widthAnchor constraintEqualToConstant:kToolbarButtonWidth]]; + if (!IsIPadIdiom()) { + self.forwardButton.imageEdgeInsets = + UIEdgeInsetsMakeDirected(0, kForwardButtonImageInset, 0, 0); + } [self.forwardButton addTarget:self.dispatcher action:@selector(goForward) forControlEvents:UIControlEventTouchUpInside]; @@ -380,8 +389,7 @@ self.tabSwitchStripButton = [self.buttonFactory tabSwitcherStripToolbarButton]; self.tabSwitchStripButton.visibilityMask = - ToolbarComponentVisibilityCompactWidth | - ToolbarComponentVisibilityRegularWidth; + ToolbarComponentVisibilityIPhoneOnly; [buttonConstraints addObject:[self.tabSwitchStripButton.widthAnchor constraintEqualToConstant:kToolbarButtonWidth]]; @@ -395,7 +403,7 @@ ToolbarComponentVisibilityRegularWidth; [buttonConstraints addObject:[self.toolsMenuButton.widthAnchor - constraintEqualToConstant:kToolbarButtonWidth]]; + constraintEqualToConstant:kToolsMenuButtonWidth]]; [self.toolsMenuButton addTarget:self.dispatcher action:@selector(showToolsMenu) forControlEvents:UIControlEventTouchUpInside]; @@ -709,7 +717,7 @@ constraintEqualToAnchor:self.view.trailingAnchor], [self.locationBarContainerStackView.topAnchor constraintEqualToAnchor:self.topSafeAnchor - constant:kVerticalMargin], + constant:kLocationBarVerticalMargin], ]; } return _expandedToolbarConstraints;
diff --git a/ios/chrome/browser/ui/toolbar/toolbar_controller.mm b/ios/chrome/browser/ui/toolbar/toolbar_controller.mm index d5e6b04..e1abae8 100644 --- a/ios/chrome/browser/ui/toolbar/toolbar_controller.mm +++ b/ios/chrome/browser/ui/toolbar/toolbar_controller.mm
@@ -202,8 +202,7 @@ toolsMenuButton_ = [[ToolbarToolsMenuButton alloc] initWithFrame:toolsMenuButtonFrame - style:style_ - small:NO]; + style:style_]; [toolsMenuButton_ addTarget:self.dispatcher action:@selector(showToolsMenu) forControlEvents:UIControlEventTouchUpInside];
diff --git a/media/audio/audio_device_thread.cc b/media/audio/audio_device_thread.cc index 7232151..09c3420 100644 --- a/media/audio/audio_device_thread.cc +++ b/media/audio/audio_device_thread.cc
@@ -31,6 +31,7 @@ AudioDeviceThread::Callback::Callback(const AudioParameters& audio_parameters, base::SharedMemoryHandle memory, + bool read_only_memory, uint32_t segment_length, uint32_t total_segments) : audio_parameters_(audio_parameters), @@ -41,7 +42,7 @@ // CHECK that the shared memory is large enough. The memory allocated // must be at least as large as expected. shared_memory_((CHECK(memory_length_ <= memory.GetSize()), memory), - false) { + read_only_memory) { CHECK_GT(total_segments_, 0u); thread_checker_.DetachFromThread(); }
diff --git a/media/audio/audio_device_thread.h b/media/audio/audio_device_thread.h index 6e1f22d..f5aa297 100644 --- a/media/audio/audio_device_thread.h +++ b/media/audio/audio_device_thread.h
@@ -31,6 +31,7 @@ public: Callback(const AudioParameters& audio_parameters, base::SharedMemoryHandle memory, + bool read_only_memory, uint32_t segment_length, uint32_t total_segments);
diff --git a/media/audio/audio_input_device.cc b/media/audio/audio_input_device.cc index afe0bf9..052da96 100644 --- a/media/audio/audio_input_device.cc +++ b/media/audio/audio_input_device.cc
@@ -68,7 +68,7 @@ const double bytes_per_ms_; size_t current_segment_id_; uint32_t last_buffer_id_; - std::vector<std::unique_ptr<media::AudioBus>> audio_buses_; + std::vector<std::unique_ptr<const media::AudioBus>> audio_buses_; CaptureCallback* capture_callback_; // Used for informing AudioInputDevice that we have gotten data, i.e. the @@ -369,6 +369,7 @@ : AudioDeviceThread::Callback( audio_parameters, memory, + /*read only*/ true, ComputeAudioInputBufferSize(audio_parameters, 1u), total_segments), bytes_per_ms_(static_cast<double>(audio_parameters.GetBytesPerSecond()) / @@ -387,12 +388,12 @@ shared_memory_.Map(memory_length_); // Create vector of audio buses by wrapping existing blocks of memory. - uint8_t* ptr = static_cast<uint8_t*>(shared_memory_.memory()); + const uint8_t* ptr = static_cast<const uint8_t*>(shared_memory_.memory()); for (uint32_t i = 0; i < total_segments_; ++i) { - media::AudioInputBuffer* buffer = - reinterpret_cast<media::AudioInputBuffer*>(ptr); + const media::AudioInputBuffer* buffer = + reinterpret_cast<const media::AudioInputBuffer*>(ptr); audio_buses_.push_back( - media::AudioBus::WrapMemory(audio_parameters_, buffer->audio)); + media::AudioBus::WrapReadOnlyMemory(audio_parameters_, buffer->audio)); ptr += segment_length_; } @@ -407,9 +408,10 @@ // The shared memory represents parameters, size of the data buffer and the // actual data buffer containing audio data. Map the memory into this // structure and parse out parameters and the data area. - uint8_t* ptr = static_cast<uint8_t*>(shared_memory_.memory()); + const uint8_t* ptr = static_cast<const uint8_t*>(shared_memory_.memory()); ptr += current_segment_id_ * segment_length_; - AudioInputBuffer* buffer = reinterpret_cast<AudioInputBuffer*>(ptr); + const AudioInputBuffer* buffer = + reinterpret_cast<const AudioInputBuffer*>(ptr); // Usually this will be equal but in the case of low sample rate (e.g. 8kHz, // the buffer may be bigger (on mac at least)). @@ -434,7 +436,7 @@ last_buffer_id_ = buffer->params.id; // Use pre-allocated audio bus wrapping existing block of shared memory. - media::AudioBus* audio_bus = audio_buses_[current_segment_id_].get(); + const media::AudioBus* audio_bus = audio_buses_[current_segment_id_].get(); // Regularly inform that we have gotten data. frames_since_last_got_data_callback_ += audio_bus->frames();
diff --git a/media/audio/audio_output_device.cc b/media/audio/audio_output_device.cc index 02d5431..8f9c1bbf 100644 --- a/media/audio/audio_output_device.cc +++ b/media/audio/audio_output_device.cc
@@ -461,8 +461,9 @@ : AudioDeviceThread::Callback( audio_parameters, memory, + /*read only*/ false, ComputeAudioOutputBufferSize(audio_parameters), - 1), + /*segment count*/ 1), render_callback_(render_callback), callback_num_(0) {}
diff --git a/media/base/audio_bus.cc b/media/base/audio_bus.cc index a3e1cb7..b490718a 100644 --- a/media/base/audio_bus.cc +++ b/media/base/audio_bus.cc
@@ -8,6 +8,7 @@ #include <stdint.h> #include <limits> +#include <utility> #include "base/logging.h" #include "base/memory/ptr_util.h" @@ -148,6 +149,16 @@ static_cast<float*>(data))); } +std::unique_ptr<const AudioBus> AudioBus::WrapReadOnlyMemory( + const AudioParameters& params, + const void* data) { + // Note: const_cast is generally dangerous but is used in this case since + // AudioBus accomodates both read-only and read/write use cases. A const + // AudioBus object is returned to ensure noone accidentally writes to the + // read-only data. + return WrapMemory(params, const_cast<void*>(data)); +} + void AudioBus::SetChannelData(int channel, float* data) { CHECK(can_set_channel_data_); CHECK(data);
diff --git a/media/base/audio_bus.h b/media/base/audio_bus.h index 6efaa588..b78eabc 100644 --- a/media/base/audio_bus.h +++ b/media/base/audio_bus.h
@@ -55,6 +55,9 @@ void* data); static std::unique_ptr<AudioBus> WrapMemory(const AudioParameters& params, void* data); + static std::unique_ptr<const AudioBus> WrapReadOnlyMemory( + const AudioParameters& params, + const void* data); // Based on the given number of channels and frames, calculates the minimum // required size in bytes of a contiguous block of memory to be passed to
diff --git a/media/mojo/services/mojo_audio_input_stream.cc b/media/mojo/services/mojo_audio_input_stream.cc index 00dbbbc..87b98972 100644 --- a/media/mojo/services/mojo_audio_input_stream.cc +++ b/media/mojo/services/mojo_audio_input_stream.cc
@@ -76,14 +76,15 @@ DCHECK(foreign_socket); base::SharedMemoryHandle foreign_memory_handle = - base::SharedMemory::DuplicateHandle(shared_memory->handle()); + shared_memory->GetReadOnlyHandle(); if (!base::SharedMemory::IsHandleValid(foreign_memory_handle)) { OnStreamError(/*not used*/ 0); return; } mojo::ScopedSharedBufferHandle buffer_handle = mojo::WrapSharedMemoryHandle( - foreign_memory_handle, shared_memory->requested_size(), false); + foreign_memory_handle, shared_memory->requested_size(), + /*read_only*/ true); mojo::ScopedHandle socket_handle = mojo::WrapPlatformFile(foreign_socket->Release());
diff --git a/media/mojo/services/mojo_audio_input_stream_unittest.cc b/media/mojo/services/mojo_audio_input_stream_unittest.cc index a1f3ebab..51e135a 100644 --- a/media/mojo/services/mojo_audio_input_stream_unittest.cc +++ b/media/mojo/services/mojo_audio_input_stream_unittest.cc
@@ -114,7 +114,7 @@ mojo::UnwrapSharedMemoryHandle(std::move(shared_buffer), &shmem_handle, &memory_length, &read_only), MOJO_RESULT_OK); - EXPECT_FALSE(read_only); + EXPECT_TRUE(read_only); buffer_ = base::MakeUnique<base::SharedMemory>(shmem_handle, read_only); GotNotification(initially_muted); @@ -173,7 +173,10 @@ base::WrapUnique(delegate_)); EXPECT_TRUE( base::CancelableSyncSocket::CreatePair(&local_, foreign_socket_.get())); - EXPECT_TRUE(mem_.CreateAnonymous(kShmemSize)); + base::SharedMemoryCreateOptions shmem_options; + shmem_options.size = kShmemSize; + shmem_options.share_read_only = true; + EXPECT_TRUE(mem_.Create(shmem_options)); EXPECT_CALL(mock_delegate_factory_, MockCreateDelegate(NotNull())) .WillOnce(SaveArg<0>(&delegate_event_handler_)); }