diff --git a/DEPS b/DEPS index 34ed576..a8aa20c 100644 --- a/DEPS +++ b/DEPS
@@ -64,7 +64,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling PDFium # and whatever else without interference from each other. - 'pdfium_revision': 'defe54dbf60459972ff1c295af332ccdbbe15a70', + 'pdfium_revision': '53827cbbfebae66dd31f7aa30d3ee5c88716897a', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling openmax_dl # and whatever else without interference from each other. @@ -96,7 +96,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling catapult # and whatever else without interference from each other. - 'catapult_revision': '32bdd960945af3580a61f9640bbfe72677e458ac', + 'catapult_revision': '93941b45f0e4d47db31547f735f27375bf6b7da4', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling libFuzzer # and whatever else without interference from each other.
diff --git a/ash/shell.cc b/ash/shell.cc index 61379d5..6ef34b2b 100644 --- a/ash/shell.cc +++ b/ash/shell.cc
@@ -609,6 +609,9 @@ Shell::~Shell() { TRACE_EVENT0("shutdown", "ash::Shell::Destructor"); + for (auto& observer : shell_observers_) + observer.OnShellDestroying(); + const Config config = shell_port_->GetAshConfig(); if (config != Config::MASH)
diff --git a/ash/shell_observer.h b/ash/shell_observer.h index c76ede7..563f2a59 100644 --- a/ash/shell_observer.h +++ b/ash/shell_observer.h
@@ -69,6 +69,9 @@ // Called at the end of Shell::Init. virtual void OnShellInitialized() {} + // Called early on in ~Shell. + virtual void OnShellDestroying() {} + protected: virtual ~ShellObserver() {} };
diff --git a/ash/system/session/tray_session_length_limit.cc b/ash/system/session/tray_session_length_limit.cc index 185a9ddf..873b981 100644 --- a/ash/system/session/tray_session_length_limit.cc +++ b/ash/system/session/tray_session_length_limit.cc
@@ -73,11 +73,20 @@ tray_bubble_view_ = nullptr; } +void TraySessionLengthLimit::OnSessionStateChanged( + session_manager::SessionState state) { + Update(); +} + void TraySessionLengthLimit::OnSessionLengthLimitChanged() { Update(); } void TraySessionLengthLimit::Update() { + // Don't show notification or tray item until the user is logged in. + if (!Shell::Get()->session_controller()->IsActiveUserSessionStarted()) + return; + UpdateState(); UpdateNotification(); UpdateTrayBubbleView();
diff --git a/ash/system/session/tray_session_length_limit.h b/ash/system/session/tray_session_length_limit.h index 23c5ff8..3bd8032a 100644 --- a/ash/system/session/tray_session_length_limit.h +++ b/ash/system/session/tray_session_length_limit.h
@@ -35,6 +35,7 @@ void OnDefaultViewDestroyed() override; // SessionObserver: + void OnSessionStateChanged(session_manager::SessionState state) override; void OnSessionLengthLimitChanged() override; private:
diff --git a/ash/system/session/tray_session_length_limit_unittest.cc b/ash/system/session/tray_session_length_limit_unittest.cc index 60d35c41..f27574d 100644 --- a/ash/system/session/tray_session_length_limit_unittest.cc +++ b/ash/system/session/tray_session_length_limit_unittest.cc
@@ -164,5 +164,24 @@ .should_make_spoken_feedback_for_popup_updates); } +class TraySessionLengthLimitLoginTest : public TraySessionLengthLimitTest { + public: + TraySessionLengthLimitLoginTest() { set_start_session(false); } + + private: + DISALLOW_COPY_AND_ASSIGN(TraySessionLengthLimitLoginTest); +}; + +TEST_F(TraySessionLengthLimitLoginTest, NotificationShownAfterLogin) { + UpdateSessionLengthLimitInMin(15); + + // No notifications before login. + EXPECT_FALSE(GetNotification()); + + // Notification is shown after login. + SetSessionStarted(true); + EXPECT_TRUE(GetNotification()); +} + } // namespace test } // namespace ash
diff --git a/build/android/lint/suppressions.xml b/build/android/lint/suppressions.xml index e6f8531..c58889a 100644 --- a/build/android/lint/suppressions.xml +++ b/build/android/lint/suppressions.xml
@@ -104,6 +104,8 @@ <issue id="IconDensities"> <!-- The large assets below only include a few densities to reduce APK size. --> <ignore regexp=": data_reduction_illustration.png, physical_web_logo.png, physical_web_logo_anim1.png, physical_web_logo_anim2.png$"/> + <!-- This is intentional to save on WebAPKs' size. --> + <ignore regexp="chrome/android/webapk/shell_apk/res/drawable-*"/> <!-- crbug.com/457918 is tracking missing assets --> <ignore regexp="chrome/android/java/res/drawable-xxhdpi"/> <ignore regexp="chrome/android/java/res/drawable-xxxhdpi"/>
diff --git a/cc/surfaces/primary_begin_frame_source.cc b/cc/surfaces/primary_begin_frame_source.cc index 900ec98..726513e 100644 --- a/cc/surfaces/primary_begin_frame_source.cc +++ b/cc/surfaces/primary_begin_frame_source.cc
@@ -65,7 +65,9 @@ } bool PrimaryBeginFrameSource::IsThrottled() const { - return begin_frame_source_.IsThrottled(); + return current_begin_frame_source_ + ? current_begin_frame_source_->IsThrottled() + : true; } void PrimaryBeginFrameSource::OnNeedsBeginFrames(bool needs_begin_frames) {
diff --git a/chrome/android/BUILD.gn b/chrome/android/BUILD.gn index 6bee9fc..22dc801e6 100644 --- a/chrome/android/BUILD.gn +++ b/chrome/android/BUILD.gn
@@ -311,6 +311,7 @@ "//chrome/browser/android/webapk/chrome_webapk_host.h", "//chrome/browser/android/webapk/webapk_install_service.h", "//chrome/browser/banners/app_banner_settings_helper.h", + "//chrome/browser/notifications/notification_channels_provider_android.h", "//chrome/browser/notifications/notification_platform_bridge_android.cc", "//chrome/browser/ntp_snippets/ntp_snippets_metrics.h", "//chrome/browser/profiles/profile_metrics.h",
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationSettingsBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationSettingsBridge.java new file mode 100644 index 0000000..d4c23971 --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationSettingsBridge.java
@@ -0,0 +1,70 @@ +// 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.notifications; + +import org.chromium.base.BuildInfo; +import org.chromium.base.annotations.CalledByNative; + +/** + * Interface for native code to interact with Android notification channels. + */ +public class NotificationSettingsBridge { + // TODO(awdf): Remove this and check BuildInfo.sdk_int() from native instead, once SdkVersion + // enum includes Android O. + @CalledByNative + static boolean shouldUseChannelSettings() { + return BuildInfo.isAtLeastO(); + } + + /** + * Creates a notification channel for the given origin. + * @param origin The site origin to be used as the channel name. + * @param enabled True if the channel should be initially enabled, false if + * it should start off as blocked. + * @return true if the channel was successfully created, false otherwise. + */ + @CalledByNative + static boolean createChannel(String origin, boolean enabled) { + // TODO(crbug.com/700377) Actually construct a channel. + return false; + } + + @CalledByNative + static @NotificationChannelStatus int getChannelStatus(String origin) { + // TODO(crbug.com/700377) Actually check channel status. + return NotificationChannelStatus.UNAVAILABLE; + } + + @CalledByNative + static SiteChannel[] getSiteChannels() { + // TODO(crbug.com/700377) Actually get site channels. + return new SiteChannel[] {}; + } + + @CalledByNative + static void deleteChannel(String origin) { + // TODO(crbug.com/700377) Actually delete channel. + } + + static class SiteChannel { + private final String mOrigin; + private final @NotificationChannelStatus int mStatus; + + private SiteChannel(String origin, @NotificationChannelStatus int status) { + mOrigin = origin; + mStatus = status; + } + + @CalledByNative("SiteChannel") + private String getOrigin() { + return mOrigin; + } + + @CalledByNative("SiteChannel") + private @NotificationChannelStatus int getStatus() { + return mStatus; + } + } +}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/services/gcm/InvalidationGcmUpstreamSender.java b/chrome/android/java/src/org/chromium/chrome/browser/services/gcm/InvalidationGcmUpstreamSender.java index ea2ff86..19b46f19 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/services/gcm/InvalidationGcmUpstreamSender.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/services/gcm/InvalidationGcmUpstreamSender.java
@@ -10,11 +10,14 @@ import android.os.AsyncTask; import android.os.Bundle; import android.os.Parcel; +import android.support.annotation.MainThread; import android.util.Log; import com.google.android.gms.gcm.GoogleCloudMessaging; import com.google.ipc.invalidation.ticl.android2.channel.GcmUpstreamSenderService; +import org.chromium.base.ThreadUtils; +import org.chromium.chrome.browser.init.ProcessInitializationHandler; import org.chromium.chrome.browser.signin.OAuth2TokenService; import org.chromium.components.signin.AccountManagerHelper; import org.chromium.components.signin.ChromeSigninController; @@ -36,6 +39,23 @@ @Override public void deliverMessage(final String to, final Bundle data) { + final Bundle dataToSend = createDeepCopy(data); + final Context applicationContext = getApplicationContext(); + + ThreadUtils.postOnUiThread(new Runnable() { + @Override + public void run() { + doDeliverMessage(applicationContext, to, dataToSend); + } + }); + } + + @MainThread + private void doDeliverMessage( + final Context applicationContext, final String to, final Bundle data) { + ThreadUtils.assertOnUiThread(); + ProcessInitializationHandler.getInstance().initializePreNative(); + @Nullable Account account = ChromeSigninController.get().getSignedInUser(); if (account == null) { @@ -45,9 +65,6 @@ return; } - final Bundle dataToSend = createDeepCopy(data); - final Context applicationContext = getApplicationContext(); - // Attempt to retrieve a token for the user. OAuth2TokenService.getOAuth2AccessToken(this, account, SyncConstants.CHROME_SYNC_OAUTH2_SCOPE, @@ -57,7 +74,7 @@ new AsyncTask<Void, Void, Void>() { @Override protected Void doInBackground(Void... voids) { - sendUpstreamMessage(to, dataToSend, token, applicationContext); + sendUpstreamMessage(to, data, token, applicationContext); return null; } }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java b/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java index 93dfb74..a6064e3fa 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
@@ -1099,10 +1099,10 @@ private static boolean isVrCoreCompatible( final VrCoreVersionChecker versionChecker, final Tab tabToShowInfobarIn) { final int vrCoreCompatibility = versionChecker.getVrCoreInfo().compatibility; - - if (vrCoreCompatibility == VrCoreCompatibility.VR_NOT_AVAILABLE - || vrCoreCompatibility == VrCoreCompatibility.VR_OUT_OF_DATE) { - // This is somewhat slow, so do it async. + boolean needsUpdate = vrCoreCompatibility == VrCoreCompatibility.VR_NOT_AVAILABLE + || vrCoreCompatibility == VrCoreCompatibility.VR_OUT_OF_DATE; + if (tabToShowInfobarIn != null && needsUpdate) { + ThreadUtils.assertOnUiThread(); new Handler().post(new Runnable() { @Override public void run() { @@ -1115,9 +1115,6 @@ } private static void promptToUpdateVrServices(int vrCoreCompatibility, Tab tab) { - if (tab == null) { - return; - } final Activity activity = tab.getActivity(); String infobarText; String buttonText;
diff --git a/chrome/android/java_sources.gni b/chrome/android/java_sources.gni index c5aa7ed3..3c3d5d5c 100644 --- a/chrome/android/java_sources.gni +++ b/chrome/android/java_sources.gni
@@ -598,6 +598,7 @@ "java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java", "java/src/org/chromium/chrome/browser/notifications/NotificationJobService.java", "java/src/org/chromium/chrome/browser/notifications/NotificationService.java", + "java/src/org/chromium/chrome/browser/notifications/NotificationSettingsBridge.java", "java/src/org/chromium/chrome/browser/notifications/NotificationSystemStatusUtil.java", "java/src/org/chromium/chrome/browser/notifications/NotificationUmaTracker.java", "java/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilder.java",
diff --git a/chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkMetaDataKeys.java b/chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkMetaDataKeys.java index 0b6f76ed..558da25 100644 --- a/chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkMetaDataKeys.java +++ b/chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkMetaDataKeys.java
@@ -10,6 +10,8 @@ public final class WebApkMetaDataKeys { public static final String SHELL_APK_VERSION = "org.chromium.webapk.shell_apk.shellApkVersion"; public static final String RUNTIME_HOST = "org.chromium.webapk.shell_apk.runtimeHost"; + public static final String RUNTIME_HOST_APPLICATION_NAME = + "org.chromium.webapk.shell_apk.runtimeHostApplicationName"; public static final String START_URL = "org.chromium.webapk.shell_apk.startUrl"; public static final String NAME = "org.chromium.webapk.shell_apk.name"; public static final String SHORT_NAME = "org.chromium.webapk.shell_apk.shortName";
diff --git a/chrome/android/webapk/shell_apk/AndroidManifest.xml b/chrome/android/webapk/shell_apk/AndroidManifest.xml index fc379458..44f4572 100644 --- a/chrome/android/webapk/shell_apk/AndroidManifest.xml +++ b/chrome/android/webapk/shell_apk/AndroidManifest.xml
@@ -33,6 +33,7 @@ </activity> <meta-data android:name="org.chromium.webapk.shell_apk.shellApkVersion" android:value="{{{shell_apk_version}}}" /> {{#runtime_host}}<meta-data android:name="org.chromium.webapk.shell_apk.runtimeHost" android:value="{{{runtime_host}}}" />{{/runtime_host}} + {{#runtime_host_application_name}}<meta-data android:name="org.chromium.webapk.shell_apk.runtimeHostApplicationName" android:value="{{{runtime_host_application_name}}}" />{{/runtime_host_application_name}} <meta-data android:name="org.chromium.webapk.shell_apk.startUrl" android:value="{{{start_url}}}" /> <meta-data android:name="org.chromium.webapk.shell_apk.name" android:value="{{{name}}}" /> <meta-data android:name="org.chromium.webapk.shell_apk.shortName" android:value="{{{short_name}}}" />
diff --git a/chrome/android/webapk/shell_apk/BUILD.gn b/chrome/android/webapk/shell_apk/BUILD.gn index e737cd3..446a9cb 100644 --- a/chrome/android/webapk/shell_apk/BUILD.gn +++ b/chrome/android/webapk/shell_apk/BUILD.gn
@@ -10,6 +10,9 @@ # The browser that the WebAPK will be bound to. webapk_runtime_host = "com.google.android.apps.chrome" + # The application name of the browser that the WebAPK will be bound to. + webapk_runtime_host_application_name = "Chromium" + # The Url of the Web Manifest file. webapk_web_manifest_url = "https://www.template.com/manifest.json" @@ -59,6 +62,7 @@ "shell_apk_version=$template_shell_apk_version", "manifest_package=org.chromium.webapk", "runtime_host=$webapk_runtime_host", + "runtime_host_application_name=$webapk_runtime_host_application_name", "start_url=$webapk_start_url", "name=$webapk_name", "short_name=$webapk_short_name", @@ -146,6 +150,7 @@ "src/org/chromium/webapk/shell_apk/HostBrowserClassLoader.java", "src/org/chromium/webapk/shell_apk/ChooseHostBrowserDialog.java", "src/org/chromium/webapk/shell_apk/MainActivity.java", + "src/org/chromium/webapk/shell_apk/InstallHostBrowserDialog.java", "src/org/chromium/webapk/shell_apk/WebApkSandboxedProcessService.java", "src/org/chromium/webapk/shell_apk/WebApkSandboxedProcessService0.java", "src/org/chromium/webapk/shell_apk/WebApkSandboxedProcessService1.java",
diff --git a/chrome/android/webapk/shell_apk/res/drawable-hdpi/last_resort_runtime_host_logo.png b/chrome/android/webapk/shell_apk/res/drawable-hdpi/last_resort_runtime_host_logo.png new file mode 100644 index 0000000..22af647 --- /dev/null +++ b/chrome/android/webapk/shell_apk/res/drawable-hdpi/last_resort_runtime_host_logo.png Binary files differ
diff --git a/chrome/android/webapk/shell_apk/res/layout/choose_host_browser_dialog_list.xml b/chrome/android/webapk/shell_apk/res/layout/host_browser_list_item.xml similarity index 100% rename from chrome/android/webapk/shell_apk/res/layout/choose_host_browser_dialog_list.xml rename to chrome/android/webapk/shell_apk/res/layout/host_browser_list_item.xml
diff --git a/chrome/android/webapk/shell_apk/shell_apk_version.gni b/chrome/android/webapk/shell_apk/shell_apk_version.gni index fb7d2bb..95fb06a 100644 --- a/chrome/android/webapk/shell_apk/shell_apk_version.gni +++ b/chrome/android/webapk/shell_apk/shell_apk_version.gni
@@ -6,7 +6,7 @@ # (including AndroidManifest.xml) is updated. This version should be incremented # prior to uploading a new ShellAPK to the WebAPK Minting Server. # Does not affect Chrome.apk -template_shell_apk_version = 6 +template_shell_apk_version = 7 # The ShellAPK version expected by Chrome. Chrome will try to update the WebAPK # if the WebAPK's ShellAPK version is less than |expected_shell_apk_version|.
diff --git a/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/ChooseHostBrowserDialog.java b/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/ChooseHostBrowserDialog.java index d63dd27..6802038 100644 --- a/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/ChooseHostBrowserDialog.java +++ b/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/ChooseHostBrowserDialog.java
@@ -4,11 +4,13 @@ package org.chromium.webapk.shell_apk; -import android.app.Activity; import android.app.AlertDialog; import android.content.Context; import android.content.DialogInterface; +import android.content.pm.PackageManager; +import android.content.pm.ResolveInfo; import android.graphics.Color; +import android.graphics.drawable.Drawable; import android.view.ContextThemeWrapper; import android.view.LayoutInflater; import android.view.View; @@ -19,6 +21,9 @@ import android.widget.ListView; import android.widget.TextView; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; import java.util.List; /** @@ -27,52 +32,81 @@ */ public class ChooseHostBrowserDialog { /** - * A listener to receive updates when user chooses a host browser for the WebAPK, or dismiss the + * A listener which is notified when user chooses a host browser for the WebAPK, or dismiss the * dialog. */ public interface DialogListener { - void onHostBrowserSelected(String packageName); + void onHostBrowserSelected(String selectedHostBrowser); void onQuit(); } - /** Listens to which browser is chosen by the user to launch WebAPK. */ - private DialogListener mListener; + /** Stores information about a potential host browser for the WebAPK. */ + public static class BrowserItem { + private String mPackageName; + private CharSequence mApplicationLabel; + private Drawable mIcon; + private boolean mSupportsWebApks; + + public BrowserItem(String packageName, CharSequence applicationLabel, Drawable icon, + boolean supportsWebApks) { + mPackageName = packageName; + mApplicationLabel = applicationLabel; + mIcon = icon; + mSupportsWebApks = supportsWebApks; + } + + /** Returns the package name of a browser. */ + public String getPackageName() { + return mPackageName; + } + + /** Returns the application name of a browser. */ + public CharSequence getApplicationName() { + return mApplicationLabel; + } + + /** Returns a drawable of the browser icon. */ + public Drawable getApplicationIcon() { + return mIcon; + } + + /** Returns whether the browser supports WebAPKs. */ + public boolean supportsWebApks() { + return mSupportsWebApks; + } + } /** * Shows the dialog for choosing a host browser. - * @param activity The current activity in which to create the dialog. - * @param url URL of the WebAPK that is shown on the dialog. + * @param context The current Context. + * @param listener The listener for the dialog. + * @param infos The list of ResolvedInfos of the browsers that are shown on the dialog. + * @param url URL of the WebAPK for which the dialog is shown. */ - public void show(Activity activity, String url) { - if (!(activity instanceof DialogListener)) { - throw new IllegalArgumentException( - activity.toString() + " must implement DialogListener"); - } - - mListener = (DialogListener) activity; - final List<WebApkUtils.BrowserItem> browserItems = - WebApkUtils.getBrowserInfosForHostBrowserSelection(activity.getPackageManager()); + public static void show( + Context context, final DialogListener listener, List<ResolveInfo> infos, String url) { + final List<BrowserItem> browserItems = + getBrowserInfosForHostBrowserSelection(context.getPackageManager(), infos); // The dialog contains: // 1) a description of the dialog. // 2) a list of browsers for user to choose from. - View view = - LayoutInflater.from(activity).inflate(R.layout.choose_host_browser_dialog, null); + View view = LayoutInflater.from(context).inflate(R.layout.choose_host_browser_dialog, null); TextView desc = (TextView) view.findViewById(R.id.desc); ListView browserList = (ListView) view.findViewById(R.id.browser_list); - desc.setText(activity.getString(R.string.choose_host_browser, url)); - browserList.setAdapter(new BrowserArrayAdapter(activity, browserItems, url)); + desc.setText(context.getString(R.string.choose_host_browser, url)); + browserList.setAdapter(new BrowserArrayAdapter(context, browserItems, url)); // The context theme wrapper is needed for pre-L. - AlertDialog.Builder builder = new AlertDialog.Builder(new ContextThemeWrapper( - activity, android.R.style.Theme_DeviceDefault_Light_Dialog)); - builder.setTitle(activity.getString(R.string.choose_host_browser_dialog_title, url)) + AlertDialog.Builder builder = new AlertDialog.Builder( + new ContextThemeWrapper(context, android.R.style.Theme_DeviceDefault_Light_Dialog)); + builder.setTitle(context.getString(R.string.choose_host_browser_dialog_title, url)) .setView(view) .setNegativeButton(R.string.choose_host_browser_dialog_quit, new DialogInterface.OnClickListener() { @Override public void onClick(DialogInterface dialog, int which) { - mListener.onQuit(); + listener.onQuit(); } }); @@ -80,9 +114,9 @@ browserList.setOnItemClickListener(new AdapterView.OnItemClickListener() { @Override public void onItemClick(AdapterView<?> parent, View view, int position, long id) { - WebApkUtils.BrowserItem browserItem = browserItems.get(position); + BrowserItem browserItem = browserItems.get(position); if (browserItem.supportsWebApks()) { - mListener.onHostBrowserSelected(browserItem.getPackageName()); + listener.onHostBrowserSelected(browserItem.getPackageName()); dialog.cancel(); } } @@ -91,15 +125,41 @@ dialog.show(); }; + /** Returns a list of BrowserItem for all of the installed browsers. */ + private static List<BrowserItem> getBrowserInfosForHostBrowserSelection( + PackageManager packageManager, List<ResolveInfo> resolveInfos) { + List<BrowserItem> browsers = new ArrayList<>(); + List<String> browsersSupportingWebApk = WebApkUtils.getBrowsersSupportingWebApk(); + + for (ResolveInfo info : resolveInfos) { + browsers.add(new BrowserItem(info.activityInfo.packageName, + info.loadLabel(packageManager), info.loadIcon(packageManager), + browsersSupportingWebApk.contains(info.activityInfo.packageName))); + } + + if (browsers.size() <= 1) return browsers; + + Collections.sort(browsers, new Comparator<BrowserItem>() { + @Override + public int compare(BrowserItem a, BrowserItem b) { + if (a.mSupportsWebApks == b.mSupportsWebApks) { + return a.getPackageName().compareTo(b.getPackageName()); + } + return a.mSupportsWebApks ? -1 : 1; + } + }); + + return browsers; + } + /** Item adaptor for the list of browsers. */ - private static class BrowserArrayAdapter extends ArrayAdapter<WebApkUtils.BrowserItem> { - private List<WebApkUtils.BrowserItem> mBrowsers; + private static class BrowserArrayAdapter extends ArrayAdapter<BrowserItem> { + private List<BrowserItem> mBrowsers; private Context mContext; private String mUrl; - public BrowserArrayAdapter( - Context context, List<WebApkUtils.BrowserItem> browsers, String url) { - super(context, R.layout.choose_host_browser_dialog_list, browsers); + public BrowserArrayAdapter(Context context, List<BrowserItem> browsers, String url) { + super(context, R.layout.host_browser_list_item, browsers); mContext = context; mBrowsers = browsers; mUrl = url; @@ -109,12 +169,12 @@ public View getView(int position, View convertView, ViewGroup parent) { if (convertView == null) { convertView = LayoutInflater.from(mContext).inflate( - R.layout.choose_host_browser_dialog_list, null); + R.layout.host_browser_list_item, null); } TextView name = (TextView) convertView.findViewById(R.id.browser_name); ImageView icon = (ImageView) convertView.findViewById(R.id.browser_icon); - WebApkUtils.BrowserItem item = mBrowsers.get(position); + BrowserItem item = mBrowsers.get(position); name.setEnabled(item.supportsWebApks()); if (item.supportsWebApks()) { @@ -136,4 +196,4 @@ return mBrowsers.get(position).supportsWebApks(); } } -} \ No newline at end of file +}
diff --git a/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/InstallHostBrowserDialog.java b/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/InstallHostBrowserDialog.java new file mode 100644 index 0000000..2dcf615 --- /dev/null +++ b/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/InstallHostBrowserDialog.java
@@ -0,0 +1,71 @@ +// 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.webapk.shell_apk; + +import android.app.AlertDialog; +import android.content.Context; +import android.content.DialogInterface; +import android.graphics.Color; +import android.view.ContextThemeWrapper; +import android.view.LayoutInflater; +import android.view.View; +import android.widget.ImageView; +import android.widget.TextView; + +/** Shows the dialog to install a host browser for launching WebAPK. */ +public class InstallHostBrowserDialog { + /** + * A listener which is notified when user chooses to install the host browser, or dismiss the + * dialog. + */ + public interface DialogListener { + void onConfirmInstall(String packageName); + void onConfirmQuit(); + } + + /** + * Shows the dialog to install a host browser. + * @param context The current context. + * @param listener The listener for the dialog. + * @param url URL of the WebAPK for which the dialog is shown. + * @param hostBrowserPackageName The package name of the host browser. + * @param hostBrowserApplicationName The application name of the host browser. + * @param hostBrowserIconId The resource id of the icon of the host browser. + */ + public static void show(Context context, final DialogListener listener, String url, + final String hostBrowserPackageName, String hostBrowserApplicationName, + int hostBrowserIconId) { + View view = LayoutInflater.from(context).inflate(R.layout.host_browser_list_item, null); + TextView name = (TextView) view.findViewById(R.id.browser_name); + ImageView icon = (ImageView) view.findViewById(R.id.browser_icon); + + name.setText(hostBrowserApplicationName); + name.setTextColor(Color.BLACK); + icon.setImageResource(hostBrowserIconId); + + // The context theme wrapper is needed for pre-L. + AlertDialog.Builder builder = new AlertDialog.Builder( + new ContextThemeWrapper(context, android.R.style.Theme_DeviceDefault_Light_Dialog)); + builder.setTitle(context.getString(R.string.install_host_browser_dialog_title, url)) + .setView(view) + .setNegativeButton(R.string.choose_host_browser_dialog_quit, + new DialogInterface.OnClickListener() { + @Override + public void onClick(DialogInterface dialog, int which) { + listener.onConfirmQuit(); + } + }) + .setPositiveButton(R.string.install_host_browser_dialog_install_button, + new DialogInterface.OnClickListener() { + @Override + public void onClick(DialogInterface dialog, int which) { + listener.onConfirmInstall(hostBrowserPackageName); + } + }); + + AlertDialog dialog = builder.create(); + dialog.show(); + }; +}
diff --git a/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java b/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java index a665663..693cb38 100644 --- a/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java +++ b/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java
@@ -6,7 +6,10 @@ import android.app.Activity; import android.content.ActivityNotFoundException; +import android.content.Context; import android.content.Intent; +import android.content.SharedPreferences; +import android.content.pm.ResolveInfo; import android.net.Uri; import android.os.AsyncTask; import android.os.Bundle; @@ -19,12 +22,14 @@ import java.io.File; import java.net.MalformedURLException; import java.net.URL; -import java.util.Set; +import java.util.List; /** * WebAPK's main Activity. */ -public class MainActivity extends Activity implements ChooseHostBrowserDialog.DialogListener { +public class MainActivity extends Activity { + private static final String LAST_RESORT_HOST_BROWSER = "com.android.chrome"; + private static final String LAST_RESORT_HOST_BROWSER_APPLICATION_NAME = "Google Chrome"; private static final String TAG = "cr_MainActivity"; /** @@ -62,18 +67,26 @@ * @param packageName Package to install. * @return The intent. */ - public static Intent createInstallIntent(String packageName) { + private static Intent createInstallIntent(String packageName) { String marketUrl = "market://details?id=" + packageName; - return new Intent(Intent.ACTION_VIEW, Uri.parse(marketUrl)); + Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse(marketUrl)); + intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); + return intent; } @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); + + Bundle metadata = WebApkUtils.readMetaData(this); + if (metadata == null) { + finish(); + return; + } + mOverrideUrl = getOverrideUrl(); - mStartUrl = (mOverrideUrl != null) - ? mOverrideUrl - : WebApkUtils.readMetaDataFromManifest(this, WebApkMetaDataKeys.START_URL); + mStartUrl = (mOverrideUrl != null) ? mOverrideUrl + : metadata.getString(WebApkMetaDataKeys.START_URL); if (mStartUrl == null) { finish(); return; @@ -87,7 +100,7 @@ String runtimeHost = WebApkUtils.getHostBrowserPackageName(this); if (!TextUtils.isEmpty(runtimeHostInPreferences) && !runtimeHostInPreferences.equals(runtimeHost)) { - WebApkUtils.deleteSharedPref(this); + deleteSharedPref(this); deleteInternalStorageAsync(); } @@ -106,32 +119,24 @@ return; } - ChooseHostBrowserDialog dialog = new ChooseHostBrowserDialog(); - dialog.show(this, hostUrl); - } - - @Override - public void onHostBrowserSelected(String selectedHostBrowser) { - Set<String> installedBrowsers = WebApkUtils.getInstalledBrowsers(getPackageManager()); - if (installedBrowsers.contains(selectedHostBrowser)) { - launchInHostBrowser(selectedHostBrowser); + List<ResolveInfo> infos = WebApkUtils.getInstalledBrowserResolveInfos(getPackageManager()); + if (hasBrowserSupportingWebApks(infos)) { + showChooseHostBrowserDialog(infos, hostUrl); } else { - installBrowser(selectedHostBrowser); + showInstallHostBrowserDialog(hostUrl, metadata); } - // It is safe to cache the selected host browser to the share pref if user didn't install - // the browser in {@link installBrowser}. On the next launch, - // {@link WebApkUtils#getHostBrowserPackageName()} will catch it, and the dialog to choose - // host browser will show again. If user did install the browser chosen, saving the - // selected host browser to the shared pref can avoid showing the dialog. - WebApkUtils.writeHostBrowserToSharedPref(this, selectedHostBrowser); - finish(); } - @Override - public void onQuit() { - finish(); + /** Deletes the SharedPreferences. */ + private void deleteSharedPref(Context context) { + SharedPreferences sharedPref = + context.getSharedPreferences(WebApkConstants.PREF_PACKAGE, Context.MODE_PRIVATE); + SharedPreferences.Editor editor = sharedPref.edit(); + editor.clear(); + editor.apply(); } + /** Deletes the internal storage asynchronously. */ private void deleteInternalStorageAsync() { new AsyncTask<Void, Void, Void>() { @Override @@ -208,4 +213,66 @@ } return null; } + + /** Returns whether there is any installed browser supporting WebAPKs. */ + private static boolean hasBrowserSupportingWebApks(List<ResolveInfo> resolveInfos) { + List<String> browsersSupportingWebApk = WebApkUtils.getBrowsersSupportingWebApk(); + for (ResolveInfo info : resolveInfos) { + if (browsersSupportingWebApk.contains(info.activityInfo.packageName)) { + return true; + } + } + return false; + } + + /** Shows a dialog to choose the host browser. */ + private void showChooseHostBrowserDialog(List<ResolveInfo> infos, String hostUrl) { + ChooseHostBrowserDialog.DialogListener listener = + new ChooseHostBrowserDialog.DialogListener() { + @Override + public void onHostBrowserSelected(String selectedHostBrowser) { + launchInHostBrowser(selectedHostBrowser); + WebApkUtils.writeHostBrowserToSharedPref( + MainActivity.this, selectedHostBrowser); + finish(); + } + @Override + public void onQuit() { + finish(); + } + }; + ChooseHostBrowserDialog.show(this, listener, infos, hostUrl); + } + + /** Shows a dialog to install the host browser. */ + private void showInstallHostBrowserDialog(String hostUrl, Bundle metadata) { + String lastResortHostBrowserPackageName = + metadata.getString(WebApkMetaDataKeys.RUNTIME_HOST); + String lastResortHostBrowserApplicationName = + metadata.getString(WebApkMetaDataKeys.RUNTIME_HOST_APPLICATION_NAME); + + if (TextUtils.isEmpty(lastResortHostBrowserPackageName)) { + // WebAPKs without runtime host specified in the AndroidManifest.xml always install + // Google Chrome as the default host browser. + lastResortHostBrowserPackageName = LAST_RESORT_HOST_BROWSER; + lastResortHostBrowserApplicationName = LAST_RESORT_HOST_BROWSER_APPLICATION_NAME; + } + + InstallHostBrowserDialog.DialogListener listener = + new InstallHostBrowserDialog.DialogListener() { + @Override + public void onConfirmInstall(String packageName) { + installBrowser(packageName); + WebApkUtils.writeHostBrowserToSharedPref(MainActivity.this, packageName); + finish(); + } + @Override + public void onConfirmQuit() { + finish(); + } + }; + + InstallHostBrowserDialog.show(this, listener, hostUrl, lastResortHostBrowserPackageName, + lastResortHostBrowserApplicationName, R.drawable.last_resort_runtime_host_logo); + } }
diff --git a/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/WebApkUtils.java b/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/WebApkUtils.java index bc8c754d..2ff261f 100644 --- a/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/WebApkUtils.java +++ b/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/WebApkUtils.java
@@ -11,8 +11,8 @@ import android.content.pm.PackageManager; import android.content.pm.PackageManager.NameNotFoundException; import android.content.pm.ResolveInfo; -import android.graphics.drawable.Drawable; import android.net.Uri; +import android.os.Bundle; import android.text.TextUtils; import org.chromium.webapk.lib.common.WebApkConstants; @@ -20,8 +20,6 @@ import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; -import java.util.Comparator; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -40,42 +38,6 @@ Arrays.asList("com.google.android.apps.chrome", "com.android.chrome", "com.chrome.beta", "com.chrome.dev", "com.chrome.canary")); - /** Stores information about a potential host browser for the WebAPK. */ - public static class BrowserItem { - private String mPackageName; - private CharSequence mApplicationLabel; - private Drawable mIcon; - private boolean mSupportsWebApks; - - public BrowserItem(String packageName, CharSequence applicationLabel, Drawable icon, - boolean supportsWebApks) { - mPackageName = packageName; - mApplicationLabel = applicationLabel; - mIcon = icon; - mSupportsWebApks = supportsWebApks; - } - - /** Returns the package name of a browser. */ - public String getPackageName() { - return mPackageName; - } - - /** Returns the application name of a browser. */ - public CharSequence getApplicationName() { - return mApplicationLabel; - } - - /** Returns a drawable of the browser icon. */ - public Drawable getApplicationIcon() { - return mIcon; - } - - /** Returns whether the browser supports WebAPKs. */ - public boolean supportsWebApks() { - return mSupportsWebApks; - } - } - /** * Caches the package name of the host browser. {@link sHostPackage} might refer to a browser * which has been uninstalled. A notification can keep the WebAPK process alive after the host @@ -89,6 +51,14 @@ } /** + * Returns a list of browsers that support WebAPKs. TODO(hanxi): Replace this function once we + * figure out a better way to know which browser supports WebAPKs. + */ + public static List<String> getBrowsersSupportingWebApk() { + return sBrowsersSupportingWebApk; + } + + /** * Returns a Context for the host browser that was specified when building the WebAPK. * @param context A context. * @return The remote context. Returns null on an error. @@ -123,6 +93,14 @@ /** Returns the <meta-data> value in the Android Manifest for {@link key}. */ public static String readMetaDataFromManifest(Context context, String key) { + Bundle metadata = readMetaData(context); + if (metadata == null) return null; + + return metadata.getString(key); + } + + /** Returns the <meta-data> in the Android Manifest. */ + public static Bundle readMetaData(Context context) { ApplicationInfo ai = null; try { ai = context.getPackageManager().getApplicationInfo( @@ -130,7 +108,7 @@ } catch (NameNotFoundException e) { return null; } - return ai.metaData.getString(key); + return ai.metaData; } /** @@ -171,44 +149,11 @@ return null; } - /** - * Returns a list of browsers to choose host browser from. The list includes all the installed - * browsers, and if none of the installed browser supports WebAPKs, Chrome will be added to the - * list as well. - */ - public static List<BrowserItem> getBrowserInfosForHostBrowserSelection( - PackageManager packageManager) { + /** Returns a list of ResolveInfo for all of the installed browsers. */ + public static List<ResolveInfo> getInstalledBrowserResolveInfos(PackageManager packageManager) { Intent browserIntent = new Intent(Intent.ACTION_VIEW, Uri.parse("http://")); - List<ResolveInfo> resolvedActivityList = packageManager.queryIntentActivities( + return packageManager.queryIntentActivities( browserIntent, PackageManager.MATCH_DEFAULT_ONLY); - - boolean hasBrowserSupportingWebApks = false; - List<BrowserItem> browsers = new ArrayList<>(); - for (ResolveInfo info : resolvedActivityList) { - boolean supportsWebApk = false; - if (sBrowsersSupportingWebApk.contains(info.activityInfo.packageName)) { - supportsWebApk = true; - hasBrowserSupportingWebApks = true; - } - browsers.add(new BrowserItem(info.activityInfo.packageName, - info.loadLabel(packageManager), info.loadIcon(packageManager), supportsWebApk)); - } - - Collections.sort(browsers, new Comparator<BrowserItem>() { - @Override - public int compare(BrowserItem a, BrowserItem b) { - if (a.mSupportsWebApks == b.mSupportsWebApks) { - return a.getPackageName().compareTo(b.getPackageName()); - } - return a.mSupportsWebApks ? -1 : 1; - } - }); - - if (hasBrowserSupportingWebApks) return browsers; - - // TODO(hanxi): add Chrome's icon to WebAPKs. - browsers.add(new BrowserItem("com.android.chrome", "Chrome", null, true)); - return browsers; } /** @@ -240,13 +185,11 @@ } /** Returns a set of package names of all the installed browsers on the device. */ - public static Set<String> getInstalledBrowsers(PackageManager packageManager) { - Intent browserIntent = new Intent(Intent.ACTION_VIEW, Uri.parse("http://")); - List<ResolveInfo> resolvedActivityList = packageManager.queryIntentActivities( - browserIntent, PackageManager.MATCH_DEFAULT_ONLY); + private static Set<String> getInstalledBrowsers(PackageManager packageManager) { + List<ResolveInfo> resolvedInfos = getInstalledBrowserResolveInfos(packageManager); Set<String> packagesSupportingWebApks = new HashSet<String>(); - for (ResolveInfo info : resolvedActivityList) { + for (ResolveInfo info : resolvedInfos) { packagesSupportingWebApks.add(info.activityInfo.packageName); } return packagesSupportingWebApks; @@ -276,13 +219,4 @@ editor.putString(SHARED_PREF_RUNTIME_HOST, hostPackage); editor.apply(); } - - /** Deletes the SharedPreferences. */ - public static void deleteSharedPref(Context context) { - SharedPreferences sharedPref = - context.getSharedPreferences(WebApkConstants.PREF_PACKAGE, Context.MODE_PRIVATE); - SharedPreferences.Editor editor = sharedPref.edit(); - editor.clear(); - editor.apply(); - } }
diff --git a/chrome/android/webapk/strings/android_webapk_strings.grd b/chrome/android/webapk/strings/android_webapk_strings.grd index 4d5808a..1ca3d61f 100644 --- a/chrome/android/webapk/strings/android_webapk_strings.grd +++ b/chrome/android/webapk/strings/android_webapk_strings.grd
@@ -106,6 +106,12 @@ <message name="IDS_HOST_BROWSER_ITEM_NOT_SUPPORTING_WEBAPKS" desc="Text for the host browser item that doesn't support WebAPKs on the choose host browser dialog. "> <ph name="BROWSER_NAME">%1$s<ex>Chrome</ex></ph>\nUnsupported by <ph name="WEBAPK_SITE">%2$s<ex>Housing.com</ex></ph> </message> + <message name="IDS_INSTALL_HOST_BROWSER_DIALOG_TITLE" desc="Title for the dialog to install the host browser to launch the WebAPK."> + <ph name="WEBAPK_SITE">%1$s<ex>Housing.com</ex></ph> needs the following app to run + </message> + <message name="IDS_INSTALL_HOST_BROWSER_DIALOG_INSTALL_BUTTON" desc="Text for the install button on the dialog. "> + INSTALL + </message> </messages> </release> </grit>
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn index 19eac32..aa464ff 100644 --- a/chrome/browser/BUILD.gn +++ b/chrome/browser/BUILD.gn
@@ -762,6 +762,8 @@ "notifications/non_persistent_notification_handler.h", "notifications/notification.cc", "notifications/notification.h", + "notifications/notification_channels_provider_android.cc", + "notifications/notification_channels_provider_android.h", "notifications/notification_common.cc", "notifications/notification_common.h", "notifications/notification_delegate.h", @@ -770,8 +772,6 @@ "notifications/notification_display_service_factory.cc", "notifications/notification_display_service_factory.h", "notifications/notification_handler.h", - "notifications/notification_object_proxy.cc", - "notifications/notification_object_proxy.h", "notifications/notification_permission_context.cc", "notifications/notification_permission_context.h", "notifications/notification_platform_bridge.h", @@ -782,8 +782,6 @@ "notifications/notifier_state_tracker.h", "notifications/notifier_state_tracker_factory.cc", "notifications/notifier_state_tracker_factory.h", - "notifications/persistent_notification_delegate.cc", - "notifications/persistent_notification_delegate.h", "notifications/persistent_notification_handler.cc", "notifications/persistent_notification_handler.h", "notifications/platform_notification_service_impl.cc", @@ -4258,6 +4256,7 @@ "../android/java/src/org/chromium/chrome/browser/net/spdyproxy/DataReductionProxySettings.java", "../android/java/src/org/chromium/chrome/browser/notifications/ActionInfo.java", "../android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java", + "../android/java/src/org/chromium/chrome/browser/notifications/NotificationSettingsBridge.java", "../android/java/src/org/chromium/chrome/browser/ntp/ContentSuggestionsNotificationHelper.java", "../android/java/src/org/chromium/chrome/browser/ntp/ForeignSessionHelper.java", "../android/java/src/org/chromium/chrome/browser/ntp/LogoBridge.java",
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc index 64b0bb2..3e72025c 100644 --- a/chrome/browser/about_flags.cc +++ b/chrome/browser/about_flags.cc
@@ -115,6 +115,7 @@ #if defined(OS_CHROMEOS) #include "chromeos/chromeos_switches.h" #include "components/arc/arc_features.h" +#include "components/ui_devtools/switches.h" #include "third_party/cros_system_api/switches/chrome_switches.h" #endif // OS_CHROMEOS @@ -2734,6 +2735,13 @@ // when the flag is manually enabled in a local build. "AutofillCreditCardDropdownVariations")}, #endif // OS_ANDROID + +#if defined(OS_CHROMEOS) + {ui::devtools::kEnableUiDevTools, flag_descriptions::kUiDevToolsName, + flag_descriptions::kUiDevToolsDescription, kOsCrOS, + SINGLE_VALUE_TYPE(ui::devtools::kEnableUiDevTools)}, +#endif // defined(OS_CHROMEOS) + {"enable-autofill-credit-card-last-used-date-display", flag_descriptions::kEnableAutofillCreditCardLastUsedDateDisplayName, flag_descriptions::kEnableAutofillCreditCardLastUsedDateDisplayDescription, @@ -3115,6 +3123,12 @@ channel != version_info::Channel::UNKNOWN) { return true; } + + // enable-ui-devtools is only available on for non Stable channels. + if (!strcmp(ui::devtools::kEnableUiDevTools, entry.internal_name) && + channel == version_info::Channel::STABLE) { + return true; + } #endif // defined(OS_CHROMEOS) // data-reduction-proxy-lo-fi and enable-data-reduction-proxy-lite-page
diff --git a/chrome/browser/content_settings/host_content_settings_map_factory.cc b/chrome/browser/content_settings/host_content_settings_map_factory.cc index a346556..b9e9a44 100644 --- a/chrome/browser/content_settings/host_content_settings_map_factory.cc +++ b/chrome/browser/content_settings/host_content_settings_map_factory.cc
@@ -30,6 +30,10 @@ #include "chrome/browser/supervised_user/supervised_user_settings_service_factory.h" #endif +#if defined(OS_ANDROID) +#include "chrome/browser/notifications/notification_channels_provider_android.h" +#endif // OS_ANDROID + HostContentSettingsMapFactory::HostContentSettingsMapFactory() : RefcountedBrowserContextKeyedServiceFactory( "HostContentSettingsMap", @@ -109,6 +113,15 @@ } #endif // BUILDFLAG(ENABLE_SUPERVISED_USERS) +#if defined(OS_ANDROID) + if (base::FeatureList::IsEnabled(features::kSiteNotificationChannels)) { + auto channels_provider = + base::MakeUnique<NotificationChannelsProviderAndroid>(); + settings_map->RegisterProvider( + HostContentSettingsMap::NOTIFICATION_ANDROID_PROVIDER, + std::move(channels_provider)); + } +#endif // defined (OS_ANDROID) return settings_map; }
diff --git a/chrome/browser/engagement/site_engagement_service_unittest.cc b/chrome/browser/engagement/site_engagement_service_unittest.cc index 740e9c14..e029822 100644 --- a/chrome/browser/engagement/site_engagement_service_unittest.cc +++ b/chrome/browser/engagement/site_engagement_service_unittest.cc
@@ -557,7 +557,7 @@ settings_map->SetContentSettingDefaultScope( url3, GURL(), CONTENT_SETTINGS_TYPE_NOTIFICATIONS, - content_settings::ResourceIdentifier(), CONTENT_SETTING_ASK); + content_settings::ResourceIdentifier(), CONTENT_SETTING_DEFAULT); EXPECT_EQ(5, service_->GetScore(url1)); EXPECT_EQ(0, service_->GetScore(url2));
diff --git a/chrome/browser/extensions/api/desktop_capture/desktop_capture_apitest.cc b/chrome/browser/extensions/api/desktop_capture/desktop_capture_apitest.cc index a81d0d3..da76a6d 100644 --- a/chrome/browser/extensions/api/desktop_capture/desktop_capture_apitest.cc +++ b/chrome/browser/extensions/api/desktop_capture/desktop_capture_apitest.cc
@@ -171,7 +171,8 @@ } // namespace // Flaky on Windows: http://crbug.com/301887 -#if defined(OS_WIN) +// Fails on Ozone Chrome OS: http://crbug.com/718512 +#if defined(OS_WIN) || (defined(OS_CHROMEOS) && defined(USE_OZONE)) #define MAYBE_ChooseDesktopMedia DISABLED_ChooseDesktopMedia #else #define MAYBE_ChooseDesktopMedia ChooseDesktopMedia
diff --git a/chrome/browser/flag_descriptions.cc b/chrome/browser/flag_descriptions.cc index a9789a6..b396780d 100644 --- a/chrome/browser/flag_descriptions.cc +++ b/chrome/browser/flag_descriptions.cc
@@ -3203,6 +3203,11 @@ "Disable new window behavior for the accessibility keyboard " "in non-sticky mode (do not change work area in non-sticky mode)."; +const char kUiDevToolsName[] = "Enable native UI inspection"; +const char kUiDevToolsDescription[] = + "Enables inspection of native UI elements. For local inspection use " + "chrome://inspect#other"; + #endif // defined(OS_CHROMEOS) } // namespace flag_descriptions
diff --git a/chrome/browser/flag_descriptions.h b/chrome/browser/flag_descriptions.h index 8e14959356..87430a03 100644 --- a/chrome/browser/flag_descriptions.h +++ b/chrome/browser/flag_descriptions.h
@@ -1348,6 +1348,9 @@ extern const char kTouchscreenCalibrationName[]; extern const char kTouchscreenCalibrationDescription[]; +extern const char kUiDevToolsName[]; +extern const char kUiDevToolsDescription[]; + extern const char kUseMusName[]; extern const char kUseMusDescription[]; extern const char kEnableMashDescription[];
diff --git a/chrome/browser/notifications/message_center_display_service.cc b/chrome/browser/notifications/message_center_display_service.cc index e04fca35..e7445d7 100644 --- a/chrome/browser/notifications/message_center_display_service.cc +++ b/chrome/browser/notifications/message_center_display_service.cc
@@ -8,9 +8,11 @@ #include "chrome/browser/notifications/message_center_display_service.h" #include "chrome/browser/notifications/notification.h" +#include "chrome/browser/notifications/notification_handler.h" #include "chrome/browser/notifications/notification_ui_manager.h" #include "chrome/browser/profiles/profile.h" #include "content/public/browser/browser_thread.h" +#include "content/public/browser/notification_event_dispatcher.h" MessageCenterDisplayService::MessageCenterDisplayService( Profile* profile, @@ -28,6 +30,9 @@ // TODO(miguelg): MCDS should stop relying on the |notification|'s delegate // for Close/Click operations once the Notification object becomes a mojom // type. + + NotificationHandler* handler = GetNotificationHandler(notification_type); + handler->OnShow(profile_, notification_id); ui_manager_->Add(notification, profile_); }
diff --git a/chrome/browser/notifications/notification_channels_provider_android.cc b/chrome/browser/notifications/notification_channels_provider_android.cc new file mode 100644 index 0000000..f85426c --- /dev/null +++ b/chrome/browser/notifications/notification_channels_provider_android.cc
@@ -0,0 +1,213 @@ +// 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. + +#include "chrome/browser/notifications/notification_channels_provider_android.h" + +#include "base/android/jni_android.h" +#include "base/android/jni_string.h" +#include "base/logging.h" +#include "base/macros.h" +#include "base/strings/string_util.h" +#include "chrome/browser/content_settings/host_content_settings_map_factory.h" +#include "chrome/browser/profiles/profile.h" +#include "components/content_settings/core/browser/content_settings_details.h" +#include "components/content_settings/core/browser/content_settings_rule.h" +#include "components/content_settings/core/browser/content_settings_utils.h" +#include "components/content_settings/core/browser/host_content_settings_map.h" +#include "components/content_settings/core/common/content_settings.h" +#include "components/content_settings/core/common/content_settings_pattern.h" +#include "jni/NotificationSettingsBridge_jni.h" +#include "url/gurl.h" +#include "url/origin.h" +#include "url/url_constants.h" + +using base::android::AttachCurrentThread; +using base::android::ConvertUTF8ToJavaString; +using base::android::ScopedJavaLocalRef; + +namespace { + +class NotificationChannelsBridgeImpl + : public NotificationChannelsProviderAndroid::NotificationChannelsBridge { + public: + NotificationChannelsBridgeImpl() = default; + ~NotificationChannelsBridgeImpl() override = default; + + bool ShouldUseChannelSettings() override { + return Java_NotificationSettingsBridge_shouldUseChannelSettings( + AttachCurrentThread()); + } + + void CreateChannel(const std::string& origin, bool enabled) override { + JNIEnv* env = AttachCurrentThread(); + Java_NotificationSettingsBridge_createChannel( + env, ConvertUTF8ToJavaString(env, origin), enabled); + } + + NotificationChannelStatus GetChannelStatus( + const std::string& origin) override { + JNIEnv* env = AttachCurrentThread(); + return static_cast<NotificationChannelStatus>( + Java_NotificationSettingsBridge_getChannelStatus( + env, ConvertUTF8ToJavaString(env, origin))); + } + + void DeleteChannel(const std::string& origin) override { + JNIEnv* env = AttachCurrentThread(); + Java_NotificationSettingsBridge_deleteChannel( + env, ConvertUTF8ToJavaString(env, origin)); + } + + std::vector<NotificationChannel> GetChannels() override { + JNIEnv* env = AttachCurrentThread(); + ScopedJavaLocalRef<jobjectArray> raw_channels = + Java_NotificationSettingsBridge_getSiteChannels(env); + jsize num_channels = env->GetArrayLength(raw_channels.obj()); + std::vector<NotificationChannel> channels; + for (jsize i = 0; i < num_channels; ++i) { + jobject jchannel = env->GetObjectArrayElement(raw_channels.obj(), i); + channels.emplace_back( + ConvertJavaStringToUTF8(Java_SiteChannel_getOrigin(env, jchannel)), + static_cast<NotificationChannelStatus>( + Java_SiteChannel_getStatus(env, jchannel))); + } + return channels; + } +}; + +ContentSetting ChannelStatusToContentSetting(NotificationChannelStatus status) { + switch (status) { + case NotificationChannelStatus::ENABLED: + return CONTENT_SETTING_ALLOW; + case NotificationChannelStatus::BLOCKED: + return CONTENT_SETTING_BLOCK; + case NotificationChannelStatus::UNAVAILABLE: + NOTREACHED(); + } + return CONTENT_SETTING_DEFAULT; +} + +class ChannelsRuleIterator : public content_settings::RuleIterator { + public: + explicit ChannelsRuleIterator(std::vector<NotificationChannel> channels) + : channels_(std::move(channels)), index_(0) {} + + ~ChannelsRuleIterator() override = default; + + bool HasNext() const override { return index_ < channels_.size(); } + + content_settings::Rule Next() override { + DCHECK(HasNext()); + DCHECK_NE(channels_[index_].status_, + NotificationChannelStatus::UNAVAILABLE); + content_settings::Rule rule = content_settings::Rule( + ContentSettingsPattern::FromString(channels_[index_].origin_), + ContentSettingsPattern::Wildcard(), + new base::Value( + ChannelStatusToContentSetting(channels_[index_].status_))); + index_++; + return rule; + } + + private: + std::vector<NotificationChannel> channels_; + size_t index_; + DISALLOW_COPY_AND_ASSIGN(ChannelsRuleIterator); +}; + +} // anonymous namespace + +NotificationChannelsProviderAndroid::NotificationChannelsProviderAndroid() + : NotificationChannelsProviderAndroid( + base::MakeUnique<NotificationChannelsBridgeImpl>()) {} + +NotificationChannelsProviderAndroid::NotificationChannelsProviderAndroid( + std::unique_ptr<NotificationChannelsBridge> bridge) + : bridge_(std::move(bridge)), + should_use_channels_(bridge_->ShouldUseChannelSettings()) {} + +NotificationChannelsProviderAndroid::~NotificationChannelsProviderAndroid() = + default; + +std::unique_ptr<content_settings::RuleIterator> +NotificationChannelsProviderAndroid::GetRuleIterator( + ContentSettingsType content_type, + const content_settings::ResourceIdentifier& resource_identifier, + bool incognito) const { + if (content_type != CONTENT_SETTINGS_TYPE_NOTIFICATIONS || incognito || + !should_use_channels_) { + return nullptr; + } + std::vector<NotificationChannel> channels = bridge_->GetChannels(); + return channels.empty() + ? nullptr + : base::MakeUnique<ChannelsRuleIterator>(std::move(channels)); +} + +bool NotificationChannelsProviderAndroid::SetWebsiteSetting( + const ContentSettingsPattern& primary_pattern, + const ContentSettingsPattern& secondary_pattern, + ContentSettingsType content_type, + const content_settings::ResourceIdentifier& resource_identifier, + base::Value* value) { + if (content_type != CONTENT_SETTINGS_TYPE_NOTIFICATIONS || + !should_use_channels_) { + return false; + } + // This provider only handles settings for specific origins. + if (primary_pattern == ContentSettingsPattern::Wildcard() && + secondary_pattern == ContentSettingsPattern::Wildcard() && + resource_identifier.empty()) { + return false; + } + url::Origin origin = url::Origin(GURL(primary_pattern.ToString())); + DCHECK(!origin.unique()); + const std::string origin_string = origin.Serialize(); + ContentSetting setting = content_settings::ValueToContentSetting(value); + switch (setting) { + case CONTENT_SETTING_ALLOW: + CreateChannelIfRequired(origin_string, + NotificationChannelStatus::ENABLED); + break; + case CONTENT_SETTING_BLOCK: + CreateChannelIfRequired(origin_string, + NotificationChannelStatus::BLOCKED); + break; + case CONTENT_SETTING_DEFAULT: + bridge_->DeleteChannel(origin_string); + break; + default: + // We rely on notification settings being one of ALLOW/BLOCK/DEFAULT. + NOTREACHED(); + break; + } + return true; +} + +void NotificationChannelsProviderAndroid::ClearAllContentSettingsRules( + ContentSettingsType content_type) { + // TODO(crbug.com/700377): If |content_type| == NOTIFICATIONS, delete + // all channels. +} + +void NotificationChannelsProviderAndroid::ShutdownOnUIThread() { + RemoveAllObservers(); +} + +void NotificationChannelsProviderAndroid::CreateChannelIfRequired( + const std::string& origin_string, + NotificationChannelStatus new_channel_status) { + // TODO(awdf): Maybe check cached incognito status here to make sure + // channels are never created in incognito mode. + auto old_channel_status = bridge_->GetChannelStatus(origin_string); + if (old_channel_status == NotificationChannelStatus::UNAVAILABLE) { + bridge_->CreateChannel( + origin_string, + new_channel_status == NotificationChannelStatus::ENABLED); + } else { + // TODO(awdf): Maybe remove this DCHECK - channel status could change any + // time so this may be vulnerable to a race condition. + DCHECK(old_channel_status == new_channel_status); + } +}
diff --git a/chrome/browser/notifications/notification_channels_provider_android.h b/chrome/browser/notifications/notification_channels_provider_android.h new file mode 100644 index 0000000..896f350 --- /dev/null +++ b/chrome/browser/notifications/notification_channels_provider_android.h
@@ -0,0 +1,79 @@ +// 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. + +#ifndef CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_CHANNELS_PROVIDER_ANDROID_H_ +#define CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_CHANNELS_PROVIDER_ANDROID_H_ + +#include <string> + +#include "base/macros.h" +#include "components/content_settings/core/browser/content_settings_observable_provider.h" +#include "components/content_settings/core/browser/content_settings_observer.h" +#include "components/content_settings/core/browser/content_settings_rule.h" +#include "components/content_settings/core/common/content_settings.h" +#include "components/content_settings/core/common/content_settings_types.h" +#include "components/keyed_service/core/keyed_service.h" + +// A Java counterpart will be generated for this enum. +// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.notifications +enum NotificationChannelStatus { ENABLED, BLOCKED, UNAVAILABLE }; + +struct NotificationChannel { + NotificationChannel(std::string origin, NotificationChannelStatus status) + : origin_(origin), status_(status) {} + std::string origin_; + NotificationChannelStatus status_ = NotificationChannelStatus::UNAVAILABLE; +}; + +// This class provides notification content settings from system notification +// channels on Android O+. This provider takes precedence over pref-provided +// content settings, but defers to supervised user and policy settings - see +// ordering of the ProviderType enum values in HostContentSettingsMap. +class NotificationChannelsProviderAndroid + : public content_settings::ObservableProvider { + public: + // Helper class to make the JNI calls. + class NotificationChannelsBridge { + public: + virtual ~NotificationChannelsBridge() = default; + virtual bool ShouldUseChannelSettings() = 0; + virtual void CreateChannel(const std::string& origin, bool enabled) = 0; + virtual NotificationChannelStatus GetChannelStatus( + const std::string& origin) = 0; + virtual void DeleteChannel(const std::string& origin) = 0; + virtual std::vector<NotificationChannel> GetChannels() = 0; + }; + + NotificationChannelsProviderAndroid(); + ~NotificationChannelsProviderAndroid() override; + + // ProviderInterface methods: + std::unique_ptr<content_settings::RuleIterator> GetRuleIterator( + ContentSettingsType content_type, + const content_settings::ResourceIdentifier& resource_identifier, + bool incognito) const override; + bool SetWebsiteSetting( + const ContentSettingsPattern& primary_pattern, + const ContentSettingsPattern& secondary_pattern, + ContentSettingsType content_type, + const content_settings::ResourceIdentifier& resource_identifier, + base::Value* value) override; + void ClearAllContentSettingsRules(ContentSettingsType content_type) override; + void ShutdownOnUIThread() override; + + private: + explicit NotificationChannelsProviderAndroid( + std::unique_ptr<NotificationChannelsBridge> bridge); + friend class NotificationChannelsProviderAndroidTest; + + void CreateChannelIfRequired(const std::string& origin_string, + NotificationChannelStatus new_channel_status); + + std::unique_ptr<NotificationChannelsBridge> bridge_; + bool should_use_channels_; + + DISALLOW_COPY_AND_ASSIGN(NotificationChannelsProviderAndroid); +}; + +#endif // CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_CHANNELS_PROVIDER_ANDROID_H_
diff --git a/chrome/browser/notifications/notification_channels_provider_android_unittest.cc b/chrome/browser/notifications/notification_channels_provider_android_unittest.cc new file mode 100644 index 0000000..991f1a4 --- /dev/null +++ b/chrome/browser/notifications/notification_channels_provider_android_unittest.cc
@@ -0,0 +1,219 @@ +// 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. + +#include "chrome/browser/notifications/notification_channels_provider_android.h" + +#include "base/memory/ptr_util.h" +#include "base/values.h" +#include "components/content_settings/core/browser/content_settings_pref.h" +#include "components/content_settings/core/browser/content_settings_rule.h" +#include "components/content_settings/core/browser/content_settings_utils.h" +#include "components/content_settings/core/common/content_settings_pattern.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "url/gurl.h" + +using ::testing::Return; + +namespace { +const char kTestOrigin[] = "https://example.com"; +} // namespace + +class MockNotificationChannelsBridge + : public NotificationChannelsProviderAndroid::NotificationChannelsBridge { + public: + ~MockNotificationChannelsBridge() = default; + MOCK_METHOD0(ShouldUseChannelSettings, bool()); + MOCK_METHOD2(CreateChannel, void(const std::string&, bool)); + MOCK_METHOD1(GetChannelStatus, NotificationChannelStatus(const std::string&)); + MOCK_METHOD1(DeleteChannel, void(const std::string&)); + MOCK_METHOD0(GetChannels, std::vector<NotificationChannel>()); +}; + +class NotificationChannelsProviderAndroidTest : public testing::Test { + public: + NotificationChannelsProviderAndroidTest() + : mock_bridge_(new MockNotificationChannelsBridge()) {} + ~NotificationChannelsProviderAndroidTest() override { + channels_provider_->ShutdownOnUIThread(); + } + + protected: + // No leak because ownership is passed to channels_provider_ in constructor. + MockNotificationChannelsBridge* mock_bridge_; + std::unique_ptr<NotificationChannelsProviderAndroid> channels_provider_; + void InitChannelsProvider(bool should_use_channels) { + EXPECT_CALL(*mock_bridge_, ShouldUseChannelSettings()) + .WillOnce(Return(should_use_channels)); + // Can't use base::MakeUnique because the provider's constructor is private. + channels_provider_ = + base::WrapUnique(new NotificationChannelsProviderAndroid( + base::WrapUnique(mock_bridge_))); + } +}; + +TEST_F(NotificationChannelsProviderAndroidTest, + SetWebsiteSettingWhenChannelsShouldNotBeUsed_NoopAndReturnsFalse) { + this->InitChannelsProvider(false /* should_use_channels */); + bool result = channels_provider_->SetWebsiteSetting( + ContentSettingsPattern::FromString(kTestOrigin), ContentSettingsPattern(), + CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(), + new base::Value(CONTENT_SETTING_BLOCK)); + + EXPECT_FALSE(result); +} + +TEST_F(NotificationChannelsProviderAndroidTest, + SetWebsiteSettingAllowedWhenChannelUnavailable_CreatesEnabledChannel) { + InitChannelsProvider(true /* should_use_channels */); + EXPECT_CALL(*mock_bridge_, GetChannelStatus(kTestOrigin)) + .WillOnce(Return(NotificationChannelStatus::UNAVAILABLE)); + EXPECT_CALL(*mock_bridge_, CreateChannel(kTestOrigin, true /* enabled */)); + + bool result = channels_provider_->SetWebsiteSetting( + ContentSettingsPattern::FromString(kTestOrigin), ContentSettingsPattern(), + CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(), + new base::Value(CONTENT_SETTING_ALLOW)); + + EXPECT_TRUE(result); +} + +TEST_F(NotificationChannelsProviderAndroidTest, + SetWebsiteSettingBlockedWhenChannelUnavailable_CreatesDisabledChannel) { + InitChannelsProvider(true /* should_use_channels */); + EXPECT_CALL(*mock_bridge_, GetChannelStatus(kTestOrigin)) + .WillOnce(Return(NotificationChannelStatus::UNAVAILABLE)); + EXPECT_CALL(*mock_bridge_, CreateChannel(kTestOrigin, false /* enabled */)); + + bool result = channels_provider_->SetWebsiteSetting( + ContentSettingsPattern::FromString(kTestOrigin), ContentSettingsPattern(), + CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(), + new base::Value(CONTENT_SETTING_BLOCK)); + + EXPECT_TRUE(result); +} + +TEST_F(NotificationChannelsProviderAndroidTest, + SetWebsiteSettingAllowedWhenChannelAllowed_NoopAndReturnsTrue) { + InitChannelsProvider(true /* should_use_channels */); + EXPECT_CALL(*mock_bridge_, GetChannelStatus(kTestOrigin)) + .WillOnce(Return(NotificationChannelStatus::ENABLED)); + + bool result = channels_provider_->SetWebsiteSetting( + ContentSettingsPattern::FromString(kTestOrigin), ContentSettingsPattern(), + CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(), + new base::Value(CONTENT_SETTING_ALLOW)); + + EXPECT_TRUE(result); +} + +TEST_F(NotificationChannelsProviderAndroidTest, + SetWebsiteSettingBlockedWhenChannelBlocked_NoopAndReturnsTrue) { + InitChannelsProvider(true /* should_use_channels */); + EXPECT_CALL(*mock_bridge_, GetChannelStatus(kTestOrigin)) + .WillOnce(Return(NotificationChannelStatus::BLOCKED)); + + bool result = channels_provider_->SetWebsiteSetting( + ContentSettingsPattern::FromString(kTestOrigin), ContentSettingsPattern(), + CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(), + new base::Value(CONTENT_SETTING_BLOCK)); + + EXPECT_TRUE(result); +} + +TEST_F(NotificationChannelsProviderAndroidTest, + SetWebsiteSettingDefault_DeletesChannelAndReturnsTrue) { + InitChannelsProvider(true /* should_use_channels */); + EXPECT_CALL(*mock_bridge_, DeleteChannel(kTestOrigin)); + bool result = channels_provider_->SetWebsiteSetting( + ContentSettingsPattern::FromString(kTestOrigin), ContentSettingsPattern(), + CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(), nullptr); + + EXPECT_TRUE(result); +} + +TEST_F(NotificationChannelsProviderAndroidTest, + GetRuleIteratorWhenChannelsShouldNotBeUsed) { + InitChannelsProvider(false /* should_use_channels */); + EXPECT_FALSE(channels_provider_->GetRuleIterator( + CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(), + false /* incognito */)); +} + +TEST_F(NotificationChannelsProviderAndroidTest, GetRuleIteratorForIncognito) { + InitChannelsProvider(true /* should_use_channels */); + EXPECT_FALSE( + channels_provider_->GetRuleIterator(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, + std::string(), true /* incognito */)); +} + +TEST_F(NotificationChannelsProviderAndroidTest, + GetRuleIteratorWhenNoChannelsExist) { + InitChannelsProvider(true /* should_use_channels */); + EXPECT_CALL(*mock_bridge_, GetChannels()); + EXPECT_FALSE(channels_provider_->GetRuleIterator( + CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(), + false /* incognito */)); +} + +TEST_F(NotificationChannelsProviderAndroidTest, + GetRuleIteratorWhenOneBlockedChannelExists) { + InitChannelsProvider(true /* should_use_channels */); + std::vector<NotificationChannel> channels; + channels.emplace_back(kTestOrigin, NotificationChannelStatus::BLOCKED); + EXPECT_CALL(*mock_bridge_, GetChannels()).WillOnce(Return(channels)); + std::unique_ptr<content_settings::RuleIterator> result = + channels_provider_->GetRuleIterator(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, + std::string(), false /* incognito */); + EXPECT_TRUE(result->HasNext()); + content_settings::Rule rule = result->Next(); + EXPECT_EQ(ContentSettingsPattern::FromString(kTestOrigin), + rule.primary_pattern); + EXPECT_EQ(CONTENT_SETTING_BLOCK, + content_settings::ValueToContentSetting(rule.value.get())); + EXPECT_FALSE(result->HasNext()); +} + +TEST_F(NotificationChannelsProviderAndroidTest, + GetRuleIteratorWhenOneAllowedChannelExists) { + InitChannelsProvider(true /* should_use_channels */); + std::vector<NotificationChannel> channels; + channels.emplace_back(kTestOrigin, NotificationChannelStatus::ENABLED); + EXPECT_CALL(*mock_bridge_, GetChannels()).WillOnce(Return(channels)); + std::unique_ptr<content_settings::RuleIterator> result = + channels_provider_->GetRuleIterator(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, + std::string(), false /* incognito */); + EXPECT_TRUE(result->HasNext()); + content_settings::Rule rule = result->Next(); + EXPECT_EQ(ContentSettingsPattern::FromString(kTestOrigin), + rule.primary_pattern); + EXPECT_EQ(CONTENT_SETTING_ALLOW, + content_settings::ValueToContentSetting(rule.value.get())); + EXPECT_FALSE(result->HasNext()); +} + +TEST_F(NotificationChannelsProviderAndroidTest, + GetRuleIteratorWhenMultipleChannelsExist) { + InitChannelsProvider(true /* should_use_channels */); + std::vector<NotificationChannel> channels; + channels.emplace_back("https://abc.com", NotificationChannelStatus::ENABLED); + channels.emplace_back("https://xyz.com", NotificationChannelStatus::BLOCKED); + EXPECT_CALL(*mock_bridge_, GetChannels()).WillOnce(Return(channels)); + std::unique_ptr<content_settings::RuleIterator> result = + channels_provider_->GetRuleIterator(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, + std::string(), false /* incognito */); + EXPECT_TRUE(result->HasNext()); + content_settings::Rule first_rule = result->Next(); + EXPECT_EQ(ContentSettingsPattern::FromString("https://abc.com"), + first_rule.primary_pattern); + EXPECT_EQ(CONTENT_SETTING_ALLOW, + content_settings::ValueToContentSetting(first_rule.value.get())); + EXPECT_TRUE(result->HasNext()); + content_settings::Rule second_rule = result->Next(); + EXPECT_EQ(ContentSettingsPattern::FromString("https://xyz.com"), + second_rule.primary_pattern); + EXPECT_EQ(CONTENT_SETTING_BLOCK, + content_settings::ValueToContentSetting(second_rule.value.get())); + EXPECT_FALSE(result->HasNext()); +}
diff --git a/chrome/browser/notifications/notification_object_proxy.cc b/chrome/browser/notifications/notification_object_proxy.cc deleted file mode 100644 index 55a8b84..0000000 --- a/chrome/browser/notifications/notification_object_proxy.cc +++ /dev/null
@@ -1,49 +0,0 @@ -// Copyright (c) 2012 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. - -#include "chrome/browser/notifications/notification_object_proxy.h" - -#include <utility> - -#include "base/logging.h" -#include "chrome/browser/notifications/platform_notification_service_impl.h" -#include "content/public/browser/desktop_notification_delegate.h" -#include "url/gurl.h" - -NotificationObjectProxy::NotificationObjectProxy( - content::BrowserContext* browser_context, - const std::string& notification_id, - const GURL& origin, - std::unique_ptr<content::DesktopNotificationDelegate> delegate) - : WebNotificationDelegate(browser_context, notification_id, origin), - delegate_(std::move(delegate)), - displayed_(false) {} - -NotificationObjectProxy::~NotificationObjectProxy() {} - -void NotificationObjectProxy::Display() { - // This method is called each time the notification is shown to the user - // but we only want to fire the event the first time. - if (displayed_) - return; - - displayed_ = true; - - delegate_->NotificationDisplayed(); -} - -void NotificationObjectProxy::Close(bool by_user) { - delegate_->NotificationClosed(); -} - -void NotificationObjectProxy::Click() { - delegate_->NotificationClick(); -} - -void NotificationObjectProxy::ButtonClick(int button_index) { - // Notification buttons not are supported for non persistent notifications. - DCHECK_EQ(button_index, 0); - - NotificationCommon::OpenNotificationSettings(browser_context()); -}
diff --git a/chrome/browser/notifications/notification_object_proxy.h b/chrome/browser/notifications/notification_object_proxy.h deleted file mode 100644 index 02eab40a..0000000 --- a/chrome/browser/notifications/notification_object_proxy.h +++ /dev/null
@@ -1,51 +0,0 @@ -// Copyright (c) 2012 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. - -#ifndef CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_OBJECT_PROXY_H_ -#define CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_OBJECT_PROXY_H_ - -#include <memory> -#include <string> - -#include "base/macros.h" -#include "chrome/browser/notifications/web_notification_delegate.h" - -class GURL; - -namespace content { -class BrowserContext; -class DesktopNotificationDelegate; -} - -// A NotificationObjectProxy stands in for the JavaScript Notification object -// which corresponds to a notification toast on the desktop. It can be signaled -// when various events occur regarding the desktop notification, and the -// attached JS listeners will be invoked in the renderer or worker process. -class NotificationObjectProxy : public WebNotificationDelegate { - public: - // Creates a Proxy object with the necessary callback information. The Proxy - // will take ownership of |delegate|. - NotificationObjectProxy( - content::BrowserContext* browser_context, - const std::string& notification_id, - const GURL& origin, - std::unique_ptr<content::DesktopNotificationDelegate> delegate); - - // NotificationDelegate implementation. - void Display() override; - void Close(bool by_user) override; - void Click() override; - void ButtonClick(int button_index) override; - - protected: - ~NotificationObjectProxy() override; - - private: - std::unique_ptr<content::DesktopNotificationDelegate> delegate_; - bool displayed_; - - DISALLOW_COPY_AND_ASSIGN(NotificationObjectProxy); -}; - -#endif // CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_OBJECT_PROXY_H_
diff --git a/chrome/browser/notifications/notification_platform_bridge_android.cc b/chrome/browser/notifications/notification_platform_bridge_android.cc index 7ed7fcf..7e928f2 100644 --- a/chrome/browser/notifications/notification_platform_bridge_android.cc +++ b/chrome/browser/notifications/notification_platform_bridge_android.cc
@@ -19,7 +19,6 @@ #include "chrome/browser/notifications/notification_common.h" #include "chrome/browser/notifications/notification_display_service.h" #include "chrome/browser/notifications/notification_display_service_factory.h" -#include "chrome/browser/notifications/persistent_notification_delegate.h" #include "chrome/browser/notifications/platform_notification_service_impl.h" #include "chrome/browser/profiles/profile_manager.h" #include "chrome/common/chrome_switches.h"
diff --git a/chrome/browser/notifications/notification_platform_bridge_mac.mm b/chrome/browser/notifications/notification_platform_bridge_mac.mm index 7dc5255..7aa66e0 100644 --- a/chrome/browser/notifications/notification_platform_bridge_mac.mm +++ b/chrome/browser/notifications/notification_platform_bridge_mac.mm
@@ -24,7 +24,6 @@ #include "chrome/browser/notifications/notification_common.h" #include "chrome/browser/notifications/notification_display_service.h" #include "chrome/browser/notifications/notification_display_service_factory.h" -#include "chrome/browser/notifications/persistent_notification_delegate.h" #include "chrome/browser/notifications/platform_notification_service_impl.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile_manager.h"
diff --git a/chrome/browser/notifications/persistent_notification_delegate.cc b/chrome/browser/notifications/persistent_notification_delegate.cc deleted file mode 100644 index b38f3c1..0000000 --- a/chrome/browser/notifications/persistent_notification_delegate.cc +++ /dev/null
@@ -1,54 +0,0 @@ -// Copyright 2014 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. - -#include "chrome/browser/notifications/persistent_notification_delegate.h" - -#include "base/strings/nullable_string16.h" -#include "chrome/browser/notifications/platform_notification_service_impl.h" -#include "url/gurl.h" - -PersistentNotificationDelegate::PersistentNotificationDelegate( - content::BrowserContext* browser_context, - const std::string& notification_id, - const GURL& origin, - int notification_settings_index) - : WebNotificationDelegate(browser_context, notification_id, origin), - notification_settings_index_(notification_settings_index) {} - -PersistentNotificationDelegate::~PersistentNotificationDelegate() {} - -void PersistentNotificationDelegate::Display() {} - -void PersistentNotificationDelegate::Close(bool by_user) { - PlatformNotificationServiceImpl::GetInstance()->OnPersistentNotificationClose( - browser_context(), id(), origin(), by_user); -} - -void PersistentNotificationDelegate::Click() { - PlatformNotificationServiceImpl::GetInstance()->OnPersistentNotificationClick( - browser_context(), id(), origin(), -1 /* action_index */, - base::NullableString16() /* reply */); -} - -void PersistentNotificationDelegate::ButtonClick(int button_index) { - DCHECK_GE(button_index, 0); - if (button_index == notification_settings_index_) { - NotificationCommon::OpenNotificationSettings(browser_context()); - return; - } - - PlatformNotificationServiceImpl::GetInstance()->OnPersistentNotificationClick( - browser_context(), id(), origin(), button_index, - base::NullableString16() /* reply */); -} - -void PersistentNotificationDelegate::ButtonClickWithReply( - int button_index, - const base::string16& reply) { - DCHECK_GE(button_index, 0); - DCHECK_NE(button_index, notification_settings_index_); - PlatformNotificationServiceImpl::GetInstance()->OnPersistentNotificationClick( - browser_context(), id(), origin(), button_index, - base::NullableString16(reply, false /* is_null */)); -}
diff --git a/chrome/browser/notifications/persistent_notification_delegate.h b/chrome/browser/notifications/persistent_notification_delegate.h deleted file mode 100644 index ace6d1b..0000000 --- a/chrome/browser/notifications/persistent_notification_delegate.h +++ /dev/null
@@ -1,46 +0,0 @@ -// Copyright 2014 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. - -#ifndef CHROME_BROWSER_NOTIFICATIONS_PERSISTENT_NOTIFICATION_DELEGATE_H_ -#define CHROME_BROWSER_NOTIFICATIONS_PERSISTENT_NOTIFICATION_DELEGATE_H_ - -#include <string> - -#include "base/macros.h" -#include "chrome/browser/notifications/web_notification_delegate.h" - -class GURL; - -namespace content { -class BrowserContext; -} - -// Delegate responsible for listening to the click event on persistent -// notifications, to forward them to the PlatformNotificationService so that -// JavaScript events can be fired on the associated Service Worker. -class PersistentNotificationDelegate : public WebNotificationDelegate { - public: - PersistentNotificationDelegate(content::BrowserContext* browser_context, - const std::string& notification_id, - const GURL& origin, - int notification_settings_index); - - // NotificationDelegate implementation. - void Display() override; - void Close(bool by_user) override; - void Click() override; - void ButtonClick(int button_index) override; - void ButtonClickWithReply(int button_index, - const base::string16& reply) override; - - protected: - ~PersistentNotificationDelegate() override; - - private: - int notification_settings_index_; - - DISALLOW_COPY_AND_ASSIGN(PersistentNotificationDelegate); -}; - -#endif // CHROME_BROWSER_NOTIFICATIONS_PERSISTENT_NOTIFICATION_DELEGATE_H_
diff --git a/chrome/browser/notifications/platform_notification_service_impl.cc b/chrome/browser/notifications/platform_notification_service_impl.cc index cce7df8..91f9040e 100644 --- a/chrome/browser/notifications/platform_notification_service_impl.cc +++ b/chrome/browser/notifications/platform_notification_service_impl.cc
@@ -19,8 +19,7 @@ #include "chrome/browser/notifications/native_notification_delegate.h" #include "chrome/browser/notifications/notification_common.h" #include "chrome/browser/notifications/notification_display_service_factory.h" -#include "chrome/browser/notifications/notification_object_proxy.h" -#include "chrome/browser/notifications/persistent_notification_delegate.h" +#include "chrome/browser/notifications/web_notification_delegate.h" #include "chrome/browser/permissions/permission_decision_auto_blocker.h" #include "chrome/browser/permissions/permission_manager.h" #include "chrome/browser/permissions/permission_result.h" @@ -40,7 +39,6 @@ #include "components/content_settings/core/common/content_settings_types.h" #include "components/prefs/pref_service.h" #include "content/public/browser/browser_thread.h" -#include "content/public/browser/desktop_notification_delegate.h" #include "content/public/browser/notification_event_dispatcher.h" #include "content/public/common/notification_resources.h" #include "content/public/common/platform_notification_data.h" @@ -315,7 +313,6 @@ const GURL& origin, const content::PlatformNotificationData& notification_data, const content::NotificationResources& notification_resources, - std::unique_ptr<content::DesktopNotificationDelegate> delegate, base::Closure* cancel_callback) { DCHECK_CURRENTLY_ON(BrowserThread::UI); @@ -330,20 +327,8 @@ DCHECK_EQ(0u, notification_data.actions.size()); DCHECK_EQ(0u, notification_resources.action_icons.size()); - // Temporary change while the delegates are merged. - NotificationDelegate* notification_delegate; -#if BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS) - if (base::FeatureList::IsEnabled(features::kNativeNotifications) && - g_browser_process->notification_platform_bridge()) { - notification_delegate = new NativeNotificationDelegate(notification_id); - } else { - notification_delegate = new NotificationObjectProxy( - browser_context, notification_id, origin, std::move(delegate)); - } -#else - notification_delegate = new NotificationObjectProxy( - browser_context, notification_id, origin, std::move(delegate)); -#endif // BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS) + NotificationDelegate* notification_delegate = new WebNotificationDelegate( + NotificationCommon::NON_PERSISTENT, profile, notification_id, origin); Notification notification = CreateNotificationFromData( profile, GURL() /* service_worker_scope */, origin, notification_data, @@ -381,12 +366,8 @@ Profile* profile = Profile::FromBrowserContext(browser_context); DCHECK(profile); - // The notification settings button will be appended after the developer- - // supplied buttons, available in |notification_data.actions|. - int settings_button_index = notification_data.actions.size(); - - PersistentNotificationDelegate* delegate = new PersistentNotificationDelegate( - browser_context, notification_id, origin, settings_button_index); + NotificationDelegate* delegate = new WebNotificationDelegate( + NotificationCommon::PERSISTENT, profile, notification_id, origin); Notification notification = CreateNotificationFromData( profile, service_worker_scope, origin, notification_data,
diff --git a/chrome/browser/notifications/platform_notification_service_impl.h b/chrome/browser/notifications/platform_notification_service_impl.h index 5a5253c..3bfa09e 100644 --- a/chrome/browser/notifications/platform_notification_service_impl.h +++ b/chrome/browser/notifications/platform_notification_service_impl.h
@@ -84,7 +84,6 @@ const GURL& origin, const content::PlatformNotificationData& notification_data, const content::NotificationResources& notification_resources, - std::unique_ptr<content::DesktopNotificationDelegate> delegate, base::Closure* cancel_callback) override; void DisplayPersistentNotification( content::BrowserContext* browser_context,
diff --git a/chrome/browser/notifications/platform_notification_service_unittest.cc b/chrome/browser/notifications/platform_notification_service_unittest.cc index c51c1e8..cd2715e 100644 --- a/chrome/browser/notifications/platform_notification_service_unittest.cc +++ b/chrome/browser/notifications/platform_notification_service_unittest.cc
@@ -17,7 +17,7 @@ #include "build/build_config.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/notifications/message_center_display_service.h" -#include "chrome/browser/notifications/notification_delegate.h" +#include "chrome/browser/notifications/native_notification_delegate.h" #include "chrome/browser/notifications/notification_display_service_factory.h" #include "chrome/browser/notifications/notification_test_util.h" #include "chrome/browser/notifications/platform_notification_service_impl.h" @@ -27,7 +27,6 @@ #include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile_manager.h" #include "components/content_settings/core/browser/host_content_settings_map.h" -#include "content/public/browser/desktop_notification_delegate.h" #include "content/public/common/notification_resources.h" #include "content/public/common/platform_notification_data.h" #include "content/public/test/test_browser_thread_bundle.h" @@ -56,26 +55,6 @@ const char kNotificationId[] = "my-notification-id"; const int kNotificationVibrationPattern[] = { 100, 200, 300 }; -class MockDesktopNotificationDelegate - : public content::DesktopNotificationDelegate { - public: - MockDesktopNotificationDelegate() : displayed_(false) {} - - ~MockDesktopNotificationDelegate() override {} - - // content::DesktopNotificationDelegate implementation. - void NotificationDisplayed() override { displayed_ = true; } - void NotificationClosed() override {} - void NotificationClick() override {} - - bool displayed() const { return displayed_; } - - private: - bool displayed_; - - DISALLOW_COPY_AND_ASSIGN(MockDesktopNotificationDelegate); -}; - } // namespace class PlatformNotificationServiceTest : public testing::Test { @@ -110,30 +89,22 @@ } protected: - // Displays a simple, fake notifications and returns a weak pointer to the - // delegate receiving events for it (ownership is transferred to the service). - MockDesktopNotificationDelegate* CreateSimplePageNotification() const { - return CreateSimplePageNotificationWithCloseClosure(nullptr); + // Displays a simple, fake notifications. + void CreateSimplePageNotification() const { + CreateSimplePageNotificationWithCloseClosure(nullptr); } - // Displays a simple, fake notification and returns a weak pointer to the - // delegate receiving events for it (ownership is transferred to the service). - // The close closure may be specified if so desired. - MockDesktopNotificationDelegate* CreateSimplePageNotificationWithCloseClosure( + // Displays a simple, fake notification. + // The close closure may be specified if desired. + void CreateSimplePageNotificationWithCloseClosure( base::Closure* close_closure) const { PlatformNotificationData notification_data; notification_data.title = base::ASCIIToUTF16("My Notification"); notification_data.body = base::ASCIIToUTF16("Hello, world!"); - MockDesktopNotificationDelegate* delegate = - new MockDesktopNotificationDelegate(); - - service()->DisplayNotification(profile(), kNotificationId, - GURL("https://chrome.com/"), - notification_data, NotificationResources(), - base::WrapUnique(delegate), close_closure); - - return delegate; + service()->DisplayNotification( + profile(), kNotificationId, GURL("https://chrome.com/"), + notification_data, NotificationResources(), close_closure); } // Returns the Platform Notification Service these unit tests are for. @@ -183,19 +154,6 @@ std::unique_ptr<std::set<std::string>> displayed_notifications_; }; -// Native, non persistent notifications don't have delegates any more -#if !defined(OS_MACOSX) -#if defined(OS_ANDROID) -// http://crbug.com/729247 -#define DisplayPageDisplayedEvent DISABLED_DisplayPageDisplayedEvent -#endif -TEST_F(PlatformNotificationServiceTest, DisplayPageDisplayedEvent) { - auto* delegate = CreateSimplePageNotification(); - - EXPECT_EQ(1u, GetNotificationCount()); - EXPECT_TRUE(delegate->displayed()); -} -#endif // !defined(OS_MACOSX) TEST_F(PlatformNotificationServiceTest, DisplayPageCloseClosure) { base::Closure close_closure; @@ -244,12 +202,9 @@ notification_data.vibration_pattern = vibration_pattern; notification_data.silent = true; - MockDesktopNotificationDelegate* delegate - = new MockDesktopNotificationDelegate(); service()->DisplayNotification(profile(), kNotificationId, GURL("https://chrome.com/"), notification_data, - NotificationResources(), - base::WrapUnique(delegate), nullptr); + NotificationResources(), nullptr); ASSERT_EQ(1u, GetNotificationCount()); @@ -400,7 +355,7 @@ Notification notification = service()->CreateNotificationFromData( profile(), GURL() /* service_worker_scope */, GURL("https://chrome.com/"), notification_data, NotificationResources(), - new MockNotificationDelegate("hello")); + new NativeNotificationDelegate("hello")); EXPECT_TRUE(notification.context_message().empty()); // Create a mocked extension. @@ -420,11 +375,10 @@ EXPECT_TRUE(registry->AddEnabled(extension)); notification = service()->CreateNotificationFromData( - profile(), - GURL() /* service_worker_scope */, + profile(), GURL() /* service_worker_scope */, GURL("chrome-extension://honijodknafkokifofgiaalefdiedpko/main.html"), notification_data, NotificationResources(), - new MockNotificationDelegate("hello")); + new NativeNotificationDelegate("hello")); EXPECT_EQ("NotificationTest", base::UTF16ToUTF8(notification.context_message())); }
diff --git a/chrome/browser/notifications/web_notification_delegate.cc b/chrome/browser/notifications/web_notification_delegate.cc index 86671a5..8d4e792 100644 --- a/chrome/browser/notifications/web_notification_delegate.cc +++ b/chrome/browser/notifications/web_notification_delegate.cc
@@ -6,7 +6,9 @@ #include "base/feature_list.h" #include "base/metrics/histogram_macros.h" -#include "chrome/browser/notifications/notification_common.h" +#include "base/strings/nullable_string16.h" +#include "chrome/browser/notifications/notification_display_service.h" +#include "chrome/browser/notifications/notification_display_service_factory.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_list.h" @@ -27,12 +29,13 @@ } // namespace features - WebNotificationDelegate::WebNotificationDelegate( - content::BrowserContext* browser_context, + NotificationCommon::Type notification_type, + Profile* profile, const std::string& notification_id, const GURL& origin) - : browser_context_(browser_context), + : notification_type_(notification_type), + profile_(profile), notification_id_(notification_id), origin_(origin) {} @@ -44,7 +47,7 @@ bool WebNotificationDelegate::SettingsClick() { #if !defined(OS_CHROMEOS) - NotificationCommon::OpenNotificationSettings(browser_context_); + NotificationCommon::OpenNotificationSettings(profile_); return true; #else return false; @@ -57,13 +60,11 @@ bool WebNotificationDelegate::ShouldDisplayOverFullscreen() const { #if !defined(OS_ANDROID) - Profile* profile = Profile::FromBrowserContext(browser_context_); - DCHECK(profile); // Check to see if this notification comes from a webpage that is displaying // fullscreen content. for (auto* browser : *BrowserList::GetInstance()) { // Only consider the browsers for the profile that created the notification - if (browser->profile() != profile) + if (browser->profile() != profile_) continue; const content::WebContents* active_contents = @@ -98,3 +99,41 @@ return false; } + +void WebNotificationDelegate::Display() {} + +void WebNotificationDelegate::Close(bool by_user) { + auto* display_service = + NotificationDisplayServiceFactory::GetForProfile(profile_); + display_service->ProcessNotificationOperation( + NotificationCommon::CLOSE, notification_type_, origin().spec(), + notification_id_, -1, base::NullableString16()); +} + +void WebNotificationDelegate::Click() { + auto* display_service = + NotificationDisplayServiceFactory::GetForProfile(profile_); + display_service->ProcessNotificationOperation( + NotificationCommon::CLICK, notification_type_, origin().spec(), + notification_id_, -1, base::NullableString16()); +} + +void WebNotificationDelegate::ButtonClick(int button_index) { + DCHECK(button_index >= 0); + auto* display_service = + NotificationDisplayServiceFactory::GetForProfile(profile_); + display_service->ProcessNotificationOperation( + NotificationCommon::CLICK, notification_type_, origin().spec(), + notification_id_, button_index, base::NullableString16()); +} + +void WebNotificationDelegate::ButtonClickWithReply( + int button_index, + const base::string16& reply) { + auto* display_service = + NotificationDisplayServiceFactory::GetForProfile(profile_); + display_service->ProcessNotificationOperation( + NotificationCommon::CLICK, notification_type_, origin().spec(), + notification_id_, button_index, + base::NullableString16(reply, false /* is_null */)); +}
diff --git a/chrome/browser/notifications/web_notification_delegate.h b/chrome/browser/notifications/web_notification_delegate.h index d4629fc6..592ef79 100644 --- a/chrome/browser/notifications/web_notification_delegate.h +++ b/chrome/browser/notifications/web_notification_delegate.h
@@ -9,12 +9,11 @@ #include "base/feature_list.h" #include "base/macros.h" +#include "chrome/browser/notifications/notification_common.h" #include "chrome/browser/notifications/notification_delegate.h" #include "url/gurl.h" -namespace content { -class BrowserContext; -} +class Profile; namespace features { @@ -22,31 +21,33 @@ } // namespace features -// Base class for the PersistentNotificationDelegate and the -// NotificationObjectProxy. All common functionality for displaying web -// notifications is found here. -// TODO(peter, crbug.com/596161): Migrate this functionality offered by the -// delegate to the NotificationDisplayService. +// Delegate class for Web Notifications. class WebNotificationDelegate : public NotificationDelegate { public: + WebNotificationDelegate(NotificationCommon::Type notification_type, + Profile* profile, + const std::string& notification_id, + const GURL& origin); + // NotificationDelegate implementation. std::string id() const override; bool SettingsClick() override; bool ShouldDisplaySettingsButton() override; bool ShouldDisplayOverFullscreen() const override; + void Display() override; + void Close(bool by_user) override; + void Click() override; + void ButtonClick(int button_index) override; + void ButtonClickWithReply(int button_index, + const base::string16& reply) override; protected: - WebNotificationDelegate(content::BrowserContext* browser_context, - const std::string& notification_id, - const GURL& origin); - ~WebNotificationDelegate() override; - - content::BrowserContext* browser_context() { return browser_context_; } const GURL& origin() { return origin_; } private: - content::BrowserContext* browser_context_; + NotificationCommon::Type notification_type_; + Profile* profile_; std::string notification_id_; GURL origin_;
diff --git a/chrome/browser/safe_browsing/notification_image_reporter_unittest.cc b/chrome/browser/safe_browsing/notification_image_reporter_unittest.cc index 031d0b45..5a3774af 100644 --- a/chrome/browser/safe_browsing/notification_image_reporter_unittest.cc +++ b/chrome/browser/safe_browsing/notification_image_reporter_unittest.cc
@@ -171,6 +171,9 @@ feature_list_ = base::MakeUnique<base::test::ScopedFeatureList>(); if (level == SBER_LEVEL_SCOUT) feature_list_->InitWithFeatures({safe_browsing::kOnlyShowScoutOptIn}, {}); + else + // Explicitly disable CanShowScoutOptIn, which is on by default. + feature_list_->InitWithFeatures({}, {safe_browsing::kCanShowScoutOptIn}); InitializeSafeBrowsingPrefs(profile_->GetPrefs()); SetExtendedReportingPref(profile_->GetPrefs(), level != SBER_LEVEL_OFF);
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc index c6e948a..cb7dafba 100644 --- a/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc +++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
@@ -1251,8 +1251,12 @@ if (expect_threat_details) SetReportSentCallback(threat_report_sent_runner->QuitClosure()); + // Turn on both SBER and Scout prefs so we're independent of the Scout + // rollout. browser()->profile()->GetPrefs()->SetBoolean( - prefs::kSafeBrowsingExtendedReportingEnabled, true); // set up SBER + prefs::kSafeBrowsingExtendedReportingEnabled, true); + browser()->profile()->GetPrefs()->SetBoolean( + prefs::kSafeBrowsingScoutReportingEnabled, true); GURL url = SetupWarningAndNavigate(browser()); // not incognito EXPECT_TRUE(hit_report_sent()); }
diff --git a/chrome/browser/ui/views/payments/credit_card_editor_view_controller_browsertest.cc b/chrome/browser/ui/views/payments/credit_card_editor_view_controller_browsertest.cc index 2a3fd75..f1a1fb26 100644 --- a/chrome/browser/ui/views/payments/credit_card_editor_view_controller_browsertest.cc +++ b/chrome/browser/ui/views/payments/credit_card_editor_view_controller_browsertest.cc
@@ -17,6 +17,7 @@ #include "components/autofill/core/browser/field_types.h" #include "components/autofill/core/browser/personal_data_manager.h" #include "components/autofill/core/browser/test_autofill_clock.h" +#include "components/autofill/core/browser/test_region_data_loader.h" #include "components/payments/content/payment_request.h" #include "components/payments/content/payment_request_spec.h" #include "components/payments/core/autofill_payment_instrument.h" @@ -673,6 +674,15 @@ ASSERT_NE(nullptr, billing_address_combobox); EXPECT_FALSE(billing_address_combobox->enabled()); + // Add some region data to load synchonously. + autofill::TestRegionDataLoader test_region_data_loader_; + SetRegionDataLoader(&test_region_data_loader_); + test_region_data_loader_.set_synchronous_callback(true); + std::vector<std::pair<std::string, std::string>> regions1; + regions1.push_back(std::make_pair("AL", "Alabama")); + regions1.push_back(std::make_pair("CA", "California")); + test_region_data_loader_.SetRegionData(regions1); + // Click to open the address editor ResetEventObserver(DialogEvent::SHIPPING_ADDRESS_EDITOR_OPENED); ClickOnDialogViewAndWait(DialogViewID::ADD_BILLING_ADDRESS_BUTTON); @@ -683,6 +693,8 @@ autofill::ADDRESS_HOME_STREET_ADDRESS); SetEditorTextfieldValue(base::ASCIIToUTF16("BobCity"), autofill::ADDRESS_HOME_CITY); + SetComboboxValue(base::UTF8ToUTF16("California"), + autofill::ADDRESS_HOME_STATE); SetEditorTextfieldValue(base::ASCIIToUTF16("BobZip"), autofill::ADDRESS_HOME_ZIP); SetEditorTextfieldValue(base::ASCIIToUTF16("5755555555"),
diff --git a/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc b/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc index 0a0b9fd4..444ac2e 100644 --- a/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc +++ b/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc
@@ -248,9 +248,10 @@ if (chosen_country == countries_[chosen_country_index_].second) break; } - // Make sure the the country was actually found in |countries_|, otherwise - // set |chosen_country_index_| as the default country at index 0. - if (chosen_country_index_ >= countries_.size()) { + // Make sure the the country was actually found in |countries_| and was not + // empty, otherwise set |chosen_country_index_| to index 0, which is the + // default country based on the locale. + if (chosen_country_index_ >= countries_.size() || chosen_country.empty()) { // But only if there is at least one country. if (countries_.size() > 0) { LOG(ERROR) << "Unexpected country: " << chosen_country; @@ -518,6 +519,13 @@ } return false; } + + if (field_.type == autofill::ADDRESS_HOME_STATE && + value == l10n_util::GetStringUTF16(IDS_AUTOFILL_LOADING_REGIONS)) { + // Wait for the regions to be loaded or timeout before assessing validity. + return false; + } + // As long as other field types are non-empty, they are valid. return true; }
diff --git a/chrome/browser/ui/views/payments/shipping_address_editor_view_controller_browsertest.cc b/chrome/browser/ui/views/payments/shipping_address_editor_view_controller_browsertest.cc index 68fd515..58b81875 100644 --- a/chrome/browser/ui/views/payments/shipping_address_editor_view_controller_browsertest.cc +++ b/chrome/browser/ui/views/payments/shipping_address_editor_view_controller_browsertest.cc
@@ -271,9 +271,12 @@ regions.push_back(std::make_pair("code", kAnyState)); test_region_data_loader_.SendAsynchronousData(regions); - std::string country_code(GetSelectedCountryCode()); - SetCommonFields(); + SetComboboxValue(base::UTF8ToUTF16("United States"), + autofill::ADDRESS_HOME_COUNTRY); + SetComboboxValue(base::UTF8ToUTF16(kAnyState), autofill::ADDRESS_HOME_STATE); + + std::string country_code(GetSelectedCountryCode()); ResetEventObserver(DialogEvent::BACK_TO_PAYMENT_SHEET_NAVIGATION); @@ -347,12 +350,14 @@ autofill::RegionComboboxModel* region_model = static_cast<autofill::RegionComboboxModel*>(region_combobox->model()); if (use_regions1) { - ASSERT_EQ(1, region_model->GetItemCount()); - EXPECT_EQ(base::ASCIIToUTF16("region1a"), region_model->GetItemAt(0)); - } else { ASSERT_EQ(2, region_model->GetItemCount()); - EXPECT_EQ(base::ASCIIToUTF16("region2a"), region_model->GetItemAt(0)); - EXPECT_EQ(base::ASCIIToUTF16("region2b"), region_model->GetItemAt(1)); + EXPECT_EQ(base::ASCIIToUTF16("---"), region_model->GetItemAt(0)); + EXPECT_EQ(base::ASCIIToUTF16("region1a"), region_model->GetItemAt(1)); + } else { + ASSERT_EQ(3, region_model->GetItemCount()); + EXPECT_EQ(base::ASCIIToUTF16("---"), region_model->GetItemAt(0)); + EXPECT_EQ(base::ASCIIToUTF16("region2a"), region_model->GetItemAt(1)); + EXPECT_EQ(base::ASCIIToUTF16("region2b"), region_model->GetItemAt(2)); } use_regions1 = !use_regions1; } @@ -608,9 +613,17 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest, AddInternationalPhoneNumberFromOtherCountry) { InvokePaymentRequestUI(); + SetRegionDataLoader(&test_region_data_loader_); + test_region_data_loader_.set_synchronous_callback(true); + std::vector<std::pair<std::string, std::string>> regions1; + regions1.push_back(std::make_pair("AL", "Alabama")); + regions1.push_back(std::make_pair("CA", "California")); + test_region_data_loader_.SetRegionData(regions1); OpenShippingAddressEditorScreen(); SetCommonFields(); + SetComboboxValue(base::UTF8ToUTF16("California"), + autofill::ADDRESS_HOME_STATE); // Set an Australian phone number in international format. SetEditorTextfieldValue(base::UTF8ToUTF16("+61 2 9374 4000"), @@ -681,4 +694,226 @@ static_cast<views::Label*>(error_label)->text()); } +// Tests that if the a profile has a country and no state, the editor makes the +// user pick a state. This should also disable the "Done" button. +IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest, + CountryButNoState) { + // Add address with a country but no state. + autofill::AutofillProfile profile = autofill::test::GetFullProfile(); + profile.SetInfo(autofill::AutofillType(autofill::ADDRESS_HOME_STATE), + base::ASCIIToUTF16(""), kLocale); + AddAutofillProfile(profile); + + InvokePaymentRequestUI(); + SetRegionDataLoader(&test_region_data_loader_); + + test_region_data_loader_.set_synchronous_callback(false); + OpenShippingAddressSectionScreen(); + + // There should be an error label for the address. + views::View* sheet = dialog_view()->GetViewByID( + static_cast<int>(DialogViewID::SHIPPING_ADDRESS_SHEET_LIST_VIEW)); + ASSERT_EQ(1, sheet->child_count()); + views::View* error_label = sheet->child_at(0)->GetViewByID( + static_cast<int>(DialogViewID::PROFILE_LABEL_ERROR)); + EXPECT_EQ(base::ASCIIToUTF16("Enter a valid address"), + static_cast<views::Label*>(error_label)->text()); + + ResetEventObserver(DialogEvent::SHIPPING_ADDRESS_EDITOR_OPENED); + ClickOnChildInListViewAndWait(/*child_index=*/0, /*num_children=*/1, + DialogViewID::SHIPPING_ADDRESS_SHEET_LIST_VIEW); + std::vector<std::pair<std::string, std::string>> regions1; + regions1.push_back(std::make_pair("AL", "Alabama")); + regions1.push_back(std::make_pair("CA", "California")); + test_region_data_loader_.SendAsynchronousData(regions1); + // Expect that the country is set correctly. + EXPECT_EQ(base::ASCIIToUTF16("United States"), + GetComboboxValue(autofill::ADDRESS_HOME_COUNTRY)); + + // Expect that no state is selected. + EXPECT_EQ(base::ASCIIToUTF16("---"), + GetComboboxValue(autofill::ADDRESS_HOME_STATE)); + + // Expect that the save button is disabled. + views::View* save_button = dialog_view()->GetViewByID( + static_cast<int>(DialogViewID::SAVE_ADDRESS_BUTTON)); + EXPECT_FALSE(save_button->enabled()); +} + +// TODO(crbug.com/730652): This address should be invalid. +// Tests that if the a profile has a country and an invalid state for the +// country, the address is considered valid. +IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest, + CountryAndInvalidState) { + // Add address with a country but no state. + autofill::AutofillProfile profile = autofill::test::GetFullProfile(); + profile.SetInfo(autofill::AutofillType(autofill::ADDRESS_HOME_STATE), + base::ASCIIToUTF16("INVALIDSTATE"), kLocale); + AddAutofillProfile(profile); + + LOG(ERROR) << profile.GetInfo( + autofill::AutofillType(autofill::ADDRESS_HOME_COUNTRY), kLocale); + + InvokePaymentRequestUI(); + SetRegionDataLoader(&test_region_data_loader_); + test_region_data_loader_.set_synchronous_callback(false); + OpenShippingAddressSectionScreen(); + + // There should be no error label. + views::View* sheet = dialog_view()->GetViewByID( + static_cast<int>(DialogViewID::SHIPPING_ADDRESS_SHEET_LIST_VIEW)); + ASSERT_EQ(1, sheet->child_count()); + EXPECT_EQ(nullptr, sheet->child_at(0)->GetViewByID( + static_cast<int>(DialogViewID::PROFILE_LABEL_ERROR))); +} + +// Tests that if the a profile has no country and no state, the editor sets the +// country and lets the user pick a state. This should also disable the "Done" +// button. +IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest, + NoCountryNoState) { + // Add address without a country or no state. + autofill::AutofillProfile profile = autofill::test::GetFullProfile(); + profile.SetInfo(autofill::AutofillType(autofill::ADDRESS_HOME_COUNTRY), + base::ASCIIToUTF16(""), kLocale); + profile.SetInfo(autofill::AutofillType(autofill::ADDRESS_HOME_STATE), + base::ASCIIToUTF16(""), kLocale); + AddAutofillProfile(profile); + + InvokePaymentRequestUI(); + SetRegionDataLoader(&test_region_data_loader_); + + test_region_data_loader_.set_synchronous_callback(false); + OpenShippingAddressSectionScreen(); + + // There should be an error label for the address. + views::View* sheet = dialog_view()->GetViewByID( + static_cast<int>(DialogViewID::SHIPPING_ADDRESS_SHEET_LIST_VIEW)); + ASSERT_EQ(1, sheet->child_count()); + views::View* error_label = sheet->child_at(0)->GetViewByID( + static_cast<int>(DialogViewID::PROFILE_LABEL_ERROR)); + EXPECT_EQ(base::ASCIIToUTF16("Enter a valid address"), + static_cast<views::Label*>(error_label)->text()); + + ResetEventObserver(DialogEvent::SHIPPING_ADDRESS_EDITOR_OPENED); + ClickOnChildInListViewAndWait(/*child_index=*/0, /*num_children=*/1, + DialogViewID::SHIPPING_ADDRESS_SHEET_LIST_VIEW); + std::vector<std::pair<std::string, std::string>> regions1; + regions1.push_back(std::make_pair("AL", "Alabama")); + regions1.push_back(std::make_pair("CA", "California")); + test_region_data_loader_.SendAsynchronousData(regions1); + + // Expect that the default country was selected. + EXPECT_FALSE(GetComboboxValue(autofill::ADDRESS_HOME_COUNTRY).empty()); + + // Expect that no state is selected. + EXPECT_EQ(base::ASCIIToUTF16("---"), + GetComboboxValue(autofill::ADDRESS_HOME_STATE)); + + // Expect that the save button is disabled. + views::View* save_button = dialog_view()->GetViewByID( + static_cast<int>(DialogViewID::SAVE_ADDRESS_BUTTON)); + EXPECT_FALSE(save_button->enabled()); +} + +// Tests that if the a profile has no country and an invalid state for the +// default country, the editor sets the country and lets the user pick a state. +// This should also disable the "Done" button. +IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest, + NoCountryInvalidState) { + // Add address without a country or no state. + autofill::AutofillProfile profile = autofill::test::GetFullProfile(); + profile.SetInfo(autofill::AutofillType(autofill::ADDRESS_HOME_COUNTRY), + base::ASCIIToUTF16(""), kLocale); + profile.SetInfo(autofill::AutofillType(autofill::ADDRESS_HOME_STATE), + base::ASCIIToUTF16("INVALIDSTATE"), kLocale); + AddAutofillProfile(profile); + + InvokePaymentRequestUI(); + SetRegionDataLoader(&test_region_data_loader_); + + test_region_data_loader_.set_synchronous_callback(false); + OpenShippingAddressSectionScreen(); + + // There should be an error label for the address. + views::View* sheet = dialog_view()->GetViewByID( + static_cast<int>(DialogViewID::SHIPPING_ADDRESS_SHEET_LIST_VIEW)); + ASSERT_EQ(1, sheet->child_count()); + views::View* error_label = sheet->child_at(0)->GetViewByID( + static_cast<int>(DialogViewID::PROFILE_LABEL_ERROR)); + EXPECT_EQ(base::ASCIIToUTF16("Enter a valid address"), + static_cast<views::Label*>(error_label)->text()); + + ResetEventObserver(DialogEvent::SHIPPING_ADDRESS_EDITOR_OPENED); + ClickOnChildInListViewAndWait(/*child_index=*/0, /*num_children=*/1, + DialogViewID::SHIPPING_ADDRESS_SHEET_LIST_VIEW); + std::vector<std::pair<std::string, std::string>> regions1; + regions1.push_back(std::make_pair("AL", "Alabama")); + regions1.push_back(std::make_pair("CA", "California")); + test_region_data_loader_.SendAsynchronousData(regions1); + + // Expect that the default country was selected. + EXPECT_FALSE(GetComboboxValue(autofill::ADDRESS_HOME_COUNTRY).empty()); + + // Expect that no state is selected. + EXPECT_EQ(base::ASCIIToUTF16("---"), + GetComboboxValue(autofill::ADDRESS_HOME_STATE)); + + // Expect that the save button is disabled. + views::View* save_button = dialog_view()->GetViewByID( + static_cast<int>(DialogViewID::SAVE_ADDRESS_BUTTON)); + EXPECT_FALSE(save_button->enabled()); +} + +// TODO(crbug.com/730165): The profile should be considered valid. +// Tests that if the a profile has no country but has a valid state for the +// default country, the editor sets the country and the state for the user. +// This should also enable the "Done" button. +IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest, + NoCountryValidState) { + // Add address without a country but a valid state for the default country. + autofill::AutofillProfile profile = autofill::test::GetFullProfile(); + profile.SetInfo(autofill::AutofillType(autofill::ADDRESS_HOME_COUNTRY), + base::ASCIIToUTF16(""), kLocale); + profile.SetInfo(autofill::AutofillType(autofill::ADDRESS_HOME_STATE), + base::ASCIIToUTF16("California"), kLocale); + AddAutofillProfile(profile); + + InvokePaymentRequestUI(); + SetRegionDataLoader(&test_region_data_loader_); + + // TODO(crbug.com/730166): This should work with async callback too. + test_region_data_loader_.set_synchronous_callback(true); + std::vector<std::pair<std::string, std::string>> regions; + regions.push_back(std::make_pair("AL", "Alabama")); + regions.push_back(std::make_pair("CA", "California")); + test_region_data_loader_.SetRegionData(regions); + OpenShippingAddressSectionScreen(); + + // There should be an error label for the address. + views::View* sheet = dialog_view()->GetViewByID( + static_cast<int>(DialogViewID::SHIPPING_ADDRESS_SHEET_LIST_VIEW)); + ASSERT_EQ(1, sheet->child_count()); + views::View* error_label = sheet->child_at(0)->GetViewByID( + static_cast<int>(DialogViewID::PROFILE_LABEL_ERROR)); + EXPECT_EQ(base::ASCIIToUTF16("Enter a valid address"), + static_cast<views::Label*>(error_label)->text()); + + ResetEventObserver(DialogEvent::SHIPPING_ADDRESS_EDITOR_OPENED); + ClickOnChildInListViewAndWait(/*child_index=*/0, /*num_children=*/1, + DialogViewID::SHIPPING_ADDRESS_SHEET_LIST_VIEW); + + // Expect that the default country was selected. + EXPECT_FALSE(GetComboboxValue(autofill::ADDRESS_HOME_COUNTRY).empty()); + + // Expect that the state was selected. + EXPECT_EQ(base::ASCIIToUTF16("California"), + GetComboboxValue(autofill::ADDRESS_HOME_STATE)); + + // Expect that the save button is enabled, since the profile is now valid. + views::View* save_button = dialog_view()->GetViewByID( + static_cast<int>(DialogViewID::SAVE_ADDRESS_BUTTON)); + EXPECT_TRUE(save_button->enabled()); +} + } // namespace payments
diff --git a/chrome/common/chrome_features.cc b/chrome/common/chrome_features.cc index e696257..d07f061 100644 --- a/chrome/common/chrome_features.cc +++ b/chrome/common/chrome_features.cc
@@ -305,6 +305,13 @@ const base::Feature kSafeSearchUrlReporting{"SafeSearchUrlReporting", base::FEATURE_DISABLED_BY_DEFAULT}; +#if defined(OS_ANDROID) +// Enables separate notification channels in Android O for notifications from +// different origins, instead of sending them all to a single 'Sites' channel. +const base::Feature kSiteNotificationChannels{ + "SiteNotificationChannels", base::FEATURE_DISABLED_BY_DEFAULT}; +#endif // defined (OS_ANDROID) + // A new user experience for transitioning into fullscreen and mouse pointer // lock states. const base::Feature kSimplifiedFullscreenUI{"ViewsSimplifiedFullscreenUI",
diff --git a/chrome/common/chrome_features.h b/chrome/common/chrome_features.h index 96861ed..1ba9a8b 100644 --- a/chrome/common/chrome_features.h +++ b/chrome/common/chrome_features.h
@@ -166,6 +166,10 @@ extern const base::Feature kSiteDetails; +#if defined(OS_ANDROID) +extern const base::Feature kSiteNotificationChannels; +#endif + #if defined(SYZYASAN) extern const base::Feature kSyzyasanDeferredFree; #endif
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index f5cd21b..6d128ec 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn
@@ -3118,6 +3118,7 @@ "../browser/net/spdyproxy/data_reduction_proxy_settings_unittest_android.cc", "../browser/net/url_info_unittest.cc", "../browser/notifications/desktop_notification_profile_util_unittest.cc", + "../browser/notifications/notification_channels_provider_android_unittest.cc", "../browser/notifications/notification_permission_context_unittest.cc", "../browser/notifications/notification_platform_bridge_mac_unittest.mm", "../browser/notifications/platform_notification_service_unittest.cc",
diff --git a/components/autofill/android/java/src/org/chromium/components/autofill/AutofillKeyboardAccessory.java b/components/autofill/android/java/src/org/chromium/components/autofill/AutofillKeyboardAccessory.java index ab59f564..7f0ea2a5 100644 --- a/components/autofill/android/java/src/org/chromium/components/autofill/AutofillKeyboardAccessory.java +++ b/components/autofill/android/java/src/org/chromium/components/autofill/AutofillKeyboardAccessory.java
@@ -230,6 +230,7 @@ } } }); + announceForAccessibility(container.getContentDescription()); } /**
diff --git a/components/autofill/core/browser/region_combobox_model.cc b/components/autofill/core/browser/region_combobox_model.cc index 570fc67..977a692d 100644 --- a/components/autofill/core/browser/region_combobox_model.cc +++ b/components/autofill/core/browser/region_combobox_model.cc
@@ -87,6 +87,7 @@ // Some countries expose a state field but have no region names available. if (regions.size() > 0) { failed_to_load_data_ = false; + regions_.push_back(std::make_pair("", "---")); for (auto* const region : regions) { regions_.push_back(std::make_pair(region->key(), region->name())); }
diff --git a/components/autofill/core/browser/region_combobox_model_unittest.cc b/components/autofill/core/browser/region_combobox_model_unittest.cc index 704fd56..a38ca40 100644 --- a/components/autofill/core/browser/region_combobox_model_unittest.cc +++ b/components/autofill/core/browser/region_combobox_model_unittest.cc
@@ -35,9 +35,10 @@ test_region_data_loader.SendAsynchronousData(regions); - EXPECT_EQ(2, model.GetItemCount()); - EXPECT_EQ(base::ASCIIToUTF16(kQuebec), model.GetItemAt(0)); - EXPECT_EQ(base::ASCIIToUTF16(kOntario), model.GetItemAt(1)); + EXPECT_EQ(3, model.GetItemCount()); + EXPECT_EQ(base::ASCIIToUTF16("---"), model.GetItemAt(0)); + EXPECT_EQ(base::ASCIIToUTF16(kQuebec), model.GetItemAt(1)); + EXPECT_EQ(base::ASCIIToUTF16(kOntario), model.GetItemAt(2)); EXPECT_FALSE(model.failed_to_load_data()); }
diff --git a/components/autofill/core/browser/test_region_data_loader.cc b/components/autofill/core/browser/test_region_data_loader.cc index 7747356f..de8e3fd3 100644 --- a/components/autofill/core/browser/test_region_data_loader.cc +++ b/components/autofill/core/browser/test_region_data_loader.cc
@@ -17,7 +17,7 @@ autofill::RegionDataLoader::RegionDataLoaded callback, int64_t unused_timeout_ms) { if (synchronous_callback_) { - callback.Run(std::vector<const ::i18n::addressinput::RegionData*>()); + SendRegionData(regions_, callback); } else { country_code_ = country_code; callback_ = callback; @@ -37,12 +37,19 @@ if (callback_.is_null()) return; + SendRegionData(regions, callback_); + callback_.Reset(); +} + +void TestRegionDataLoader::SendRegionData( + const std::vector<std::pair<std::string, std::string>>& regions, + autofill::RegionDataLoader::RegionDataLoaded callback) { ::i18n::addressinput::RegionData root_region(""); for (const auto& region : regions) { root_region.AddSubRegion(region.first, region.second); } - callback_.Run(root_region.sub_regions()); - callback_.Reset(); + + callback.Run(root_region.sub_regions()); } } // namespace autofill
diff --git a/components/autofill/core/browser/test_region_data_loader.h b/components/autofill/core/browser/test_region_data_loader.h index c2d2ae94..cf4472e 100644 --- a/components/autofill/core/browser/test_region_data_loader.h +++ b/components/autofill/core/browser/test_region_data_loader.h
@@ -33,10 +33,20 @@ synchronous_callback_ = synchronous_callback; } + void SetRegionData( + std::vector<std::pair<std::string, std::string>>& regions) { + regions_ = regions; + } + void SendAsynchronousData( const std::vector<std::pair<std::string, std::string>>& regions); private: + void SendRegionData( + const std::vector<std::pair<std::string, std::string>>& regions, + autofill::RegionDataLoader::RegionDataLoaded callback); + + std::vector<std::pair<std::string, std::string>> regions_; std::string country_code_; autofill::RegionDataLoader::RegionDataLoaded callback_; bool synchronous_callback_{false};
diff --git a/components/content_settings/core/browser/host_content_settings_map.cc b/components/content_settings/core/browser/host_content_settings_map.cc index 52a6708..92c7e2a3 100644 --- a/components/content_settings/core/browser/host_content_settings_map.cc +++ b/components/content_settings/core/browser/host_content_settings_map.cc
@@ -55,6 +55,7 @@ {"policy", content_settings::SETTING_SOURCE_POLICY}, {"supervised_user", content_settings::SETTING_SOURCE_SUPERVISED}, {"extension", content_settings::SETTING_SOURCE_EXTENSION}, + {"notification_android", content_settings::SETTING_SOURCE_USER}, {"preference", content_settings::SETTING_SOURCE_USER}, {"default", content_settings::SETTING_SOURCE_USER}, }; @@ -384,6 +385,8 @@ std::unique_ptr<base::Value> value) { DCHECK(SupportsResourceIdentifier(content_type) || resource_identifier.empty()); + // TODO(crbug.com/731126): Verify that assumptions for notification content + // settings are met. UsedContentSettingsProviders(); for (const auto& provider_pair : content_settings_providers_) { @@ -910,4 +913,4 @@ void HostContentSettingsMap::SetClockForTesting( std::unique_ptr<base::Clock> clock) { pref_provider_->SetClockForTesting(std::move(clock)); -} \ No newline at end of file +}
diff --git a/components/content_settings/core/browser/host_content_settings_map.h b/components/content_settings/core/browser/host_content_settings_map.h index b5c7798b..b9f4bfb 100644 --- a/components/content_settings/core/browser/host_content_settings_map.h +++ b/components/content_settings/core/browser/host_content_settings_map.h
@@ -57,6 +57,7 @@ POLICY_PROVIDER, SUPERVISED_PROVIDER, CUSTOM_EXTENSION_PROVIDER, + NOTIFICATION_ANDROID_PROVIDER, PREF_PROVIDER, DEFAULT_PROVIDER, NUM_PROVIDER_TYPES
diff --git a/components/exo/surface.cc b/components/exo/surface.cc index 771e0aa..b6edb5b 100644 --- a/components/exo/surface.cc +++ b/components/exo/surface.cc
@@ -185,10 +185,6 @@ //////////////////////////////////////////////////////////////////////////////// // Surface, public: -// TODO(fsamuel): exo should not use context_factory_private. Instead, we should -// request a CompositorFrameSink from the aura::Window. Setting up the -// BeginFrame hierarchy should be an internal implementation detail of aura or -// mus in aura-mus. Surface::Surface() : window_(new aura::Window(new CustomWindowDelegate(this))) { window_->SetType(aura::client::WINDOW_TYPE_CONTROL); window_->SetName("ExoSurface"); @@ -808,6 +804,10 @@ } content_size_ = layer_size; + // We need update window_'s bounds with content size, because the + // CompositorFrameSink may not update the window's size base the size of + // the lastest submitted CompositorFrame. + window_->SetBounds(gfx::Rect(window_->bounds().origin(), content_size_)); // TODO(jbauman): Figure out how this interacts with the pixel size of // CopyOutputRequests on the layer. gfx::Size contents_surface_size = layer_size;
diff --git a/components/ntp_snippets/remote/json_request.cc b/components/ntp_snippets/remote/json_request.cc index 3f7af6e8..2ea9f4f7 100644 --- a/components/ntp_snippets/remote/json_request.cc +++ b/components/ntp_snippets/remote/json_request.cc
@@ -52,8 +52,6 @@ // Variation parameter for disabling the retry. const char kBackground5xxRetriesName[] = "background_5xx_retries_count"; -const int kMaxExcludedIds = 100; - // Variation parameter for sending LanguageModel info to the server. const char kSendTopLanguagesName[] = "send_top_languages"; @@ -321,9 +319,6 @@ auto excluded = base::MakeUnique<base::ListValue>(); for (const auto& id : params_.excluded_ids) { excluded->AppendString(id); - if (excluded->GetSize() >= kMaxExcludedIds) { - break; - } } request->Set("excludedSuggestionIds", std::move(excluded));
diff --git a/components/ntp_snippets/remote/json_request_unittest.cc b/components/ntp_snippets/remote/json_request_unittest.cc index d19f2c8b..3791ccd 100644 --- a/components/ntp_snippets/remote/json_request_unittest.cc +++ b/components/ntp_snippets/remote/json_request_unittest.cc
@@ -148,7 +148,7 @@ "}")); } -TEST_F(JsonRequestTest, BuildRequestExcludedIds) { +TEST_F(JsonRequestTest, ShouldNotTruncateExcludedIdsList) { JsonRequest::Builder builder; RequestParams params; params.interactive_request = false; @@ -180,9 +180,27 @@ " \"080\", \"081\", \"082\", \"083\", \"084\"," " \"085\", \"086\", \"087\", \"088\", \"089\"," " \"090\", \"091\", \"092\", \"093\", \"094\"," - " \"095\", \"096\", \"097\", \"098\", \"099\"" - // Truncated to 100 entries. Currently, they happen to - // be those lexically first. + " \"095\", \"096\", \"097\", \"098\", \"099\"," + " \"100\", \"101\", \"102\", \"103\", \"104\"," + " \"105\", \"106\", \"107\", \"108\", \"109\"," + " \"110\", \"111\", \"112\", \"113\", \"114\"," + " \"115\", \"116\", \"117\", \"118\", \"119\"," + " \"120\", \"121\", \"122\", \"123\", \"124\"," + " \"125\", \"126\", \"127\", \"128\", \"129\"," + " \"130\", \"131\", \"132\", \"133\", \"134\"," + " \"135\", \"136\", \"137\", \"138\", \"139\"," + " \"140\", \"141\", \"142\", \"143\", \"144\"," + " \"145\", \"146\", \"147\", \"148\", \"149\"," + " \"150\", \"151\", \"152\", \"153\", \"154\"," + " \"155\", \"156\", \"157\", \"158\", \"159\"," + " \"160\", \"161\", \"162\", \"163\", \"164\"," + " \"165\", \"166\", \"167\", \"168\", \"169\"," + " \"170\", \"171\", \"172\", \"173\", \"174\"," + " \"175\", \"176\", \"177\", \"178\", \"179\"," + " \"180\", \"181\", \"182\", \"183\", \"184\"," + " \"185\", \"186\", \"187\", \"188\", \"189\"," + " \"190\", \"191\", \"192\", \"193\", \"194\"," + " \"195\", \"196\", \"197\", \"198\", \"199\"" " ]," " \"userActivenessClass\": \"ACTIVE_NTP_USER\"" "}"));
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc b/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc index 87766dd..a3a8b9f 100644 --- a/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc +++ b/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
@@ -48,6 +48,10 @@ // Number of archived suggestions we keep around in memory. const int kMaxArchivedSuggestionCount = 200; +// Maximal number of dismissed suggestions to exclude when fetching new +// suggestions from the server. This limit exists to decrease data usage. +const int kMaxExcludedDismissedIds = 100; + // Keys for storing CategoryContent info in prefs. const char kCategoryContentId[] = "id"; const char kCategoryContentTitle[] = "title"; @@ -193,6 +197,17 @@ callback.Run(status, std::vector<ContentSuggestion>()); } +void AddDismissedIdsToRequest(const RemoteSuggestion::PtrVector& dismissed, + RequestParams* request_params) { + // The latest ids are added first, because they are more relevant. + for (auto it = dismissed.rbegin(); it != dismissed.rend(); ++it) { + if (request_params->excluded_ids.size() == kMaxExcludedDismissedIds) { + break; + } + request_params->excluded_ids.insert((*it)->id()); + } +} + } // namespace CachedImageFetcher::CachedImageFetcher( @@ -433,7 +448,7 @@ MarkEmptyCategoriesAsLoading(); - RequestParams params = BuildFetchParams(); + RequestParams params = BuildFetchParams(/*fetched_category=*/base::nullopt); params.interactive_request = interactive_request; suggestions_fetcher_->FetchSnippets( params, @@ -466,7 +481,7 @@ }, base::Unretained(remote_suggestions_scheduler_), callback); - RequestParams params = BuildFetchParams(); + RequestParams params = BuildFetchParams(category); params.excluded_ids.insert(known_suggestion_ids.begin(), known_suggestion_ids.end()); params.interactive_request = true; @@ -479,15 +494,23 @@ } // Builds default fetcher params. -RequestParams RemoteSuggestionsProviderImpl::BuildFetchParams() const { +RequestParams RemoteSuggestionsProviderImpl::BuildFetchParams( + base::Optional<Category> fetched_category) const { RequestParams result; result.language_code = application_language_code_; result.count_to_fetch = kMaxSuggestionCount; + // If this is a fetch for a specific category, its dismissed suggestions are + // added first to truncate them less. + if (fetched_category.has_value()) { + DCHECK(category_contents_.count(*fetched_category)); + AddDismissedIdsToRequest( + category_contents_.find(*fetched_category)->second.dismissed, &result); + } for (const auto& map_entry : category_contents_) { - const CategoryContent& content = map_entry.second; - for (const auto& dismissed_suggestion : content.dismissed) { - result.excluded_ids.insert(dismissed_suggestion->id()); + if (fetched_category.has_value() && map_entry.first == *fetched_category) { + continue; } + AddDismissedIdsToRequest(map_entry.second.dismissed, &result); } return result; }
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider_impl.h b/components/ntp_snippets/remote/remote_suggestions_provider_impl.h index 2540c3aba..d84d00d 100644 --- a/components/ntp_snippets/remote/remote_suggestions_provider_impl.h +++ b/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
@@ -17,6 +17,7 @@ #include "base/callback_forward.h" #include "base/gtest_prod_util.h" #include "base/macros.h" +#include "base/optional.h" #include "base/time/clock.h" #include "base/time/time.h" #include "components/image_fetcher/core/image_fetcher_delegate.h" @@ -409,7 +410,9 @@ void RestoreCategoriesFromPrefs(); void StoreCategoriesToPrefs(); - RequestParams BuildFetchParams() const; + // Absence of fetched category corresponds to fetching all categories. + RequestParams BuildFetchParams( + base::Optional<Category> fetched_category) const; void MarkEmptyCategoriesAsLoading();
diff --git a/components/policy/core/common/cloud/component_cloud_policy_store.cc b/components/policy/core/common/cloud/component_cloud_policy_store.cc index 08c3735..762f557 100644 --- a/components/policy/core/common/cloud/component_cloud_policy_store.cc +++ b/components/policy/core/common/cloud/component_cloud_policy_store.cc
@@ -177,12 +177,11 @@ } } -bool ComponentCloudPolicyStore::Store( - const PolicyNamespace& ns, - const std::string& serialized_policy, - std::unique_ptr<em::PolicyData> policy_data, - const std::string& secure_hash, - const std::string& data) { +bool ComponentCloudPolicyStore::Store(const PolicyNamespace& ns, + const std::string& serialized_policy, + const em::PolicyData* policy_data, + const std::string& secure_hash, + const std::string& data) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); const DomainConstants* constants = GetDomainConstants(ns.domain); PolicyMap policy;
diff --git a/components/policy/core/common/cloud/component_cloud_policy_store.h b/components/policy/core/common/cloud/component_cloud_policy_store.h index db3e1c89..b2ce91ca 100644 --- a/components/policy/core/common/cloud/component_cloud_policy_store.h +++ b/components/policy/core/common/cloud/component_cloud_policy_store.h
@@ -93,7 +93,7 @@ // data was stored in the cache. bool Store(const PolicyNamespace& ns, const std::string& serialized_policy_proto, - std::unique_ptr<enterprise_management::PolicyData> policy_data, + const enterprise_management::PolicyData* policy_data, const std::string& secure_hash, const std::string& data);
diff --git a/components/policy/core/common/cloud/component_cloud_policy_store_unittest.cc b/components/policy/core/common/cloud/component_cloud_policy_store_unittest.cc index b84206c..cbe231ed 100644 --- a/components/policy/core/common/cloud/component_cloud_policy_store_unittest.cc +++ b/components/policy/core/common/cloud/component_cloud_policy_store_unittest.cc
@@ -134,7 +134,7 @@ nullptr /* payload */)); EXPECT_CALL(store_delegate_, OnComponentCloudPolicyStoreUpdated()); EXPECT_TRUE(store->Store(kTestPolicyNS, CreateSerializedResponse(), - CreatePolicyData(), TestPolicyHash(), + CreatePolicyData().get(), TestPolicyHash(), kTestPolicy)); Mock::VerifyAndClearExpectations(&store_delegate_); EXPECT_TRUE(store->policy().Equals(expected_bundle_)); @@ -173,7 +173,8 @@ TEST_F(ComponentCloudPolicyStoreTest, ValidatePolicyWrongTimestamp) { EXPECT_CALL(store_delegate_, OnComponentCloudPolicyStoreUpdated()); EXPECT_TRUE(store_->Store(kTestPolicyNS, CreateSerializedResponse(), - CreatePolicyData(), TestPolicyHash(), kTestPolicy)); + CreatePolicyData().get(), TestPolicyHash(), + kTestPolicy)); const int64_t kPastTimestamp = (base::Time() + base::TimeDelta::FromDays(1)).ToJavaTime(); @@ -432,7 +433,7 @@ builder_.policy_data().set_policy_type(dm_protocol::kChromeUserPolicyType); EXPECT_FALSE( store_->Store(PolicyNamespace(POLICY_DOMAIN_CHROME, kTestExtension), - CreateSerializedResponse(), CreatePolicyData(), + CreateSerializedResponse(), CreatePolicyData().get(), TestPolicyHash(), kTestPolicy)); // Store policy with the wrong hash. @@ -440,19 +441,20 @@ dm_protocol::kChromeExtensionPolicyType); builder_.payload().set_secure_hash("badash"); EXPECT_FALSE(store_->Store(kTestPolicyNS, CreateSerializedResponse(), - CreatePolicyData(), "badash", kTestPolicy)); + CreatePolicyData().get(), "badash", kTestPolicy)); // Store policy without a hash. builder_.payload().clear_secure_hash(); EXPECT_FALSE(store_->Store(kTestPolicyNS, CreateSerializedResponse(), - CreatePolicyData(), std::string(), kTestPolicy)); + CreatePolicyData().get(), std::string(), + kTestPolicy)); // Store policy with invalid JSON data. static const char kInvalidData[] = "{ not json }"; const std::string invalid_data_hash = crypto::SHA256HashString(kInvalidData); builder_.payload().set_secure_hash(invalid_data_hash); EXPECT_FALSE(store_->Store(kTestPolicyNS, CreateSerializedResponse(), - CreatePolicyData(), invalid_data_hash, + CreatePolicyData().get(), invalid_data_hash, kInvalidData)); // All of those failed. @@ -463,7 +465,8 @@ builder_.payload().set_secure_hash(TestPolicyHash()); EXPECT_CALL(store_delegate_, OnComponentCloudPolicyStoreUpdated()); EXPECT_TRUE(store_->Store(kTestPolicyNS, CreateSerializedResponse(), - CreatePolicyData(), TestPolicyHash(), kTestPolicy)); + CreatePolicyData().get(), TestPolicyHash(), + kTestPolicy)); Mock::VerifyAndClearExpectations(&store_delegate_); EXPECT_FALSE(IsStoreEmpty(*store_)); EXPECT_TRUE(store_->policy().Equals(expected_bundle_)); @@ -485,7 +488,8 @@ // Store some policies. EXPECT_CALL(store_delegate_, OnComponentCloudPolicyStoreUpdated()); EXPECT_TRUE(store_->Store(kTestPolicyNS, CreateSerializedResponse(), - CreatePolicyData(), TestPolicyHash(), kTestPolicy)); + CreatePolicyData().get(), TestPolicyHash(), + kTestPolicy)); Mock::VerifyAndClearExpectations(&store_delegate_); EXPECT_FALSE(IsStoreEmpty(*store_)); EXPECT_TRUE(store_->policy().Equals(expected_bundle_)); @@ -505,7 +509,8 @@ // Store a valid policy. EXPECT_CALL(store_delegate_, OnComponentCloudPolicyStoreUpdated()); EXPECT_TRUE(store_->Store(kTestPolicyNS, CreateSerializedResponse(), - CreatePolicyData(), TestPolicyHash(), kTestPolicy)); + CreatePolicyData().get(), TestPolicyHash(), + kTestPolicy)); Mock::VerifyAndClearExpectations(&store_delegate_); EXPECT_FALSE(IsStoreEmpty(*store_)); EXPECT_TRUE(store_->policy().Equals(expected_bundle_));
diff --git a/components/policy/core/common/cloud/component_cloud_policy_updater.cc b/components/policy/core/common/cloud/component_cloud_policy_updater.cc index 9901476d..2dd8d66 100644 --- a/components/policy/core/common/cloud/component_cloud_policy_updater.cc +++ b/components/policy/core/common/cloud/component_cloud_policy_updater.cc
@@ -89,10 +89,11 @@ // Make a request to fetch policy for this component. If another fetch // request is already pending for the component, it will be canceled. external_policy_data_updater_.FetchExternalData( - key, ExternalPolicyDataUpdater::Request( - data.download_url(), data.secure_hash(), kPolicyDataMaxSize), + key, + ExternalPolicyDataUpdater::Request( + data.download_url(), data.secure_hash(), kPolicyDataMaxSize), base::Bind(&ComponentCloudPolicyStore::Store, base::Unretained(store_), - ns, serialized_response, base::Passed(&policy_data), + ns, serialized_response, base::Owned(policy_data.release()), data.secure_hash())); } }
diff --git a/components/policy/core/common/cloud/component_cloud_policy_updater_unittest.cc b/components/policy/core/common/cloud/component_cloud_policy_updater_unittest.cc index 28fad2f..a1b01ce 100644 --- a/components/policy/core/common/cloud/component_cloud_policy_updater_unittest.cc +++ b/components/policy/core/common/cloud/component_cloud_policy_updater_unittest.cc
@@ -13,7 +13,8 @@ #include "base/files/scoped_temp_dir.h" #include "base/memory/ptr_util.h" #include "base/sequenced_task_runner.h" -#include "base/test/test_simple_task_runner.h" +#include "base/test/test_mock_time_task_runner.h" +#include "base/time/time.h" #include "base/values.h" #include "components/policy/core/common/cloud/cloud_policy_constants.h" #include "components/policy/core/common/cloud/component_cloud_policy_store.h" @@ -79,22 +80,23 @@ std::unique_ptr<em::PolicyFetchResponse> CreateResponse(); - const PolicyNamespace kTestPolicyNS; - scoped_refptr<base::TestSimpleTaskRunner> task_runner_; - base::ScopedTempDir temp_dir_; - std::unique_ptr<ResourceCache> cache_; + const PolicyNamespace kTestPolicyNS{POLICY_DOMAIN_EXTENSIONS, kTestExtension}; + scoped_refptr<base::TestMockTimeTaskRunner> task_runner_; std::unique_ptr<ComponentCloudPolicyStore> store_; MockComponentCloudPolicyStoreDelegate store_delegate_; net::TestURLFetcherFactory fetcher_factory_; - std::unique_ptr<ExternalPolicyDataFetcherBackend> fetcher_backend_; std::unique_ptr<ComponentCloudPolicyUpdater> updater_; ComponentPolicyBuilder builder_; PolicyBundle expected_bundle_; + + private: + base::ScopedTempDir temp_dir_; + std::unique_ptr<ResourceCache> cache_; + std::unique_ptr<ExternalPolicyDataFetcherBackend> fetcher_backend_; std::string public_key_; }; -ComponentCloudPolicyUpdaterTest::ComponentCloudPolicyUpdaterTest() - : kTestPolicyNS(POLICY_DOMAIN_EXTENSIONS, kTestExtension) { +ComponentCloudPolicyUpdaterTest::ComponentCloudPolicyUpdaterTest() { builder_.SetDefaultSigningKey(); builder_.policy_data().set_policy_type( dm_protocol::kChromeExtensionPolicyType); @@ -115,7 +117,7 @@ void ComponentCloudPolicyUpdaterTest::SetUp() { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); - task_runner_ = new base::TestSimpleTaskRunner(); + task_runner_ = new base::TestMockTimeTaskRunner(); cache_.reset(new ResourceCache(temp_dir_.GetPath(), task_runner_)); store_.reset(new ComponentCloudPolicyStore(&store_delegate_, cache_.get())); store_->SetCredentials(ComponentPolicyBuilder::kFakeUsername, @@ -320,8 +322,7 @@ builder_.Build(); EXPECT_CALL(store_delegate_, OnComponentCloudPolicyStoreUpdated()); EXPECT_TRUE( - store_->Store(kTestPolicyNS, builder_.GetBlob(), - base::MakeUnique<em::PolicyData>(builder_.policy_data()), + store_->Store(kTestPolicyNS, builder_.GetBlob(), &builder_.policy_data(), crypto::SHA256HashString(kTestPolicy), kTestPolicy)); Mock::VerifyAndClearExpectations(&store_delegate_); @@ -334,7 +335,7 @@ EXPECT_FALSE(fetcher_factory_.GetFetcherByID(0)); } -TEST_F(ComponentCloudPolicyUpdaterTest, PolicyDataInvalid) { +TEST_F(ComponentCloudPolicyUpdaterTest, DataTooLarge) { // Submit three policy fetch responses. updater_->UpdateExternalPolicy(kTestPolicyNS, CreateResponse()); builder_.payload().set_download_url(kTestDownload2); @@ -429,8 +430,8 @@ EXPECT_FALSE(fetcher_factory_.GetFetcherByID(1)); // Verify that the policy is no longer being served. - const PolicyBundle empty_bundle; - EXPECT_TRUE(store_->policy().Equals(empty_bundle)); + const PolicyBundle kEmptyBundle; + EXPECT_TRUE(store_->policy().Equals(kEmptyBundle)); } TEST_F(ComponentCloudPolicyUpdaterTest, NoPolicy) { @@ -467,4 +468,98 @@ EXPECT_FALSE(fetcher_factory_.GetFetcherByID(0)); } +TEST_F(ComponentCloudPolicyUpdaterTest, RetryAfterDataTooLarge) { + // Submit a policy fetch response. + updater_->UpdateExternalPolicy(kTestPolicyNS, CreateResponse()); + task_runner_->RunUntilIdle(); + + // Verify that the first download has been started. + net::TestURLFetcher* fetcher = fetcher_factory_.GetFetcherByID(0); + ASSERT_TRUE(fetcher); + EXPECT_EQ(GURL(kTestDownload), fetcher->GetOriginalURL()); + + // Indicate that the policy data size will exceed allowed maximum. + fetcher->delegate()->OnURLFetchDownloadProgress(fetcher, 6 * 1024 * 1024, -1, + 6 * 1024 * 1024); + task_runner_->RunUntilIdle(); + + // After 12 hours (minus some random jitter), the next download attempt + // happens. + EXPECT_FALSE(fetcher_factory_.GetFetcherByID(1)); + task_runner_->FastForwardBy(base::TimeDelta::FromHours(12)); + fetcher = fetcher_factory_.GetFetcherByID(1); + ASSERT_TRUE(fetcher); + EXPECT_EQ(GURL(kTestDownload), fetcher->GetOriginalURL()); + + // Complete the download. + fetcher->set_response_code(200); + fetcher->SetResponseString(kTestPolicy); + EXPECT_CALL(store_delegate_, OnComponentCloudPolicyStoreUpdated()); + fetcher->delegate()->OnURLFetchComplete(fetcher); + task_runner_->RunUntilIdle(); + Mock::VerifyAndClearExpectations(&store_delegate_); + + // Verify that the downloaded policy is being served. + EXPECT_TRUE(store_->policy().Equals(expected_bundle_)); +} + +TEST_F(ComponentCloudPolicyUpdaterTest, RetryAfterDataValidationFails) { + // Submit a policy fetch response that is calculated for an empty (that is, + // invalid) JSON. + builder_.payload().set_secure_hash(crypto::SHA256HashString(std::string())); + updater_->UpdateExternalPolicy(kTestPolicyNS, CreateResponse()); + task_runner_->RunUntilIdle(); + + // Verify that the first download has been started. + net::TestURLFetcher* fetcher = fetcher_factory_.GetFetcherByID(0); + ASSERT_TRUE(fetcher); + EXPECT_EQ(GURL(kTestDownload), fetcher->GetOriginalURL()); + + // Complete the download with an invalid (empty) JSON. + fetcher->set_response_code(200); + fetcher->SetResponseString(std::string()); + fetcher->delegate()->OnURLFetchComplete(fetcher); + task_runner_->RunUntilIdle(); + + // Verify that no policy is being served. + const PolicyBundle kEmptyBundle; + EXPECT_TRUE(store_->policy().Equals(kEmptyBundle)); + + // After 12 hours (minus some random jitter), the next download attempt + // happens. + EXPECT_FALSE(fetcher_factory_.GetFetcherByID(1)); + task_runner_->FastForwardBy(base::TimeDelta::FromHours(12)); + fetcher = fetcher_factory_.GetFetcherByID(1); + ASSERT_TRUE(fetcher); + EXPECT_EQ(GURL(kTestDownload), fetcher->GetOriginalURL()); + + // Complete the download with an invalid (empty) JSON. This tests against the + // regression that was tracked at https://crbug.com/706781. + fetcher->set_response_code(200); + fetcher->SetResponseString(std::string()); + fetcher->delegate()->OnURLFetchComplete(fetcher); + task_runner_->RunUntilIdle(); + + // Submit a policy fetch response that is calculated for the correct JSON. + builder_.payload().set_secure_hash(crypto::SHA256HashString(kTestPolicy)); + updater_->UpdateExternalPolicy(kTestPolicyNS, CreateResponse()); + task_runner_->RunUntilIdle(); + + // The next download attempt has been started. + fetcher = fetcher_factory_.GetFetcherByID(2); + ASSERT_TRUE(fetcher); + EXPECT_EQ(GURL(kTestDownload), fetcher->GetOriginalURL()); + + // Complete the download. + fetcher->set_response_code(200); + fetcher->SetResponseString(kTestPolicy); + EXPECT_CALL(store_delegate_, OnComponentCloudPolicyStoreUpdated()); + fetcher->delegate()->OnURLFetchComplete(fetcher); + task_runner_->RunUntilIdle(); + Mock::VerifyAndClearExpectations(&store_delegate_); + + // Verify that the downloaded policy is being served. + EXPECT_TRUE(store_->policy().Equals(expected_bundle_)); +} + } // namespace policy
diff --git a/components/safe_browsing/common/safe_browsing_prefs.cc b/components/safe_browsing/common/safe_browsing_prefs.cc index 83f7588..ef7d79e 100644 --- a/components/safe_browsing/common/safe_browsing_prefs.cc +++ b/components/safe_browsing/common/safe_browsing_prefs.cc
@@ -156,7 +156,7 @@ const char kSwitchForceScoutGroup[] = "force-scout-group"; const base::Feature kCanShowScoutOptIn{"CanShowScoutOptIn", - base::FEATURE_DISABLED_BY_DEFAULT}; + base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kOnlyShowScoutOptIn{"OnlyShowScoutOptIn", base::FEATURE_DISABLED_BY_DEFAULT};
diff --git a/components/ui_devtools/devtools_server.cc b/components/ui_devtools/devtools_server.cc index 2bab50aa..fd0e35b 100644 --- a/components/ui_devtools/devtools_server.cc +++ b/components/ui_devtools/devtools_server.cc
@@ -34,6 +34,7 @@ int GetUiDevToolsPort() { DCHECK(IsUiDevToolsEnabled()); + // This value is duplicated in the chrome://flags description. constexpr int kDefaultPort = 9223; int port; if (!base::StringToInt(
diff --git a/components/ui_devtools/switches.h b/components/ui_devtools/switches.h index 8dcb6980..d0bb577 100644 --- a/components/ui_devtools/switches.h +++ b/components/ui_devtools/switches.h
@@ -5,10 +5,12 @@ #ifndef COMPONENTS_UI_DEVTOOLS_SWITCHES_H_ #define COMPONENTS_UI_DEVTOOLS_SWITCHES_H_ +#include "components/ui_devtools/devtools_export.h" + namespace ui { namespace devtools { -extern const char kEnableUiDevTools[]; +extern UI_DEVTOOLS_EXPORT const char kEnableUiDevTools[]; } // namespace devtools } // namespace ui
diff --git a/components/viz/client/client_compositor_frame_sink.cc b/components/viz/client/client_compositor_frame_sink.cc index 3fe17a6..13e1faf 100644 --- a/components/viz/client/client_compositor_frame_sink.cc +++ b/components/viz/client/client_compositor_frame_sink.cc
@@ -32,7 +32,8 @@ compositor_frame_sink_info_(std::move(compositor_frame_sink_info)), client_request_(std::move(client_request)), client_binding_(this), - enable_surface_synchronization_(enable_surface_synchronization) { + enable_surface_synchronization_(enable_surface_synchronization), + weak_factory_(this) { DETACH_FROM_THREAD(thread_checker_); } @@ -49,12 +50,19 @@ compositor_frame_sink_info_(std::move(compositor_frame_sink_info)), client_request_(std::move(client_request)), client_binding_(this), - enable_surface_synchronization_(enable_surface_synchronization) { + enable_surface_synchronization_(enable_surface_synchronization), + weak_factory_(this) { DETACH_FROM_THREAD(thread_checker_); } ClientCompositorFrameSink::~ClientCompositorFrameSink() {} +base::WeakPtr<ClientCompositorFrameSink> +ClientCompositorFrameSink::GetWeakPtr() { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + return weak_factory_.GetWeakPtr(); +} + bool ClientCompositorFrameSink::BindToClient( cc::CompositorFrameSinkClient* client) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); @@ -87,6 +95,7 @@ void ClientCompositorFrameSink::SetLocalSurfaceId( const cc::LocalSurfaceId& local_surface_id) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DCHECK(local_surface_id.is_valid()); DCHECK(enable_surface_synchronization_); local_surface_id_ = local_surface_id;
diff --git a/components/viz/client/client_compositor_frame_sink.h b/components/viz/client/client_compositor_frame_sink.h index 236767c..25a6dd6 100644 --- a/components/viz/client/client_compositor_frame_sink.h +++ b/components/viz/client/client_compositor_frame_sink.h
@@ -6,6 +6,7 @@ #define COMPONENTS_VIZ_CLIENT_CLIENT_COMPOSITOR_FRAME_SINK_H_ #include "base/macros.h" +#include "base/memory/weak_ptr.h" #include "cc/ipc/mojo_compositor_frame_sink.mojom.h" #include "cc/output/compositor_frame_sink.h" #include "cc/output/context_provider.h" @@ -46,6 +47,8 @@ ~ClientCompositorFrameSink() override; + base::WeakPtr<ClientCompositorFrameSink> GetWeakPtr(); + // cc::CompositorFrameSink implementation. bool BindToClient(cc::CompositorFrameSinkClient* client) override; void DetachFromClient() override; @@ -74,6 +77,8 @@ THREAD_CHECKER(thread_checker_); const bool enable_surface_synchronization_; + base::WeakPtrFactory<ClientCompositorFrameSink> weak_factory_; + DISALLOW_COPY_AND_ASSIGN(ClientCompositorFrameSink); };
diff --git a/content/browser/BUILD.gn b/content/browser/BUILD.gn index 8b0191e..8f609b8f 100644 --- a/content/browser/BUILD.gn +++ b/content/browser/BUILD.gn
@@ -1057,8 +1057,6 @@ "notifications/notification_id_generator.h", "notifications/notification_message_filter.cc", "notifications/notification_message_filter.h", - "notifications/page_notification_delegate.cc", - "notifications/page_notification_delegate.h", "notifications/platform_notification_context_impl.cc", "notifications/platform_notification_context_impl.h", "notifications/type_converters.cc",
diff --git a/content/browser/notifications/notification_event_dispatcher_impl.cc b/content/browser/notifications/notification_event_dispatcher_impl.cc index 47779cfe..a02d719 100644 --- a/content/browser/notifications/notification_event_dispatcher_impl.cc +++ b/content/browser/notifications/notification_event_dispatcher_impl.cc
@@ -394,6 +394,15 @@ const std::string& notification_id, int renderer_id, int non_persistent_id) { + if (non_persistent_ids_.count(notification_id) && + non_persistent_ids_[notification_id] != non_persistent_id) { + // Notify close for a previously displayed notification with the same id, + // this can happen when replacing a non-persistent notification with the + // same tag since from the JS point of view there will be two notification + // objects and the old one needs to receive the close event. + // TODO(miguelg) this is probably not the right layer to do this. + DispatchNonPersistentCloseEvent(notification_id); + } renderer_ids_[notification_id] = renderer_id; non_persistent_ids_[notification_id] = non_persistent_id; }
diff --git a/content/browser/notifications/notification_message_filter.cc b/content/browser/notifications/notification_message_filter.cc index 9f28e1b..c4182128 100644 --- a/content/browser/notifications/notification_message_filter.cc +++ b/content/browser/notifications/notification_message_filter.cc
@@ -12,14 +12,12 @@ #include "content/browser/bad_message.h" #include "content/browser/notifications/notification_event_dispatcher_impl.h" #include "content/browser/notifications/notification_id_generator.h" -#include "content/browser/notifications/page_notification_delegate.h" #include "content/browser/notifications/platform_notification_context_impl.h" #include "content/browser/service_worker/service_worker_context_wrapper.h" #include "content/common/platform_notification_messages.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/content_browser_client.h" -#include "content/public/browser/desktop_notification_delegate.h" #include "content/public/browser/notification_database_data.h" #include "content/public/browser/platform_notification_service.h" #include "content/public/browser/render_process_host.h" @@ -168,22 +166,16 @@ GetNotificationIdGenerator()->GenerateForNonPersistentNotification( origin, notification_data.tag, non_persistent_notification_id, process_id_); - NotificationEventDispatcherImpl* event_dispatcher = content::NotificationEventDispatcherImpl::GetInstance(); non_persistent__notification_shown_ = true; event_dispatcher->RegisterNonPersistentNotification( notification_id, process_id_, non_persistent_notification_id); - std::unique_ptr<DesktopNotificationDelegate> delegate( - new PageNotificationDelegate(process_id_, non_persistent_notification_id, - notification_id)); - base::Closure close_closure; service->DisplayNotification(browser_context_, notification_id, origin, SanitizeNotificationData(notification_data), - notification_resources, std::move(delegate), - &close_closure); + notification_resources, &close_closure); if (!close_closure.is_null()) close_closures_[notification_id] = close_closure;
diff --git a/content/browser/notifications/page_notification_delegate.cc b/content/browser/notifications/page_notification_delegate.cc deleted file mode 100644 index 4785829..0000000 --- a/content/browser/notifications/page_notification_delegate.cc +++ /dev/null
@@ -1,55 +0,0 @@ -// Copyright 2014 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. - -#include "content/browser/notifications/page_notification_delegate.h" - -#include "content/browser/notifications/notification_message_filter.h" -#include "content/browser/renderer_host/render_process_host_impl.h" -#include "content/common/platform_notification_messages.h" -#include "content/public/browser/render_process_host.h" - -namespace content { - -PageNotificationDelegate::PageNotificationDelegate( - int render_process_id, - int non_persistent_notification_id, - const std::string& notification_id) - : render_process_id_(render_process_id), - non_persistent_notification_id_(non_persistent_notification_id), - notification_id_(notification_id) {} - -PageNotificationDelegate::~PageNotificationDelegate() {} - -void PageNotificationDelegate::NotificationDisplayed() { - RenderProcessHost* sender = RenderProcessHost::FromID(render_process_id_); - if (!sender) - return; - - sender->Send( - new PlatformNotificationMsg_DidShow(non_persistent_notification_id_)); -} - -void PageNotificationDelegate::NotificationClosed() { - RenderProcessHost* sender = RenderProcessHost::FromID(render_process_id_); - if (!sender) - return; - - sender->Send( - new PlatformNotificationMsg_DidClose(non_persistent_notification_id_)); - - static_cast<RenderProcessHostImpl*>(sender) - ->notification_message_filter() - ->DidCloseNotification(notification_id_); -} - -void PageNotificationDelegate::NotificationClick() { - RenderProcessHost* sender = RenderProcessHost::FromID(render_process_id_); - if (!sender) - return; - - sender->Send( - new PlatformNotificationMsg_DidClick(non_persistent_notification_id_)); -} - -} // namespace content
diff --git a/content/browser/notifications/page_notification_delegate.h b/content/browser/notifications/page_notification_delegate.h deleted file mode 100644 index 497255b..0000000 --- a/content/browser/notifications/page_notification_delegate.h +++ /dev/null
@@ -1,37 +0,0 @@ -// Copyright 2014 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. - -#ifndef CONTENT_BROWSER_NOTIFICATIONS_PAGE_NOTIFICATION_DELEGATE_H_ -#define CONTENT_BROWSER_NOTIFICATIONS_PAGE_NOTIFICATION_DELEGATE_H_ - -#include <string> - -#include "content/public/browser/desktop_notification_delegate.h" - -namespace content { - -// A delegate used by the notification service to report the results of platform -// notification interactions to Web Notifications whose lifetime is tied to -// that of the page displaying them. -class PageNotificationDelegate : public DesktopNotificationDelegate { - public: - PageNotificationDelegate(int render_process_id, - int non_persistent_notification_id, - const std::string& notification_id); - ~PageNotificationDelegate() override; - - // DesktopNotificationDelegate implementation. - void NotificationDisplayed() override; - void NotificationClosed() override; - void NotificationClick() override; - - private: - int render_process_id_; - int non_persistent_notification_id_; - std::string notification_id_; -}; - -} // namespace content - -#endif // CONTENT_BROWSER_NOTIFICATIONS_PAGE_NOTIFICATION_DELEGATE_H_
diff --git a/content/public/browser/BUILD.gn b/content/public/browser/BUILD.gn index dbae369..8ae5576 100644 --- a/content/public/browser/BUILD.gn +++ b/content/public/browser/BUILD.gn
@@ -88,7 +88,6 @@ "cookie_store_factory.h", "desktop_media_id.cc", "desktop_media_id.h", - "desktop_notification_delegate.h", "devtools_agent_host.h", "devtools_agent_host_client.h", "devtools_agent_host_observer.cc",
diff --git a/content/public/browser/desktop_notification_delegate.h b/content/public/browser/desktop_notification_delegate.h deleted file mode 100644 index 48030709..0000000 --- a/content/public/browser/desktop_notification_delegate.h +++ /dev/null
@@ -1,28 +0,0 @@ -// Copyright 2014 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. - -#ifndef CONTENT_PUBLIC_BROWSER_DESKTOP_NOTIFICATION_DELEGATE_H_ -#define CONTENT_PUBLIC_BROWSER_DESKTOP_NOTIFICATION_DELEGATE_H_ - -namespace content { - -// A delegate used by ContentBrowserClient::ShowDesktopNotification to report -// the result of a desktop notification. -class DesktopNotificationDelegate { - public: - virtual ~DesktopNotificationDelegate() {} - - // The notification was shown. - virtual void NotificationDisplayed() = 0; - - // The notification was closed. - virtual void NotificationClosed() = 0; - - // The user clicked on the notification. - virtual void NotificationClick() = 0; -}; - -} // namespace content - -#endif // CONTENT_PUBLIC_BROWSER_DESKTOP_NOTIFICATION_DELEGATE_H_
diff --git a/content/public/browser/platform_notification_service.h b/content/public/browser/platform_notification_service.h index 7689ff6..a8dcc86 100644 --- a/content/public/browser/platform_notification_service.h +++ b/content/public/browser/platform_notification_service.h
@@ -21,7 +21,6 @@ namespace content { class BrowserContext; -class DesktopNotificationDelegate; struct NotificationResources; struct PlatformNotificationData; class ResourceContext; @@ -63,7 +62,6 @@ const GURL& origin, const PlatformNotificationData& notification_data, const NotificationResources& notification_resources, - std::unique_ptr<DesktopNotificationDelegate> delegate, base::Closure* cancel_callback) = 0; // Displays the persistent notification described in |notification_data| to
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index ffdd088..67ae44a 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc
@@ -3404,9 +3404,6 @@ datasource->UpdateNavigation( redirect_start, redirect_end, fetch_start, !navigation_state->request_params().redirects.empty()); - // TODO(clamy) We need to provide additional timing values for the - // Navigation Timing API to work with browser-side navigations. - // UnloadEventStart and UnloadEventEnd are still missing. } // PlzNavigate: update the source location before processing the navigation
diff --git a/content/test/mock_platform_notification_service.cc b/content/test/mock_platform_notification_service.cc index 47990a2..58db696 100644 --- a/content/test/mock_platform_notification_service.cc +++ b/content/test/mock_platform_notification_service.cc
@@ -8,7 +8,6 @@ #include "base/strings/nullable_string16.h" #include "base/strings/utf_string_conversions.h" #include "content/public/browser/browser_thread.h" -#include "content/public/browser/desktop_notification_delegate.h" #include "content/public/browser/notification_event_dispatcher.h" #include "content/public/browser/permission_type.h" #include "content/public/common/persistent_notification_status.h" @@ -34,7 +33,6 @@ const GURL& origin, const PlatformNotificationData& notification_data, const NotificationResources& notification_resources, - std::unique_ptr<DesktopNotificationDelegate> delegate, base::Closure* cancel_callback) { DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK(cancel_callback); @@ -43,10 +41,10 @@ weak_factory_.GetWeakPtr(), notification_id); ReplaceNotificationIfNeeded(notification_id); + non_persistent_notifications_.insert(notification_id); - non_persistent_notifications_[notification_id] = std::move(delegate); - non_persistent_notifications_[notification_id]->NotificationDisplayed(); - + NotificationEventDispatcher::GetInstance()->DispatchNonPersistentShowEvent( + notification_id); notification_id_map_[base::UTF16ToUTF8(notification_data.title)] = notification_id; } @@ -90,7 +88,7 @@ displayed_notifications->insert(kv.first); BrowserThread::PostTask( - content::BrowserThread::UI, FROM_HERE, + BrowserThread::UI, FROM_HERE, base::Bind(callback, base::Passed(&displayed_notifications), true /* supports_synchronization */)); } @@ -100,7 +98,6 @@ int action_index, const base::NullableString16& reply) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - const auto notification_id_iter = notification_id_map_.find(title); if (notification_id_iter == notification_id_map_.end()) return; @@ -115,15 +112,14 @@ DCHECK(non_persistent_iter == non_persistent_notifications_.end()); const PersistentNotification& notification = persistent_iter->second; - content::NotificationEventDispatcher::GetInstance() - ->DispatchNotificationClickEvent( - notification.browser_context, notification_id, notification.origin, - action_index, reply, base::Bind(&OnEventDispatchComplete)); + NotificationEventDispatcher::GetInstance()->DispatchNotificationClickEvent( + notification.browser_context, notification_id, notification.origin, + action_index, reply, base::Bind(&OnEventDispatchComplete)); } else if (non_persistent_iter != non_persistent_notifications_.end()) { DCHECK_EQ(action_index, -1) << "Action buttons are only supported for " "persistent notifications"; - - non_persistent_iter->second->NotificationClick(); + NotificationEventDispatcher::GetInstance()->DispatchNonPersistentClickEvent( + notification_id); } } @@ -142,10 +138,9 @@ return; const PersistentNotification& notification = persistent_iter->second; - content::NotificationEventDispatcher::GetInstance() - ->DispatchNotificationCloseEvent( - notification.browser_context, notification_id, notification.origin, - by_user, base::Bind(&OnEventDispatchComplete)); + NotificationEventDispatcher::GetInstance()->DispatchNotificationCloseEvent( + notification.browser_context, notification_id, notification.origin, + by_user, base::Bind(&OnEventDispatchComplete)); } blink::mojom::PermissionStatus @@ -169,26 +164,19 @@ void MockPlatformNotificationService::Close( const std::string& notification_id) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - auto iterator = non_persistent_notifications_.find(notification_id); - if (iterator == non_persistent_notifications_.end()) - return; - - iterator->second->NotificationClosed(); + const auto non_persistent_iter = + non_persistent_notifications_.find(notification_id); + if (non_persistent_iter == non_persistent_notifications_.end()) { + NotificationEventDispatcher::GetInstance()->DispatchNonPersistentCloseEvent( + notification_id); + non_persistent_notifications_.erase(non_persistent_iter); + } } void MockPlatformNotificationService::ReplaceNotificationIfNeeded( const std::string& notification_id) { - const auto persistent_iter = persistent_notifications_.find(notification_id); - const auto non_persistent_iter = - non_persistent_notifications_.find(notification_id); - - if (persistent_iter != persistent_notifications_.end()) { - DCHECK(non_persistent_iter == non_persistent_notifications_.end()); - persistent_notifications_.erase(persistent_iter); - } else if (non_persistent_iter != non_persistent_notifications_.end()) { - non_persistent_iter->second->NotificationClosed(); - non_persistent_notifications_.erase(non_persistent_iter); - } + persistent_notifications_.erase(notification_id); + non_persistent_notifications_.erase(notification_id); } blink::mojom::PermissionStatus MockPlatformNotificationService::CheckPermission(
diff --git a/content/test/mock_platform_notification_service.h b/content/test/mock_platform_notification_service.h index 64496cd..076d016 100644 --- a/content/test/mock_platform_notification_service.h +++ b/content/test/mock_platform_notification_service.h
@@ -8,6 +8,7 @@ #include <stdint.h> #include <string> #include <unordered_map> +#include <unordered_set> #include "base/callback.h" #include "base/macros.h" @@ -22,7 +23,6 @@ namespace content { -class DesktopNotificationDelegate; struct NotificationResources; struct PlatformNotificationData; @@ -60,7 +60,6 @@ const GURL& origin, const PlatformNotificationData& notification_data, const NotificationResources& notification_resources, - std::unique_ptr<DesktopNotificationDelegate> delegate, base::Closure* cancel_callback) override; void DisplayPersistentNotification( BrowserContext* browser_context, @@ -96,8 +95,7 @@ std::unordered_map<std::string, PersistentNotification> persistent_notifications_; - std::unordered_map<std::string, std::unique_ptr<DesktopNotificationDelegate>> - non_persistent_notifications_; + std::unordered_set<std::string> non_persistent_notifications_; // Mapping of titles to notification ids giving test a usable identifier. std::unordered_map<std::string, std::string> notification_id_map_;
diff --git a/docs/task_scheduler_migration.md b/docs/task_scheduler_migration.md index 9861bde..c08ccc6 100644 --- a/docs/task_scheduler_migration.md +++ b/docs/task_scheduler_migration.md
@@ -25,6 +25,20 @@ of plumbing without otherwise hurting testing as documented [here](threading_and_tasks.md#TaskRunner-ownership-encourage-no-dependency-injection). +Replace methods that used to +```cpp + DCHECK(content::BrowserThread::GetBlockingPool()->RunsTasksInCurrentSequence()); +``` +and make them use +```cpp + base::ThreadsRestrictions::AssertIOAllowed(); +``` + +The TaskScheduler API intentionally doesn't provide a +TaskScheduler::RunsTasksInCurrentSequence() equivalent as ultimately everything +will run in TaskScheduler and that'd be meaningless... As such prefer asserting +the properties your task needs as documentation rather than where it runs. + ## BrowserThreads All BrowserThreads but UI/IO are being migrated to TaskScheduler @@ -45,13 +59,15 @@ (threading_and_tasks.md#Prefer-Sequences-to-Threads) sequenced context. * Note: if your tasks use COM APIs (Component Object Model on Windows), you'll need to use CreateCOMSTATaskRunnerWithTraits() and sequencing will - not be an option. + not be an option (there are DCHECKs in place that will fire if your task + uses COM without being on a COM initialized TaskRunner). ## Relevant single-thread -> sequence mappings * base::SingleThreadTaskRunner -> base::SequencedTaskRunner * base::ThreadTaskRunnerHandle -> base::SequencedTaskRunnerHandle * base::ThreadChecker -> base::SequenceChecker +* base::ThreadLocalStorage::Slot -> base::SequenceLocalStorageSlot (coming [very soon](https://chromium-review.googlesource.com/c/527322/)) * BrowserThread::DeleteOnThread -> base::DeleteOnTaskRunner / base::RefCountedDeleteOnSequence * CreateSingleThreadTaskRunnerWithTraits() -> CreateSequencedTaskRunnerWithTraits() * Every CreateSingleThreadTaskRunnerWithTraits() usage should be accompanied @@ -75,3 +91,11 @@ TaskScheduler::FlushForTesting() * If you need the TaskScheduler to not run anything until explicitly asked to use ScopedTaskEnvironment::ExecutionMode::QUEUED. + +## Other known migration hurdles and recommended paradigms +* Everything in a file/component needs to run on the same sequence but there + isn't a clear place to own/access the common SequencedTaskRunner => + [base::Lazy*TaskRunner](https://chromium-review.googlesource.com/c/524141/). +* For anything else, ping [base/task_scheduler/OWNERS](https://cs.chromium.org/chromium/src/base/task_scheduler/OWNERS) + or [scheduler-dev@chromium.org](https://groups.google.com/a/chromium.org/forum/#!forum/scheduler-dev), + thanks!
diff --git a/docs/threading_and_tasks.md b/docs/threading_and_tasks.md index 9b28921..b29e601 100644 --- a/docs/threading_and_tasks.md +++ b/docs/threading_and_tasks.md
@@ -67,6 +67,9 @@ use of base::CreateSingleThreadTaskRunnerWithTraits() with a TODO against your bug to use base::CreateSequencedTaskRunnerWithTraits() when fixed). +Detailed documentation on how to migrate from single-threaded contexts to +sequenced contexts can be found [here](task_scheduler_migration.md). + The discussion below covers all of these ways to execute tasks in details. ## Posting a Parallel Task
diff --git a/ios/chrome/browser/content_suggestions/BUILD.gn b/ios/chrome/browser/content_suggestions/BUILD.gn index 40ff459..ed24baa 100644 --- a/ios/chrome/browser/content_suggestions/BUILD.gn +++ b/ios/chrome/browser/content_suggestions/BUILD.gn
@@ -34,6 +34,7 @@ "//ios/chrome/browser/reading_list", "//ios/chrome/browser/ui", "//ios/chrome/browser/ui/alert_coordinator", + "//ios/chrome/browser/ui/collection_view/cells", "//ios/chrome/browser/ui/commands", "//ios/chrome/browser/ui/content_suggestions", "//ios/chrome/browser/ui/content_suggestions/cells",
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm index 5213895..a01b334 100644 --- a/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm
@@ -204,8 +204,12 @@ // is fixed. For now hidding the ink prevent cell interaction. - (UIColor*)collectionView:(UICollectionView*)collectionView inkColorAtIndexPath:(NSIndexPath*)indexPath { + ContentSuggestionType itemType = [self.collectionUpdater + contentSuggestionTypeForItem:[self.collectionViewModel + itemAtIndexPath:indexPath]]; if ([self.collectionUpdater - shouldUseCustomStyleForSection:indexPath.section]) { + shouldUseCustomStyleForSection:indexPath.section] || + itemType == ContentSuggestionTypeEmpty) { return [UIColor clearColor]; } return nil;
diff --git a/ios/chrome/browser/ui/reading_list/BUILD.gn b/ios/chrome/browser/ui/reading_list/BUILD.gn index ef4bca6..d9dd70b6 100644 --- a/ios/chrome/browser/ui/reading_list/BUILD.gn +++ b/ios/chrome/browser/ui/reading_list/BUILD.gn
@@ -6,11 +6,8 @@ sources = [ "offline_page_native_content.h", "offline_page_native_content.mm", - "reading_list_collection_view_controller.h", - "reading_list_collection_view_controller.mm", "reading_list_collection_view_item.h", "reading_list_collection_view_item.mm", - "reading_list_collection_view_item_accessibility_delegate.h", "reading_list_coordinator.h", "reading_list_coordinator.mm", "reading_list_mediator.h", @@ -22,17 +19,13 @@ "reading_list_side_swipe_provider.mm", "reading_list_utils.h", "reading_list_utils.mm", - "reading_list_view_controller.h", - "reading_list_view_controller.mm", ] deps = [ ":reading_list_ui", - ":resources", "//base", "//components/favicon/core", "//components/reading_list/core", "//components/reading_list/ios", - "//components/strings", "//components/url_formatter", "//ios/chrome/app/strings", "//ios/chrome/browser", @@ -43,8 +36,6 @@ "//ios/chrome/browser/ui/alert_coordinator", "//ios/chrome/browser/ui/favicon", "//ios/chrome/browser/ui/favicon:favicon_ui", - "//ios/chrome/browser/ui/keyboard", - "//ios/chrome/browser/ui/material_components", "//ios/chrome/browser/ui/side_swipe", "//ios/chrome/browser/ui/static_content", "//ios/chrome/browser/ui/util", @@ -54,9 +45,6 @@ "//ui/strings", "//url", ] - public_deps = [ - "//ios/chrome/browser/ui/collection_view", - ] allow_circular_includes_from = [ "//ios/chrome/browser/ui/side_swipe" ] libs = [ "UIKit.framework" ] configs += [ "//build/config/compiler:enable_arc" ] @@ -69,12 +57,17 @@ "number_badge_view.mm", "reading_list_collection_view_cell.h", "reading_list_collection_view_cell.mm", + "reading_list_collection_view_controller.h", + "reading_list_collection_view_controller.mm", + "reading_list_collection_view_item_accessibility_delegate.h", "reading_list_data_sink.h", "reading_list_data_source.h", "reading_list_empty_collection_background.h", "reading_list_empty_collection_background.mm", "reading_list_toolbar.h", "reading_list_toolbar.mm", + "reading_list_view_controller.h", + "reading_list_view_controller.mm", ] deps = [ ":resources", @@ -88,12 +81,16 @@ "//ios/chrome/browser/ui/collection_view/cells", "//ios/chrome/browser/ui/colors", "//ios/chrome/browser/ui/favicon:favicon_ui", + "//ios/chrome/browser/ui/keyboard", "//ios/chrome/browser/ui/util", "//ios/chrome/common", "//ios/third_party/material_components_ios", "//ios/third_party/material_roboto_font_loader_ios", "//ui/base", ] + public_deps = [ + "//ios/chrome/browser/ui/collection_view", + ] libs = [ "UIKit.framework" ] }
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm index 710b5349..bc4b7d1 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm +++ b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm
@@ -4,38 +4,23 @@ #import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.h" -#include "base/bind.h" #include "base/logging.h" -#import "base/mac/foundation_util.h" #include "base/metrics/histogram_macros.h" #include "base/metrics/user_metrics.h" #include "base/metrics/user_metrics_action.h" -#include "base/time/time.h" -#include "components/reading_list/core/reading_list_entry.h" #include "components/strings/grit/components_strings.h" -#include "ios/chrome/browser/reading_list/offline_url_utils.h" #import "ios/chrome/browser/ui/alert_coordinator/action_sheet_coordinator.h" #import "ios/chrome/browser/ui/collection_view/cells/collection_view_item+collection_view_controller.h" #import "ios/chrome/browser/ui/collection_view/cells/collection_view_text_item.h" #import "ios/chrome/browser/ui/collection_view/collection_view_model.h" -#import "ios/chrome/browser/ui/favicon/favicon_attributes_provider.h" -#import "ios/chrome/browser/ui/favicon/favicon_view.h" -#import "ios/chrome/browser/ui/material_components/utils.h" -#import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.h" #import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_item_accessibility_delegate.h" #import "ios/chrome/browser/ui/reading_list/reading_list_data_sink.h" #import "ios/chrome/browser/ui/reading_list/reading_list_data_source.h" #import "ios/chrome/browser/ui/reading_list/reading_list_empty_collection_background.h" #import "ios/chrome/browser/ui/reading_list/reading_list_toolbar.h" -#import "ios/chrome/browser/ui/uikit_ui_util.h" -#include "ios/chrome/browser/ui/url_loader.h" #include "ios/chrome/grit/ios_strings.h" -#import "ios/third_party/material_components_ios/src/components/AppBar/src/MaterialAppBar.h" #import "ios/third_party/material_components_ios/src/components/Palettes/src/MaterialPalettes.h" -#include "ios/web/public/referrer.h" -#include "ios/web/public/web_state/web_state.h" #include "ui/base/l10n/l10n_util_mac.h" -#include "ui/base/window_open_disposition.h" #if !defined(__has_feature) || !__has_feature(objc_arc) #error "This file requires ARC support." @@ -53,13 +38,12 @@ ItemTypeItem, }; -// Typedef for a block taking a GURL as parameter and returning nothing. -typedef void (^EntryUpdater)(const GURL&); - +// Typedef for a block taking a CollectionViewItem as parameter and returning +// nothing. +typedef void (^EntryUpdater)(CollectionViewItem* item); } @interface ReadingListCollectionViewController ()< - ReadingListCollectionViewItemAccessibilityDelegate, ReadingListDataSink> { // Toolbar with the actions. @@ -80,7 +64,7 @@ // Loads all the items in all sections. - (void)loadItems; // Fills section |sectionIdentifier| with the items from |array|. -- (void)loadItemsFromArray:(NSArray<ReadingListCollectionViewItem*>*)array +- (void)loadItemsFromArray:(NSArray<CollectionViewItem*>*)array toSection:(SectionIdentifier)sectionIdentifier; // Reloads the data if a changed occured during editing - (void)applyPendingUpdates; @@ -127,8 +111,6 @@ // the data source updates are suspended during this time. - (void)updateIndexPaths:(NSArray<NSIndexPath*>*)indexPaths usingEntryUpdater:(EntryUpdater)updater; -// Logs the deletions histograms for the entry with |url|. -- (void)logDeletionHistogramsForEntry:(const GURL&)url; // Move all the items from |sourceSectionIdentifier| to // |destinationSectionIdentifier| and removes the empty section from the // collection. @@ -404,7 +386,7 @@ } } -- (void)loadItemsFromArray:(NSArray<ReadingListCollectionViewItem*>*)items +- (void)loadItemsFromArray:(NSArray<CollectionViewItem*>*)items toSection:(SectionIdentifier)sectionIdentifier { if (items.count == 0) { return; @@ -413,20 +395,19 @@ [model addSectionWithIdentifier:sectionIdentifier]; [model setHeader:[self headerForSection:sectionIdentifier] forSectionWithIdentifier:sectionIdentifier]; - for (ReadingListCollectionViewItem* item in items) { + for (CollectionViewItem* item in items) { item.type = ItemTypeItem; [self.dataSource fetchFaviconForItem:item]; - item.accessibilityDelegate = self; [model addItem:item toSectionWithIdentifier:sectionIdentifier]; } } - (void)loadItems { - NSMutableArray<ReadingListCollectionViewItem*>* readArray = - [NSMutableArray array]; - NSMutableArray<ReadingListCollectionViewItem*>* unreadArray = - [NSMutableArray array]; - [self.dataSource fillReadItems:readArray unreadItems:unreadArray]; + NSMutableArray<CollectionViewItem*>* readArray = [NSMutableArray array]; + NSMutableArray<CollectionViewItem*>* unreadArray = [NSMutableArray array]; + [self.dataSource fillReadItems:readArray + unreadItems:unreadArray + withDelegate:self]; [self loadItemsFromArray:unreadArray toSection:SectionIdentifierUnread]; [self loadItemsFromArray:readArray toSection:SectionIdentifierRead]; @@ -654,8 +635,8 @@ } [self updateItemsInSectionIdentifier:SectionIdentifierUnread - usingEntryUpdater:^(const GURL& url) { - [self.dataSource setReadStatus:YES forURL:url]; + usingEntryUpdater:^(CollectionViewItem* item) { + [self.dataSource setReadStatus:YES forItem:item]; }]; [self exitEditingModeAnimated:YES]; @@ -670,8 +651,8 @@ } [self updateItemsInSectionIdentifier:SectionIdentifierRead - usingEntryUpdater:^(const GURL& url) { - [self.dataSource setReadStatus:NO forURL:url]; + usingEntryUpdater:^(CollectionViewItem* item) { + [self.dataSource setReadStatus:NO forItem:item]; }]; [self exitEditingModeAnimated:YES]; @@ -684,8 +665,8 @@ NSArray* sortedIndexPaths = [indexPaths sortedArrayUsingSelector:@selector(compare:)]; [self updateIndexPaths:sortedIndexPaths - usingEntryUpdater:^(const GURL& url) { - [self.dataSource setReadStatus:YES forURL:url]; + usingEntryUpdater:^(CollectionViewItem* item) { + [self.dataSource setReadStatus:YES forItem:item]; }]; [self exitEditingModeAnimated:YES]; @@ -697,8 +678,8 @@ NSArray* sortedIndexPaths = [indexPaths sortedArrayUsingSelector:@selector(compare:)]; [self updateIndexPaths:sortedIndexPaths - usingEntryUpdater:^(const GURL& url) { - [self.dataSource setReadStatus:NO forURL:url]; + usingEntryUpdater:^(CollectionViewItem* item) { + [self.dataSource setReadStatus:NO forItem:item]; }]; [self exitEditingModeAnimated:YES]; @@ -713,9 +694,8 @@ } [self updateItemsInSectionIdentifier:SectionIdentifierRead - usingEntryUpdater:^(const GURL& url) { - [self logDeletionHistogramsForEntry:url]; - [self.dataSource removeEntryWithURL:url]; + usingEntryUpdater:^(CollectionViewItem* item) { + [self.dataSource removeEntryFromItem:item]; }]; [self exitEditingModeAnimated:YES]; @@ -738,9 +718,8 @@ - (void)deleteItemsAtIndexPaths:(NSArray*)indexPaths { [self updateIndexPaths:indexPaths - usingEntryUpdater:^(const GURL& url) { - [self logDeletionHistogramsForEntry:url]; - [self.dataSource removeEntryWithURL:url]; + usingEntryUpdater:^(CollectionViewItem* item) { + [self.dataSource removeEntryFromItem:item]; }]; [self exitEditingModeAnimated:YES]; @@ -767,10 +746,8 @@ [self.collectionViewModel itemsInSectionWithIdentifier:identifier]; // Read the objects in reverse order to keep the order (last modified first). for (id item in [readItems reverseObjectEnumerator]) { - ReadingListCollectionViewItem* readingListItem = - base::mac::ObjCCastStrict<ReadingListCollectionViewItem>(item); if (updater) - updater(readingListItem.url); + updater(item); } [self.dataSource endBatchUpdates]; } @@ -780,41 +757,13 @@ [self.dataSource beginBatchUpdates]; // Read the objects in reverse order to keep the order (last modified first). for (NSIndexPath* index in [indexPaths reverseObjectEnumerator]) { - CollectionViewItem* cell = [self.collectionViewModel itemAtIndexPath:index]; - ReadingListCollectionViewItem* readingListItem = - base::mac::ObjCCastStrict<ReadingListCollectionViewItem>(cell); + CollectionViewItem* item = [self.collectionViewModel itemAtIndexPath:index]; if (updater) - updater(readingListItem.url); + updater(item); } [self.dataSource endBatchUpdates]; } -- (void)logDeletionHistogramsForEntry:(const GURL&)url { - const ReadingListEntry* entry = [self.dataSource entryWithURL:url]; - - if (!entry) - return; - - int64_t firstRead = entry->FirstReadTime(); - if (firstRead > 0) { - // Log 0 if the entry has never been read. - firstRead = (base::Time::Now() - base::Time::UnixEpoch()).InMicroseconds() - - firstRead; - // Convert it to hours. - firstRead = firstRead / base::Time::kMicrosecondsPerHour; - } - UMA_HISTOGRAM_COUNTS_10000("ReadingList.FirstReadAgeOnDeletion", firstRead); - - int64_t age = (base::Time::Now() - base::Time::UnixEpoch()).InMicroseconds() - - entry->CreationTime(); - // Convert it to hours. - age = age / base::Time::kMicrosecondsPerHour; - if (entry->IsRead()) - UMA_HISTOGRAM_COUNTS_10000("ReadingList.Read.AgeOnDeletion", age); - else - UMA_HISTOGRAM_COUNTS_10000("ReadingList.Unread.AgeOnDeletion", age); -} - - (void)moveItemsFromSection:(SectionIdentifier)sourceSectionIdentifier toSection:(SectionIdentifier)destinationSectionIdentifier { NSInteger sourceSection = [self.collectionViewModel
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_coordinator.mm b/ios/chrome/browser/ui/reading_list/reading_list_coordinator.mm index 22dc97b..3aa96e6 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_coordinator.mm +++ b/ios/chrome/browser/ui/reading_list/reading_list_coordinator.mm
@@ -267,11 +267,7 @@ (ReadingListCollectionViewController*) readingListCollectionViewController openItemOfflineInNewTab:(CollectionViewItem*)item { - ReadingListCollectionViewItem* readingListItem = - base::mac::ObjCCastStrict<ReadingListCollectionViewItem>(item); - const ReadingListEntry* entry = - [readingListCollectionViewController.dataSource - entryWithURL:readingListItem.url]; + const ReadingListEntry* entry = [self.mediator entryFromItem:item]; if (!entry) { return; @@ -304,8 +300,7 @@ UMA_HISTOGRAM_BOOLEAN("ReadingList.OfflineVersionDisplayed", true); const GURL updateURL = entryURL; - [readingListCollectionViewController.dataSource setReadStatus:YES - forURL:updateURL]; + [self.mediator markEntryRead:updateURL]; } // Opens |URL| in a new tab |incognito| or not.
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_data_source.h b/ios/chrome/browser/ui/reading_list/reading_list_data_source.h index 88931b70..bf612a20a 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_data_source.h +++ b/ios/chrome/browser/ui/reading_list/reading_list_data_source.h
@@ -7,11 +7,9 @@ #include <memory> -class GURL; -class ReadingListEntry; @class CollectionViewItem; -@class ReadingListCollectionViewItem; @protocol ReadingListDataSink; +@protocol ReadingListCollectionViewItemAccessibilityDelegate; // Data Source for the Reading List UI, providing the data sink with the data to // be displayed. Handle the interactions with the model. @@ -31,25 +29,23 @@ // Mark all entries as seen and stop sending updates to the data sink. - (void)dataSinkWillBeDismissed; -// Set the read status of the entry identified with |URL|. -- (void)setReadStatus:(BOOL)read forURL:(const GURL&)URL; +// Set the read status of the entry associated with |item|. +- (void)setReadStatus:(BOOL)read forItem:(nonnull CollectionViewItem*)item; -// Removes the entry associated with |URL|. -- (void)removeEntryWithURL:(const GURL&)URL; +// Removes the entry associated with |item| and logs the deletion. +- (void)removeEntryFromItem:(nonnull CollectionViewItem*)item; // Fills the |readArray| and |unreadArray| with the corresponding items from the // model. The items are sorted most recent first. -- (void)fillReadItems: - (nullable NSMutableArray<ReadingListCollectionViewItem*>*)readArray - unreadItems:(nullable NSMutableArray<ReadingListCollectionViewItem*>*) - unreadArray; - -// TODO(crbug.com/721758): Return ReadingListItem directly. -- (const ReadingListEntry* _Nullable)entryWithURL:(const GURL&)URL; +- (void)fillReadItems:(nullable NSMutableArray<CollectionViewItem*>*)readArray + unreadItems:(nullable NSMutableArray<CollectionViewItem*>*)unreadArray + withDelegate: + (nullable id<ReadingListCollectionViewItemAccessibilityDelegate>) + delegate; // Fetches the |faviconURL| of this |item|, notifies the data sink when // receiving the favicon. -- (void)fetchFaviconForItem:(nonnull ReadingListCollectionViewItem*)item; +- (void)fetchFaviconForItem:(nonnull CollectionViewItem*)item; // Prepares the data source for batch updates. The UI is not notified for the // updates happenning between |-beginBatchUpdates| and |-endBatchUpdates|.
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_mediator.h b/ios/chrome/browser/ui/reading_list/reading_list_mediator.h index 6042d87..2e66c90 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_mediator.h +++ b/ios/chrome/browser/ui/reading_list/reading_list_mediator.h
@@ -13,6 +13,8 @@ class LargeIconService; } +class GURL; +class ReadingListEntry; class ReadingListModel; // Mediator between the Model and the UI. @@ -31,6 +33,9 @@ - (nullable const ReadingListEntry*)entryFromItem: (nonnull CollectionViewItem*)item; +// Marks the entry with |URL| as read. +- (void)markEntryRead:(const GURL&)URL; + @end #endif // IOS_CHROME_BROWSER_UI_READING_LIST_READING_LIST_MEDIATOR_H_
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm b/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm index b32ef24..a57b953 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm +++ b/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm
@@ -16,6 +16,7 @@ #import "ios/chrome/browser/ui/favicon/favicon_attributes_provider.h" #import "ios/chrome/browser/ui/favicon/favicon_view.h" #import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.h" +#import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_item_accessibility_delegate.h" #import "ios/chrome/browser/ui/reading_list/reading_list_data_sink.h" #import "ios/chrome/browser/ui/reading_list/reading_list_utils.h" @@ -77,6 +78,10 @@ return self.model->GetEntryByURL(readingListItem.url); } +- (void)markEntryRead:(const GURL&)URL { + self.model->SetReadStatus(URL, true); +} + #pragma mark - ReadingListDataSource - (BOOL)isEntryRead:(CollectionViewItem*)item { @@ -98,21 +103,27 @@ self.dataSink = nil; } -- (void)setReadStatus:(BOOL)read forURL:(const GURL&)URL { - self.model->SetReadStatus(URL, read); +- (void)setReadStatus:(BOOL)read forItem:(CollectionViewItem*)item { + ReadingListCollectionViewItem* readingListItem = + base::mac::ObjCCastStrict<ReadingListCollectionViewItem>(item); + self.model->SetReadStatus(readingListItem.url, read); } - (const ReadingListEntry*)entryWithURL:(const GURL&)URL { return self.model->GetEntryByURL(URL); } -- (void)removeEntryWithURL:(const GURL&)URL { - self.model->RemoveEntryByURL(URL); +- (void)removeEntryFromItem:(CollectionViewItem*)item { + ReadingListCollectionViewItem* readingListItem = + base::mac::ObjCCastStrict<ReadingListCollectionViewItem>(item); + [self logDeletionOfItem:readingListItem]; + self.model->RemoveEntryByURL(readingListItem.url); } -- (void)fillReadItems:(NSMutableArray<ReadingListCollectionViewItem*>*)readArray - unreadItems: - (NSMutableArray<ReadingListCollectionViewItem*>*)unreadArray { +- (void)fillReadItems:(NSMutableArray<CollectionViewItem*>*)readArray + unreadItems:(NSMutableArray<CollectionViewItem*>*)unreadArray + withDelegate: + (id<ReadingListCollectionViewItemAccessibilityDelegate>)delegate { std::vector<const ReadingListEntry*> readEntries; std::vector<const ReadingListEntry*> unreadEntries; @@ -130,18 +141,22 @@ std::sort(unreadEntries.begin(), unreadEntries.end(), EntrySorter); for (const ReadingListEntry* entry : readEntries) { - [readArray addObject:[self cellItemForReadingListEntry:entry]]; + [readArray addObject:[self cellItemForReadingListEntry:entry + withDelegate:delegate]]; } for (const ReadingListEntry* entry : unreadEntries) { - [unreadArray addObject:[self cellItemForReadingListEntry:entry]]; + [unreadArray addObject:[self cellItemForReadingListEntry:entry + withDelegate:delegate]]; } DCHECK(self.model->Keys().size() == [readArray count] + [unreadArray count]); } -- (void)fetchFaviconForItem:(ReadingListCollectionViewItem*)item { - __weak ReadingListCollectionViewItem* weakItem = item; +- (void)fetchFaviconForItem:(CollectionViewItem*)item { + ReadingListCollectionViewItem* readingListItem = + base::mac::ObjCCastStrict<ReadingListCollectionViewItem>(item); + __weak ReadingListCollectionViewItem* weakItem = readingListItem; __weak ReadingListMediator* weakSelf = self; void (^completionBlock)(FaviconAttributes* attributes) = ^(FaviconAttributes* attributes) { @@ -156,8 +171,9 @@ [strongSelf.dataSink itemHasChangedAfterDelay:strongItem]; }; - [self.attributesProvider fetchFaviconAttributesForURL:item.faviconPageURL - completion:completionBlock]; + [self.attributesProvider + fetchFaviconAttributesForURL:readingListItem.faviconPageURL + completion:completionBlock]; } - (void)beginBatchUpdates { @@ -241,8 +257,11 @@ #pragma mark - Private // Creates a ReadingListCollectionViewItem from a ReadingListEntry |entry|. -- (ReadingListCollectionViewItem*)cellItemForReadingListEntry: - (const ReadingListEntry*)entry { +- (ReadingListCollectionViewItem*) +cellItemForReadingListEntry:(const ReadingListEntry*)entry + withDelegate: + (id<ReadingListCollectionViewItemAccessibilityDelegate>) + delegate { const GURL& url = entry->URL(); ReadingListCollectionViewItem* item = [[ReadingListCollectionViewItem alloc] initWithType:0 @@ -269,16 +288,15 @@ has_distillation_details ? entry->DistillationTime() : 0; item.distillationSize = has_distillation_details ? entry->DistillationSize() : 0; + item.accessibilityDelegate = delegate; return item; } // Whether the data source has changed. - (BOOL)hasDataSourceChanged { - NSMutableArray<ReadingListCollectionViewItem*>* readArray = - [NSMutableArray array]; - NSMutableArray<ReadingListCollectionViewItem*>* unreadArray = - [NSMutableArray array]; - [self fillReadItems:readArray unreadItems:unreadArray]; + NSMutableArray<CollectionViewItem*>* readArray = [NSMutableArray array]; + NSMutableArray<CollectionViewItem*>* unreadArray = [NSMutableArray array]; + [self fillReadItems:readArray unreadItems:unreadArray withDelegate:nil]; return [self currentSection:[self.dataSink readItems] isDifferentOfArray:readArray] || @@ -291,7 +309,7 @@ // URL of the elements. If an element exist in both, the one in |currentSection| // will be overwriten with the informations contained in the one from|array|. - (BOOL)currentSection:(NSArray<CollectionViewItem*>*)currentSection - isDifferentOfArray:(NSArray<ReadingListCollectionViewItem*>*)array { + isDifferentOfArray:(NSArray<CollectionViewItem*>*)array { if (currentSection.count != array.count) return YES; @@ -326,4 +344,31 @@ return NO; } +// Logs the deletions histograms for the entry associated with |item|. +- (void)logDeletionOfItem:(CollectionViewItem*)item { + const ReadingListEntry* entry = [self entryFromItem:item]; + + if (!entry) + return; + + int64_t firstRead = entry->FirstReadTime(); + if (firstRead > 0) { + // Log 0 if the entry has never been read. + firstRead = (base::Time::Now() - base::Time::UnixEpoch()).InMicroseconds() - + firstRead; + // Convert it to hours. + firstRead = firstRead / base::Time::kMicrosecondsPerHour; + } + UMA_HISTOGRAM_COUNTS_10000("ReadingList.FirstReadAgeOnDeletion", firstRead); + + int64_t age = (base::Time::Now() - base::Time::UnixEpoch()).InMicroseconds() - + entry->CreationTime(); + // Convert it to hours. + age = age / base::Time::kMicrosecondsPerHour; + if (entry->IsRead()) + UMA_HISTOGRAM_COUNTS_10000("ReadingList.Read.AgeOnDeletion", age); + else + UMA_HISTOGRAM_COUNTS_10000("ReadingList.Unread.AgeOnDeletion", age); +} + @end
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_mediator_unittest.mm b/ios/chrome/browser/ui/reading_list/reading_list_mediator_unittest.mm index 7bd20967..2127d33 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_mediator_unittest.mm +++ b/ios/chrome/browser/ui/reading_list/reading_list_mediator_unittest.mm
@@ -14,10 +14,12 @@ #include "components/url_formatter/url_formatter.h" #include "ios/chrome/browser/favicon/ios_chrome_large_icon_service_factory.h" #import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.h" +#import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_item_accessibility_delegate.h" #include "ios/web/public/test/test_web_thread_bundle.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" +#import "third_party/ocmock/OCMock/OCMock.h" #if !defined(__has_feature) || !__has_feature(objc_arc) #error "This file requires ARC support." @@ -79,20 +81,29 @@ TEST_F(ReadingListMediatorTest, fillItems) { // Setup. - NSMutableArray<ReadingListCollectionViewItem*>* readArray = - [NSMutableArray array]; - NSMutableArray<ReadingListCollectionViewItem*>* unreadArray = - [NSMutableArray array]; + NSMutableArray<CollectionViewItem*>* readArray = [NSMutableArray array]; + NSMutableArray<CollectionViewItem*>* unreadArray = [NSMutableArray array]; + id mockDelegate = OCMProtocolMock( + @protocol(ReadingListCollectionViewItemAccessibilityDelegate)); // Action. - [mediator_ fillReadItems:readArray unreadItems:unreadArray]; + [mediator_ fillReadItems:readArray + unreadItems:unreadArray + withDelegate:mockDelegate]; // Tests. EXPECT_EQ(3U, [unreadArray count]); EXPECT_EQ(2U, [readArray count]); - EXPECT_TRUE([unreadArray[0].title + NSArray<ReadingListCollectionViewItem*>* rlReadArray = [readArray copy]; + NSArray<ReadingListCollectionViewItem*>* rlUneadArray = [unreadArray copy]; + EXPECT_TRUE([rlUneadArray[0].title isEqualToString:base::SysUTF16ToNSString(url_formatter::FormatUrl( no_title_entry_url_.GetOrigin()))]); - EXPECT_TRUE([readArray[0].title isEqualToString:@"read2"]); - EXPECT_TRUE([readArray[1].title isEqualToString:@"read1"]); + EXPECT_TRUE([rlReadArray[0].title isEqualToString:@"read2"]); + EXPECT_TRUE([rlReadArray[1].title isEqualToString:@"read1"]); + EXPECT_EQ(mockDelegate, rlReadArray[0].accessibilityDelegate); + EXPECT_EQ(mockDelegate, rlReadArray[1].accessibilityDelegate); + EXPECT_EQ(mockDelegate, rlUneadArray[0].accessibilityDelegate); + EXPECT_EQ(mockDelegate, rlUneadArray[1].accessibilityDelegate); + EXPECT_EQ(mockDelegate, rlUneadArray[2].accessibilityDelegate); }
diff --git a/net/cookies/cookie_store_unittest.h b/net/cookies/cookie_store_unittest.h index e8ac9882..bc28a4f 100644 --- a/net/cookies/cookie_store_unittest.h +++ b/net/cookies/cookie_store_unittest.h
@@ -587,12 +587,21 @@ // IE and Safari do not. Assert the expected policy here. TYPED_TEST_P(CookieStoreTest, DomainWithTrailingDotTest) { CookieStore* cs = this->GetCookieStore(); - EXPECT_FALSE(this->SetCookie(cs, this->http_www_google_.url(), - "a=1; domain=.www.google.com.")); - EXPECT_FALSE(this->SetCookie(cs, this->http_www_google_.url(), - "b=2; domain=.www.google.com..")); - this->MatchCookieLines(std::string(), - this->GetCookies(cs, this->http_www_google_.url())); + if (TypeParam::preserves_trailing_dots) { + EXPECT_FALSE(this->SetCookie(cs, this->http_www_google_.url(), + "a=1; domain=.www.google.izzle.")); + EXPECT_FALSE(this->SetCookie(cs, this->http_www_google_.url(), + "b=2; domain=.www.google.izzle..")); + this->MatchCookieLines(std::string(), + this->GetCookies(cs, this->http_www_google_.url())); + } else { + EXPECT_TRUE(this->SetCookie(cs, this->http_www_google_.url(), + "a=1; domain=.www.google.izzle.")); + EXPECT_FALSE(this->SetCookie(cs, this->http_www_google_.url(), + "b=2; domain=.www.google.izzle..")); + this->MatchCookieLines("a=1", + this->GetCookies(cs, this->http_www_google_.url())); + } } // Test that cookies can bet set on higher level domains.
diff --git a/services/ui/ws/window_server.cc b/services/ui/ws/window_server.cc index 2aaa85e..322cb0c 100644 --- a/services/ui/ws/window_server.cc +++ b/services/ui/ws/window_server.cc
@@ -882,10 +882,9 @@ HandleTemporaryReferenceForNewSurface(surface_info.id(), window); - if (!window->parent()) - return; - - WindowTree* window_tree = GetTreeWithId(window->parent()->id().client_id); + // We always use the owner of the window's id (even for an embedded window), + // because an embedded window's id is allocated by the parent's window tree. + WindowTree* window_tree = GetTreeWithId(window->id().client_id); if (window_tree) window_tree->ProcessWindowSurfaceChanged(window, surface_info); }
diff --git a/services/ui/ws/window_tree.cc b/services/ui/ws/window_tree.cc index 5b7166a..ee3cbc1 100644 --- a/services/ui/ws/window_tree.cc +++ b/services/ui/ws/window_tree.cc
@@ -963,14 +963,9 @@ void WindowTree::ProcessWindowSurfaceChanged( ServerWindow* window, const cc::SurfaceInfo& surface_info) { - ServerWindow* parent_window = window->parent(); - ClientWindowId client_window_id, parent_client_window_id; - if (!IsWindowKnown(window, &client_window_id) || - !IsWindowKnown(parent_window, &parent_client_window_id) || - !created_window_map_.count(parent_window->id())) { + ClientWindowId client_window_id; + if (!IsWindowKnown(window, &client_window_id)) return; - } - client()->OnWindowSurfaceChanged(client_window_id.id, surface_info); }
diff --git a/services/ui/ws/window_tree_client_unittest.cc b/services/ui/ws/window_tree_client_unittest.cc index a42ee87..ca6d030 100644 --- a/services/ui/ws/window_tree_client_unittest.cc +++ b/services/ui/ws/window_tree_client_unittest.cc
@@ -2187,46 +2187,78 @@ // Establish the second client at 1,100. ASSERT_NO_FATAL_FAILURE(EstablishSecondClientWithRoot(window_1_100)); + changes2()->clear(); // 1,100 is the id in the wt_client1's id space. The new client should see // 2,1 (the server id). const Id window_1_100_in_ws2 = BuildWindowId(client_id_1(), 1); EXPECT_EQ(window_1_100_in_ws2, wt_client2()->root_window_id()); + // Submit a CompositorFrame to window_1_100_in_ws2 (the embedded window in + // wt2) and make sure the server gets it. + { + cc::mojom::MojoCompositorFrameSinkPtr surface_ptr; + cc::mojom::MojoCompositorFrameSinkClientRequest client_request; + cc::mojom::MojoCompositorFrameSinkClientPtr surface_client_ptr; + client_request = mojo::MakeRequest(&surface_client_ptr); + wt2()->AttachCompositorFrameSink(window_1_100_in_ws2, + mojo::MakeRequest(&surface_ptr), + std::move(surface_client_ptr)); + cc::CompositorFrame compositor_frame; + std::unique_ptr<cc::RenderPass> render_pass = cc::RenderPass::Create(); + gfx::Rect frame_rect(0, 0, 100, 100); + render_pass->SetNew(1, frame_rect, frame_rect, gfx::Transform()); + compositor_frame.render_pass_list.push_back(std::move(render_pass)); + compositor_frame.metadata.device_scale_factor = 1.f; + compositor_frame.metadata.begin_frame_ack = + cc::BeginFrameAck(0, 1, 1, true); + cc::LocalSurfaceId local_surface_id(1, base::UnguessableToken::Create()); + surface_ptr->SubmitCompositorFrame(local_surface_id, + std::move(compositor_frame)); + } + // Make sure the parent connection gets the surface ID. + wt_client1()->WaitForChangeCount(1); + // Verify that the submitted frame is for |window_2_101|. + EXPECT_EQ(window_1_100_in_ws2, + changes1()->back().surface_id.frame_sink_id().client_id()); + changes1()->clear(); + // The first window created in the second client gets a server id of 2,1 // regardless of the id the client uses. const Id window_2_101 = wt_client2()->NewWindow(101); ASSERT_TRUE(wt_client2()->AddWindow(window_1_100_in_ws2, window_2_101)); - const Id window_2_101_in_ws1 = BuildWindowId(client_id_2(), 1); + const Id window_2_101_in_ws2 = BuildWindowId(client_id_2(), 1); wt_client1()->WaitForChangeCount(1); - EXPECT_EQ("HierarchyChanged window=" + IdToString(window_2_101_in_ws1) + + EXPECT_EQ("HierarchyChanged window=" + IdToString(window_2_101_in_ws2) + " old_parent=null new_parent=" + IdToString(window_1_100), SingleChangeToDescription(*changes1())); - changes1()->clear(); - - // Submit a CompositorFrame to window_2_101 and make sure server gets it. - cc::mojom::MojoCompositorFrameSinkPtr surface_ptr; - cc::mojom::MojoCompositorFrameSinkClientRequest client_request; - cc::mojom::MojoCompositorFrameSinkClientPtr surface_client_ptr; - client_request = mojo::MakeRequest(&surface_client_ptr); - wt2()->AttachCompositorFrameSink(window_2_101, - mojo::MakeRequest(&surface_ptr), - std::move(surface_client_ptr)); - cc::CompositorFrame compositor_frame; - std::unique_ptr<cc::RenderPass> render_pass = cc::RenderPass::Create(); - gfx::Rect frame_rect(0, 0, 100, 100); - render_pass->SetNew(1, frame_rect, frame_rect, gfx::Transform()); - compositor_frame.render_pass_list.push_back(std::move(render_pass)); - compositor_frame.metadata.device_scale_factor = 1.f; - compositor_frame.metadata.begin_frame_ack = cc::BeginFrameAck(0, 1, 1, true); - cc::LocalSurfaceId local_surface_id(1, base::UnguessableToken::Create()); - surface_ptr->SubmitCompositorFrame(local_surface_id, - std::move(compositor_frame)); + // Submit a CompositorFrame to window_2_101_in_ws2 (a regular window in + // wt2) and make sure client gets it. + { + cc::mojom::MojoCompositorFrameSinkPtr surface_ptr; + cc::mojom::MojoCompositorFrameSinkClientRequest client_request; + cc::mojom::MojoCompositorFrameSinkClientPtr surface_client_ptr; + client_request = mojo::MakeRequest(&surface_client_ptr); + wt2()->AttachCompositorFrameSink(window_2_101, + mojo::MakeRequest(&surface_ptr), + std::move(surface_client_ptr)); + cc::CompositorFrame compositor_frame; + std::unique_ptr<cc::RenderPass> render_pass = cc::RenderPass::Create(); + gfx::Rect frame_rect(0, 0, 100, 100); + render_pass->SetNew(1, frame_rect, frame_rect, gfx::Transform()); + compositor_frame.render_pass_list.push_back(std::move(render_pass)); + compositor_frame.metadata.device_scale_factor = 1.f; + compositor_frame.metadata.begin_frame_ack = + cc::BeginFrameAck(0, 1, 1, true); + cc::LocalSurfaceId local_surface_id(2, base::UnguessableToken::Create()); + surface_ptr->SubmitCompositorFrame(local_surface_id, + std::move(compositor_frame)); + } // Make sure the parent connection gets the surface ID. - wt_client1()->WaitForChangeCount(1); + wt_client2()->WaitForChangeCount(1); // Verify that the submitted frame is for |window_2_101|. - EXPECT_EQ(window_2_101_in_ws1, - changes1()->back().surface_id.frame_sink_id().client_id()); + EXPECT_EQ(window_2_101_in_ws2, + changes2()->back().surface_id.frame_sink_id().client_id()); } // Verifies when an unknown window with a known child is added to a hierarchy
diff --git a/testing/buildbot/chromium.chromiumos.json b/testing/buildbot/chromium.chromiumos.json index 589224b..addb373 100644 --- a/testing/buildbot/chromium.chromiumos.json +++ b/testing/buildbot/chromium.chromiumos.json
@@ -85,6 +85,16 @@ "test": "base_unittests" }, { + "args": [ + "--ozone-platform=headless" + ], + "swarming": { + "can_use_on_swarming_builders": true, + "shards": 5 + }, + "test": "browser_tests" + }, + { "swarming": { "can_use_on_swarming_builders": true },
diff --git a/testing/buildbot/chromium.fyi.json b/testing/buildbot/chromium.fyi.json index 4f12f78..534b296 100644 --- a/testing/buildbot/chromium.fyi.json +++ b/testing/buildbot/chromium.fyi.json
@@ -10549,6 +10549,81 @@ } ] }, + "Linux ARM (dbg)": { + "gtest_tests": [ + { + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "nacl_helper_nonsfi_unittests" + }, + { + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "nacl_loader_unittests" + }, + { + "args": [ + "--test-launcher-print-test-stdio=always" + ], + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "sandbox_linux_unittests" + } + ] + }, + "Linux ARM64": { + "gtest_tests": [ + { + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "nacl_helper_nonsfi_unittests" + }, + { + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "nacl_loader_unittests" + }, + { + "args": [ + "--test-launcher-print-test-stdio=always" + ], + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "sandbox_linux_unittests" + } + ] + }, + "Linux ARM64 (dbg)": { + "gtest_tests": [ + { + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "nacl_helper_nonsfi_unittests" + }, + { + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "nacl_loader_unittests" + }, + { + "args": [ + "--test-launcher-print-test-stdio=always" + ], + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "sandbox_linux_unittests" + } + ] + }, "Linux Clang Analyzer": { "additional_compile_targets": [ "chrome"
diff --git a/testing/buildbot/chromium.perf.json b/testing/buildbot/chromium.perf.json index 1646f82c..228fae0b 100644 --- a/testing/buildbot/chromium.perf.json +++ b/testing/buildbot/chromium.perf.json
@@ -19596,6 +19596,65 @@ }, { "args": [ + "blink_perf.bindings", + "-v", + "--upload-results", + "--output-format=chartjson", + "--browser=android-chromium" + ], + "isolate_name": "telemetry_perf_tests", + "name": "blink_perf.bindings", + "override_compile_targets": [ + "telemetry_perf_tests" + ], + "swarming": { + "can_use_on_swarming_builders": true, + "dimension_sets": [ + { + "android_devices": "1", + "id": "build47-b1--device4", + "os": "Android", + "pool": "Chrome-perf" + } + ], + "expiration": 36000, + "hard_timeout": 10800, + "ignore_task_failure": false, + "io_timeout": 3600 + } + }, + { + "args": [ + "blink_perf.bindings", + "-v", + "--upload-results", + "--output-format=chartjson", + "--browser=reference", + "--output-trace-tag=_ref" + ], + "isolate_name": "telemetry_perf_tests", + "name": "blink_perf.bindings.reference", + "override_compile_targets": [ + "telemetry_perf_tests" + ], + "swarming": { + "can_use_on_swarming_builders": true, + "dimension_sets": [ + { + "android_devices": "1", + "id": "build47-b1--device4", + "os": "Android", + "pool": "Chrome-perf" + } + ], + "expiration": 36000, + "hard_timeout": 10800, + "ignore_task_failure": true, + "io_timeout": 3600 + } + }, + { + "args": [ "blink_perf.css", "-v", "--upload-results", @@ -20206,6 +20265,65 @@ }, { "args": [ + "dromaeo.domcorequery", + "-v", + "--upload-results", + "--output-format=chartjson", + "--browser=android-chromium" + ], + "isolate_name": "telemetry_perf_tests", + "name": "dromaeo.domcorequery", + "override_compile_targets": [ + "telemetry_perf_tests" + ], + "swarming": { + "can_use_on_swarming_builders": true, + "dimension_sets": [ + { + "android_devices": "1", + "id": "build47-b1--device4", + "os": "Android", + "pool": "Chrome-perf" + } + ], + "expiration": 36000, + "hard_timeout": 10800, + "ignore_task_failure": false, + "io_timeout": 3600 + } + }, + { + "args": [ + "dromaeo.domcorequery", + "-v", + "--upload-results", + "--output-format=chartjson", + "--browser=reference", + "--output-trace-tag=_ref" + ], + "isolate_name": "telemetry_perf_tests", + "name": "dromaeo.domcorequery.reference", + "override_compile_targets": [ + "telemetry_perf_tests" + ], + "swarming": { + "can_use_on_swarming_builders": true, + "dimension_sets": [ + { + "android_devices": "1", + "id": "build47-b1--device4", + "os": "Android", + "pool": "Chrome-perf" + } + ], + "expiration": 36000, + "hard_timeout": 10800, + "ignore_task_failure": true, + "io_timeout": 3600 + } + }, + { + "args": [ "dromaeo.domcoretraverse", "-v", "--upload-results", @@ -21789,6 +21907,65 @@ }, { "args": [ + "smoothness.gpu_rasterization.tough_filters_cases", + "-v", + "--upload-results", + "--output-format=chartjson", + "--browser=android-chromium" + ], + "isolate_name": "telemetry_perf_tests", + "name": "smoothness.gpu_rasterization.tough_filters_cases", + "override_compile_targets": [ + "telemetry_perf_tests" + ], + "swarming": { + "can_use_on_swarming_builders": true, + "dimension_sets": [ + { + "android_devices": "1", + "id": "build47-b1--device4", + "os": "Android", + "pool": "Chrome-perf" + } + ], + "expiration": 36000, + "hard_timeout": 10800, + "ignore_task_failure": false, + "io_timeout": 3600 + } + }, + { + "args": [ + "smoothness.gpu_rasterization.tough_filters_cases", + "-v", + "--upload-results", + "--output-format=chartjson", + "--browser=reference", + "--output-trace-tag=_ref" + ], + "isolate_name": "telemetry_perf_tests", + "name": "smoothness.gpu_rasterization.tough_filters_cases.reference", + "override_compile_targets": [ + "telemetry_perf_tests" + ], + "swarming": { + "can_use_on_swarming_builders": true, + "dimension_sets": [ + { + "android_devices": "1", + "id": "build47-b1--device4", + "os": "Android", + "pool": "Chrome-perf" + } + ], + "expiration": 36000, + "hard_timeout": 10800, + "ignore_task_failure": true, + "io_timeout": 3600 + } + }, + { + "args": [ "smoothness.gpu_rasterization.tough_path_rendering_cases", "-v", "--upload-results", @@ -23441,6 +23618,65 @@ }, { "args": [ + "thread_times.key_silk_cases", + "-v", + "--upload-results", + "--output-format=chartjson", + "--browser=android-chromium" + ], + "isolate_name": "telemetry_perf_tests", + "name": "thread_times.key_silk_cases", + "override_compile_targets": [ + "telemetry_perf_tests" + ], + "swarming": { + "can_use_on_swarming_builders": true, + "dimension_sets": [ + { + "android_devices": "1", + "id": "build47-b1--device4", + "os": "Android", + "pool": "Chrome-perf" + } + ], + "expiration": 36000, + "hard_timeout": 10800, + "ignore_task_failure": false, + "io_timeout": 3600 + } + }, + { + "args": [ + "thread_times.key_silk_cases", + "-v", + "--upload-results", + "--output-format=chartjson", + "--browser=reference", + "--output-trace-tag=_ref" + ], + "isolate_name": "telemetry_perf_tests", + "name": "thread_times.key_silk_cases.reference", + "override_compile_targets": [ + "telemetry_perf_tests" + ], + "swarming": { + "can_use_on_swarming_builders": true, + "dimension_sets": [ + { + "android_devices": "1", + "id": "build47-b1--device4", + "os": "Android", + "pool": "Chrome-perf" + } + ], + "expiration": 36000, + "hard_timeout": 10800, + "ignore_task_failure": true, + "io_timeout": 3600 + } + }, + { + "args": [ "thread_times.polymer", "-v", "--upload-results",
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index 59db9a8..646192c 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -919,6 +919,8 @@ crbug.com/543110 [ Mac ] fast/text/international/text-shaping-arabic.html [ Failure ] +crbug.com/731731 inspector-protocol/layers/paint-profiler-load-empty.html [ Failure Pass ] + # Will be re-enabled and rebaselined once we remove the '--enable-file-cookies' flag. crbug.com/470482 fast/cookies/local-file-can-set-cookies.html [ Skip ] @@ -996,6 +998,66 @@ crbug.com/441840 external/wpt/css/css-shapes-1/shape-outside/values/shape-outside-polygon-004.html [ Failure ] crbug.com/441840 [ Linux Win ] external/wpt/css/css-shapes-1/shape-outside/values/shape-outside-shape-arguments-000.html [ Failure ] +crbug.com/731696 images/color-profile-border-fade.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-background-image-cover.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-layer-filter.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-image-shape.html [ NeedsRebaseline ] +crbug.com/731696 virtual/exotic-color-space/images/motion-jpeg-single-frame.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-image.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-image-canvas.html [ NeedsRebaseline ] +crbug.com/731696 images/jpeg-with-color-profile.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-iframe.html [ NeedsRebaseline ] +crbug.com/731696 virtual/gpu-rasterization/images/color-profile-background-image-space.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-animate-rotate.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-image-filter-all.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-animate.html [ NeedsRebaseline ] +crbug.com/731696 virtual/gpu-rasterization/images/color-profile-svg.html [ NeedsRebaseline ] +crbug.com/731696 virtual/gpu-rasterization/images/color-profile-image-canvas-svg.html [ NeedsRebaseline ] +crbug.com/731696 virtual/exotic-color-space/images/color-profile-image-profile-match.html [ NeedsRebaseline ] +crbug.com/731696 virtual/gpu-rasterization/images/color-profile-image-canvas-pattern.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-svg-fill-text.html [ NeedsRebaseline ] +crbug.com/731696 images/webp-color-profile-lossy-alpha.html [ NeedsRebaseline ] +crbug.com/731696 virtual/exotic-color-space/images/color-profile-image-canvas.html [ NeedsRebaseline ] +crbug.com/731696 images/cHRM_color_spin.html [ NeedsRebaseline ] +crbug.com/731696 images/webp-color-profile-lossless.html [ NeedsRebaseline ] +crbug.com/731696 images/paletted-png-with-color-profile.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-clip.html [ NeedsRebaseline ] +crbug.com/731696 virtual/gpu-rasterization/images/color-profile-munsell-adobe-to-srgb.html [ NeedsRebaseline ] +crbug.com/731696 images/cross-fade-background-size.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-mask-image-svg.html [ NeedsRebaseline ] +crbug.com/731696 images/color-jpeg-with-color-profile.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-background-clip-text.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-image-canvas-svg.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-image-pseudo-content.html [ NeedsRebaseline ] +crbug.com/731696 virtual/exotic-color-space/images/color-profile-drag-image.html [ NeedsRebaseline ] +crbug.com/731696 virtual/exotic-color-space/images/color-profile-image-canvas-svg.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-layer.html [ NeedsRebaseline ] +crbug.com/731696 virtual/exotic-color-space/images/jpeg-yuv-progressive-image.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-background-image-repeat.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-image-svg-resource-url.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-group.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-svg-foreign-object.html [ NeedsRebaseline ] +crbug.com/731696 images/png-suite/test.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-background-image-cross-fade.html [ NeedsRebaseline ] +crbug.com/731696 virtual/exotic-color-space/images/color-profile-image-canvas-pattern.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-image-canvas-pattern.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-filter.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-object.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-drag-image.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-border-image.html [ NeedsRebaseline ] +crbug.com/731696 virtual/gpu-rasterization/images/cross-fade-background-size.html [ NeedsRebaseline ] +crbug.com/731696 images/webp-color-profile-lossy.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-munsell-adobe-to-srgb.html [ NeedsRebaseline ] +crbug.com/731696 virtual/gpu-rasterization/images/color-profile-drag-image.html [ NeedsRebaseline ] +crbug.com/731696 images/png-with-color-profile.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-image-object-fit.html [ NeedsRebaseline ] +crbug.com/731696 virtual/gpu-rasterization/images/color-profile-image-canvas.html [ NeedsRebaseline ] +crbug.com/731696 virtual/gpu-rasterization/images/color-profile-svg-fill-text.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-background-image-cross-fade-png.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-border-radius.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-background-image-space.html [ NeedsRebaseline ] +crbug.com/731696 images/color-profile-svg.html [ NeedsRebaseline ] + crbug.com/306730 external/wpt/css/css-text-3/i18n/css3-text-line-break-jazh-136.html [ Failure ] crbug.com/306730 external/wpt/css/css-text-3/i18n/css3-text-line-break-jazh-137.html [ Failure ] crbug.com/306730 external/wpt/css/css-text-3/i18n/css3-text-line-break-jazh-142.html [ Failure ]
diff --git a/third_party/WebKit/LayoutTests/http/tests/worklet/webexposed/global-interface-listing-paint-worklet-expected.txt b/third_party/WebKit/LayoutTests/http/tests/worklet/webexposed/global-interface-listing-paint-worklet-expected.txt index 35294a2..2439768 100644 --- a/third_party/WebKit/LayoutTests/http/tests/worklet/webexposed/global-interface-listing-paint-worklet-expected.txt +++ b/third_party/WebKit/LayoutTests/http/tests/worklet/webexposed/global-interface-listing-paint-worklet-expected.txt
@@ -50,7 +50,6 @@ CONSOLE MESSAGE: line 147: getter ay CONSOLE MESSAGE: line 147: method constructor CONSOLE MESSAGE: line 147: interface CSSStyleValue -CONSOLE MESSAGE: line 147: static method parse CONSOLE MESSAGE: line 147: method constructor CONSOLE MESSAGE: line 147: method toString CONSOLE MESSAGE: line 147: interface CSSTransformComponent
diff --git a/third_party/WebKit/Source/core/css/cssom/CSSKeywordValue.idl b/third_party/WebKit/Source/core/css/cssom/CSSKeywordValue.idl index 1f0b894..5f882b55 100644 --- a/third_party/WebKit/Source/core/css/cssom/CSSKeywordValue.idl +++ b/third_party/WebKit/Source/core/css/cssom/CSSKeywordValue.idl
@@ -7,8 +7,7 @@ // https://drafts.css-houdini.org/css-typed-om/#keywordvalue-objects [ Constructor(DOMString keyword), - Exposed=(Window,PaintWorklet), - RuntimeEnabled=CSSTypedOM, + Exposed(Window CSSTypedOM, PaintWorklet CSSPaintAPI), RaisesException=Constructor ] interface CSSKeywordValue : CSSStyleValue { attribute DOMString value;
diff --git a/third_party/WebKit/Source/core/css/cssom/CSSNumericValue.idl b/third_party/WebKit/Source/core/css/cssom/CSSNumericValue.idl index edb1a00..0f26ef9 100644 --- a/third_party/WebKit/Source/core/css/cssom/CSSNumericValue.idl +++ b/third_party/WebKit/Source/core/css/cssom/CSSNumericValue.idl
@@ -5,8 +5,7 @@ // CSSNumericValue is the base class for numeric and length typed CSS Values. // https://drafts.css-houdini.org/css-typed-om/#numeric-objects [ - Exposed=(Window,PaintWorklet), - RuntimeEnabled=CSSTypedOM + Exposed(Window CSSTypedOM, PaintWorklet CSSPaintAPI), ] interface CSSNumericValue : CSSStyleValue { [RaisesException, NewObject] CSSNumericValue add(CSSNumericValue value); [RaisesException, NewObject] CSSNumericValue sub(CSSNumericValue value); @@ -15,5 +14,6 @@ [RaisesException, NewObject] CSSNumericValue to(DOMString unit); - [RaisesException, NewObject] static CSSNumericValue parse(DOMString cssText); + // Putting Exposed=Window in the next line makes |parse| not exposed to PaintWorklet. + [RaisesException, NewObject, Exposed=Window] static CSSNumericValue parse(DOMString cssText); };
diff --git a/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.idl b/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.idl index a51541dd..42622420 100644 --- a/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.idl +++ b/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.idl
@@ -7,8 +7,7 @@ // Spec: https://drafts.css-houdini.org/css-typed-om/#positionvalue-objects [ Constructor(CSSNumericValue x, CSSNumericValue y), - Exposed=(Window,PaintWorklet), - RuntimeEnabled=CSSTypedOM + Exposed(Window CSSTypedOM, PaintWorklet CSSPaintAPI), ] interface CSSPositionValue : CSSStyleValue { readonly attribute CSSNumericValue x; readonly attribute CSSNumericValue y;
diff --git a/third_party/WebKit/Source/core/css/cssom/CSSResourceValue.idl b/third_party/WebKit/Source/core/css/cssom/CSSResourceValue.idl index 85e0272..d8f2996 100644 --- a/third_party/WebKit/Source/core/css/cssom/CSSResourceValue.idl +++ b/third_party/WebKit/Source/core/css/cssom/CSSResourceValue.idl
@@ -10,8 +10,7 @@ }; [ - Exposed=(Window,PaintWorklet), - RuntimeEnabled=CSSTypedOM, + Exposed(Window CSSTypedOM, PaintWorklet CSSPaintAPI), ] interface CSSResourceValue : CSSStyleValue { readonly attribute CSSResourceState state; };
diff --git a/third_party/WebKit/Source/core/css/cssom/CSSStyleValue.idl b/third_party/WebKit/Source/core/css/cssom/CSSStyleValue.idl index 582f76d7..14ecb15 100644 --- a/third_party/WebKit/Source/core/css/cssom/CSSStyleValue.idl +++ b/third_party/WebKit/Source/core/css/cssom/CSSStyleValue.idl
@@ -7,10 +7,10 @@ // base CSSStyleValues. // Spec: https://drafts.css-houdini.org/css-typed-om/#stylevalue-objects [ - Exposed=(Window,PaintWorklet), - RuntimeEnabled=CSSTypedOM, + Exposed(Window CSSTypedOM, PaintWorklet CSSPaintAPI), ] interface CSSStyleValue { stringifier; // TODO(meade): Should be (CSSStyleValue or sequence<CSSStyleValue>)? instead of object?. Fix when the code generator supports this. - [RaisesException, CallWith=ScriptState] static object? parse(DOMString property, DOMString cssText); + // Putting Exposed=Window in the next line makes |parse| not exposed to PaintWorklet. + [RaisesException, CallWith=ScriptState, Exposed=Window] static object? parse(DOMString property, DOMString cssText); };
diff --git a/third_party/WebKit/Source/core/css/cssom/CSSTransformValue.idl b/third_party/WebKit/Source/core/css/cssom/CSSTransformValue.idl index c024b23..9cd03ce6 100644 --- a/third_party/WebKit/Source/core/css/cssom/CSSTransformValue.idl +++ b/third_party/WebKit/Source/core/css/cssom/CSSTransformValue.idl
@@ -5,8 +5,7 @@ [ Constructor(), Constructor(sequence<CSSTransformComponent> transformComponents), - Exposed=(Window,PaintWorklet), - RuntimeEnabled=CSSTypedOM + Exposed(Window CSSTypedOM, PaintWorklet CSSPaintAPI), ] interface CSSTransformValue : CSSStyleValue { // https://github.com/w3c/css-houdini-drafts/issues/358 readonly attribute unsigned long length;
diff --git a/third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.idl b/third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.idl index 98ba6a0f..d5ec81b3 100644 --- a/third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.idl +++ b/third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.idl
@@ -6,8 +6,7 @@ // They represent a list of string fragments and variable references. // Spec: https://drafts.css-houdini.org/css-typed-om/#unparsedvalue-objects [ - Exposed=(Window,PaintWorklet), - RuntimeEnabled=CSSTypedOM, + Exposed(Window CSSTypedOM, PaintWorklet CSSPaintAPI), ] interface CSSUnparsedValue : CSSStyleValue { // https://github.com/w3c/css-houdini-drafts/issues/358 readonly attribute unsigned long length;
diff --git a/third_party/WebKit/Source/core/css/cssom/StylePropertyMapReadonly.idl b/third_party/WebKit/Source/core/css/cssom/StylePropertyMapReadonly.idl index 3237e0b..fdf354b 100644 --- a/third_party/WebKit/Source/core/css/cssom/StylePropertyMapReadonly.idl +++ b/third_party/WebKit/Source/core/css/cssom/StylePropertyMapReadonly.idl
@@ -3,8 +3,7 @@ // found in the LICENSE file. [ - Exposed=(Window,PaintWorklet), - RuntimeEnabled=CSSTypedOM, + Exposed(Window CSSTypedOM, PaintWorklet CSSPaintAPI), ] interface StylePropertyMapReadonly { [RaisesException] CSSStyleValue? get(DOMString property); [RaisesException] sequence<CSSStyleValue> getAll(DOMString property);
diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp index 9daa4f8..252d0aa 100644 --- a/third_party/WebKit/Source/core/dom/Document.cpp +++ b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -2908,7 +2908,7 @@ } HTMLBodyElement* Document::FirstBodyElement() const { - if (!documentElement()) + if (!documentElement() || !isHTMLHtmlElement(documentElement())) return 0; for (HTMLElement* child =
diff --git a/third_party/WebKit/Source/core/loader/LinkLoader.h b/third_party/WebKit/Source/core/loader/LinkLoader.h index 43a29583..35a4ad81 100644 --- a/third_party/WebKit/Source/core/loader/LinkLoader.h +++ b/third_party/WebKit/Source/core/loader/LinkLoader.h
@@ -50,8 +50,8 @@ class PrerenderHandle; struct ViewportDescriptionWrapper; -// The LinkLoader can load link rel types icon, dns-prefetch, subresource, -// prefetch and prerender. +// The LinkLoader can load link rel types icon, dns-prefetch, prefetch, and +// prerender. class CORE_EXPORT LinkLoader final : public GarbageCollectedFinalized<LinkLoader>, public ResourceOwner<Resource, ResourceClient>,
diff --git a/third_party/WebKit/Source/core/page/PagePopupClientTest.cpp b/third_party/WebKit/Source/core/page/PagePopupClientTest.cpp index 738fb24f..f40346eb 100644 --- a/third_party/WebKit/Source/core/page/PagePopupClientTest.cpp +++ b/third_party/WebKit/Source/core/page/PagePopupClientTest.cpp
@@ -14,9 +14,10 @@ PagePopupClient::AddJavaScriptString( String::FromUTF8("abc\r\n'\"</script>\t\f\v\xE2\x80\xA8\xE2\x80\xA9"), buffer.Get()); + const Vector<char> contiguous = buffer->Copy(); EXPECT_EQ( "\"abc\\r\\n'\\\"\\x3C/script>\\u0009\\u000C\\u000B\\u2028\\u2029\"", - std::string(buffer->Data(), buffer->size())); + std::string(contiguous.data(), contiguous.size())); } } // namespace blink
diff --git a/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp index 9f9308e0..4e0b191 100644 --- a/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp +++ b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
@@ -16,8 +16,8 @@ String full_path = testing::BlinkRootDir(); full_path.append("/Source/core/paint/test_data/"); full_path.append(file_name); - RefPtr<SharedBuffer> input_buffer = testing::ReadFromFile(full_path); - SetBodyInnerHTML(String(input_buffer->Data(), input_buffer->size())); + const Vector<char> input_buffer = testing::ReadFromFile(full_path)->Copy(); + SetBodyInnerHTML(String(input_buffer.data(), input_buffer.size())); } const TransformPaintPropertyNode*
diff --git a/third_party/WebKit/Source/platform/SharedBuffer.cpp b/third_party/WebKit/Source/platform/SharedBuffer.cpp index 39707b6..add05a7 100644 --- a/third_party/WebKit/Source/platform/SharedBuffer.cpp +++ b/third_party/WebKit/Source/platform/SharedBuffer.cpp
@@ -140,21 +140,17 @@ buffer_.clear(); } -PassRefPtr<SharedBuffer> SharedBuffer::Copy() const { - RefPtr<SharedBuffer> clone(AdoptRef(new SharedBuffer)); - clone->size_ = size_; - clone->buffer_.ReserveInitialCapacity(size_); - clone->buffer_.Append(buffer_.data(), buffer_.size()); - if (!segments_.IsEmpty()) { - const char* segment = 0; - size_t position = buffer_.size(); - while (size_t segment_size = GetSomeDataInternal(segment, position)) { - clone->buffer_.Append(segment, segment_size); - position += segment_size; - } - DCHECK_EQ(position, clone->size()); - } - return clone.Release(); +Vector<char> SharedBuffer::Copy() const { + Vector<char> buffer; + buffer.ReserveInitialCapacity(size_); + + ForEachSegment([&buffer](const char* segment, size_t segment_size, + size_t segment_offset) { + buffer.Append(segment, segment_size); + }); + + DCHECK_EQ(buffer.size(), size_); + return buffer; } void SharedBuffer::MergeSegmentsIntoBuffer() const {
diff --git a/third_party/WebKit/Source/platform/SharedBuffer.h b/third_party/WebKit/Source/platform/SharedBuffer.h index 486792b7..cefa8b9 100644 --- a/third_party/WebKit/Source/platform/SharedBuffer.h +++ b/third_party/WebKit/Source/platform/SharedBuffer.h
@@ -70,6 +70,8 @@ ~SharedBuffer(); + // DEPRECATED: use a segment iterator or Copy() instead. + // // Calling this function will force internal segmented buffers to be merged // into a flat buffer. Use getSomeData() whenever possible for better // performance. @@ -90,7 +92,9 @@ void Clear(); - PassRefPtr<SharedBuffer> Copy() const; + // Copies the segmented data into a contiguous buffer. Use GetSomeData() or + // ForEachSegment() whenever possible, as they are cheaper. + Vector<char> Copy() const; // Return the number of consecutive bytes after "position". "data" // points to the first byte.
diff --git a/third_party/WebKit/Source/platform/SharedBufferTest.cpp b/third_party/WebKit/Source/platform/SharedBufferTest.cpp index 269ebf3..8fb9b0e4 100644 --- a/third_party/WebKit/Source/platform/SharedBufferTest.cpp +++ b/third_party/WebKit/Source/platform/SharedBufferTest.cpp
@@ -132,12 +132,14 @@ // copy(). ASSERT_EQ(length * 4, shared_buffer->size()); - RefPtr<SharedBuffer> clone = shared_buffer->Copy(); - ASSERT_EQ(length * 4, clone->size()); - ASSERT_EQ(0, memcmp(clone->Data(), shared_buffer->Data(), clone->size())); + Vector<char> clone = shared_buffer->Copy(); + ASSERT_EQ(length * 4, clone.size()); + const Vector<char> contiguous = shared_buffer->Copy(); + ASSERT_EQ(contiguous.size(), shared_buffer->size()); + ASSERT_EQ(0, memcmp(clone.data(), contiguous.data(), clone.size())); - clone->Append(test_data.data(), length); - ASSERT_EQ(length * 5, clone->size()); + clone.Append(test_data.data(), length); + ASSERT_EQ(length * 5, clone.size()); } TEST(SharedBufferTest, constructorWithSizeOnly) {
diff --git a/third_party/WebKit/Source/platform/graphics/DecodingImageGenerator.cpp b/third_party/WebKit/Source/platform/graphics/DecodingImageGenerator.cpp index cb35511..88ac3e7 100644 --- a/third_party/WebKit/Source/platform/graphics/DecodingImageGenerator.cpp +++ b/third_party/WebKit/Source/platform/graphics/DecodingImageGenerator.cpp
@@ -62,46 +62,6 @@ return data_->GetAsSkData().release(); } -static void doColorSpaceXform(const SkImageInfo& dst_info, - void* pixels, - size_t row_bytes, - SkColorSpace* src_color_space, - SkTransferFunctionBehavior behavior) { - TRACE_EVENT0("blink", "DecodingImageGenerator::getPixels - apply xform"); - std::unique_ptr<SkColorSpaceXform> xform = - SkColorSpaceXform::New(src_color_space, dst_info.colorSpace()); - - const bool post_xform_premul = - (dst_info.alphaType() == kPremul_SkAlphaType) && - (behavior == SkTransferFunctionBehavior::kIgnore); - - uint32_t* row = reinterpret_cast<uint32_t*>(pixels); - for (int y = 0; y < dst_info.height(); y++) { - SkColorSpaceXform::ColorFormat format = - SkColorSpaceXform::kRGBA_8888_ColorFormat; - if (kN32_SkColorType == kBGRA_8888_SkColorType) { - format = SkColorSpaceXform::kBGRA_8888_ColorFormat; - } - SkAlphaType alpha_type = - post_xform_premul ? kUnpremul_SkAlphaType : dst_info.alphaType(); - bool xformed = - xform->apply(format, row, format, row, dst_info.width(), alpha_type); - DCHECK(xformed); - - // To be compatible with dst space blending, premultiply in the dst space. - if (post_xform_premul) { - for (int x = 0; x < dst_info.width(); x++) { - row[x] = - SkPreMultiplyARGB(SkGetPackedA32(row[x]), SkGetPackedR32(row[x]), - SkGetPackedG32(row[x]), SkGetPackedB32(row[x])); - } - } - - row = reinterpret_cast<uint32_t*>( - (reinterpret_cast<uint8_t*>(row) + row_bytes)); - } -} - bool DecodingImageGenerator::onGetPixels(const SkImageInfo& dst_info, void* pixels, size_t row_bytes, @@ -140,14 +100,20 @@ } PlatformInstrumentation::WillDecodeLazyPixelRef(uniqueID()); - bool decoded = frame_generator_->DecodeAndScale( + const bool decoded = frame_generator_->DecodeAndScale( data_.Get(), all_data_received_, frame_index_, decode_info, pixels, row_bytes, alpha_option); PlatformInstrumentation::DidDecodeLazyPixelRef(); if (decoded && needs_color_xform) { - doColorSpaceXform(dst_info, pixels, row_bytes, decode_color_space, - options.fBehavior); + TRACE_EVENT0("blink", "DecodingImageGenerator::getPixels - apply xform"); + SkPixmap src(decode_info, pixels, row_bytes); + + // kIgnore ensures that we perform the premultiply (if necessary) in the dst + // space. + const bool converted = src.readPixels(dst_info, pixels, row_bytes, 0, 0, + SkTransferFunctionBehavior::kIgnore); + DCHECK(converted); } return decoded;
diff --git a/third_party/WebKit/Source/platform/graphics/DeferredImageDecoderTest.cpp b/third_party/WebKit/Source/platform/graphics/DeferredImageDecoderTest.cpp index 905b2d38..c580df1 100644 --- a/third_party/WebKit/Source/platform/graphics/DeferredImageDecoderTest.cpp +++ b/third_party/WebKit/Source/platform/graphics/DeferredImageDecoderTest.cpp
@@ -383,14 +383,16 @@ } TEST_F(DeferredImageDecoderTest, data) { - RefPtr<SharedBuffer> original_data = + RefPtr<SharedBuffer> original_buffer = SharedBuffer::Create(data_->Data(), data_->size()); - EXPECT_EQ(original_data->size(), data_->size()); - lazy_decoder_->SetData(original_data, false); - RefPtr<SharedBuffer> new_data = lazy_decoder_->Data(); - EXPECT_EQ(original_data->size(), new_data->size()); - EXPECT_EQ(0, std::memcmp(original_data->Data(), new_data->Data(), - new_data->size())); + EXPECT_EQ(original_buffer->size(), data_->size()); + lazy_decoder_->SetData(original_buffer, false); + RefPtr<SharedBuffer> new_buffer = lazy_decoder_->Data(); + EXPECT_EQ(original_buffer->size(), new_buffer->size()); + const Vector<char> original_data = original_buffer->Copy(); + const Vector<char> new_data = new_buffer->Copy(); + EXPECT_EQ(0, std::memcmp(original_data.data(), new_data.data(), + new_buffer->size())); } } // namespace blink
diff --git a/third_party/WebKit/Source/platform/graphics/DeferredImageDecoderTestWoPlatform.cpp b/third_party/WebKit/Source/platform/graphics/DeferredImageDecoderTestWoPlatform.cpp index 1fb0783..10ddba7c 100644 --- a/third_party/WebKit/Source/platform/graphics/DeferredImageDecoderTestWoPlatform.cpp +++ b/third_party/WebKit/Source/platform/graphics/DeferredImageDecoderTestWoPlatform.cpp
@@ -35,11 +35,10 @@ static void MixImages(const char* file_name, size_t bytes_for_first_frame, size_t later_frame) { - RefPtr<SharedBuffer> file = ReadFile(file_name); - ASSERT_NE(file, nullptr); + const Vector<char> file = ReadFile(file_name)->Copy(); RefPtr<SharedBuffer> partial_file = - SharedBuffer::Create(file->Data(), bytes_for_first_frame); + SharedBuffer::Create(file.data(), bytes_for_first_frame); std::unique_ptr<DeferredImageDecoder> decoder = DeferredImageDecoder::Create( partial_file, false, ImageDecoder::kAlphaPremultiplied, ColorBehavior::Ignore()); @@ -47,7 +46,7 @@ sk_sp<SkImage> partial_image = decoder->CreateFrameAtIndex(0); RefPtr<SharedBuffer> almost_complete_file = - SharedBuffer::Create(file->Data(), file->size() - 1); + SharedBuffer::Create(file.data(), file.size() - 1); decoder->SetData(almost_complete_file, false); sk_sp<SkImage> image_with_more_data = decoder->CreateFrameAtIndex(later_frame);
diff --git a/third_party/WebKit/Source/platform/image-decoders/ImageDecoderTestHelpers.cpp b/third_party/WebKit/Source/platform/image-decoders/ImageDecoderTestHelpers.cpp index 41643a03..02c5760 100644 --- a/third_party/WebKit/Source/platform/image-decoders/ImageDecoderTestHelpers.cpp +++ b/third_party/WebKit/Source/platform/image-decoders/ImageDecoderTestHelpers.cpp
@@ -55,13 +55,13 @@ } void TestByteByByteDecode(DecoderCreator create_decoder, - SharedBuffer* data, + SharedBuffer* shared_data, size_t expected_frame_count, int expected_repetition_count) { - ASSERT_TRUE(data->Data()); + const Vector<char> data = shared_data->Copy(); Vector<unsigned> baseline_hashes; - CreateDecodingBaseline(create_decoder, data, &baseline_hashes); + CreateDecodingBaseline(create_decoder, shared_data, &baseline_hashes); std::unique_ptr<ImageDecoder> decoder = create_decoder(); @@ -71,15 +71,15 @@ // Pass data to decoder byte by byte. RefPtr<SharedBuffer> source_data[2] = {SharedBuffer::Create(), SharedBuffer::Create()}; - const char* source = data->Data(); + const char* source = data.data(); - for (size_t length = 1; length <= data->size() && !decoder->Failed(); + for (size_t length = 1; length <= data.size() && !decoder->Failed(); ++length) { source_data[0]->Append(source, 1u); source_data[1]->Append(source++, 1u); // Alternate the buffers to cover the JPEGImageDecoder::OnSetData restart // code. - decoder->SetData(source_data[length & 1].Get(), length == data->size()); + decoder->SetData(source_data[length & 1].Get(), length == data.size()); EXPECT_LE(frame_count, decoder->FrameCount()); frame_count = decoder->FrameCount(); @@ -115,8 +115,10 @@ // not break decoding at a critical point: in between a call to decode the size // (when the decoder stops while it may still have input data to read) and a // call to do a full decode. -static void TestMergeBuffer(DecoderCreator create_decoder, SharedBuffer* data) { - const unsigned hash = CreateDecodingBaseline(create_decoder, data); +static void TestMergeBuffer(DecoderCreator create_decoder, + SharedBuffer* shared_data) { + const unsigned hash = CreateDecodingBaseline(create_decoder, shared_data); + const Vector<char> data = shared_data->Copy(); // In order to do any verification, this test needs to move the data owned // by the SharedBuffer. A way to guarantee that is to create a new one, and @@ -124,7 +126,7 @@ // results in writing the data into a segment, skipping the internal // contiguous buffer. RefPtr<SharedBuffer> segmented_data = SharedBuffer::Create(); - segmented_data->Append(data->Data(), data->size()); + segmented_data->Append(data.data(), data.size()); std::unique_ptr<ImageDecoder> decoder = create_decoder(); decoder->SetData(segmented_data.Get(), true); @@ -202,7 +204,8 @@ size_t frame_count = decoder->FrameCount(); // ... and then decode frames from 'reallocated_data'. - RefPtr<SharedBuffer> reallocated_data = data->Copy(); + Vector<char> copy = data->Copy(); + RefPtr<SharedBuffer> reallocated_data = SharedBuffer::AdoptVector(copy); ASSERT_TRUE(reallocated_data.Get()); data->Clear(); decoder->SetData(reallocated_data.Get(), true); @@ -225,7 +228,8 @@ // the data to check that IsSizeAvailable() changes state only when that // offset is reached. Also check other decoder state. RefPtr<SharedBuffer> temp_data = SharedBuffer::Create(); - const char* source = data->Data(); + const Vector<char> source_buffer = data->Copy(); + const char* source = source_buffer.data(); for (size_t length = 1; length <= frame_offset; ++length) { temp_data->Append(source++, 1u); decoder->SetData(temp_data.Get(), false); @@ -250,9 +254,10 @@ } static void TestProgressiveDecoding(DecoderCreator create_decoder, - SharedBuffer* full_data, + SharedBuffer* full_buffer, size_t increment) { - const size_t full_length = full_data->size(); + const Vector<char> full_data = full_buffer->Copy(); + const size_t full_length = full_data.size(); std::unique_ptr<ImageDecoder> decoder; @@ -261,7 +266,7 @@ // Compute hashes when the file is truncated. RefPtr<SharedBuffer> data = SharedBuffer::Create(); - const char* source = full_data->Data(); + const char* source = full_data.data(); for (size_t i = 1; i <= full_length; i += increment) { decoder = create_decoder(); data->Append(source++, 1u); @@ -277,7 +282,7 @@ // Compute hashes when the file is progressively decoded. decoder = create_decoder(); data = SharedBuffer::Create(); - source = full_data->Data(); + source = full_data.data(); for (size_t i = 1; i <= full_length; i += increment) { data->Append(source++, 1u); decoder->SetData(data.Get(), i == full_length); @@ -295,13 +300,14 @@ void TestUpdateRequiredPreviousFrameAfterFirstDecode( DecoderCreator create_decoder, - SharedBuffer* full_data) { + SharedBuffer* full_buffer) { + const Vector<char> full_data = full_buffer->Copy(); std::unique_ptr<ImageDecoder> decoder = create_decoder(); // Give it data that is enough to parse but not decode in order to check the // status of RequiredPreviousFrameIndex before decoding. RefPtr<SharedBuffer> data = SharedBuffer::Create(); - const char* source = full_data->Data(); + const char* source = full_data.data(); do { data->Append(source++, 1u); decoder->SetData(data.Get(), false); @@ -317,7 +323,7 @@ decoder->FrameBufferAtIndex(i)->RequiredPreviousFrameIndex()); } - decoder->SetData(full_data, true); + decoder->SetData(full_buffer, true); for (size_t i = 0; i < frame_count; ++i) { EXPECT_EQ(kNotFound, decoder->FrameBufferAtIndex(i)->RequiredPreviousFrameIndex()); @@ -326,16 +332,17 @@ void TestResumePartialDecodeAfterClearFrameBufferCache( DecoderCreator create_decoder, - SharedBuffer* full_data) { + SharedBuffer* full_buffer) { + const Vector<char> full_data = full_buffer->Copy(); Vector<unsigned> baseline_hashes; - CreateDecodingBaseline(create_decoder, full_data, &baseline_hashes); + CreateDecodingBaseline(create_decoder, full_buffer, &baseline_hashes); size_t frame_count = baseline_hashes.size(); std::unique_ptr<ImageDecoder> decoder = create_decoder(); // Let frame 0 be partially decoded. RefPtr<SharedBuffer> data = SharedBuffer::Create(); - const char* source = full_data->Data(); + const char* source = full_data.data(); do { data->Append(source++, 1u); decoder->SetData(data.Get(), false); @@ -344,7 +351,7 @@ ImageFrame::kFrameEmpty); // Skip to the last frame and clear. - decoder->SetData(full_data, true); + decoder->SetData(full_buffer, true); EXPECT_EQ(frame_count, decoder->FrameCount()); ImageFrame* last_frame = decoder->FrameBufferAtIndex(frame_count - 1); EXPECT_EQ(baseline_hashes[frame_count - 1], HashBitmap(last_frame->Bitmap()));
diff --git a/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoderTest.cpp b/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoderTest.cpp index 79ced47..05e56e9f 100644 --- a/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoderTest.cpp +++ b/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoderTest.cpp
@@ -124,15 +124,15 @@ TEST(GIFImageDecoderTest, parseByteByByte) { std::unique_ptr<ImageDecoder> decoder = CreateDecoder(); - RefPtr<SharedBuffer> data = ReadFile(kLayoutTestResourcesDir, "animated.gif"); - ASSERT_TRUE(data.Get()); + const Vector<char> data = + ReadFile(kLayoutTestResourcesDir, "animated.gif")->Copy(); size_t frame_count = 0; // Pass data to decoder byte by byte. - for (size_t length = 1; length <= data->size(); ++length) { - RefPtr<SharedBuffer> temp_data = SharedBuffer::Create(data->Data(), length); - decoder->SetData(temp_data.Get(), length == data->size()); + for (size_t length = 1; length <= data.size(); ++length) { + RefPtr<SharedBuffer> temp_data = SharedBuffer::Create(data.data(), length); + decoder->SetData(temp_data.Get(), length == data.size()); EXPECT_LE(frame_count, decoder->FrameCount()); frame_count = decoder->FrameCount(); @@ -171,12 +171,12 @@ TEST(GIFImageDecoderTest, allDataReceivedTruncation) { std::unique_ptr<ImageDecoder> decoder = CreateDecoder(); - RefPtr<SharedBuffer> data = ReadFile(kLayoutTestResourcesDir, "animated.gif"); - ASSERT_TRUE(data.Get()); + const Vector<char> data = + ReadFile(kLayoutTestResourcesDir, "animated.gif")->Copy(); - ASSERT_GE(data->size(), 10u); + ASSERT_GE(data.size(), 10u); RefPtr<SharedBuffer> temp_data = - SharedBuffer::Create(data->Data(), data->size() - 10); + SharedBuffer::Create(data.data(), data.size() - 10); decoder->SetData(temp_data.Get(), true); EXPECT_EQ(2u, decoder->FrameCount()); @@ -205,12 +205,14 @@ TEST(GIFImageDecoderTest, frameIsCompleteLoading) { std::unique_ptr<ImageDecoder> decoder = CreateDecoder(); - RefPtr<SharedBuffer> data = ReadFile(kLayoutTestResourcesDir, "animated.gif"); - ASSERT_TRUE(data.Get()); + RefPtr<SharedBuffer> data_buffer = + ReadFile(kLayoutTestResourcesDir, "animated.gif"); + ASSERT_TRUE(data_buffer.Get()); + const Vector<char> data = data_buffer->Copy(); - ASSERT_GE(data->size(), 10u); + ASSERT_GE(data.size(), 10u); RefPtr<SharedBuffer> temp_data = - SharedBuffer::Create(data->Data(), data->size() - 10); + SharedBuffer::Create(data.data(), data.size() - 10); decoder->SetData(temp_data.Get(), false); EXPECT_EQ(2u, decoder->FrameCount()); @@ -218,7 +220,7 @@ EXPECT_TRUE(decoder->FrameIsCompleteAtIndex(0)); EXPECT_FALSE(decoder->FrameIsCompleteAtIndex(1)); - decoder->SetData(data.Get(), true); + decoder->SetData(data_buffer.Get(), true); EXPECT_EQ(2u, decoder->FrameCount()); EXPECT_TRUE(decoder->FrameIsCompleteAtIndex(0)); EXPECT_TRUE(decoder->FrameIsCompleteAtIndex(1)); @@ -327,18 +329,19 @@ } TEST(GIFImageDecoderTest, firstFrameHasGreaterSizeThanScreenSize) { - RefPtr<SharedBuffer> full_data = ReadFile( - kDecodersTestingDir, "first-frame-has-greater-size-than-screen-size.gif"); - ASSERT_TRUE(full_data.Get()); + const Vector<char> full_data = + ReadFile(kDecodersTestingDir, + "first-frame-has-greater-size-than-screen-size.gif") + ->Copy(); std::unique_ptr<ImageDecoder> decoder; IntSize frame_size; // Compute hashes when the file is truncated. - for (size_t i = 1; i <= full_data->size(); ++i) { + for (size_t i = 1; i <= full_data.size(); ++i) { decoder = CreateDecoder(); - RefPtr<SharedBuffer> data = SharedBuffer::Create(full_data->Data(), i); - decoder->SetData(data.Get(), i == full_data->size()); + RefPtr<SharedBuffer> data = SharedBuffer::Create(full_data.data(), i); + decoder->SetData(data.Get(), i == full_data.size()); if (decoder->IsSizeAvailable() && !frame_size.Width() && !frame_size.Height()) { @@ -357,16 +360,18 @@ } TEST(GIFImageDecoderTest, bitmapAlphaType) { - RefPtr<SharedBuffer> full_data = ReadFile(kDecodersTestingDir, "radient.gif"); - ASSERT_TRUE(full_data.Get()); + RefPtr<SharedBuffer> full_data_buffer = + ReadFile(kDecodersTestingDir, "radient.gif"); + ASSERT_TRUE(full_data_buffer.Get()); + const Vector<char> full_data = full_data_buffer->Copy(); // Empirically chosen truncation size: // a) large enough to produce a partial frame && // b) small enough to not fully decode the frame const size_t kTruncateSize = 800; - ASSERT_TRUE(kTruncateSize < full_data->size()); + ASSERT_TRUE(kTruncateSize < full_data.size()); RefPtr<SharedBuffer> partial_data = - SharedBuffer::Create(full_data->Data(), kTruncateSize); + SharedBuffer::Create(full_data.data(), kTruncateSize); std::unique_ptr<ImageDecoder> premul_decoder = WTF::WrapUnique( new GIFImageDecoder(ImageDecoder::kAlphaPremultiplied, @@ -393,9 +398,9 @@ EXPECT_EQ(unpremul_frame->Bitmap().alphaType(), kUnpremul_SkAlphaType); // Fully decoded frame => the frame alpha type is known (opaque). - premul_decoder->SetData(full_data.Get(), true); + premul_decoder->SetData(full_data_buffer.Get(), true); ASSERT_TRUE(premul_decoder->FrameCount()); - unpremul_decoder->SetData(full_data.Get(), true); + unpremul_decoder->SetData(full_data_buffer.Get(), true); ASSERT_TRUE(unpremul_decoder->FrameCount()); premul_frame = premul_decoder->FrameBufferAtIndex(0); EXPECT_TRUE(premul_frame &&
diff --git a/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoderTest.cpp b/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoderTest.cpp index 37bd35d..f5b0b4a 100644 --- a/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoderTest.cpp +++ b/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoderTest.cpp
@@ -22,12 +22,12 @@ } TEST(ICOImageDecoderTests, trunctedIco) { - RefPtr<SharedBuffer> data = - ReadFile("/LayoutTests/images/resources/png-in-ico.ico"); - ASSERT_FALSE(data->IsEmpty()); + const Vector<char> data = + ReadFile("/LayoutTests/images/resources/png-in-ico.ico")->Copy(); + ASSERT_FALSE(data.IsEmpty()); RefPtr<SharedBuffer> truncated_data = - SharedBuffer::Create(data->Data(), data->size() / 2); + SharedBuffer::Create(data.data(), data.size() / 2); auto decoder = CreateDecoder(); decoder->SetData(truncated_data.Get(), false); @@ -40,19 +40,19 @@ } TEST(ICOImageDecoderTests, errorInPngInIco) { - RefPtr<SharedBuffer> data = - ReadFile("/LayoutTests/images/resources/png-in-ico.ico"); - ASSERT_FALSE(data->IsEmpty()); + const Vector<char> data = + ReadFile("/LayoutTests/images/resources/png-in-ico.ico")->Copy(); + ASSERT_FALSE(data.IsEmpty()); // Modify the file to have a broken CRC in IHDR. constexpr size_t kCrcOffset = 22 + 29; constexpr size_t kCrcSize = 4; RefPtr<SharedBuffer> modified_data = - SharedBuffer::Create(data->Data(), kCrcOffset); + SharedBuffer::Create(data.data(), kCrcOffset); Vector<char> bad_crc(kCrcSize, 0); modified_data->Append(bad_crc); - modified_data->Append(data->Data() + kCrcOffset + kCrcSize, - data->size() - kCrcOffset - kCrcSize); + modified_data->Append(data.data() + kCrcOffset + kCrcSize, + data.size() - kCrcOffset - kCrcSize); auto decoder = CreateDecoder(); decoder->SetData(modified_data.Get(), true);
diff --git a/third_party/WebKit/Source/platform/image-decoders/webp/WEBPImageDecoderTest.cpp b/third_party/WebKit/Source/platform/image-decoders/webp/WEBPImageDecoderTest.cpp index db4ea63..4ba3df7 100644 --- a/third_party/WebKit/Source/platform/image-decoders/webp/WEBPImageDecoderTest.cpp +++ b/third_party/WebKit/Source/platform/image-decoders/webp/WEBPImageDecoderTest.cpp
@@ -284,11 +284,11 @@ TEST(AnimatedWebPTests, truncatedInBetweenFrame) { std::unique_ptr<ImageDecoder> decoder = CreateDecoder(); - RefPtr<SharedBuffer> full_data = - ReadFile("/LayoutTests/images/resources/invalid-animated-webp4.webp"); - ASSERT_TRUE(full_data.Get()); + const Vector<char> full_data = + ReadFile("/LayoutTests/images/resources/invalid-animated-webp4.webp") + ->Copy(); RefPtr<SharedBuffer> data = - SharedBuffer::Create(full_data->Data(), full_data->size() - 1); + SharedBuffer::Create(full_data.data(), full_data.size() - 1); decoder->SetData(data.Get(), false); ImageFrame* frame = decoder->FrameBufferAtIndex(1); @@ -305,15 +305,16 @@ TEST(AnimatedWebPTests, reproCrash) { std::unique_ptr<ImageDecoder> decoder = CreateDecoder(); - RefPtr<SharedBuffer> full_data = + RefPtr<SharedBuffer> full_data_buffer = ReadFile("/LayoutTests/images/resources/invalid_vp8_vp8x.webp"); - ASSERT_TRUE(full_data.Get()); + ASSERT_TRUE(full_data_buffer.Get()); + const Vector<char> full_data = full_data_buffer->Copy(); // Parse partial data up to which error in bitstream is not detected. const size_t kPartialSize = 32768; - ASSERT_GT(full_data->size(), kPartialSize); + ASSERT_GT(full_data.size(), kPartialSize); RefPtr<SharedBuffer> data = - SharedBuffer::Create(full_data->Data(), kPartialSize); + SharedBuffer::Create(full_data.data(), kPartialSize); decoder->SetData(data.Get(), false); EXPECT_EQ(1u, decoder->FrameCount()); ImageFrame* frame = decoder->FrameBufferAtIndex(0); @@ -322,7 +323,7 @@ EXPECT_FALSE(decoder->Failed()); // Parse full data now. The error in bitstream should now be detected. - decoder->SetData(full_data.Get(), true); + decoder->SetData(full_data_buffer.Get(), true); EXPECT_EQ(1u, decoder->FrameCount()); frame = decoder->FrameBufferAtIndex(0); ASSERT_TRUE(frame); @@ -339,13 +340,14 @@ TEST(AnimatedWebPTests, frameIsCompleteAndDuration) { std::unique_ptr<ImageDecoder> decoder = CreateDecoder(); - RefPtr<SharedBuffer> data = + RefPtr<SharedBuffer> data_buffer = ReadFile("/LayoutTests/images/resources/webp-animated.webp"); - ASSERT_TRUE(data.Get()); + ASSERT_TRUE(data_buffer.Get()); + const Vector<char> data = data_buffer->Copy(); - ASSERT_GE(data->size(), 10u); + ASSERT_GE(data.size(), 10u); RefPtr<SharedBuffer> temp_data = - SharedBuffer::Create(data->Data(), data->size() - 10); + SharedBuffer::Create(data.data(), data.size() - 10); decoder->SetData(temp_data.Get(), false); EXPECT_EQ(2u, decoder->FrameCount()); @@ -355,7 +357,7 @@ EXPECT_TRUE(decoder->FrameIsCompleteAtIndex(1)); EXPECT_EQ(500, decoder->FrameDurationAtIndex(1)); - decoder->SetData(data.Get(), true); + decoder->SetData(data_buffer.Get(), true); EXPECT_EQ(3u, decoder->FrameCount()); EXPECT_TRUE(decoder->FrameIsCompleteAtIndex(0)); EXPECT_EQ(1000, decoder->FrameDurationAtIndex(0));
diff --git a/third_party/WebKit/Source/platform/wtf/allocator/Partitions.cpp b/third_party/WebKit/Source/platform/wtf/allocator/Partitions.cpp index 9a8ea36d..b96cc6e 100644 --- a/third_party/WebKit/Source/platform/wtf/allocator/Partitions.cpp +++ b/third_party/WebKit/Source/platform/wtf/allocator/Partitions.cpp
@@ -70,13 +70,17 @@ return; PartitionPurgeMemoryGeneric(ArrayBufferPartition(), - base::PartitionPurgeDecommitEmptyPages); + base::PartitionPurgeDecommitEmptyPages | + base::PartitionPurgeDiscardUnusedSystemPages); PartitionPurgeMemoryGeneric(BufferPartition(), - base::PartitionPurgeDecommitEmptyPages); + base::PartitionPurgeDecommitEmptyPages | + base::PartitionPurgeDiscardUnusedSystemPages); PartitionPurgeMemoryGeneric(FastMallocPartition(), - base::PartitionPurgeDecommitEmptyPages); + base::PartitionPurgeDecommitEmptyPages | + base::PartitionPurgeDiscardUnusedSystemPages); PartitionPurgeMemory(LayoutPartition(), - base::PartitionPurgeDecommitEmptyPages); + base::PartitionPurgeDecommitEmptyPages | + base::PartitionPurgeDiscardUnusedSystemPages); } void Partitions::ReportMemoryUsageHistogram() {
diff --git a/third_party/WebKit/Source/web/tests/FrameSerializerTest.cpp b/third_party/WebKit/Source/web/tests/FrameSerializerTest.cpp index 664d45e..b287f03 100644 --- a/third_party/WebKit/Source/web/tests/FrameSerializerTest.cpp +++ b/third_party/WebKit/Source/web/tests/FrameSerializerTest.cpp
@@ -151,8 +151,10 @@ String GetSerializedData(const char* url, const char* mime_type = 0) { const SerializedResource* resource = GetResource(url, mime_type); - if (resource) - return String(resource->data->Data(), resource->data->size()); + if (resource) { + const Vector<char> data = resource->data->Copy(); + return String(data.data(), data.size()); + } return String(); }
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/git.py b/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/git.py index dd18b09..399a5ea 100644 --- a/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/git.py +++ b/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/git.py
@@ -256,11 +256,6 @@ def _status_regexp(self, expected_types): return '^(?P<status>[%s])\t(?P<filename>.+)$' % expected_types - @staticmethod - def supports_local_commits(): - # TODO(qyearsley): Remove this. - return True - def display_name(self): return 'git'
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/git_mock.py b/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/git_mock.py index 11faf9a..40da047 100644 --- a/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/git_mock.py +++ b/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/git_mock.py
@@ -48,9 +48,6 @@ def delete_branch(self, name): pass - def supports_local_commits(self): - return True - def exists(self, path): # TestRealMain.test_real_main (and several other rebaseline tests) are sensitive to this return value. # We should make those tests more robust, but for now we just return True always (since no test needs otherwise).
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/tool/webkit_patch.py b/third_party/WebKit/Tools/Scripts/webkitpy/tool/webkit_patch.py index 267ffed..b8c7881 100644 --- a/third_party/WebKit/Tools/Scripts/webkitpy/tool/webkit_patch.py +++ b/third_party/WebKit/Tools/Scripts/webkitpy/tool/webkit_patch.py
@@ -113,11 +113,6 @@ command.set_option_parser(option_parser) (options, args) = command.parse_args(args) - (should_execute, failure_reason) = self._should_execute_command(command) - if not should_execute: - _log.error(failure_reason) - return 0 # FIXME: Should this really be 0? - result = command.check_arguments_and_execute(options, args, self) return result @@ -148,22 +143,11 @@ for option in global_options: option_parser.add_option(option) - def _should_execute_command(self, command): - if command.requires_local_commits and not self.git().supports_local_commits(): - failure_reason = '%s requires local commits using %s in %s.' % ( - command.name, self.git().display_name(), self.git().checkout_root) - return (False, failure_reason) - return (True, None) - def name(self): return optparse.OptionParser().get_prog_name() def should_show_in_main_help(self, command): - if not command.show_in_main_help: - return False - if command.requires_local_commits: - return self.git().supports_local_commits() - return True + return command.show_in_main_help def command_by_name(self, command_name): for command in self.commands:
diff --git a/tools/mb/mb_config.pyl b/tools/mb/mb_config.pyl index 1238161..0204d8ed 100644 --- a/tools/mb/mb_config.pyl +++ b/tools/mb/mb_config.pyl
@@ -194,6 +194,9 @@ 'Libfuzzer Upload Linux UBSan': 'release_libfuzzer_ubsan', 'Libfuzzer Upload Mac ASan': 'release_libfuzzer_mac_asan', 'Linux ARM': 'release_bot_arm', + 'Linux ARM (dbg)': 'debug_bot_arm', + 'Linux ARM64': 'release_bot_arm64', + 'Linux ARM64 (dbg)': 'debug_bot_arm64', 'Linux Clang Analyzer': 'linux_chromium_analysis', 'Linux deterministic': 'release_bot', 'Linux deterministic (dbg)': 'debug_bot', @@ -517,7 +520,12 @@ 'chromeos_x86-generic_chromium_compile_only_ng': 'cros_chrome_sdk', 'closure_compilation': 'closure_compilation', 'fuchsia': 'release_trybot_fuchsia', + # TODO(kjellander): Remove linux_arm after tryserver restart. 'linux_arm': 'release_trybot_arm', + 'linux_arm_dbg': 'debug_trybot_arm', + 'linux_arm_rel': 'release_trybot_arm', + 'linux_arm64_dbg': 'debug_trybot_arm64', + 'linux_arm64_rel': 'release_trybot_arm64', 'linux_chromium_archive_rel_ng': 'release_bot', 'linux_chromium_asan_rel_ng': 'asan_lsan_release_trybot', 'linux_chromium_browser_side_navigation_rel': 'release_trybot', @@ -1181,6 +1189,14 @@ 'debug_bot', ], + 'debug_bot_arm': [ + 'debug_bot', 'arm', + ], + + 'debug_bot_arm64': [ + 'debug_bot', 'arm64', + ], + 'debug_bot_fuchsia': [ 'debug_bot', 'fuchsia', ], @@ -1203,6 +1219,14 @@ 'debug_trybot', ], + 'debug_trybot_arm': [ + 'debug_trybot', 'arm', + ], + + 'debug_trybot_arm64': [ + 'debug_trybot', 'arm64', + ], + 'debug_trybot_x86': [ 'debug_trybot', 'x86', ], @@ -1396,6 +1420,10 @@ 'release_bot', 'arm', ], + 'release_bot_arm64': [ + 'release_bot', 'arm64', + ], + 'release_bot_chrome_with_codecs': [ 'release_bot', 'chrome_with_codecs', ], @@ -1450,6 +1478,10 @@ 'release_trybot', 'arm', ], + 'release_trybot_arm64': [ + 'release_trybot', 'arm64', + ], + 'release_trybot_fuchsia': [ 'release_trybot', 'fuchsia', ],
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index 4d914829..f17facaf 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml
@@ -22789,6 +22789,7 @@ <int value="1062357243" label="remember-cert-error-decisions"/> <int value="1064288458" label="OfflineRecentPages:enabled"/> <int value="1067618884" label="enable-experimental-input-view-features"/> + <int value="1067990299" label="enable-ui-devtools"/> <int value="1070164693" label="MidiManagerDynamicInstantiation:disabled"/> <int value="1070300488" label="disable-webgl"/> <int value="1070449228" label="ContextualSuggestionsCarousel:enabled"/>
diff --git a/tools/perf/core/perf_data_generator.py b/tools/perf/core/perf_data_generator.py index 4dd75c7..49479bd7 100755 --- a/tools/perf/core/perf_data_generator.py +++ b/tools/perf/core/perf_data_generator.py
@@ -694,7 +694,6 @@ # Devices which are broken right now. Tests will not be scheduled on them. # Please add a comment with a bug for replacing the device. BLACKLISTED_DEVICES = [ - 'build47-b1--device4', # crbug.com/731168 ]
diff --git a/tools/perf/page_sets/image_decoding_cases.py b/tools/perf/page_sets/image_decoding_cases.py index dd1eb7e6..826858cc 100644 --- a/tools/perf/page_sets/image_decoding_cases.py +++ b/tools/perf/page_sets/image_decoding_cases.py
@@ -6,8 +6,9 @@ class ImageDecodingCasesPage(page_module.Page): - def __init__(self, url, page_set): - super(ImageDecodingCasesPage, self).__init__(url=url, page_set=page_set) + def __init__(self, url, page_set, name): + super(ImageDecodingCasesPage, self).__init__( + url=url, page_set=page_set, name=name) def RunPageInteractions(self, action_runner): with action_runner.CreateInteraction('DecodeImage'): @@ -18,11 +19,11 @@ """ A directed benchmark of accelerated jpeg image decoding performance """ def __init__(self): - super(ImageDecodingCasesPageSet, self).__init__() + super(ImageDecodingCasesPageSet, self).__init__(verify_names=True) urls_list = [ - 'file://image_decoding_cases/yuv_decoding.html' + ('file://image_decoding_cases/yuv_decoding.html', 'yuv_decoding.html') ] - for url in urls_list: - self.AddStory(ImageDecodingCasesPage(url, self)) + for url, name in urls_list: + self.AddStory(ImageDecodingCasesPage(url, self, name))
diff --git a/tools/perf/page_sets/key_desktop_move_cases.py b/tools/perf/page_sets/key_desktop_move_cases.py index a226a62..1cd6811 100644 --- a/tools/perf/page_sets/key_desktop_move_cases.py +++ b/tools/perf/page_sets/key_desktop_move_cases.py
@@ -9,6 +9,8 @@ class KeyDesktopMoveCasesPage(page_module.Page): def __init__(self, url, page_set, name='', credentials=None): + if name == '': + name = url super(KeyDesktopMoveCasesPage, self).__init__( url=url, page_set=page_set, name=name, credentials_path='data/credentials.json', @@ -116,7 +118,8 @@ def __init__(self): super(KeyDesktopMoveCasesPageSet, self).__init__( archive_data_file='data/key_desktop_move_cases.json', - cloud_storage_bucket=story.PARTNER_BUCKET) + cloud_storage_bucket=story.PARTNER_BUCKET, + verify_names=True) self.AddStory(GmailMouseScrollPage(self)) self.AddStory(GoogleMapsPage(self))
diff --git a/tools/perf/page_sets/key_mobile_sites_smooth.py b/tools/perf/page_sets/key_mobile_sites_smooth.py index 2ba7134..3d009c5 100644 --- a/tools/perf/page_sets/key_mobile_sites_smooth.py +++ b/tools/perf/page_sets/key_mobile_sites_smooth.py
@@ -25,6 +25,8 @@ def __init__(self, url, page_set, name='', tags=None, action_on_load_complete=False): + if name == '': + name = url super(KeyMobileSitesSmoothPage, self).__init__( url=url, page_set=page_set, name=name, credentials_path='data/credentials.json', tags=tags, @@ -141,7 +143,8 @@ def __init__(self): super(KeyMobileSitesSmoothPageSet, self).__init__( archive_data_file='data/key_mobile_sites_smooth.json', - cloud_storage_bucket=story.PARTNER_BUCKET) + cloud_storage_bucket=story.PARTNER_BUCKET, + verify_names=True) # Add pages with predefined classes that contain custom navigation logic.
diff --git a/tools/perf/page_sets/maps.py b/tools/perf/page_sets/maps.py index 9cd1897..cb112fb 100644 --- a/tools/perf/page_sets/maps.py +++ b/tools/perf/page_sets/maps.py
@@ -11,11 +11,13 @@ class MapsPage(page_module.Page): def __init__(self, page_set): + url = 'http://localhost:8000/performance.html' super(MapsPage, self).__init__( - url='http://localhost:8000/performance.html', + url=url, page_set=page_set, shared_page_state_class=( - webgl_supported_shared_state.WebGLSupportedSharedState)) + webgl_supported_shared_state.WebGLSupportedSharedState), + name=url) self.archive_data_file = 'data/maps.json' @property @@ -40,6 +42,7 @@ def __init__(self): super(MapsPageSet, self).__init__( archive_data_file='data/maps.json', - cloud_storage_bucket=story.PUBLIC_BUCKET) + cloud_storage_bucket=story.PUBLIC_BUCKET, + verify_names=True) self.AddStory(MapsPage(self))
diff --git a/tools/perf/page_sets/pathological_mobile_sites.py b/tools/perf/page_sets/pathological_mobile_sites.py index 9ccb7d6..bbef7db 100644 --- a/tools/perf/page_sets/pathological_mobile_sites.py +++ b/tools/perf/page_sets/pathological_mobile_sites.py
@@ -11,7 +11,8 @@ def __init__(self, url, page_set): super(PathologicalMobileSitesPage, self).__init__( url=url, page_set=page_set, credentials_path='data/credentials.json', - shared_page_state_class=shared_page_state.SharedMobilePageState) + shared_page_state_class=shared_page_state.SharedMobilePageState, + name=url) def RunPageInteractions(self, action_runner): with action_runner.CreateGestureInteraction('ScrollAction'): @@ -25,7 +26,8 @@ def __init__(self): super(PathologicalMobileSitesPageSet, self).__init__( archive_data_file='data/pathological_mobile_sites.json', - cloud_storage_bucket=story.PARTNER_BUCKET) + cloud_storage_bucket=story.PARTNER_BUCKET, + verify_names=True) sites = ['http://edition.cnn.com', 'http://m.espn.go.com/nhl/rankings',
diff --git a/tools/perf/page_sets/top_25_smooth.py b/tools/perf/page_sets/top_25_smooth.py index f327ff1..ad73095 100644 --- a/tools/perf/page_sets/top_25_smooth.py +++ b/tools/perf/page_sets/top_25_smooth.py
@@ -26,6 +26,8 @@ class TopSmoothPage(page_module.Page): def __init__(self, url, page_set, name='', credentials=None): + if name == '': + name = url super(TopSmoothPage, self).__init__( url=url, page_set=page_set, name=name, shared_page_state_class=shared_page_state.SharedDesktopPageState, @@ -112,7 +114,8 @@ def __init__(self, techcrunch=True): super(Top25SmoothPageSet, self).__init__( archive_data_file='data/top_25_smooth.json', - cloud_storage_bucket=story.PARTNER_BUCKET) + cloud_storage_bucket=story.PARTNER_BUCKET, + verify_names=True) desktop_state_class = shared_page_state.SharedDesktopPageState
diff --git a/tools/perf/page_sets/tough_ad_cases.py b/tools/perf/page_sets/tough_ad_cases.py index cf96357..f1748d01 100644 --- a/tools/perf/page_sets/tough_ad_cases.py +++ b/tools/perf/page_sets/tough_ad_cases.py
@@ -13,7 +13,8 @@ def __init__(self, url, page_set): super(SwiffyPage, self).__init__(url=url, page_set=page_set, - make_javascript_deterministic=False) + make_javascript_deterministic=False, + name=url) def RunNavigateSteps(self, action_runner): super(SwiffyPage, self).RunNavigateSteps(action_runner) @@ -42,13 +43,17 @@ y_scroll_distance_multiplier=0.5, scroll=False, wait_for_interactive_or_better=False): + name = url + if not name.startswith('http'): + name = url.split('/')[-1] super(AdPage, self).__init__( url=url, page_set=page_set, make_javascript_deterministic=make_javascript_deterministic, shared_page_state_class=( repeatable_synthesize_scroll_gesture_shared_state.\ - RepeatableSynthesizeScrollGestureSharedState)) + RepeatableSynthesizeScrollGestureSharedState), + name=name) self._y_scroll_distance_multiplier = y_scroll_distance_multiplier self._scroll = scroll self._wait_for_interactive_or_better = wait_for_interactive_or_better @@ -114,7 +119,8 @@ def __init__(self): super(SyntheticToughAdCasesPageSet, self).__init__( archive_data_file='data/tough_ad_cases.json', - cloud_storage_bucket=story.INTERNAL_BUCKET) + cloud_storage_bucket=story.INTERNAL_BUCKET, + verify_names=True) base_url = 'http://localhost:8000' @@ -144,7 +150,8 @@ def __init__(self): super(SyntheticToughWebglAdCasesPageSet, self).__init__( archive_data_file='data/tough_ad_cases.json', - cloud_storage_bucket=story.INTERNAL_BUCKET) + cloud_storage_bucket=story.INTERNAL_BUCKET, + verify_names=True) base_url = 'http://localhost:8000' @@ -174,7 +181,8 @@ def __init__(self, scroll=False): super(ToughAdCasesPageSet, self).__init__( archive_data_file='data/tough_ad_cases.json', - cloud_storage_bucket=story.INTERNAL_BUCKET) + cloud_storage_bucket=story.INTERNAL_BUCKET, + verify_names=True) self.AddStory(AdPage('file://tough_ad_cases/' 'swiffy_collection.html', self, make_javascript_deterministic=False,
diff --git a/tools/perf/page_sets/tough_animation_cases.py b/tools/perf/page_sets/tough_animation_cases.py index 6e831b2..2adafbaf 100644 --- a/tools/perf/page_sets/tough_animation_cases.py +++ b/tools/perf/page_sets/tough_animation_cases.py
@@ -8,7 +8,8 @@ class ToughAnimationCasesPage(page_module.Page): def __init__(self, url, page_set, need_measurement_ready): - super(ToughAnimationCasesPage, self).__init__(url=url, page_set=page_set) + super(ToughAnimationCasesPage, self).__init__(url=url, page_set=page_set, + name=url.split('/')[-1]) self.archive_data_file = 'data/tough_animation_cases.json' self._need_measurement_ready = need_measurement_ready @@ -30,7 +31,8 @@ def __init__(self): super(ToughAnimationCasesPageSet, self).__init__( archive_data_file='data/tough_animation_cases.json', - cloud_storage_bucket=story.PARTNER_BUCKET) + cloud_storage_bucket=story.PARTNER_BUCKET, + verify_names=True) urls_list_one = [ # Why: Tests the balls animation implemented with SVG animations.
diff --git a/tools/perf/page_sets/tough_canvas_cases.py b/tools/perf/page_sets/tough_canvas_cases.py index 28ce0bb..d9b50b0 100644 --- a/tools/perf/page_sets/tough_canvas_cases.py +++ b/tools/perf/page_sets/tough_canvas_cases.py
@@ -8,7 +8,11 @@ class ToughCanvasCasesPage(page_module.Page): def __init__(self, url, page_set): - super(ToughCanvasCasesPage, self).__init__(url=url, page_set=page_set) + name = url + if not url.startswith('http'): + name = url[7:] + super(ToughCanvasCasesPage, self).__init__(url=url, page_set=page_set, + name=name) self.archive_data_file = 'data/tough_canvas_cases.json' def RunNavigateSteps(self, action_runner): @@ -39,7 +43,8 @@ def __init__(self): super(ToughCanvasCasesPageSet, self).__init__( archive_data_file='data/tough_canvas_cases.json', - cloud_storage_bucket=story.PARTNER_BUCKET) + cloud_storage_bucket=story.PARTNER_BUCKET, + verify_names=True) # Crashes on Galaxy Nexus. crbug.com/314131 # self.AddStory(MicrosofFirefliesPage(self))
diff --git a/tools/perf/page_sets/tough_filters_cases.py b/tools/perf/page_sets/tough_filters_cases.py index 36b41276..7aa88c8 100644 --- a/tools/perf/page_sets/tough_filters_cases.py +++ b/tools/perf/page_sets/tough_filters_cases.py
@@ -7,9 +7,7 @@ class ToughFiltersCasesPage(page_module.Page): - def __init__(self, url, page_set): - # Truncate the url to be the name, see crbug.com/662941 - name = url[:100] + def __init__(self, url, page_set, name): super(ToughFiltersCasesPage, self).__init__( url=url, page_set=page_set, name=name) @@ -36,16 +34,23 @@ def __init__(self): super(ToughFiltersCasesPageSet, self).__init__( archive_data_file='data/tough_filters_cases.json', - cloud_storage_bucket=story.PARTNER_BUCKET) + cloud_storage_bucket=story.PARTNER_BUCKET, + verify_names=True) urls_list = [ - 'http://rawgit.com/WebKit/webkit/master/PerformanceTests/Animometer/developer.html?test-interval=20&display=minimal&controller=fixed&frame-rate=50&kalman-process-error=1&kalman-measurement-error=4&time-measurement=performance&suite-name=Animometer&test-name=Focus&complexity=100', # pylint: disable=line-too-long - 'http://letmespellitoutforyou.com/samples/svg/filter_terrain.svg', - 'http://static.bobdo.net/Analog_Clock.svg', + ('http://rawgit.com/WebKit/webkit/master/PerformanceTests/Animometer/developer.html?test-interval=20&display=minimal&controller=fixed&frame-rate=50&kalman-process-error=1&kalman-measurement-error=4&time-measurement=performance&suite-name=Animometer&test-name=Focus&complexity=100', # pylint: disable=line-too-long + 'MotionMark Focus'), + ('http://letmespellitoutforyou.com/samples/svg/filter_terrain.svg', + 'Filter Terrain SVG'), + ('http://static.bobdo.net/Analog_Clock.svg', + 'Analog Clock SVG'), ] - for url in urls_list: - self.AddStory(ToughFiltersCasesPage(url, self)) + for url, name in urls_list: + self.AddStory(ToughFiltersCasesPage(url, self, name)) - self.AddStory(PirateMarkPage( - 'http://web.archive.org/web/20150502135732/http://ie.microsoft.com/testdrive/Performance/Pirates/Default.html', self)) # pylint: disable=line-too-long + pirate_url = ('http://web.archive.org/web/20150502135732/' + 'http://ie.microsoft.com/testdrive/Performance/' + 'Pirates/Default.html') + name = 'IE PirateMark' + self.AddStory(PirateMarkPage(pirate_url, self, name=name))
diff --git a/tools/perf/page_sets/tough_image_decode_cases.py b/tools/perf/page_sets/tough_image_decode_cases.py index fd20064..a140fdf 100644 --- a/tools/perf/page_sets/tough_image_decode_cases.py +++ b/tools/perf/page_sets/tough_image_decode_cases.py
@@ -31,7 +31,8 @@ def __init__(self): super(ToughImageDecodeCasesPageSet, self).__init__( archive_data_file='data/tough_image_decode_cases.json', - cloud_storage_bucket=story.PUBLIC_BUCKET) + cloud_storage_bucket=story.PUBLIC_BUCKET, + verify_names=True) page_name_list = [ 'http://localhost:9000/cats-unscaled.html',
diff --git a/tools/perf/page_sets/tough_path_rendering_cases.py b/tools/perf/page_sets/tough_path_rendering_cases.py index b8b4dd91..c99bb215 100644 --- a/tools/perf/page_sets/tough_path_rendering_cases.py +++ b/tools/perf/page_sets/tough_path_rendering_cases.py
@@ -28,7 +28,8 @@ def __init__(self): super(ToughPathRenderingCasesPageSet, self).__init__( archive_data_file='data/tough_path_rendering_cases.json', - cloud_storage_bucket=story.PARTNER_BUCKET) + cloud_storage_bucket=story.PARTNER_BUCKET, + verify_names=True) page_list = [ ('GUIMark Vector Chart Test', @@ -45,5 +46,7 @@ # Chalkboard content linked from # http://ie.microsoft.com/testdrive/Performance/Chalkboard/. - self.AddStory(ChalkboardPage( - 'https://testdrive-archive.azurewebsites.net/performance/chalkboard/', self)) + chalkboard_url = ('https://testdrive-archive.azurewebsites.net' + '/performance/chalkboard/') + name = 'IE Chalkboard' + self.AddStory(ChalkboardPage(chalkboard_url, self, name=name))
diff --git a/tools/perf/page_sets/tough_pinch_zoom_cases.py b/tools/perf/page_sets/tough_pinch_zoom_cases.py index 2aa0151..371870e2 100644 --- a/tools/perf/page_sets/tough_pinch_zoom_cases.py +++ b/tools/perf/page_sets/tough_pinch_zoom_cases.py
@@ -9,6 +9,8 @@ class ToughPinchZoomCasesPage(page_module.Page): def __init__(self, url, page_set, name=''): + if name == '': + name = url super(ToughPinchZoomCasesPage, self).__init__( url=url, page_set=page_set, name=name, shared_page_state_class=shared_page_state.SharedDesktopPageState, @@ -216,7 +218,8 @@ def __init__(self, target_scale_factor): super(ToughPinchZoomCasesPageSet, self).__init__( archive_data_file='data/tough_pinch_zoom_cases.json', - cloud_storage_bucket=story.PARTNER_BUCKET) + cloud_storage_bucket=story.PARTNER_BUCKET, + verify_names=True) self.target_scale_factor = target_scale_factor
diff --git a/tools/perf/page_sets/tough_texture_upload_cases.py b/tools/perf/page_sets/tough_texture_upload_cases.py index 5fba88b..9f6fe47 100644 --- a/tools/perf/page_sets/tough_texture_upload_cases.py +++ b/tools/perf/page_sets/tough_texture_upload_cases.py
@@ -12,7 +12,8 @@ ToughTextureUploadCasesPage, self).__init__( url=url, - page_set=page_set) + page_set=page_set, + name=url.split('/')[-1]) def RunPageInteractions(self, action_runner): with action_runner.CreateInteraction('Animation'): @@ -26,7 +27,7 @@ """ def __init__(self): - super(ToughTextureUploadCasesPageSet, self).__init__() + super(ToughTextureUploadCasesPageSet, self).__init__(verify_names=True) urls_list = [ 'file://tough_texture_upload_cases/background_color_animation.html',
diff --git a/tools/perf/page_sets/tough_webgl_cases.py b/tools/perf/page_sets/tough_webgl_cases.py index 49a0be2c..cdbeacd 100644 --- a/tools/perf/page_sets/tough_webgl_cases.py +++ b/tools/perf/page_sets/tough_webgl_cases.py
@@ -15,7 +15,8 @@ url=url, page_set=page_set, shared_page_state_class=( webgl_supported_shared_state.WebGLSupportedSharedState), - make_javascript_deterministic=False) + make_javascript_deterministic=False, + name=url) self.archive_data_file = 'data/tough_webgl_cases.json' @@ -45,7 +46,8 @@ def __init__(self): super(ToughWebglCasesPageSet, self).__init__( archive_data_file='data/tough_webgl_cases.json', - cloud_storage_bucket=story.PUBLIC_BUCKET) + cloud_storage_bucket=story.PUBLIC_BUCKET, + verify_names=True) urls_list = [ # pylint: disable=line-too-long
diff --git a/ui/aura/local/window_port_local.cc b/ui/aura/local/window_port_local.cc index 2739a87..8b2ceac 100644 --- a/ui/aura/local/window_port_local.cc +++ b/ui/aura/local/window_port_local.cc
@@ -129,11 +129,6 @@ const gfx::Size& surface_size) { DCHECK_EQ(surface_id.frame_sink_id(), frame_sink_id_); local_surface_id_ = surface_id.local_surface_id(); - // The bounds must be updated before switching to the new surface, because - // the layer may be mirrored, in which case a surface change causes the - // mirror layer to update its surface using the latest bounds. - window_->layer()->SetBounds( - gfx::Rect(window_->layer()->bounds().origin(), surface_size)); cc::SurfaceInfo surface_info(surface_id, 1.0f, surface_size); scoped_refptr<cc::SurfaceReferenceFactory> reference_factory = aura::Env::GetInstance()
diff --git a/ui/aura/mus/window_mus.h b/ui/aura/mus/window_mus.h index 2f31f33..7087bf54 100644 --- a/ui/aura/mus/window_mus.h +++ b/ui/aura/mus/window_mus.h
@@ -90,7 +90,6 @@ const cc::FrameSinkId& frame_sink_id) = 0; virtual const cc::LocalSurfaceId& GetOrAllocateLocalSurfaceId( const gfx::Size& new_size) = 0; - virtual void SetPrimarySurfaceInfo(const cc::SurfaceInfo& surface_info) = 0; virtual void SetFallbackSurfaceInfo(const cc::SurfaceInfo& surface_info) = 0; // The window was deleted on the server side. DestroyFromServer() should // result in deleting |this|. @@ -128,6 +127,8 @@ virtual void NotifyEmbeddedAppDisconnected() = 0; + virtual bool HasLocalCompositorFrameSink() = 0; + private: // Just for set_server_id(), which other places should not call. friend class WindowTreeClient;
diff --git a/ui/aura/mus/window_port_mus.cc b/ui/aura/mus/window_port_mus.cc index f47ba8a4d..03acb08f 100644 --- a/ui/aura/mus/window_port_mus.cc +++ b/ui/aura/mus/window_port_mus.cc
@@ -8,6 +8,7 @@ #include "components/viz/client/local_surface_id_provider.h" #include "ui/aura/client/aura_constants.h" #include "ui/aura/client/transient_window_client.h" +#include "ui/aura/env.h" #include "ui/aura/mus/client_surface_embedder.h" #include "ui/aura/mus/property_converter.h" #include "ui/aura/mus/window_tree_client.h" @@ -95,7 +96,7 @@ window_tree_client_->Embed(window_, std::move(client), flags, callback); } -std::unique_ptr<cc::CompositorFrameSink> +std::unique_ptr<viz::ClientCompositorFrameSink> WindowPortMus::RequestCompositorFrameSink( scoped_refptr<cc::ContextProvider> context_provider, gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager) { @@ -115,7 +116,7 @@ enable_surface_synchronization); window_tree_client_->AttachCompositorFrameSink( server_id(), std::move(sink_request), std::move(client)); - return std::move(compositor_frame_sink); + return compositor_frame_sink; } WindowPortMus::ServerChangeIdType WindowPortMus::ScheduleChange( @@ -306,18 +307,25 @@ if (frame_sink_id_.is_valid()) UpdatePrimarySurfaceInfo(); - return local_surface_id_; -} + if (local_compositor_frame_sink_) + local_compositor_frame_sink_->SetLocalSurfaceId(local_surface_id_); -void WindowPortMus::SetPrimarySurfaceInfo(const cc::SurfaceInfo& surface_info) { - primary_surface_info_ = surface_info; - UpdateClientSurfaceEmbedder(); - if (window_->delegate()) - window_->delegate()->OnWindowSurfaceChanged(surface_info); + return local_surface_id_; } void WindowPortMus::SetFallbackSurfaceInfo( const cc::SurfaceInfo& surface_info) { + if (!frame_sink_id_.is_valid()) { + // |primary_surface_info_| shold not be valid, since we didn't know the + // |frame_sink_id_|. + DCHECK(!primary_surface_info_.is_valid()); + frame_sink_id_ = surface_info.id().frame_sink_id(); + UpdatePrimarySurfaceInfo(); + } + + // The frame sink id should never be changed. + DCHECK_EQ(surface_info.id().frame_sink_id(), frame_sink_id_); + fallback_surface_info_ = surface_info; UpdateClientSurfaceEmbedder(); } @@ -426,6 +434,10 @@ observer.OnEmbeddedAppDisconnected(window_); } +bool WindowPortMus::HasLocalCompositorFrameSink() { + return !!local_compositor_frame_sink_; +} + void WindowPortMus::OnPreInit(Window* window) { window_ = window; window_tree_client_->OnWindowMusCreated(this); @@ -521,38 +533,46 @@ std::unique_ptr<cc::CompositorFrameSink> WindowPortMus::CreateCompositorFrameSink() { - // TODO(penghuang): Implement it for Mus. - return nullptr; + DCHECK_EQ(window_mus_type(), WindowMusType::LOCAL); + DCHECK(!local_compositor_frame_sink_); + auto frame_sink = RequestCompositorFrameSink( + nullptr, + aura::Env::GetInstance()->context_factory()->GetGpuMemoryBufferManager()); + local_compositor_frame_sink_ = frame_sink->GetWeakPtr(); + return frame_sink; } cc::SurfaceId WindowPortMus::GetSurfaceId() const { - // TODO(penghuang): Implement it for Mus. + // This is only used by WindowPortLocal in unit tests. return cc::SurfaceId(); } void WindowPortMus::UpdatePrimarySurfaceInfo() { - bool embeds_surface = - window_mus_type() == WindowMusType::TOP_LEVEL_IN_WM || - window_mus_type() == WindowMusType::EMBED_IN_OWNER || - window_mus_type() == WindowMusType::DISPLAY_MANUALLY_CREATED; - if (!embeds_surface) + if (window_mus_type() != WindowMusType::TOP_LEVEL_IN_WM && + window_mus_type() != WindowMusType::EMBED_IN_OWNER && + window_mus_type() != WindowMusType::DISPLAY_MANUALLY_CREATED && + window_mus_type() != WindowMusType::LOCAL) { return; + } if (!frame_sink_id_.is_valid() || !local_surface_id_.is_valid()) return; - SetPrimarySurfaceInfo(cc::SurfaceInfo( + primary_surface_info_ = cc::SurfaceInfo( cc::SurfaceId(frame_sink_id_, local_surface_id_), - ScaleFactorForDisplay(window_), last_surface_size_in_pixels_)); + ScaleFactorForDisplay(window_), last_surface_size_in_pixels_); + UpdateClientSurfaceEmbedder(); + if (window_->delegate()) + window_->delegate()->OnWindowSurfaceChanged(primary_surface_info_); } void WindowPortMus::UpdateClientSurfaceEmbedder() { - bool embeds_surface = - window_mus_type() == WindowMusType::TOP_LEVEL_IN_WM || - window_mus_type() == WindowMusType::EMBED_IN_OWNER || - window_mus_type() == WindowMusType::DISPLAY_MANUALLY_CREATED; - if (!embeds_surface) + if (window_mus_type() != WindowMusType::TOP_LEVEL_IN_WM && + window_mus_type() != WindowMusType::EMBED_IN_OWNER && + window_mus_type() != WindowMusType::DISPLAY_MANUALLY_CREATED && + window_mus_type() != WindowMusType::LOCAL) { return; + } if (!client_surface_embedder_) { client_surface_embedder_ = base::MakeUnique<ClientSurfaceEmbedder>(
diff --git a/ui/aura/mus/window_port_mus.h b/ui/aura/mus/window_port_mus.h index c72fb58..aa18c57c 100644 --- a/ui/aura/mus/window_port_mus.h +++ b/ui/aura/mus/window_port_mus.h
@@ -24,6 +24,10 @@ #include "ui/gfx/geometry/rect.h" #include "ui/platform_window/mojo/text_input_state.mojom.h" +namespace viz { +class ClientCompositorFrameSink; +} + namespace aura { class ClientSurfaceEmbedder; @@ -76,7 +80,7 @@ uint32_t flags, const ui::mojom::WindowTree::EmbedCallback& callback); - std::unique_ptr<cc::CompositorFrameSink> RequestCompositorFrameSink( + std::unique_ptr<viz::ClientCompositorFrameSink> RequestCompositorFrameSink( scoped_refptr<cc::ContextProvider> context_provider, gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager); @@ -223,7 +227,6 @@ void SetFrameSinkIdFromServer(const cc::FrameSinkId& frame_sink_id) override; const cc::LocalSurfaceId& GetOrAllocateLocalSurfaceId( const gfx::Size& surface_size_in_pixels) override; - void SetPrimarySurfaceInfo(const cc::SurfaceInfo& surface_info) override; void SetFallbackSurfaceInfo(const cc::SurfaceInfo& surface_info) override; void DestroyFromServer() override; void AddTransientChildFromServer(WindowMus* child) override; @@ -239,6 +242,7 @@ void PrepareForTransientRestack(WindowMus* window) override; void OnTransientRestackDone(WindowMus* window) override; void NotifyEmbeddedAppDisconnected() override; + bool HasLocalCompositorFrameSink() override; // WindowPort: void OnPreInit(Window* window) override; @@ -285,6 +289,11 @@ ui::CursorData cursor_; + // When a frame sink is created + // for a local aura::Window, we need keep a weak ptr of it, so we can update + // the local surface id when necessary. + base::WeakPtr<viz::ClientCompositorFrameSink> local_compositor_frame_sink_; + DISALLOW_COPY_AND_ASSIGN(WindowPortMus); };
diff --git a/ui/aura/mus/window_tree_client.cc b/ui/aura/mus/window_tree_client.cc index cc3857f..e5aa0bd45 100644 --- a/ui/aura/mus/window_tree_client.cc +++ b/ui/aura/mus/window_tree_client.cc
@@ -699,7 +699,8 @@ base::Optional<cc::LocalSurfaceId> local_surface_id; if (window->window_mus_type() == WindowMusType::TOP_LEVEL_IN_WM || window->window_mus_type() == WindowMusType::EMBED_IN_OWNER || - window->window_mus_type() == WindowMusType::DISPLAY_MANUALLY_CREATED) { + window->window_mus_type() == WindowMusType::DISPLAY_MANUALLY_CREATED || + window->HasLocalCompositorFrameSink()) { local_surface_id = window->GetOrAllocateLocalSurfaceId(new_bounds.size()); synchronizing_with_child_on_next_frame_ = true; }
diff --git a/ui/base/ime/chromeos/ime_keyboard_x11_unittest.cc b/ui/base/ime/chromeos/ime_keyboard_x11_unittest.cc deleted file mode 100644 index 32357055..0000000 --- a/ui/base/ime/chromeos/ime_keyboard_x11_unittest.cc +++ /dev/null
@@ -1,157 +0,0 @@ -// Copyright 2014 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. - -#include "ui/base/ime/chromeos/ime_keyboard.h" - -#include <X11/Xlib.h> - -#include <algorithm> -#include <memory> -#include <set> -#include <string> - -#include "base/logging.h" -#include "base/test/scoped_task_environment.h" -#include "testing/gtest/include/gtest/gtest.h" -#include "ui/gfx/x/x11_types.h" - -namespace chromeos { -namespace input_method { - -namespace { - -class ImeKeyboardTest : public testing::Test, - public ImeKeyboard::Observer { - public: - ImeKeyboardTest() - : scoped_task_environment_( - base::test::ScopedTaskEnvironment::MainThreadType::UI) {} - - void SetUp() override { - xkey_.reset(ImeKeyboard::Create()); - xkey_->AddObserver(this); - caps_changed_ = false; - } - - void TearDown() override { - xkey_->RemoveObserver(this); - xkey_.reset(); - } - - void OnCapsLockChanged(bool enabled) override { - caps_changed_ = true; - } - - void VerifyCapsLockChanged(bool changed) { - EXPECT_EQ(changed, caps_changed_); - caps_changed_ = false; - } - - std::unique_ptr<ImeKeyboard> xkey_; - base::test::ScopedTaskEnvironment scoped_task_environment_; - bool caps_changed_; -}; - -// Returns true if X display is available. -bool DisplayAvailable() { - return gfx::GetXDisplay() != NULL; -} - -} // namespace - -// Tests CheckLayoutName() function. -TEST_F(ImeKeyboardTest, TestCheckLayoutName) { - // CheckLayoutName should not accept non-alphanumeric characters - // except "()-_". - EXPECT_FALSE(ImeKeyboard::CheckLayoutNameForTesting("us!")); - EXPECT_FALSE(ImeKeyboard::CheckLayoutNameForTesting("us; /bin/sh")); - EXPECT_TRUE(ImeKeyboard::CheckLayoutNameForTesting("ab-c_12")); - - // CheckLayoutName should not accept upper-case ascii characters. - EXPECT_FALSE(ImeKeyboard::CheckLayoutNameForTesting("US")); - - // CheckLayoutName should accept lower-case ascii characters. - for (int c = 'a'; c <= 'z'; ++c) { - EXPECT_TRUE(ImeKeyboard::CheckLayoutNameForTesting(std::string(3, c))); - } - - // CheckLayoutName should accept numbers. - for (int c = '0'; c <= '9'; ++c) { - EXPECT_TRUE(ImeKeyboard::CheckLayoutNameForTesting(std::string(3, c))); - } - - // CheckLayoutName should accept a layout with a variant name. - EXPECT_TRUE(ImeKeyboard::CheckLayoutNameForTesting("us(dvorak)")); - EXPECT_TRUE(ImeKeyboard::CheckLayoutNameForTesting("jp")); -} - -TEST_F(ImeKeyboardTest, TestSetCapsLockEnabled) { - if (!DisplayAvailable()) { - // Do not fail the test to allow developers to run unit_tests without an X - // server (e.g. via ssh). Note that both try bots and waterfall always have - // an X server for running browser_tests. - DVLOG(1) << "X server is not available. Skip the test."; - return; - } - const bool initial_lock_state = xkey_->CapsLockIsEnabled(); - xkey_->SetCapsLockEnabled(true); - EXPECT_TRUE(xkey_->CapsLockIsEnabled()); - - xkey_->SetCapsLockEnabled(false); - EXPECT_FALSE(xkey_->CapsLockIsEnabled()); - VerifyCapsLockChanged(true); - - xkey_->SetCapsLockEnabled(true); - EXPECT_TRUE(xkey_->CapsLockIsEnabled()); - VerifyCapsLockChanged(true); - - xkey_->SetCapsLockEnabled(false); - EXPECT_FALSE(xkey_->CapsLockIsEnabled()); - VerifyCapsLockChanged(true); - - xkey_->SetCapsLockEnabled(false); - EXPECT_FALSE(xkey_->CapsLockIsEnabled()); - VerifyCapsLockChanged(false); - - xkey_->SetCapsLockEnabled(initial_lock_state); -} - -TEST_F(ImeKeyboardTest, TestSetAutoRepeatEnabled) { - if (!DisplayAvailable()) { - DVLOG(1) << "X server is not available. Skip the test."; - return; - } - const bool state = xkey_->GetAutoRepeatEnabled(); - xkey_->SetAutoRepeatEnabled(!state); - EXPECT_EQ(!state, xkey_->GetAutoRepeatEnabled()); - // Restore the initial state. - xkey_->SetAutoRepeatEnabled(state); - EXPECT_EQ(state, xkey_->GetAutoRepeatEnabled()); -} - -TEST_F(ImeKeyboardTest, TestSetAutoRepeatRate) { - if (!DisplayAvailable()) { - DVLOG(1) << "X server is not available. Skip the test."; - return; - } - AutoRepeatRate rate; - EXPECT_TRUE(ImeKeyboard::GetAutoRepeatRateForTesting(&rate)); - - AutoRepeatRate tmp(rate); - ++tmp.initial_delay_in_ms; - ++tmp.repeat_interval_in_ms; - EXPECT_TRUE(xkey_->SetAutoRepeatRate(tmp)); - EXPECT_TRUE(ImeKeyboard::GetAutoRepeatRateForTesting(&tmp)); - EXPECT_EQ(rate.initial_delay_in_ms + 1, tmp.initial_delay_in_ms); - EXPECT_EQ(rate.repeat_interval_in_ms + 1, tmp.repeat_interval_in_ms); - - // Restore the initial state. - EXPECT_TRUE(xkey_->SetAutoRepeatRate(rate)); - EXPECT_TRUE(ImeKeyboard::GetAutoRepeatRateForTesting(&tmp)); - EXPECT_EQ(rate.initial_delay_in_ms, tmp.initial_delay_in_ms); - EXPECT_EQ(rate.repeat_interval_in_ms, tmp.repeat_interval_in_ms); -} - -} // namespace input_method -} // namespace chromeos