diff --git a/DEPS b/DEPS index daa0d07..f3d1dc8 100644 --- a/DEPS +++ b/DEPS
@@ -44,7 +44,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other. - 'v8_revision': 'ba118aba1aeca08e62619d30832614683aceac2e', + 'v8_revision': '309a9310a4b080f4cb216ff9c7aaab71869487ad', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling swarming_client # and whatever else without interference from each other.
diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc index 51d1a1d..cbbbe7f5 100644 --- a/cc/scheduler/scheduler.cc +++ b/cc/scheduler/scheduler.cc
@@ -209,7 +209,8 @@ } void Scheduler::BeginMainFrameNotExpectedUntil(base::TimeTicks time) { - TRACE_EVENT1("cc", "Scheduler::BeginMainFrameNotExpectedUntil", "time", time); + TRACE_EVENT1("cc", "Scheduler::BeginMainFrameNotExpectedUntil", + "remaining_time", (time - Now()).InMillisecondsF()); client_->ScheduledActionBeginMainFrameNotExpectedUntil(time); }
diff --git a/chrome/android/java/res/values-v17/styles.xml b/chrome/android/java/res/values-v17/styles.xml index 0d5d88d..a3b4155 100644 --- a/chrome/android/java/res/values-v17/styles.xml +++ b/chrome/android/java/res/values-v17/styles.xml
@@ -724,4 +724,10 @@ <item name="android:padding">16dp</item> </style> + <!-- Photo Picker animations --> + + <style name="PhotoPickerDialogAnimation"> + <item name="android:windowEnterAnimation">@anim/design_bottom_sheet_slide_in</item> + <item name="android:windowExitAnimation">@null</item> + </style> </resources>
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/init/ProcessInitializationHandler.java b/chrome/android/java/src/org/chromium/chrome/browser/init/ProcessInitializationHandler.java index 161c62d..8586bb4 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/init/ProcessInitializationHandler.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/init/ProcessInitializationHandler.java
@@ -223,6 +223,8 @@ public void showPhotoPicker( Context context, PhotoPickerListener listener, boolean allowMultiple) { mDialog = new PhotoPickerDialog(context, listener, allowMultiple); + mDialog.getWindow().getAttributes().windowAnimations = + R.style.PhotoPickerDialogAnimation; mDialog.show(); }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/PlatformNetworksManager.java b/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/PlatformNetworksManager.java new file mode 100644 index 0000000..b89df8ff --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/PlatformNetworksManager.java
@@ -0,0 +1,330 @@ +// 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.omnibox.geo; + +import android.Manifest; +import android.Manifest.permission; +import android.annotation.TargetApi; +import android.content.Context; +import android.content.pm.PackageManager; +import android.net.wifi.ScanResult; +import android.net.wifi.WifiInfo; +import android.net.wifi.WifiManager; +import android.os.Build; +import android.os.Build.VERSION_CODES; +import android.os.Process; +import android.os.SystemClock; +import android.telephony.CellIdentityCdma; +import android.telephony.CellIdentityGsm; +import android.telephony.CellIdentityLte; +import android.telephony.CellIdentityWcdma; +import android.telephony.CellInfo; +import android.telephony.CellInfoCdma; +import android.telephony.CellInfoGsm; +import android.telephony.CellInfoLte; +import android.telephony.CellInfoWcdma; +import android.telephony.TelephonyManager; + +import org.chromium.base.ApiCompatibilityUtils; +import org.chromium.base.VisibleForTesting; +import org.chromium.chrome.browser.omnibox.geo.VisibleNetworks.VisibleCell; +import org.chromium.chrome.browser.omnibox.geo.VisibleNetworks.VisibleWifi; + +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.TimeUnit; + +import javax.annotation.Nullable; + +/** + * Util methods for platform networking APIs. + */ +public class PlatformNetworksManager { + @VisibleForTesting + static TimeProvider sTimeProvider = new TimeProvider(); + + /** + * Equivalent to WifiSsid.NONE which is hidden for some reason. This is returned by + * {@link WifiManager} if it cannot get the ssid for the connected wifi access point. + */ + static final String UNKNOWN_SSID = "<unknown ssid>"; + + static VisibleWifi getConnectedWifi(Context context, WifiManager wifiManager) { + if (!hasLocationAndWifiPermission(context)) { + return VisibleWifi.NO_WIFI_INFO; + } + WifiInfo wifiInfo = wifiManager.getConnectionInfo(); + if (wifiInfo == null) { + return VisibleWifi.NO_WIFI_INFO; + } + String ssid = wifiInfo.getSSID(); + if (ssid == null || UNKNOWN_SSID.equals(ssid)) { + // No SSID. + ssid = null; + } else { + // Remove double quotation if ssid has double quotation. + if (ssid.startsWith("\"") && ssid.endsWith("\"") && ssid.length() > 2) { + ssid = ssid.substring(1, ssid.length() - 1); + } + } + String bssid = wifiInfo.getBSSID(); + // It's connected, so use current time. + return VisibleWifi.create(ssid, bssid, null, sTimeProvider.getCurrentTime()); + } + + static Set<VisibleWifi> getAllVisibleWifis(Context context, WifiManager wifiManager) { + if (!hasLocationAndWifiPermission(context)) { + return Collections.emptySet(); + } + Set<VisibleWifi> visibleWifis = new HashSet<>(); + // Do not trigger a scan, but use current visible networks from latest scan. + List<ScanResult> scanResults = wifiManager.getScanResults(); + if (scanResults == null) { + return visibleWifis; + } + long elapsedTime = sTimeProvider.getElapsedRealtime(); + long currentTime = sTimeProvider.getCurrentTime(); + for (int i = 0; i < scanResults.size(); i++) { + ScanResult scanResult = scanResults.get(i); + String bssid = scanResult.BSSID; + if (bssid == null) continue; + Long scanResultTimestamp = scanResultTimestamp(scanResult); + Long wifiTimestamp = null; + if (scanResultTimestamp != null) { + long ageMs = elapsedTime - TimeUnit.MICROSECONDS.toMillis(scanResultTimestamp); + wifiTimestamp = currentTime - ageMs; + } + visibleWifis.add( + VisibleWifi.create(scanResult.SSID, bssid, scanResult.level, wifiTimestamp)); + } + return visibleWifis; + } + + @TargetApi(VERSION_CODES.JELLY_BEAN_MR1) + @Nullable + private static Long scanResultTimestamp(ScanResult scanResult) { + if (Build.VERSION.SDK_INT < VERSION_CODES.JELLY_BEAN_MR1) { + return null; + } + return scanResult.timestamp; + } + + static Set<VisibleCell> getAllVisibleCells(Context context, TelephonyManager telephonyManager) { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1) { + // CellInfo is only available JB MR1 upwards. + return Collections.emptySet(); + } + if (!hasLocationPermission(context)) { + return Collections.emptySet(); + } + return getAllVisibleCellsPostJellyBeanMr1(telephonyManager); + } + + @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR1) + private static Set<VisibleCell> getAllVisibleCellsPostJellyBeanMr1( + TelephonyManager telephonyManager) { + Set<VisibleCell> visibleCells = new HashSet<>(); + // Retrieve visible cell networks + List<CellInfo> cellInfos = telephonyManager.getAllCellInfo(); + if (cellInfos == null) { + return visibleCells; + } + + long elapsedTime = sTimeProvider.getElapsedRealtime(); + long currentTime = sTimeProvider.getCurrentTime(); + for (int i = 0; i < cellInfos.size(); i++) { + CellInfo cellInfo = cellInfos.get(i); + VisibleCell visibleCell = + getVisibleCellPostJellyBeanMr1(cellInfo, elapsedTime, currentTime); + if (visibleCell.radioType() != VisibleCell.UNKNOWN_RADIO_TYPE) { + visibleCells.add(visibleCell); + } + } + return visibleCells; + } + + static VisibleCell getConnectedCell(Context context, TelephonyManager telephonyManager) { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1) { + // CellInfo is only available JB MR1 upwards. + return VisibleCell.UNKNOWN_VISIBLE_CELL; + } + return getConnectedCellPostJellyBeanMr1(context, telephonyManager); + } + + @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR1) + private static VisibleCell getConnectedCellPostJellyBeanMr1( + Context context, TelephonyManager telephonyManager) { + if (!hasLocationPermission(context)) { + return VisibleCell.UNKNOWN_MISSING_LOCATION_PERMISSION_VISIBLE_CELL; + } + CellInfo cellInfo = getActiveCellInfo(telephonyManager); + return getVisibleCellPostJellyBeanMr1( + cellInfo, sTimeProvider.getElapsedRealtime(), sTimeProvider.getCurrentTime()); + } + + @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR1) + private static VisibleCell getVisibleCellPostJellyBeanMr1( + @Nullable CellInfo cellInfo, long elapsedTime, long currentTime) { + if (cellInfo == null) { + return VisibleCell.UNKNOWN_VISIBLE_CELL; + } + long cellInfoAge = elapsedTime - TimeUnit.NANOSECONDS.toMillis(cellInfo.getTimeStamp()); + long cellTimestamp = currentTime - cellInfoAge; + if (cellInfo instanceof CellInfoCdma) { + CellIdentityCdma cellIdentityCdma = ((CellInfoCdma) cellInfo).getCellIdentity(); + return VisibleCell.builder(VisibleCell.CDMA_RADIO_TYPE) + .setCellId(cellIdentityCdma.getBasestationId()) + .setLocationAreaCode(cellIdentityCdma.getNetworkId()) + .setMobileNetworkCode(cellIdentityCdma.getSystemId()) + .setTimestamp(cellTimestamp) + .build(); + } + if (cellInfo instanceof CellInfoGsm) { + CellIdentityGsm cellIdentityGsm = ((CellInfoGsm) cellInfo).getCellIdentity(); + return VisibleCell.builder(VisibleCell.GSM_RADIO_TYPE) + .setCellId(cellIdentityGsm.getCid()) + .setLocationAreaCode(cellIdentityGsm.getLac()) + .setMobileCountryCode(cellIdentityGsm.getMcc()) + .setMobileNetworkCode(cellIdentityGsm.getMnc()) + .setTimestamp(cellTimestamp) + .build(); + } + if (cellInfo instanceof CellInfoLte) { + CellIdentityLte cellIdLte = ((CellInfoLte) cellInfo).getCellIdentity(); + return VisibleCell.builder(VisibleCell.LTE_RADIO_TYPE) + .setCellId(cellIdLte.getCi()) + .setMobileCountryCode(cellIdLte.getMcc()) + .setMobileNetworkCode(cellIdLte.getMnc()) + .setPhysicalCellId(cellIdLte.getPci()) + .setTrackingAreaCode(cellIdLte.getTac()) + .setTimestamp(cellTimestamp) + .build(); + } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR2 + && cellInfo instanceof CellInfoWcdma) { + // CellInfoWcdma is only usable JB MR2 upwards. + CellIdentityWcdma cellIdentityWcdma = ((CellInfoWcdma) cellInfo).getCellIdentity(); + return VisibleCell.builder(VisibleCell.WCDMA_RADIO_TYPE) + .setCellId(cellIdentityWcdma.getCid()) + .setLocationAreaCode(cellIdentityWcdma.getLac()) + .setMobileCountryCode(cellIdentityWcdma.getMcc()) + .setMobileNetworkCode(cellIdentityWcdma.getMnc()) + .setPrimaryScramblingCode(cellIdentityWcdma.getPsc()) + .setTimestamp(cellTimestamp) + .build(); + } + return VisibleCell.UNKNOWN_VISIBLE_CELL; + } + + /** + * Returns a CellInfo object representing the currently registered base stations, containing + * its identity fields and signal strength. Null if no base station is active. + */ + @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR1) + @Nullable + private static CellInfo getActiveCellInfo(TelephonyManager telephonyManager) { + int numRegisteredCellInfo = 0; + List<CellInfo> cellInfos = telephonyManager.getAllCellInfo(); + + if (cellInfos == null) { + return null; + } + CellInfo result = null; + + for (int i = 0; i < cellInfos.size(); i++) { + CellInfo cellInfo = cellInfos.get(i); + if (cellInfo.isRegistered()) { + numRegisteredCellInfo++; + if (numRegisteredCellInfo > 1) { + return null; + } + result = cellInfo; + } + } + // Only found one registered cellinfo, so we know which base station was used to measure + // network quality + return result; + } + + /** + * Computes the visible networks. + * + * @param context The application context + * @param includeAllVisibleNotConnectedNetworks Whether to include all visible networks that are + * not connected. This should only be true when performing a background non-synchronous + * call, since including not connected networks can degrade latency. + */ + public static VisibleNetworks computeVisibleNetworks( + Context context, boolean includeAllVisibleNotConnectedNetworks) { + WifiManager wifiManager = (WifiManager) context.getApplicationContext().getSystemService( + Context.WIFI_SERVICE); + TelephonyManager telephonyManager = + (TelephonyManager) context.getApplicationContext().getSystemService( + Context.TELEPHONY_SERVICE); + + VisibleWifi connectedWifi; + VisibleCell connectedCell; + Set<VisibleWifi> allVisibleWifis = null; + Set<VisibleCell> allVisibleCells = null; + + connectedWifi = getConnectedWifi(context, wifiManager); + if (connectedWifi != null && connectedWifi.bssid() == null) { + // If the connected wifi is unknown, do not use it. + connectedWifi = null; + } + connectedCell = getConnectedCell(context, telephonyManager); + if (connectedCell != null + && (connectedCell.radioType() == VisibleCell.UNKNOWN_RADIO_TYPE + || connectedCell.radioType() + == VisibleCell.UNKNOWN_MISSING_LOCATION_PERMISSION_RADIO_TYPE)) { + // If the radio type is unknown, do not use it. + connectedCell = null; + } + if (includeAllVisibleNotConnectedNetworks) { + allVisibleWifis = getAllVisibleWifis(context, wifiManager); + allVisibleCells = getAllVisibleCells(context, telephonyManager); + } + return VisibleNetworks.create( + connectedWifi, connectedCell, allVisibleWifis, allVisibleCells); + } + + private static boolean hasPermission(Context context, String permission) { + return ApiCompatibilityUtils.checkPermission( + context, permission, Process.myPid(), Process.myUid()) + == PackageManager.PERMISSION_GRANTED; + } + + private static boolean hasLocationPermission(Context context) { + return hasPermission(context, Manifest.permission.ACCESS_COARSE_LOCATION) + || hasPermission(context, Manifest.permission.ACCESS_FINE_LOCATION); + } + + private static boolean hasLocationAndWifiPermission(Context context) { + return hasLocationPermission(context) + && hasPermission(context, Manifest.permission.ACCESS_WIFI_STATE); + } + + /** + * Wrapper around static time providers that allows us to mock the implementation in + * tests. + */ + public static class TimeProvider { + /** + * Get current time in milliseconds. + */ + public long getCurrentTime() { + return System.currentTimeMillis(); + } + + /** + * Get elapsed real time in milliseconds. + */ + public long getElapsedRealtime() { + return SystemClock.elapsedRealtime(); + } + } +}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/VisibleNetworks.java b/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/VisibleNetworks.java new file mode 100644 index 0000000..0910cce6 --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/VisibleNetworks.java
@@ -0,0 +1,446 @@ +// 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.omnibox.geo; + +import android.support.annotation.IntDef; + +import org.chromium.base.ApiCompatibilityUtils; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.util.Arrays; +import java.util.Set; + +import javax.annotation.Nullable; + +/** + * Visible networks. Stores the data of connected and visible networks. + */ +public class VisibleNetworks { + private static final String TAG = "VisibleNetworks"; + + @Nullable + private final VisibleWifi mConnectedWifi; + @Nullable + private final VisibleCell mConnectedCell; + @Nullable + private final Set<VisibleWifi> mAllVisibleWifis; + @Nullable + private final Set<VisibleCell> mAllVisibleCells; + + private VisibleNetworks(@Nullable VisibleWifi connectedWifi, + @Nullable VisibleCell connectedCell, @Nullable Set<VisibleWifi> allVisibleWifis, + @Nullable Set<VisibleCell> allVisibleCells) { + this.mConnectedWifi = connectedWifi; + this.mConnectedCell = connectedCell; + this.mAllVisibleWifis = allVisibleWifis; + this.mAllVisibleCells = allVisibleCells; + } + + public static VisibleNetworks create(@Nullable VisibleWifi connectedWifi, + @Nullable VisibleCell connectedCell, @Nullable Set<VisibleWifi> allVisibleWifis, + @Nullable Set<VisibleCell> allVisibleCells) { + return new VisibleNetworks(connectedWifi, connectedCell, allVisibleWifis, allVisibleCells); + } + + /** + * Returns the connected {@link VisibleWifi} or null if the connected wifi is unknown. + */ + @Nullable + public VisibleWifi connectedWifi() { + return mConnectedWifi; + } + + /** + * Returns the connected {@link VisibleCell} or null if the connected cell is unknown. + */ + @Nullable + public VisibleCell connectedCell() { + return mConnectedCell; + } + + /** + * Returns the current set of {@link VisibleWifi}s that are visible (including not connected + * networks), or null if the set is unknown. + */ + @Nullable + public Set<VisibleWifi> allVisibleWifis() { + return mAllVisibleWifis; + } + + /** + * Returns the current set of {@link VisibleCell}s that are visible (including not connected + * networks), or null if the set is unknown. + */ + @Nullable + public Set<VisibleCell> allVisibleCells() { + return mAllVisibleCells; + } + + /** + * Returns whether this object is empty, meaning there is no visible networks at all. + */ + public final boolean isEmpty() { + Set<VisibleWifi> allVisibleWifis = allVisibleWifis(); + Set<VisibleCell> allVisibleCells = allVisibleCells(); + return connectedWifi() == null && connectedCell() == null + && (allVisibleWifis == null || allVisibleWifis.size() == 0) + && (allVisibleCells == null || allVisibleCells.size() == 0); + } + + /** + * Compares the specified object with this VisibleNetworks for equality. Returns + * {@code true} if the given object is a VisibleNetworks and has identical values for + * all of its fields. + */ + @Override + public boolean equals(Object object) { + if (!(object instanceof VisibleNetworks)) { + return false; + } + VisibleNetworks that = (VisibleNetworks) object; + return ApiCompatibilityUtils.objectEquals(this.mConnectedWifi, that.connectedWifi()) + && ApiCompatibilityUtils.objectEquals(this.mConnectedCell, that.connectedCell()) + && ApiCompatibilityUtils.objectEquals(this.mAllVisibleWifis, that.allVisibleWifis()) + && ApiCompatibilityUtils.objectEquals( + this.mAllVisibleCells, that.allVisibleCells()); + } + + private static int objectsHashCode(Object o) { + return o != null ? o.hashCode() : 0; + } + + private static int objectsHash(Object... a) { + return Arrays.hashCode(a); + } + + @Override + public int hashCode() { + return objectsHash(this.mConnectedWifi, this.mConnectedCell, + objectsHashCode(this.mAllVisibleWifis), objectsHashCode(this.mAllVisibleCells)); + } + + /** + * Specification of a visible wifi. + */ + public static class VisibleWifi { + public static final VisibleWifi NO_WIFI_INFO = VisibleWifi.create(null, null, null, null); + + @Nullable + private final String mSsid; + @Nullable + private final String mBssid; + @Nullable + private final Integer mLevel; + @Nullable + private final Long mTimestampMs; + + private VisibleWifi(@Nullable String ssid, @Nullable String bssid, @Nullable Integer level, + @Nullable Long timestampMs) { + this.mSsid = ssid; + this.mBssid = bssid; + this.mLevel = level; + this.mTimestampMs = timestampMs; + } + + public static VisibleWifi create(@Nullable String ssid, @Nullable String bssid, + @Nullable Integer level, @Nullable Long timestampMs) { + return new VisibleWifi(ssid, bssid, level, timestampMs); + } + + /** + * Returns the SSID of the visible Wifi, or null if unknown. + */ + @Nullable + public String ssid() { + return mSsid; + } + + /** + * Returns the BSSID of the visible Wifi, or null if unknown. + */ + @Nullable + public String bssid() { + return mBssid; + } + + /** + * Returns the signal level in dBm (RSSI), {@code null} if unknown. + */ + @Nullable + public Integer level() { + return mLevel; + } + + /** + * Returns the timestamp in Ms, {@code null} if unknown. + */ + @Nullable + public Long timestampMs() { + return mTimestampMs; + } + + /** + * Compares the specified object with this VisibleWifi for equality. Returns + * {@code true} if the given object is a VisibleWifi and has identical values for + * all of its fields except level and timestampMs. + */ + @Override + public boolean equals(Object object) { + if (!(object instanceof VisibleWifi)) { + return false; + } + + VisibleWifi that = (VisibleWifi) object; + return ApiCompatibilityUtils.objectEquals(this.mSsid, that.ssid()) + && ApiCompatibilityUtils.objectEquals(this.mBssid, that.bssid()); + } + + @Override + public int hashCode() { + return VisibleNetworks.objectsHash(this.mSsid, this.mBssid); + } + } + + /** + * Specification of a visible cell. + */ + public static class VisibleCell { + public static final VisibleCell UNKNOWN_VISIBLE_CELL = + VisibleCell.builder(VisibleCell.UNKNOWN_RADIO_TYPE).build(); + public static final VisibleCell UNKNOWN_MISSING_LOCATION_PERMISSION_VISIBLE_CELL = + VisibleCell.builder(VisibleCell.UNKNOWN_MISSING_LOCATION_PERMISSION_RADIO_TYPE) + .build(); + + /** + * Represents all possible values of radio type that we track. + */ + @Retention(RetentionPolicy.SOURCE) + @IntDef({UNKNOWN_RADIO_TYPE, UNKNOWN_MISSING_LOCATION_PERMISSION_RADIO_TYPE, + CDMA_RADIO_TYPE, GSM_RADIO_TYPE, LTE_RADIO_TYPE, WCDMA_RADIO_TYPE}) + public @interface RadioType {} + public static final int UNKNOWN_RADIO_TYPE = 0; + public static final int UNKNOWN_MISSING_LOCATION_PERMISSION_RADIO_TYPE = 1; + public static final int CDMA_RADIO_TYPE = 2; + public static final int GSM_RADIO_TYPE = 3; + public static final int LTE_RADIO_TYPE = 4; + public static final int WCDMA_RADIO_TYPE = 5; + + public static Builder builder(@RadioType int radioType) { + return new VisibleCell.Builder().setRadioType(radioType); + } + + @RadioType + private final int mRadioType; + @Nullable + private final Integer mCellId; + @Nullable + private final Integer mLocationAreaCode; + @Nullable + private final Integer mMobileCountryCode; + @Nullable + private final Integer mMobileNetworkCode; + @Nullable + private final Integer mPrimaryScramblingCode; + @Nullable + private final Integer mPhysicalCellId; + @Nullable + private final Integer mTrackingAreaCode; + @Nullable + private Long mTimestampMs; + + private VisibleCell(Builder builder) { + this.mRadioType = builder.mRadioType; + this.mCellId = builder.mCellId; + this.mLocationAreaCode = builder.mLocationAreaCode; + this.mMobileCountryCode = builder.mMobileCountryCode; + this.mMobileNetworkCode = builder.mMobileNetworkCode; + this.mPrimaryScramblingCode = builder.mPrimaryScramblingCode; + this.mPhysicalCellId = builder.mPhysicalCellId; + this.mTrackingAreaCode = builder.mTrackingAreaCode; + this.mTimestampMs = builder.mTimestampMs; + } + + /** + * Returns the radio type of the visible cell. + */ + @RadioType + public int radioType() { + return mRadioType; + } + + /** + * Returns the gsm cell id, {@code null} if unknown. + */ + @Nullable + public Integer cellId() { + return mCellId; + } + + /** + * Returns the gsm location area code, {@code null} if unknown. + */ + @Nullable + public Integer locationAreaCode() { + return mLocationAreaCode; + } + + /** + * Returns the mobile country code, {@code null} if unknown or GSM. + */ + @Nullable + public Integer mobileCountryCode() { + return mMobileCountryCode; + } + + /** + * Returns the mobile network code, {@code null} if unknown or GSM. + */ + @Nullable + public Integer mobileNetworkCode() { + return mMobileNetworkCode; + } + + /** + * On a UMTS network, returns the primary scrambling code of the serving cell, {@code null} + * if unknown or GSM. + */ + @Nullable + public Integer primaryScramblingCode() { + return mPrimaryScramblingCode; + } + + /** + * Returns the physical cell id, {@code null} if unknown or not LTE. + */ + @Nullable + public Integer physicalCellId() { + return mPhysicalCellId; + } + + /** + * Returns the tracking area code, {@code null} if unknown or not LTE. + */ + @Nullable + public Integer trackingAreaCode() { + return mTrackingAreaCode; + } + + /** + * Returns the timestamp in Ms, {@code null} if unknown. + */ + @Nullable + public Long timestampMs() { + return mTimestampMs; + } + + /** + * Compares the specified object with this VisibleCell for equality. Returns + * {@code true} if the given object is a VisibleWifi and has identical values for + * all of its fields except timestampMs. + */ + @Override + public boolean equals(Object object) { + if (!(object instanceof VisibleCell)) { + return false; + } + VisibleCell that = (VisibleCell) object; + return ApiCompatibilityUtils.objectEquals(this.mRadioType, that.radioType()) + && ApiCompatibilityUtils.objectEquals(this.mCellId, that.cellId()) + && ApiCompatibilityUtils.objectEquals( + this.mLocationAreaCode, that.locationAreaCode()) + && ApiCompatibilityUtils.objectEquals( + this.mMobileCountryCode, that.mobileCountryCode()) + && ApiCompatibilityUtils.objectEquals( + this.mMobileNetworkCode, that.mobileNetworkCode()) + && ApiCompatibilityUtils.objectEquals( + this.mPrimaryScramblingCode, that.primaryScramblingCode()) + && ApiCompatibilityUtils.objectEquals( + this.mPhysicalCellId, that.physicalCellId()) + && ApiCompatibilityUtils.objectEquals( + this.mTrackingAreaCode, that.trackingAreaCode()); + } + + @Override + public int hashCode() { + return VisibleNetworks.objectsHash(this.mRadioType, this.mCellId, + this.mLocationAreaCode, this.mMobileCountryCode, this.mMobileNetworkCode, + this.mPrimaryScramblingCode, this.mPhysicalCellId, this.mTrackingAreaCode); + } + + /** + * A {@link VisibleCell} builder. + */ + public static class Builder { + @RadioType + private int mRadioType; + @Nullable + private Integer mCellId; + @Nullable + private Integer mLocationAreaCode; + @Nullable + private Integer mMobileCountryCode; + @Nullable + private Integer mMobileNetworkCode; + @Nullable + private Integer mPrimaryScramblingCode; + @Nullable + private Integer mPhysicalCellId; + @Nullable + private Integer mTrackingAreaCode; + @Nullable + private Long mTimestampMs; + + public Builder setRadioType(@RadioType int radioType) { + mRadioType = radioType; + return this; + } + + public Builder setCellId(@Nullable Integer cellId) { + mCellId = cellId; + return this; + } + + public Builder setLocationAreaCode(@Nullable Integer locationAreaCode) { + mLocationAreaCode = locationAreaCode; + return this; + } + + public Builder setMobileCountryCode(@Nullable Integer mobileCountryCode) { + mMobileCountryCode = mobileCountryCode; + return this; + } + + public Builder setMobileNetworkCode(@Nullable Integer mobileNetworkCode) { + mMobileNetworkCode = mobileNetworkCode; + return this; + } + + public Builder setPrimaryScramblingCode(@Nullable Integer primaryScramblingCode) { + mPrimaryScramblingCode = primaryScramblingCode; + return this; + } + + public Builder setPhysicalCellId(@Nullable Integer physicalCellId) { + mPhysicalCellId = physicalCellId; + return this; + } + + public Builder setTrackingAreaCode(@Nullable Integer trackingAreaCode) { + mTrackingAreaCode = trackingAreaCode; + return this; + } + + public Builder setTimestamp(@Nullable Long timestampMs) { + mTimestampMs = timestampMs; + return this; + } + + public VisibleCell build() { + return new VisibleCell(this); + } + } + } +} \ No newline at end of file
diff --git a/chrome/android/java_sources.gni b/chrome/android/java_sources.gni index 2b62ebf..921618d 100644 --- a/chrome/android/java_sources.gni +++ b/chrome/android/java_sources.gni
@@ -723,6 +723,8 @@ "java/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeader.java", "java/src/org/chromium/chrome/browser/omnibox/geo/GeolocationSnackbarController.java", "java/src/org/chromium/chrome/browser/omnibox/geo/GeolocationTracker.java", + "java/src/org/chromium/chrome/browser/omnibox/geo/PlatformNetworksManager.java", + "java/src/org/chromium/chrome/browser/omnibox/geo/VisibleNetworks.java", "java/src/org/chromium/chrome/browser/page_info/CertificateChainHelper.java", "java/src/org/chromium/chrome/browser/page_info/CertificateViewer.java", "java/src/org/chromium/chrome/browser/page_info/ConnectionInfoPopup.java", @@ -1737,6 +1739,8 @@ "junit/src/org/chromium/chrome/browser/omaha/ResponseParserTest.java", "junit/src/org/chromium/chrome/browser/omaha/VersionNumberTest.java", "junit/src/org/chromium/chrome/browser/omnibox/KeyboardHideHelperUnitTest.java", + "junit/src/org/chromium/chrome/browser/omnibox/geo/VisibleNetworksTest.java", + "junit/src/org/chromium/chrome/browser/omnibox/geo/PlatformNetworksManagerTest.java", "junit/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinderTest.java", "junit/src/org/chromium/chrome/browser/payments/AutofillContactTest.java", "junit/src/org/chromium/chrome/browser/payments/AutofillContactUnitTest.java",
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/geo/PlatformNetworksManagerTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/geo/PlatformNetworksManagerTest.java new file mode 100644 index 0000000..2d668f7 --- /dev/null +++ b/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/geo/PlatformNetworksManagerTest.java
@@ -0,0 +1,550 @@ +// 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.omnibox.geo; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.when; + +import android.Manifest; +import android.content.Context; +import android.content.pm.PackageManager; +import android.net.ConnectivityManager; +import android.net.NetworkInfo; +import android.net.wifi.ScanResult; +import android.net.wifi.WifiInfo; +import android.net.wifi.WifiManager; +import android.os.Build; +import android.telephony.CellIdentityCdma; +import android.telephony.CellIdentityGsm; +import android.telephony.CellIdentityLte; +import android.telephony.CellIdentityWcdma; +import android.telephony.CellInfoCdma; +import android.telephony.CellInfoGsm; +import android.telephony.CellInfoLte; +import android.telephony.CellInfoWcdma; +import android.telephony.TelephonyManager; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.robolectric.annotation.Config; +import org.robolectric.util.ReflectionHelpers; + +import org.chromium.chrome.browser.omnibox.geo.VisibleNetworks.VisibleCell; +import org.chromium.chrome.browser.omnibox.geo.VisibleNetworks.VisibleWifi; +import org.chromium.testing.local.LocalRobolectricTestRunner; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.TimeUnit; + +/** + * Robolectric tests for {@link PlatformNetworksManager}. + */ +@RunWith(LocalRobolectricTestRunner.class) +@Config(manifest = Config.NONE) +public class PlatformNetworksManagerTest { + private static final VisibleWifi CONNECTED_WIFI = + VisibleWifi.create("ssid1", "11:11:11:11:11:11", -1, 10L); + private static final VisibleWifi NOT_CONNECTED_WIFI = + VisibleWifi.create("ssid2", "11:11:11:11:11:12", -2, 20L); + private static final long CONNECTED_WIFI_AGE = 1000L; + private static final long NOT_CONNECTED_WIFI_AGE = 2000L; + + private static final VisibleCell CDMA_CELL = VisibleCell.builder(VisibleCell.CDMA_RADIO_TYPE) + .setCellId(40) + .setLocationAreaCode(41) + .setMobileNetworkCode(43) + .setTimestamp(47L) + .build(); + private static final VisibleCell LTE_CELL = VisibleCell.builder(VisibleCell.LTE_RADIO_TYPE) + .setCellId(50) + .setMobileCountryCode(52) + .setMobileNetworkCode(53) + .setPhysicalCellId(55) + .setTrackingAreaCode(56) + .setTimestamp(57L) + .build(); + private static final VisibleCell WCDMA_CELL = VisibleCell.builder(VisibleCell.WCDMA_RADIO_TYPE) + .setCellId(60) + .setLocationAreaCode(61) + .setMobileCountryCode(62) + .setMobileNetworkCode(63) + .setPrimaryScramblingCode(64) + .setTimestamp(67L) + .build(); + private static final VisibleCell GSM_CELL = VisibleCell.builder(VisibleCell.GSM_RADIO_TYPE) + .setCellId(70) + .setLocationAreaCode(71) + .setMobileCountryCode(72) + .setMobileNetworkCode(73) + .setTimestamp(77L) + .build(); + private static final VisibleCell UNKNOWN_VISIBLE_CELL = + VisibleCell.builder(VisibleCell.UNKNOWN_RADIO_TYPE).build(); + private static final VisibleCell UNKNOWN_MISSING_LOCATION_PERMISSION_VISIBLE_CELL = + VisibleCell.builder(VisibleCell.UNKNOWN_MISSING_LOCATION_PERMISSION_RADIO_TYPE).build(); + private static final VisibleWifi UNKNOWN_VISIBLE_WIFI = + VisibleWifi.create(null, null, null, null); + private static final long LTE_CELL_AGE = 3000L; + private static final long WCDMA_CELL_AGE = 4000L; + private static final long GSM_CELL_AGE = 5000L; + private static final long CDMA_CELL_AGE = 6000L; + + private static final long CURRENT_TIME_MS = 90000000L; + private static final long CURRENT_ELAPSED_TIME_MS = 7000L; + + @Mock + private Context mContext; + @Mock + private TelephonyManager mTelephonyManager; + @Mock + private WifiManager mWifiManager; + @Mock + private NetworkInfo mCellNetworkInfo; + @Mock + private NetworkInfo mWifiNetworkInfo; + @Mock + private NetworkInfo mEthernetNetworkInfo; + @Mock + private WifiInfo mWifiInfo; + @Mock + private ScanResult mWifiScanResult; + @Mock + private ScanResult mWifiScanResultNotConnected; + @Mock + private ScanResult mWifiScanResultUnknown; + @Mock + private CellInfoLte mCellInfoLte; + @Mock + private CellInfoWcdma mCellInfoWcdma; + @Mock + private CellInfoGsm mCellInfoGsm; + @Mock + private CellInfoCdma mCellInfoCdma; + @Mock + private CellIdentityLte mCellIdentityLte; + @Mock + private CellIdentityWcdma mCellIdentityWcdma; + @Mock + private CellIdentityGsm mCellIdentityGsm; + @Mock + private CellIdentityCdma mCellIdentityCdma; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + PlatformNetworksManager.sTimeProvider = new PlatformNetworksManager.TimeProvider() { + @Override + public long getCurrentTime() { + return CURRENT_TIME_MS; + } + @Override + public long getElapsedRealtime() { + return CURRENT_ELAPSED_TIME_MS; + } + }; + when(mContext.getSystemService(Context.TELEPHONY_SERVICE)).thenReturn(mTelephonyManager); + when(mContext.getSystemService(Context.WIFI_SERVICE)).thenReturn(mWifiManager); + when(mContext.getApplicationContext()).thenReturn(mContext); + when(mCellNetworkInfo.getType()).thenReturn(ConnectivityManager.TYPE_MOBILE); + when(mWifiNetworkInfo.getType()).thenReturn(ConnectivityManager.TYPE_WIFI); + when(mEthernetNetworkInfo.getType()).thenReturn(ConnectivityManager.TYPE_ETHERNET); + // Add double quotation, since getSSID would add it. + when(mWifiInfo.getSSID()).thenReturn("\"" + CONNECTED_WIFI.ssid() + "\""); + when(mWifiInfo.getBSSID()).thenReturn(CONNECTED_WIFI.bssid()); + when(mWifiManager.getConnectionInfo()).thenReturn(mWifiInfo); + + mWifiScanResult.SSID = CONNECTED_WIFI.ssid(); + mWifiScanResult.BSSID = CONNECTED_WIFI.bssid(); + mWifiScanResult.level = CONNECTED_WIFI.level(); + mWifiScanResult.timestamp = + TimeUnit.MILLISECONDS.toMicros(CURRENT_ELAPSED_TIME_MS - CONNECTED_WIFI_AGE); + mWifiScanResultNotConnected.SSID = NOT_CONNECTED_WIFI.ssid(); + mWifiScanResultNotConnected.BSSID = NOT_CONNECTED_WIFI.bssid(); + mWifiScanResultNotConnected.level = NOT_CONNECTED_WIFI.level(); + mWifiScanResultNotConnected.timestamp = + TimeUnit.MILLISECONDS.toMicros(CURRENT_ELAPSED_TIME_MS - NOT_CONNECTED_WIFI_AGE); + mWifiScanResultUnknown.SSID = "any_value"; + mWifiScanResultUnknown.BSSID = null; // This is what makes it unknown. + mWifiScanResultUnknown.level = 0; + when(mWifiManager.getScanResults()) + .thenReturn(Arrays.asList( + mWifiScanResult, mWifiScanResultNotConnected, mWifiScanResultUnknown)); + + when(mCellIdentityLte.getCi()).thenReturn(LTE_CELL.cellId().intValue()); + when(mCellIdentityLte.getPci()).thenReturn(LTE_CELL.physicalCellId().intValue()); + when(mCellIdentityLte.getTac()).thenReturn(LTE_CELL.trackingAreaCode().intValue()); + when(mCellIdentityLte.getMcc()).thenReturn(LTE_CELL.mobileCountryCode().intValue()); + when(mCellIdentityLte.getMnc()).thenReturn(LTE_CELL.mobileNetworkCode().intValue()); + when(mCellInfoLte.getCellIdentity()).thenReturn(mCellIdentityLte); + when(mCellInfoLte.isRegistered()).thenReturn(true); + when(mCellInfoLte.getTimeStamp()) + .thenReturn(TimeUnit.MILLISECONDS.toNanos(CURRENT_ELAPSED_TIME_MS - LTE_CELL_AGE)); + when(mCellIdentityWcdma.getCid()).thenReturn(WCDMA_CELL.cellId().intValue()); + when(mCellIdentityWcdma.getLac()).thenReturn(WCDMA_CELL.locationAreaCode().intValue()); + when(mCellIdentityWcdma.getMcc()).thenReturn(WCDMA_CELL.mobileCountryCode().intValue()); + when(mCellIdentityWcdma.getMnc()).thenReturn(WCDMA_CELL.mobileNetworkCode().intValue()); + when(mCellIdentityWcdma.getPsc()).thenReturn(WCDMA_CELL.primaryScramblingCode().intValue()); + when(mCellInfoWcdma.getCellIdentity()).thenReturn(mCellIdentityWcdma); + when(mCellInfoWcdma.isRegistered()).thenReturn(false); + when(mCellInfoWcdma.getTimeStamp()) + .thenReturn( + TimeUnit.MILLISECONDS.toNanos(CURRENT_ELAPSED_TIME_MS - WCDMA_CELL_AGE)); + when(mCellIdentityGsm.getCid()).thenReturn(GSM_CELL.cellId().intValue()); + when(mCellIdentityGsm.getLac()).thenReturn(GSM_CELL.locationAreaCode().intValue()); + when(mCellIdentityGsm.getMcc()).thenReturn(GSM_CELL.mobileCountryCode().intValue()); + when(mCellIdentityGsm.getMnc()).thenReturn(GSM_CELL.mobileNetworkCode().intValue()); + when(mCellInfoGsm.getCellIdentity()).thenReturn(mCellIdentityGsm); + when(mCellInfoGsm.isRegistered()).thenReturn(false); + when(mCellInfoGsm.getTimeStamp()) + .thenReturn(TimeUnit.MILLISECONDS.toNanos(CURRENT_ELAPSED_TIME_MS - GSM_CELL_AGE)); + when(mCellIdentityCdma.getBasestationId()).thenReturn(CDMA_CELL.cellId().intValue()); + when(mCellIdentityCdma.getNetworkId()).thenReturn(CDMA_CELL.locationAreaCode().intValue()); + when(mCellIdentityCdma.getSystemId()).thenReturn(CDMA_CELL.mobileNetworkCode().intValue()); + when(mCellInfoCdma.getCellIdentity()).thenReturn(mCellIdentityCdma); + when(mCellInfoCdma.isRegistered()).thenReturn(false); + when(mCellInfoCdma.getTimeStamp()) + .thenReturn(TimeUnit.MILLISECONDS.toNanos(CURRENT_ELAPSED_TIME_MS - CDMA_CELL_AGE)); + when(mTelephonyManager.getAllCellInfo()) + .thenReturn( + Arrays.asList(mCellInfoLte, mCellInfoWcdma, mCellInfoGsm, mCellInfoCdma)); + allPermissionsGranted(); + } + + @Test + public void testGetConnectedCell_JBMR1() { + ReflectionHelpers.setStaticField( + Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.JELLY_BEAN_MR1); + VisibleCell visibleCell = + PlatformNetworksManager.getConnectedCell(mContext, mTelephonyManager); + assertEquals(LTE_CELL, visibleCell); + assertEquals(visibleCell.timestampMs(), Long.valueOf(CURRENT_TIME_MS - LTE_CELL_AGE)); + } + + @Test + public void testGetConnectedCell_preJBMR1() { + ReflectionHelpers.setStaticField( + Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.JELLY_BEAN); + VisibleCell visibleCell = + PlatformNetworksManager.getConnectedCell(mContext, mTelephonyManager); + assertEquals(UNKNOWN_VISIBLE_CELL, visibleCell); + assertNull(visibleCell.timestampMs()); + } + + @Test + public void testGetConnectedCell_allPermissionsDenied() { + ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.M); + allPermissionsDenied(); + VisibleCell visibleCell = + PlatformNetworksManager.getConnectedCell(mContext, mTelephonyManager); + assertEquals(UNKNOWN_MISSING_LOCATION_PERMISSION_VISIBLE_CELL, visibleCell); + assertNull(visibleCell.timestampMs()); + } + + @Test + public void testGetAllVisibleCells_JBMR2() { + ReflectionHelpers.setStaticField( + Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.JELLY_BEAN_MR2); + Set<VisibleCell> visibleCells = + PlatformNetworksManager.getAllVisibleCells(mContext, mTelephonyManager); + assertEquals(4, visibleCells.size()); + for (VisibleCell visibleCell : visibleCells) { + switch (visibleCell.radioType()) { + case VisibleCell.LTE_RADIO_TYPE: + assertEquals(LTE_CELL, visibleCell); + assertEquals(Long.valueOf(CURRENT_TIME_MS - LTE_CELL_AGE), + visibleCell.timestampMs()); + break; + case VisibleCell.WCDMA_RADIO_TYPE: + assertEquals(visibleCell, WCDMA_CELL); + assertEquals(Long.valueOf(CURRENT_TIME_MS - WCDMA_CELL_AGE), + visibleCell.timestampMs()); + break; + case VisibleCell.GSM_RADIO_TYPE: + assertEquals(visibleCell, GSM_CELL); + assertEquals(Long.valueOf(CURRENT_TIME_MS - GSM_CELL_AGE), + visibleCell.timestampMs()); + break; + case VisibleCell.CDMA_RADIO_TYPE: + assertEquals(visibleCell, CDMA_CELL); + assertEquals(Long.valueOf(CURRENT_TIME_MS - CDMA_CELL_AGE), + visibleCell.timestampMs()); + break; + default: + break; + } + } + } + + @Test + public void testGetAllVisibleCells_JBMR1() { + ReflectionHelpers.setStaticField( + Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.JELLY_BEAN_MR1); + Set<VisibleCell> visibleCells = + PlatformNetworksManager.getAllVisibleCells(mContext, mTelephonyManager); + // WCDMA should be ignored for pre-JBMR1 + assertEquals(visibleCells.size(), 3); + for (VisibleCell visibleCell : visibleCells) { + switch (visibleCell.radioType()) { + case VisibleCell.LTE_RADIO_TYPE: + assertEquals(LTE_CELL, visibleCell); + assertEquals(Long.valueOf(CURRENT_TIME_MS - LTE_CELL_AGE), + visibleCell.timestampMs()); + break; + case VisibleCell.GSM_RADIO_TYPE: + assertEquals(visibleCell, GSM_CELL); + assertEquals(Long.valueOf(CURRENT_TIME_MS - GSM_CELL_AGE), + visibleCell.timestampMs()); + break; + case VisibleCell.CDMA_RADIO_TYPE: + assertEquals(visibleCell, CDMA_CELL); + assertEquals(Long.valueOf(CURRENT_TIME_MS - CDMA_CELL_AGE), + visibleCell.timestampMs()); + break; + default: + break; + } + } + } + + @Test + public void testGetAllVisibleCells_JBMR1_nullResult() { + ReflectionHelpers.setStaticField( + Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.JELLY_BEAN_MR1); + when(mTelephonyManager.getAllCellInfo()).thenReturn(null); + // Null case should be handled and return an empty set. + Set<VisibleCell> visibleCells = + PlatformNetworksManager.getAllVisibleCells(mContext, mTelephonyManager); + assertEquals(0, visibleCells.size()); + } + + @Test + public void testGetAllVisibleCells_preJBMR1() { + ReflectionHelpers.setStaticField( + Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.JELLY_BEAN); + Set<VisibleCell> visibleCells = + PlatformNetworksManager.getAllVisibleCells(mContext, mTelephonyManager); + // Empty set expected + assertEquals(0, visibleCells.size()); + } + + @Test + public void testGetAllVisibleCells_allPermissionsDenied() { + ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.M); + allPermissionsDenied(); + Set<VisibleCell> visibleCells = + PlatformNetworksManager.getAllVisibleCells(mContext, mTelephonyManager); + // Empty set expected + assertEquals(0, visibleCells.size()); + } + + @Test + public void testGetConnectedWifi() { + VisibleWifi visibleWifi = PlatformNetworksManager.getConnectedWifi(mContext, mWifiManager); + assertEquals(CONNECTED_WIFI, visibleWifi); + // When we get it through get connected wifi, we should see the current time. + assertEquals(Long.valueOf(CURRENT_TIME_MS), visibleWifi.timestampMs()); + } + + @Test + public void testGetConnectedWifi_allPermissionsDenied() { + ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.M); + allPermissionsDenied(); + VisibleWifi visibleWifi = PlatformNetworksManager.getConnectedWifi(mContext, mWifiManager); + assertEquals(UNKNOWN_VISIBLE_WIFI, visibleWifi); + assertNull(visibleWifi.timestampMs()); + } + + @Test + public void testGetConnectedWifi_locationGrantedWifiDenied() { + ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.M); + locationGrantedWifiDenied(); + VisibleWifi visibleWifi = PlatformNetworksManager.getConnectedWifi(mContext, mWifiManager); + assertEquals(UNKNOWN_VISIBLE_WIFI, visibleWifi); + assertNull(visibleWifi.timestampMs()); + } + + @Test + public void testGetConnectedWifi_locationDeniedWifiGranted() { + ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.M); + locationDeniedWifiGranted(); + VisibleWifi visibleWifi = PlatformNetworksManager.getConnectedWifi(mContext, mWifiManager); + assertEquals(UNKNOWN_VISIBLE_WIFI, visibleWifi); + assertNull(visibleWifi.timestampMs()); + } + + @Test + public void testGetAllVisibleWifis() { + Set<VisibleWifi> visibleWifis = + PlatformNetworksManager.getAllVisibleWifis(mContext, mWifiManager); + assertEquals(2, visibleWifis.size()); + // When we get all wifis, we should get the scan time. + for (VisibleWifi visibleWifi : visibleWifis) { + if (visibleWifi.bssid().equals(CONNECTED_WIFI.bssid())) { + assertEquals(CONNECTED_WIFI, visibleWifi); + assertEquals(Long.valueOf(CURRENT_TIME_MS - CONNECTED_WIFI_AGE), + visibleWifi.timestampMs()); + } else { + assertEquals(NOT_CONNECTED_WIFI, visibleWifi); + assertEquals(Long.valueOf(CURRENT_TIME_MS - NOT_CONNECTED_WIFI_AGE), + visibleWifi.timestampMs()); + } + } + } + + @Test + public void testGetAllVisibleWifis_preJBMR1() { + ReflectionHelpers.setStaticField( + Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.JELLY_BEAN); + Set<VisibleWifi> visibleWifis = + PlatformNetworksManager.getAllVisibleWifis(mContext, mWifiManager); + assertEquals(2, visibleWifis.size()); + for (VisibleWifi visibleWifi : visibleWifis) { + if (visibleWifi.bssid().equals(CONNECTED_WIFI.bssid())) { + assertEquals(CONNECTED_WIFI, visibleWifi); + } else { + assertEquals(NOT_CONNECTED_WIFI, visibleWifi); + } + // We should get null timestamp in Pre-JBMR1. + assertNull(visibleWifi.timestampMs()); + } + } + + @Test + public void testGetAllVisibleWifis_allPermissionsDenied() { + ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.M); + allPermissionsDenied(); + Set<VisibleWifi> visibleWifis = + PlatformNetworksManager.getAllVisibleWifis(mContext, mWifiManager); + // Empty set expected + assertTrue(visibleWifis.isEmpty()); + } + + @Test + public void testGetAllVisibleWifis_locationGrantedWifiDenied() { + ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.M); + locationGrantedWifiDenied(); + Set<VisibleWifi> visibleWifis = + PlatformNetworksManager.getAllVisibleWifis(mContext, mWifiManager); + // Empty set expected + assertTrue(visibleWifis.isEmpty()); + } + + @Test + public void testGetAllVisibleWifis_locationDeniedWifiGranted() { + ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.M); + locationDeniedWifiGranted(); + Set<VisibleWifi> visibleWifis = + PlatformNetworksManager.getAllVisibleWifis(mContext, mWifiManager); + // Empty set expected + assertTrue(visibleWifis.isEmpty()); + } + + @Test + public void testComputeVisibleNetworks_withoutNonConnectedNetworks() { + VisibleNetworks expectedVisibleNetworks = + VisibleNetworks.create(CONNECTED_WIFI, LTE_CELL, null, null); + VisibleNetworks visibleNetworks = PlatformNetworksManager.computeVisibleNetworks( + mContext, false /* includeAllVisibleNotConnectedNetworks */); + assertEquals(expectedVisibleNetworks, visibleNetworks); + } + + @Test + public void testComputeVisibleNetworks_withNonConnectedNetworks() { + Set<VisibleCell> expectedVisibleCells = + new HashSet<VisibleCell>(Arrays.asList(LTE_CELL, WCDMA_CELL, GSM_CELL, CDMA_CELL)); + Set<VisibleWifi> expectedVisibleWifis = + new HashSet<VisibleWifi>(Arrays.asList(CONNECTED_WIFI, NOT_CONNECTED_WIFI)); + VisibleNetworks expectedVisibleNetworks = VisibleNetworks.create( + CONNECTED_WIFI, LTE_CELL, expectedVisibleWifis, expectedVisibleCells); + VisibleNetworks visibleNetworks = PlatformNetworksManager.computeVisibleNetworks( + mContext, true /* includeAllVisibleNotConnectedNetworks */); + assertEquals(expectedVisibleNetworks, visibleNetworks); + } + + @Test + public void testComputeVisibleNetworks_allPermissionsDenied() { + allPermissionsDenied(); + + VisibleNetworks visibleNetworks = PlatformNetworksManager.computeVisibleNetworks( + mContext, true /* includeAllVisibleNotConnectedNetworks */); + + assertTrue(visibleNetworks.isEmpty()); + } + + @Test + public void testComputeVisibleNetworks_locationGrantedWifiDenied() { + Set<VisibleCell> expectedVisibleCells = + new HashSet<VisibleCell>(Arrays.asList(LTE_CELL, WCDMA_CELL, GSM_CELL, CDMA_CELL)); + Set<VisibleWifi> expectedVisibleWifis = Collections.emptySet(); + VisibleNetworks expectedVisibleNetworks = + VisibleNetworks.create(null, LTE_CELL, expectedVisibleWifis, expectedVisibleCells); + locationGrantedWifiDenied(); + + VisibleNetworks visibleNetworks = PlatformNetworksManager.computeVisibleNetworks( + mContext, true /* includeAllVisibleNotConnectedNetworks */); + + assertEquals(expectedVisibleNetworks, visibleNetworks); + } + + @Test + public void testComputeVisibleNetworks_locationDeniedWifiGranted() { + Set<VisibleCell> expectedVisibleCells = Collections.emptySet(); + Set<VisibleWifi> expectedVisibleWifis = Collections.emptySet(); + locationDeniedWifiGranted(); + VisibleNetworks expectedVisibleNetworks = + VisibleNetworks.create(null, null, expectedVisibleWifis, expectedVisibleCells); + + VisibleNetworks visibleNetworks = PlatformNetworksManager.computeVisibleNetworks( + mContext, true /* includeAllVisibleNotConnectedNetworks */); + + assertEquals(expectedVisibleNetworks, visibleNetworks); + } + + private void allPermissionsGranted() { + setPermissions(true, true, true); + } + + private void allPermissionsDenied() { + setPermissions(false, false, false); + } + + private void locationGrantedWifiDenied() { + setPermissions(true, true, false); + } + + private void locationDeniedWifiGranted() { + setPermissions(false, false, true); + } + + private void setPermissions( + boolean coarseLocationGranted, boolean fineLocationGranted, boolean wifiStateGranted) { + int coarseLocationPermission = (coarseLocationGranted) ? PackageManager.PERMISSION_GRANTED + : PackageManager.PERMISSION_DENIED; + int fineLocationPermission = (fineLocationGranted) ? PackageManager.PERMISSION_GRANTED + : PackageManager.PERMISSION_DENIED; + int wifiStatePermission = (wifiStateGranted) ? PackageManager.PERMISSION_GRANTED + : PackageManager.PERMISSION_DENIED; + + when(mContext.checkPermission(eq(Manifest.permission.ACCESS_COARSE_LOCATION), + any(Integer.class), any(Integer.class))) + .thenReturn(coarseLocationPermission); + when(mContext.checkPermission(eq(Manifest.permission.ACCESS_FINE_LOCATION), + any(Integer.class), any(Integer.class))) + .thenReturn(fineLocationPermission); + when(mContext.checkPermission(eq(Manifest.permission.ACCESS_WIFI_STATE), any(Integer.class), + any(Integer.class))) + .thenReturn(wifiStatePermission); + } +} \ No newline at end of file
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/geo/VisibleNetworksTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/geo/VisibleNetworksTest.java new file mode 100644 index 0000000..18d612d --- /dev/null +++ b/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/geo/VisibleNetworksTest.java
@@ -0,0 +1,210 @@ +// 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.omnibox.geo; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.annotation.Config; + +import org.chromium.chrome.browser.omnibox.geo.VisibleNetworks.VisibleCell; +import org.chromium.chrome.browser.omnibox.geo.VisibleNetworks.VisibleCell.RadioType; +import org.chromium.chrome.browser.omnibox.geo.VisibleNetworks.VisibleWifi; +import org.chromium.testing.local.LocalRobolectricTestRunner; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +/** + * Robolectric tests for {@link VisibleNetworks}. + */ +@RunWith(LocalRobolectricTestRunner.class) +@Config(manifest = Config.NONE) +public class VisibleNetworksTest { + private static final String SSID1 = "ssid1"; + private static final String BSSID1 = "11:11:11:11:11:11"; + private static final Integer LEVEL1 = -1; + private static final Long TIMESTAMP1 = 10L; + private static final String SSID2 = "ssid2"; + private static final String BSSID2 = "11:11:11:11:11:12"; + private static final Integer LEVEL2 = -2; + private static final Long TIMESTAMP2 = 20L; + @RadioType + private static final int[] RADIO_TYPES = {VisibleCell.UNKNOWN_RADIO_TYPE, + VisibleCell.UNKNOWN_MISSING_LOCATION_PERMISSION_RADIO_TYPE, VisibleCell.CDMA_RADIO_TYPE, + VisibleCell.GSM_RADIO_TYPE, VisibleCell.LTE_RADIO_TYPE, VisibleCell.WCDMA_RADIO_TYPE}; + private static final VisibleWifi VISIBLE_WIFI1 = + VisibleWifi.create(SSID1, BSSID1, LEVEL1, TIMESTAMP1); + private static final VisibleWifi VISIBLE_WIFI2 = + VisibleWifi.create(SSID2, BSSID2, LEVEL2, TIMESTAMP2); + private static VisibleCell.Builder sVisibleCellCommunBuilder = + VisibleCell.builder(VisibleCell.GSM_RADIO_TYPE) + .setCellId(10) + .setLocationAreaCode(11) + .setMobileCountryCode(12) + .setMobileNetworkCode(13); + private static final VisibleCell VISIBLE_CELL1 = + sVisibleCellCommunBuilder.setTimestamp(10L).build(); + private static final VisibleCell VISIBLE_CELL1_DIFFERENT_TIMESTAMP = + sVisibleCellCommunBuilder.setTimestamp(20L).build(); + private static final VisibleCell VISIBLE_CELL2 = VisibleCell.builder(VisibleCell.GSM_RADIO_TYPE) + .setCellId(30) + .setLocationAreaCode(31) + .setMobileCountryCode(32) + .setMobileNetworkCode(33) + .setTimestamp(30L) + .build(); + private static Set<VisibleCell> sAllVisibleCells = + new HashSet<VisibleCell>(Arrays.asList(VISIBLE_CELL1, VISIBLE_CELL2)); + private static Set<VisibleCell> sAllVisibleCells2 = new HashSet<VisibleCell>( + Arrays.asList(VISIBLE_CELL1, VISIBLE_CELL2, VISIBLE_CELL1_DIFFERENT_TIMESTAMP)); + private static Set<VisibleWifi> sAllVisibleWifis = + new HashSet<VisibleWifi>(Arrays.asList(VISIBLE_WIFI1, VISIBLE_WIFI2)); + + private static final VisibleNetworks VISIBLE_NETWORKS1 = VisibleNetworks.create( + VISIBLE_WIFI1, VISIBLE_CELL1, sAllVisibleWifis, sAllVisibleCells); + + private static final VisibleNetworks VISIBLE_NETWORKS2 = VisibleNetworks.create( + VISIBLE_WIFI2, VISIBLE_CELL2, sAllVisibleWifis, sAllVisibleCells2); + + @Test + public void testVisibleWifiCreate() { + VisibleWifi visibleWifi = VisibleWifi.create(SSID1, BSSID1, LEVEL1, TIMESTAMP1); + assertEquals(SSID1, visibleWifi.ssid()); + assertEquals(BSSID1, visibleWifi.bssid()); + assertEquals(LEVEL1, visibleWifi.level()); + assertEquals(TIMESTAMP1, visibleWifi.timestampMs()); + } + + @Test + public void testVisibleWifiEquals() { + VisibleWifi copyOfVisibleWifi1 = VisibleWifi.create(VISIBLE_WIFI1.ssid(), + VISIBLE_WIFI1.bssid(), VISIBLE_WIFI1.level(), VISIBLE_WIFI1.timestampMs()); + + assertEquals(VISIBLE_WIFI1, copyOfVisibleWifi1); + assertNotEquals(VISIBLE_WIFI1, VISIBLE_WIFI2); + } + + @Test + public void testVisibleWifiEqualsDifferentLevelAndTimestamp() { + VisibleWifi visibleWifi3 = VisibleWifi.create(SSID2, BSSID2, LEVEL1, TIMESTAMP1); + + // visibleWifi3 has the same ssid and bssid as VISIBLE_WIFI2 but different level and + // timestamp. The level and timestamp are excluded from the VisibleWifi equality checks. + assertEquals(visibleWifi3, VISIBLE_WIFI2); + } + + @Test + public void testVisibleWifiHash() { + VisibleWifi copyOfVisibleWifi1 = VisibleWifi.create(VISIBLE_WIFI1.ssid(), + VISIBLE_WIFI1.bssid(), VISIBLE_WIFI1.level(), VISIBLE_WIFI1.timestampMs()); + + assertEquals(VISIBLE_WIFI1.hashCode(), copyOfVisibleWifi1.hashCode()); + assertNotEquals(VISIBLE_WIFI1.hashCode(), VISIBLE_WIFI2.hashCode()); + } + + @Test + public void testVisibleWifiHashDifferentLevelAndTimestamp() { + VisibleWifi visibleWifi3 = VisibleWifi.create(SSID2, BSSID2, LEVEL1, TIMESTAMP1); + // visibleWifi3 has the same ssid and bssid as VISIBLE_WIFI2 but different level and + // timestamp. The level and timestamp are excluded from the VisibleWifi hash function. + assertEquals(VISIBLE_WIFI2.hashCode(), visibleWifi3.hashCode()); + } + + @Test + public void testVisibleCellBuilder() { + for (@RadioType int radioType : RADIO_TYPES) { + VisibleCell visibleCell = VisibleCell.builder(radioType).build(); + assertEquals(radioType, visibleCell.radioType()); + } + } + + @Test + public void testVisibleCellEquals() { + VisibleCell copyOfVisibleCell1 = + VisibleCell.builder(VISIBLE_CELL1.radioType()) + .setCellId(VISIBLE_CELL1.cellId()) + .setLocationAreaCode(VISIBLE_CELL1.locationAreaCode()) + .setMobileCountryCode(VISIBLE_CELL1.mobileCountryCode()) + .setMobileNetworkCode(VISIBLE_CELL1.mobileNetworkCode()) + .setTimestamp(VISIBLE_CELL1.timestampMs()) + .build(); + assertNotEquals(VISIBLE_CELL2, VISIBLE_CELL1); + assertEquals(VISIBLE_CELL1, copyOfVisibleCell1); + } + + @Test + public void testVisibleCellEqualsDifferentTimestamp() { + // The timestamp is not included in the VisibleCell equality checks. + assertEquals(VISIBLE_CELL1, VISIBLE_CELL1_DIFFERENT_TIMESTAMP); + } + + @Test + public void testVisibleCellHash() { + VisibleCell copyOfVisibleCell1 = + VisibleCell.builder(VISIBLE_CELL1.radioType()) + .setCellId(VISIBLE_CELL1.cellId()) + .setLocationAreaCode(VISIBLE_CELL1.locationAreaCode()) + .setMobileCountryCode(VISIBLE_CELL1.mobileCountryCode()) + .setMobileNetworkCode(VISIBLE_CELL1.mobileNetworkCode()) + .setTimestamp(VISIBLE_CELL1.timestampMs()) + .build(); + + assertEquals(VISIBLE_CELL1.hashCode(), copyOfVisibleCell1.hashCode()); + assertNotEquals(VISIBLE_CELL2.hashCode(), VISIBLE_CELL1.hashCode()); + } + + @Test + public void testVisibleCellHashDifferentTimestamp() { + // The timestamp is not included in the VisibleCell hash function. + assertEquals(VISIBLE_CELL1.hashCode(), VISIBLE_CELL1_DIFFERENT_TIMESTAMP.hashCode()); + } + + @Test + public void testVisibleNetworksCreate() { + Set<VisibleCell> expectedVisibleCells = + new HashSet<VisibleCell>(Arrays.asList(VISIBLE_CELL1, VISIBLE_CELL2)); + Set<VisibleWifi> expectedVisibleWifis = + new HashSet<VisibleWifi>(Arrays.asList(VISIBLE_WIFI1, VISIBLE_WIFI2)); + VisibleNetworks visibleNetworks = VisibleNetworks.create( + VISIBLE_WIFI1, VISIBLE_CELL1, expectedVisibleWifis, expectedVisibleCells); + assertEquals(VISIBLE_WIFI1, visibleNetworks.connectedWifi()); + assertEquals(VISIBLE_CELL1, visibleNetworks.connectedCell()); + assertEquals(expectedVisibleWifis, visibleNetworks.allVisibleWifis()); + assertEquals(expectedVisibleCells, visibleNetworks.allVisibleCells()); + } + + @Test + public void testVisibleNetworksEquals() { + VisibleNetworks copyOfVisibleNetworks1 = VisibleNetworks.create( + VISIBLE_NETWORKS1.connectedWifi(), VISIBLE_NETWORKS1.connectedCell(), + VISIBLE_NETWORKS1.allVisibleWifis(), VISIBLE_NETWORKS1.allVisibleCells()); + + assertEquals(VISIBLE_NETWORKS1, copyOfVisibleNetworks1); + assertNotEquals(VISIBLE_NETWORKS1, VISIBLE_NETWORKS2); + } + + @Test + public void testVisibleNetworksHash() { + VisibleNetworks copyOfVisibleNetworks1 = VisibleNetworks.create( + VISIBLE_NETWORKS1.connectedWifi(), VISIBLE_NETWORKS1.connectedCell(), + VISIBLE_NETWORKS1.allVisibleWifis(), VISIBLE_NETWORKS1.allVisibleCells()); + + assertEquals(VISIBLE_NETWORKS1.hashCode(), copyOfVisibleNetworks1.hashCode()); + assertNotEquals(VISIBLE_NETWORKS1.hashCode(), VISIBLE_NETWORKS2.hashCode()); + } + + @Test + public void testVisibleNetworksIsEmpty() { + VisibleNetworks visibleNetworks = VisibleNetworks.create(null, null, null, null); + assertTrue(visibleNetworks.isEmpty()); + assertFalse(VISIBLE_NETWORKS1.isEmpty()); + } +} \ No newline at end of file
diff --git a/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc b/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc index c6a2dd4..711eed8 100644 --- a/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc +++ b/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc
@@ -599,19 +599,15 @@ if (error) return; - if (render_frame_host->GetParent() != nullptr) { - // Child frames may send PageLoadMetadata updates, but not PageLoadTiming - // updates. - if (!timing.IsEmpty()) - RecordInternalError(ERR_TIMING_IPC_FROM_SUBFRAME); - committed_load_->UpdateChildFrameMetadata(metadata); - return; + const bool is_main_frame = (render_frame_host->GetParent() == nullptr); + if (is_main_frame) { + committed_load_->UpdateTiming(timing, metadata); + } else { + committed_load_->UpdateSubFrameTiming(render_frame_host, timing, metadata); } - committed_load_->UpdateTiming(timing, metadata); - for (auto& observer : testing_observers_) - observer.OnTimingUpdated(true /* is_main_frame */, timing, metadata); + observer.OnTimingUpdated(is_main_frame, timing, metadata); } bool MetricsWebContentsObserver::ShouldTrackNavigation(
diff --git a/chrome/browser/page_load_metrics/metrics_web_contents_observer_unittest.cc b/chrome/browser/page_load_metrics/metrics_web_contents_observer_unittest.cc index a301058..b8282929 100644 --- a/chrome/browser/page_load_metrics/metrics_web_contents_observer_unittest.cc +++ b/chrome/browser/page_load_metrics/metrics_web_contents_observer_unittest.cc
@@ -38,12 +38,19 @@ // Simple PageLoadMetricsObserver that copies observed PageLoadTimings into the // provided std::vector, so they can be analyzed by unit tests. -class TestPageLoadMetricsObserver : public PageLoadMetricsObserver { +class TestPageLoadMetricsObserver + : public PageLoadMetricsObserver, + public MetricsWebContentsObserver::TestingObserver { public: - TestPageLoadMetricsObserver(std::vector<PageLoadTiming>* updated_timings, - std::vector<PageLoadTiming>* complete_timings, - std::vector<GURL>* observed_committed_urls) - : updated_timings_(updated_timings), + TestPageLoadMetricsObserver( + content::WebContents* web_contents, + std::vector<PageLoadTiming>* updated_timings, + std::vector<PageLoadTiming>* updated_subframe_timings, + std::vector<PageLoadTiming>* complete_timings, + std::vector<GURL>* observed_committed_urls) + : MetricsWebContentsObserver::TestingObserver(web_contents), + updated_timings_(updated_timings), + updated_subframe_timings_(updated_subframe_timings), complete_timings_(complete_timings), observed_committed_urls_(observed_committed_urls) {} @@ -59,6 +66,14 @@ updated_timings_->push_back(timing); } + void OnTimingUpdated(bool is_main_frame, + const PageLoadTiming& timing, + const PageLoadMetadata& metadata) override { + if (!is_main_frame) { + updated_subframe_timings_->push_back(timing); + } + } + void OnComplete(const PageLoadTiming& timing, const PageLoadExtraInfo& extra_info) override { complete_timings_->push_back(timing); @@ -72,6 +87,7 @@ private: std::vector<PageLoadTiming>* const updated_timings_; + std::vector<PageLoadTiming>* const updated_subframe_timings_; std::vector<PageLoadTiming>* const complete_timings_; std::vector<GURL>* const observed_committed_urls_; }; @@ -110,13 +126,16 @@ class TestPageLoadMetricsEmbedderInterface : public PageLoadMetricsEmbedderInterface { public: - TestPageLoadMetricsEmbedderInterface() : is_ntp_(false) {} + explicit TestPageLoadMetricsEmbedderInterface( + content::WebContents* web_contents) + : web_contents_(web_contents), is_ntp_(false) {} bool IsNewTabPageUrl(const GURL& url) override { return is_ntp_; } void set_is_ntp(bool is_ntp) { is_ntp_ = is_ntp; } void RegisterObservers(PageLoadTracker* tracker) override { tracker->AddObserver(base::MakeUnique<TestPageLoadMetricsObserver>( - &updated_timings_, &complete_timings_, &observed_committed_urls_)); + web_contents_, &updated_timings_, &updated_subframe_timings_, + &complete_timings_, &observed_committed_urls_)); tracker->AddObserver(base::MakeUnique<FilteringPageLoadMetricsObserver>( &completed_filtered_urls_)); } @@ -126,6 +145,9 @@ const std::vector<PageLoadTiming>& complete_timings() const { return complete_timings_; } + const std::vector<PageLoadTiming>& updated_subframe_timings() const { + return updated_subframe_timings_; + } // currently_committed_urls passed to OnStart(). const std::vector<GURL>& observed_committed_urls_from_on_start() const { @@ -139,9 +161,11 @@ private: std::vector<PageLoadTiming> updated_timings_; + std::vector<PageLoadTiming> updated_subframe_timings_; std::vector<PageLoadTiming> complete_timings_; std::vector<GURL> observed_committed_urls_; std::vector<GURL> completed_filtered_urls_; + content::WebContents* web_contents_; bool is_ntp_; }; @@ -171,7 +195,8 @@ } void AttachObserver() { - embedder_interface_ = new TestPageLoadMetricsEmbedderInterface(); + embedder_interface_ = + new TestPageLoadMetricsEmbedderInterface(web_contents()); // Owned by the web_contents. Tests must be careful not to call // SimulateTimingUpdate after they call DeleteContents() without also // calling AttachObserver() again. Otherwise they will use-after-free the @@ -203,11 +228,19 @@ return empty; } - int CountCompleteTimingReported() { - return embedder_interface_->complete_timings().size(); + const std::vector<PageLoadTiming>& updated_timings() const { + return embedder_interface_->updated_timings(); } - int CountUpdatedTimingReported() { - return embedder_interface_->updated_timings().size(); + const std::vector<PageLoadTiming>& complete_timings() const { + return embedder_interface_->complete_timings(); + } + const std::vector<PageLoadTiming>& updated_subframe_timings() const { + return embedder_interface_->updated_subframe_timings(); + } + int CountCompleteTimingReported() { return complete_timings().size(); } + int CountUpdatedTimingReported() { return updated_timings().size(); } + int CountUpdatedSubFrameTimingReported() { + return updated_subframe_timings().size(); } const std::vector<GURL>& observed_committed_urls_from_on_start() const { @@ -253,38 +286,45 @@ ASSERT_EQ(kDefaultTestUrl, observed_committed_urls_from_on_start().at(1).spec()); ASSERT_EQ(1, CountUpdatedTimingReported()); + ASSERT_EQ(0, CountUpdatedSubFrameTimingReported()); CheckNoErrorEvents(); } -TEST_F(MetricsWebContentsObserverTest, NotInMainFrame) { +TEST_F(MetricsWebContentsObserverTest, SubFrame) { PageLoadTiming timing; timing.navigation_start = base::Time::FromDoubleT(1); content::WebContentsTester* web_contents_tester = content::WebContentsTester::For(web_contents()); web_contents_tester->NavigateAndCommit(GURL(kDefaultTestUrl)); + SimulateTimingUpdate(timing); content::RenderFrameHostTester* rfh_tester = content::RenderFrameHostTester::For(main_rfh()); content::RenderFrameHost* subframe = rfh_tester->AppendChild("subframe"); + PageLoadTiming subframe_timing; + subframe_timing.navigation_start = base::Time::FromDoubleT(2); content::RenderFrameHostTester* subframe_tester = content::RenderFrameHostTester::For(subframe); subframe_tester->SimulateNavigationStart(GURL(kDefaultTestUrl2)); subframe_tester->SimulateNavigationCommit(GURL(kDefaultTestUrl2)); - SimulateTimingUpdate(timing, subframe); + SimulateTimingUpdate(subframe_timing, subframe); subframe_tester->SimulateNavigationStop(); // Navigate again to see if the timing updated for a subframe message. web_contents_tester->NavigateAndCommit(GURL(kDefaultTestUrl2)); - ASSERT_EQ(0, CountUpdatedTimingReported()); ASSERT_EQ(1, CountCompleteTimingReported()); - ASSERT_EQ(1, CountEmptyCompleteTimingReported()); - CheckErrorEvent(ERR_TIMING_IPC_FROM_SUBFRAME, 1); - CheckErrorEvent(ERR_NO_IPCS_RECEIVED, 1); - CheckTotalErrorEvents(); + ASSERT_EQ(1, CountUpdatedTimingReported()); + ASSERT_EQ(0, CountEmptyCompleteTimingReported()); + EXPECT_EQ(timing, updated_timings().at(0)); + + ASSERT_EQ(1, CountUpdatedSubFrameTimingReported()); + EXPECT_EQ(subframe_timing, updated_subframe_timings().at(0)); + + CheckNoErrorEvents(); } TEST_F(MetricsWebContentsObserverTest, SameDocumentNoTrigger) { @@ -429,29 +469,6 @@ page_load_metrics::internal::INVALID_ORDER_PARSE_START_PARSE_STOP, 1); } -TEST_F(MetricsWebContentsObserverTest, NotInMainError) { - PageLoadTiming timing; - timing.navigation_start = base::Time::FromDoubleT(1); - - content::WebContentsTester* web_contents_tester = - content::WebContentsTester::For(web_contents()); - web_contents_tester->NavigateAndCommit(GURL(kDefaultTestUrl)); - - content::RenderFrameHostTester* rfh_tester = - content::RenderFrameHostTester::For(main_rfh()); - content::RenderFrameHost* subframe = rfh_tester->AppendChild("subframe"); - - content::RenderFrameHostTester* subframe_tester = - content::RenderFrameHostTester::For(subframe); - subframe_tester->SimulateNavigationStart(GURL(kDefaultTestUrl2)); - subframe_tester->SimulateNavigationCommit(GURL(kDefaultTestUrl2)); - SimulateTimingUpdate(timing, subframe); - CheckErrorEvent(ERR_TIMING_IPC_FROM_SUBFRAME, 1); - CheckTotalErrorEvents(); - ASSERT_EQ(0, CountUpdatedTimingReported()); - ASSERT_EQ(0, CountCompleteTimingReported()); -} - TEST_F(MetricsWebContentsObserverTest, BadIPC) { PageLoadTiming timing; timing.navigation_start = base::Time::FromDoubleT(10);
diff --git a/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.h index cb6603a..f51ef0b 100644 --- a/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.h
@@ -38,8 +38,6 @@ const page_load_metrics::PageLoadExtraInfo& info) override; private: - using FrameTreeNodeId = int; - struct AdFrameData { explicit AdFrameData(FrameTreeNodeId frame_tree_node_id); size_t frame_bytes;
diff --git a/chrome/browser/page_load_metrics/observers/amp_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/amp_page_load_metrics_observer.cc index 03af34c..427b68e 100644 --- a/chrome/browser/page_load_metrics/observers/amp_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/amp_page_load_metrics_observer.cc
@@ -94,7 +94,7 @@ timing.document_timing.first_layout.value()); } -void AMPPageLoadMetricsObserver::OnFirstContentfulPaint( +void AMPPageLoadMetricsObserver::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { if (!WasStartedInForegroundOptionalEventInForeground(
diff --git a/chrome/browser/page_load_metrics/observers/amp_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/amp_page_load_metrics_observer.h index 0f0a2d1c..93941a2 100644 --- a/chrome/browser/page_load_metrics/observers/amp_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/amp_page_load_metrics_observer.h
@@ -39,7 +39,7 @@ const page_load_metrics::PageLoadExtraInfo& info) override; void OnFirstLayout(const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; - void OnFirstContentfulPaint( + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; void OnParseStart(const page_load_metrics::PageLoadTiming& timing,
diff --git a/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.cc index a7caecf..4784920 100644 --- a/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.cc
@@ -20,7 +20,7 @@ content::WebContents* web_contents) : web_contents_(web_contents) {} -void AndroidPageLoadMetricsObserver::OnFirstContentfulPaint( +void AndroidPageLoadMetricsObserver::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
diff --git a/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.h index a4677a5..8dd14aa 100644 --- a/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.h
@@ -23,7 +23,7 @@ explicit AndroidPageLoadMetricsObserver(content::WebContents* web_contents); // page_load_metrics::PageLoadMetricsObserver: - void OnFirstContentfulPaint( + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; void OnLoadEventStart(
diff --git a/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc index 35c6bee..fe5d019 100644 --- a/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
@@ -323,7 +323,7 @@ } } -void CorePageLoadMetricsObserver::OnFirstPaint( +void CorePageLoadMetricsObserver::OnFirstPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { first_paint_ = @@ -352,7 +352,7 @@ } } -void CorePageLoadMetricsObserver::OnFirstTextPaint( +void CorePageLoadMetricsObserver::OnFirstTextPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { if (WasStartedInForegroundOptionalEventInForeground( @@ -365,7 +365,7 @@ } } -void CorePageLoadMetricsObserver::OnFirstImagePaint( +void CorePageLoadMetricsObserver::OnFirstImagePaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { if (WasStartedInForegroundOptionalEventInForeground( @@ -378,7 +378,7 @@ } } -void CorePageLoadMetricsObserver::OnFirstContentfulPaint( +void CorePageLoadMetricsObserver::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { if (WasStartedInForegroundOptionalEventInForeground( @@ -469,7 +469,7 @@ } } -void CorePageLoadMetricsObserver::OnFirstMeaningfulPaint( +void CorePageLoadMetricsObserver::OnFirstMeaningfulPaintInMainFrameDocument( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { base::TimeTicks paint = info.navigation_start +
diff --git a/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.h index da546785..ccae5a7 100644 --- a/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.h
@@ -103,19 +103,19 @@ void OnFirstLayout( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstPaint( + void OnFirstPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstTextPaint( + void OnFirstTextPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstImagePaint( + void OnFirstImagePaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstContentfulPaint( + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstMeaningfulPaint( + void OnFirstMeaningfulPaintInMainFrameDocument( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; void OnParseStart(
diff --git a/chrome/browser/page_load_metrics/observers/css_scanning_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/css_scanning_page_load_metrics_observer.cc index 8dc995c..dde5c54 100644 --- a/chrome/browser/page_load_metrics/observers/css_scanning_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/css_scanning_page_load_metrics_observer.cc
@@ -35,7 +35,7 @@ return STOP_OBSERVING; } -void CssScanningMetricsObserver::OnFirstContentfulPaint( +void CssScanningMetricsObserver::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { if (!css_preload_found_) @@ -48,7 +48,7 @@ timing.parse_timing.parse_start.value()); } -void CssScanningMetricsObserver::OnFirstMeaningfulPaint( +void CssScanningMetricsObserver::OnFirstMeaningfulPaintInMainFrameDocument( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { if (!css_preload_found_)
diff --git a/chrome/browser/page_load_metrics/observers/css_scanning_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/css_scanning_page_load_metrics_observer.h index 93f652a..d8a6f15 100644 --- a/chrome/browser/page_load_metrics/observers/css_scanning_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/css_scanning_page_load_metrics_observer.h
@@ -23,10 +23,10 @@ const page_load_metrics::PageLoadExtraInfo& info) override; void OnLoadingBehaviorObserved( const page_load_metrics::PageLoadExtraInfo& info) override; - void OnFirstContentfulPaint( + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; - void OnFirstMeaningfulPaint( + void OnFirstMeaningfulPaintInMainFrameDocument( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override;
diff --git a/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc index d1fbb2f..0d70e1a8 100644 --- a/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc
@@ -354,7 +354,7 @@ internal::kHistogramFirstLayoutSuffix); } -void DataReductionProxyMetricsObserver::OnFirstPaint( +void DataReductionProxyMetricsObserver::OnFirstPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { RECORD_FOREGROUND_HISTOGRAMS_FOR_SUFFIX(info, data_, @@ -362,7 +362,7 @@ internal::kHistogramFirstPaintSuffix); } -void DataReductionProxyMetricsObserver::OnFirstTextPaint( +void DataReductionProxyMetricsObserver::OnFirstTextPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { RECORD_FOREGROUND_HISTOGRAMS_FOR_SUFFIX( @@ -370,7 +370,7 @@ internal::kHistogramFirstTextPaintSuffix); } -void DataReductionProxyMetricsObserver::OnFirstImagePaint( +void DataReductionProxyMetricsObserver::OnFirstImagePaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { RECORD_FOREGROUND_HISTOGRAMS_FOR_SUFFIX( @@ -378,7 +378,7 @@ internal::kHistogramFirstImagePaintSuffix); } -void DataReductionProxyMetricsObserver::OnFirstContentfulPaint( +void DataReductionProxyMetricsObserver::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { RECORD_FOREGROUND_HISTOGRAMS_FOR_SUFFIX( @@ -386,9 +386,10 @@ internal::kHistogramFirstContentfulPaintSuffix); } -void DataReductionProxyMetricsObserver::OnFirstMeaningfulPaint( - const page_load_metrics::PageLoadTiming& timing, - const page_load_metrics::PageLoadExtraInfo& info) { +void DataReductionProxyMetricsObserver:: + OnFirstMeaningfulPaintInMainFrameDocument( + const page_load_metrics::PageLoadTiming& timing, + const page_load_metrics::PageLoadExtraInfo& info) { RECORD_FOREGROUND_HISTOGRAMS_FOR_SUFFIX( info, data_, timing.paint_timing.first_meaningful_paint, internal::kHistogramFirstMeaningfulPaintSuffix);
diff --git a/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.h b/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.h index 338cb68c..620fb968 100644 --- a/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.h
@@ -86,18 +86,19 @@ const page_load_metrics::PageLoadExtraInfo& info) override; void OnFirstLayout(const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; - void OnFirstPaint(const page_load_metrics::PageLoadTiming& timing, - const page_load_metrics::PageLoadExtraInfo& info) override; - void OnFirstTextPaint( + void OnFirstPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; - void OnFirstImagePaint( + void OnFirstTextPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; - void OnFirstContentfulPaint( + void OnFirstImagePaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; - void OnFirstMeaningfulPaint( + void OnFirstContentfulPaintInPage( + const page_load_metrics::PageLoadTiming& timing, + const page_load_metrics::PageLoadExtraInfo& info) override; + void OnFirstMeaningfulPaintInMainFrameDocument( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; void OnParseStart(const page_load_metrics::PageLoadTiming& timing,
diff --git a/chrome/browser/page_load_metrics/observers/delay_navigation_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/delay_navigation_page_load_metrics_observer.cc index 63f1e3e..d862c31 100644 --- a/chrome/browser/page_load_metrics/observers/delay_navigation_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/delay_navigation_page_load_metrics_observer.cc
@@ -40,7 +40,7 @@ actual_delay_ = actual_delay; } -void DelayNavigationPageLoadMetricsObserver::OnFirstPaint( +void DelayNavigationPageLoadMetricsObserver::OnFirstPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { if (!delay_navigation_)
diff --git a/chrome/browser/page_load_metrics/observers/delay_navigation_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/delay_navigation_page_load_metrics_observer.h index 45c6002f..e0d32f86 100644 --- a/chrome/browser/page_load_metrics/observers/delay_navigation_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/delay_navigation_page_load_metrics_observer.h
@@ -43,8 +43,9 @@ const page_load_metrics::PageLoadExtraInfo& info) override; void OnNavigationDelayComplete(base::TimeDelta scheduled_delay, base::TimeDelta actual_delay) override; - void OnFirstPaint(const page_load_metrics::PageLoadTiming& timing, - const page_load_metrics::PageLoadExtraInfo& info) override; + void OnFirstPaintInPage( + const page_load_metrics::PageLoadTiming& timing, + const page_load_metrics::PageLoadExtraInfo& info) override; private: bool delay_navigation_ = false;
diff --git a/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc index 8d87021..3a2189c 100644 --- a/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc
@@ -76,7 +76,7 @@ } // namespace internal -void DocumentWritePageLoadMetricsObserver::OnFirstContentfulPaint( +void DocumentWritePageLoadMetricsObserver::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { if (info.main_frame_metadata.behavior_flags & @@ -90,9 +90,10 @@ } } -void DocumentWritePageLoadMetricsObserver::OnFirstMeaningfulPaint( - const page_load_metrics::PageLoadTiming& timing, - const page_load_metrics::PageLoadExtraInfo& info) { +void DocumentWritePageLoadMetricsObserver:: + OnFirstMeaningfulPaintInMainFrameDocument( + const page_load_metrics::PageLoadTiming& timing, + const page_load_metrics::PageLoadExtraInfo& info) { if (info.main_frame_metadata.behavior_flags & blink::WebLoadingBehaviorFlag:: kWebLoadingBehaviorDocumentWriteEvaluator) {
diff --git a/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.h index ff534cf..5e8b65c1 100644 --- a/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.h
@@ -24,11 +24,11 @@ DocumentWritePageLoadMetricsObserver() = default; // page_load_metrics::PageLoadMetricsObserver implementation: - void OnFirstContentfulPaint( + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstMeaningfulPaint( + void OnFirstMeaningfulPaintInMainFrameDocument( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override;
diff --git a/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc index c39d1c9..c11c958 100644 --- a/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc
@@ -521,28 +521,28 @@ logger_.OnLoadEventStart(timing, extra_info); } -void FromGWSPageLoadMetricsObserver::OnFirstPaint( +void FromGWSPageLoadMetricsObserver::OnFirstPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) { - logger_.OnFirstPaint(timing, extra_info); + logger_.OnFirstPaintInPage(timing, extra_info); } -void FromGWSPageLoadMetricsObserver::OnFirstTextPaint( +void FromGWSPageLoadMetricsObserver::OnFirstTextPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) { - logger_.OnFirstTextPaint(timing, extra_info); + logger_.OnFirstTextPaintInPage(timing, extra_info); } -void FromGWSPageLoadMetricsObserver::OnFirstImagePaint( +void FromGWSPageLoadMetricsObserver::OnFirstImagePaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) { - logger_.OnFirstImagePaint(timing, extra_info); + logger_.OnFirstImagePaintInPage(timing, extra_info); } -void FromGWSPageLoadMetricsObserver::OnFirstContentfulPaint( +void FromGWSPageLoadMetricsObserver::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) { - logger_.OnFirstContentfulPaint(timing, extra_info); + logger_.OnFirstContentfulPaintInPage(timing, extra_info); } void FromGWSPageLoadMetricsObserver::OnParseStart( @@ -692,7 +692,7 @@ } } -void FromGWSPageLoadMetricsLogger::OnFirstPaint( +void FromGWSPageLoadMetricsLogger::OnFirstPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) { if (ShouldLogForegroundEventAfterCommit(timing.paint_timing.first_paint, @@ -703,7 +703,7 @@ first_paint_triggered_ = true; } -void FromGWSPageLoadMetricsLogger::OnFirstTextPaint( +void FromGWSPageLoadMetricsLogger::OnFirstTextPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) { if (ShouldLogForegroundEventAfterCommit(timing.paint_timing.first_text_paint, @@ -713,7 +713,7 @@ } } -void FromGWSPageLoadMetricsLogger::OnFirstImagePaint( +void FromGWSPageLoadMetricsLogger::OnFirstImagePaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) { if (ShouldLogForegroundEventAfterCommit(timing.paint_timing.first_image_paint, @@ -723,7 +723,7 @@ } } -void FromGWSPageLoadMetricsLogger::OnFirstContentfulPaint( +void FromGWSPageLoadMetricsLogger::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) { if (ShouldLogForegroundEventAfterCommit(
diff --git a/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h index 9f961807..8dbe8ee98 100644 --- a/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h
@@ -73,14 +73,16 @@ const page_load_metrics::PageLoadExtraInfo& extra_info); void OnLoadEventStart(const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info); - void OnFirstPaint(const page_load_metrics::PageLoadTiming& timing, - const page_load_metrics::PageLoadExtraInfo& extra_info); - void OnFirstTextPaint(const page_load_metrics::PageLoadTiming& timing, - const page_load_metrics::PageLoadExtraInfo& extra_info); - void OnFirstImagePaint( + void OnFirstPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info); - void OnFirstContentfulPaint( + void OnFirstTextPaintInPage( + const page_load_metrics::PageLoadTiming& timing, + const page_load_metrics::PageLoadExtraInfo& extra_info); + void OnFirstImagePaintInPage( + const page_load_metrics::PageLoadTiming& timing, + const page_load_metrics::PageLoadExtraInfo& extra_info); + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info); void OnParseStart(const page_load_metrics::PageLoadTiming& timing, @@ -160,16 +162,16 @@ void OnLoadEventStart( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstPaint( + void OnFirstPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstTextPaint( + void OnFirstTextPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstImagePaint( + void OnFirstImagePaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstContentfulPaint( + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; void OnParseStart(
diff --git a/chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.cc index 92564c97..8da822e 100644 --- a/chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.cc
@@ -43,7 +43,7 @@ return CONTINUE_OBSERVING; } -void NoStatePrefetchPageLoadMetricsObserver::OnFirstContentfulPaint( +void NoStatePrefetchPageLoadMetricsObserver::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) { DCHECK(timing.paint_timing.first_contentful_paint.has_value());
diff --git a/chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.h index 0fdd257..7b67310d 100644 --- a/chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.h
@@ -45,7 +45,7 @@ private: // page_load_metrics::PageLoadMetricsObserver: ObservePolicy OnCommit(content::NavigationHandle* navigation_handle) override; - void OnFirstContentfulPaint( + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; ObservePolicy OnHidden(
diff --git a/chrome/browser/page_load_metrics/observers/omnibox_suggestion_used_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/omnibox_suggestion_used_page_load_metrics_observer.cc index df91dc07..66a311d 100644 --- a/chrome/browser/page_load_metrics/observers/omnibox_suggestion_used_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/omnibox_suggestion_used_page_load_metrics_observer.cc
@@ -60,7 +60,7 @@ : STOP_OBSERVING; } -void OmniboxSuggestionUsedMetricsObserver::OnFirstContentfulPaint( +void OmniboxSuggestionUsedMetricsObserver::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { base::TimeDelta fcp = timing.paint_timing.first_contentful_paint.value(); @@ -95,9 +95,10 @@ } } -void OmniboxSuggestionUsedMetricsObserver::OnFirstMeaningfulPaint( - const page_load_metrics::PageLoadTiming& timing, - const page_load_metrics::PageLoadExtraInfo& info) { +void OmniboxSuggestionUsedMetricsObserver:: + OnFirstMeaningfulPaintInMainFrameDocument( + const page_load_metrics::PageLoadTiming& timing, + const page_load_metrics::PageLoadExtraInfo& info) { base::TimeDelta fmp = timing.paint_timing.first_meaningful_paint.value(); if (info.started_in_foreground) {
diff --git a/chrome/browser/page_load_metrics/observers/omnibox_suggestion_used_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/omnibox_suggestion_used_page_load_metrics_observer.h index 073a09a..b36e038 100644 --- a/chrome/browser/page_load_metrics/observers/omnibox_suggestion_used_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/omnibox_suggestion_used_page_load_metrics_observer.h
@@ -20,10 +20,10 @@ const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; ObservePolicy OnCommit(content::NavigationHandle* navigation_handle) override; - void OnFirstContentfulPaint( + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; - void OnFirstMeaningfulPaint( + void OnFirstMeaningfulPaintInMainFrameDocument( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override;
diff --git a/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.cc index 99f18ab8..43d4d0d 100644 --- a/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.cc
@@ -61,7 +61,7 @@ return CONTINUE_OBSERVING; } -void PrerenderPageLoadMetricsObserver::OnFirstContentfulPaint( +void PrerenderPageLoadMetricsObserver::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) { DCHECK(!start_ticks_.is_null());
diff --git a/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.h index 1db7261..74b26fa9 100644 --- a/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.h
@@ -39,7 +39,7 @@ const GURL& currently_committed_url, bool started_in_foreground) override; ObservePolicy OnCommit(content::NavigationHandle* navigation_handle) override; - void OnFirstContentfulPaint( + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; ObservePolicy OnHidden(
diff --git a/chrome/browser/page_load_metrics/observers/previews_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/previews_page_load_metrics_observer.cc index c30bbeb..0a4f0d7 100644 --- a/chrome/browser/page_load_metrics/observers/previews_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/previews_page_load_metrics_observer.cc
@@ -94,7 +94,7 @@ timing.document_timing.first_layout.value()); } -void PreviewsPageLoadMetricsObserver::OnFirstContentfulPaint( +void PreviewsPageLoadMetricsObserver::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { if (!WasStartedInForegroundOptionalEventInForeground(
diff --git a/chrome/browser/page_load_metrics/observers/previews_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/previews_page_load_metrics_observer.h index 18063b6..4104de1 100644 --- a/chrome/browser/page_load_metrics/observers/previews_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/previews_page_load_metrics_observer.h
@@ -51,7 +51,7 @@ const page_load_metrics::PageLoadExtraInfo& info) override; void OnFirstLayout(const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; - void OnFirstContentfulPaint( + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; void OnParseStart(const page_load_metrics::PageLoadTiming& timing,
diff --git a/chrome/browser/page_load_metrics/observers/protocol_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/protocol_page_load_metrics_observer.cc index d121c7f..fbd5546 100644 --- a/chrome/browser/page_load_metrics/observers/protocol_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/protocol_page_load_metrics_observer.cc
@@ -68,7 +68,7 @@ } } -void ProtocolPageLoadMetricsObserver::OnFirstContentfulPaint( +void ProtocolPageLoadMetricsObserver::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) { switch (connection_info_) { @@ -126,7 +126,7 @@ } } -void ProtocolPageLoadMetricsObserver::OnFirstMeaningfulPaint( +void ProtocolPageLoadMetricsObserver::OnFirstMeaningfulPaintInMainFrameDocument( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) { switch (connection_info_) {
diff --git a/chrome/browser/page_load_metrics/observers/protocol_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/protocol_page_load_metrics_observer.h index 369d8d1..4d0726c 100644 --- a/chrome/browser/page_load_metrics/observers/protocol_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/protocol_page_load_metrics_observer.h
@@ -25,10 +25,10 @@ void OnParseStart( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstContentfulPaint( + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstMeaningfulPaint( + void OnFirstMeaningfulPaintInMainFrameDocument( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; void OnDomContentLoadedEventStart(
diff --git a/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.cc index cf04c79..e0c4309 100644 --- a/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.cc
@@ -70,9 +70,10 @@ return CONTINUE_OBSERVING; } -void ResourcePrefetchPredictorPageLoadMetricsObserver::OnFirstContentfulPaint( - const page_load_metrics::PageLoadTiming& timing, - const page_load_metrics::PageLoadExtraInfo& extra_info) { +void ResourcePrefetchPredictorPageLoadMetricsObserver:: + OnFirstContentfulPaintInPage( + const page_load_metrics::PageLoadTiming& timing, + const page_load_metrics::PageLoadExtraInfo& extra_info) { predictors::NavigationID navigation_id(web_contents_); predictor_->RecordFirstContentfulPaint( @@ -85,9 +86,10 @@ } } -void ResourcePrefetchPredictorPageLoadMetricsObserver::OnFirstMeaningfulPaint( - const page_load_metrics::PageLoadTiming& timing, - const page_load_metrics::PageLoadExtraInfo& extra_info) { +void ResourcePrefetchPredictorPageLoadMetricsObserver:: + OnFirstMeaningfulPaintInMainFrameDocument( + const page_load_metrics::PageLoadTiming& timing, + const page_load_metrics::PageLoadExtraInfo& extra_info) { if (record_histograms_) { PAGE_LOAD_HISTOGRAM( internal::kHistogramResourcePrefetchPredictorFirstMeaningfulPaint,
diff --git a/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.h index 125ff8c8..121b0a89 100644 --- a/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.h
@@ -47,10 +47,10 @@ ObservePolicy OnHidden( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstContentfulPaint( + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstMeaningfulPaint( + void OnFirstMeaningfulPaintInMainFrameDocument( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override;
diff --git a/chrome/browser/page_load_metrics/observers/service_worker_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/service_worker_page_load_metrics_observer.cc index bc19236..cc71145 100644 --- a/chrome/browser/page_load_metrics/observers/service_worker_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/service_worker_page_load_metrics_observer.cc
@@ -61,7 +61,7 @@ ServiceWorkerPageLoadMetricsObserver::ServiceWorkerPageLoadMetricsObserver() {} -void ServiceWorkerPageLoadMetricsObserver::OnFirstContentfulPaint( +void ServiceWorkerPageLoadMetricsObserver::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { if (!IsServiceWorkerControlled(info))
diff --git a/chrome/browser/page_load_metrics/observers/service_worker_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/service_worker_page_load_metrics_observer.h index d0e9c2f..b35d2b5 100644 --- a/chrome/browser/page_load_metrics/observers/service_worker_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/service_worker_page_load_metrics_observer.h
@@ -34,7 +34,7 @@ void OnParseStart( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; - void OnFirstContentfulPaint( + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; void OnDomContentLoadedEventStart(
diff --git a/chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.cc index 2894575..b8a6815e 100644 --- a/chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.cc
@@ -212,7 +212,7 @@ .value()); } -void SubresourceFilterMetricsObserver::OnFirstContentfulPaint( +void SubresourceFilterMetricsObserver::OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) { if (!subresource_filter_observed_) @@ -230,9 +230,10 @@ } } -void SubresourceFilterMetricsObserver::OnFirstMeaningfulPaint( - const page_load_metrics::PageLoadTiming& timing, - const page_load_metrics::PageLoadExtraInfo& info) { +void SubresourceFilterMetricsObserver:: + OnFirstMeaningfulPaintInMainFrameDocument( + const page_load_metrics::PageLoadTiming& timing, + const page_load_metrics::PageLoadExtraInfo& info) { if (!subresource_filter_observed_) return;
diff --git a/chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.h b/chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.h index f6301ab..0d0eb913 100644 --- a/chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.h
@@ -74,10 +74,10 @@ extra_request_complete_info) override; void OnParseStop(const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; - void OnFirstContentfulPaint( + void OnFirstContentfulPaintInPage( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; - void OnFirstMeaningfulPaint( + void OnFirstMeaningfulPaintInMainFrameDocument( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& info) override; void OnDomContentLoadedEventStart(
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc b/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc index 0ad8fbf..879aad45 100644 --- a/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc +++ b/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
@@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/command_line.h" +#include "base/feature_list.h" #include "base/files/scoped_temp_dir.h" #include "base/macros.h" #include "base/test/histogram_tester.h" @@ -28,9 +30,12 @@ #include "content/public/browser/browser_thread.h" #include "content/public/browser/render_process_host.h" #include "content/public/browser/render_view_host.h" +#include "content/public/common/content_features.h" +#include "content/public/common/content_switches.h" #include "content/public/test/browser_test_utils.h" #include "content/public/test/download_test_observer.h" #include "net/base/net_errors.h" +#include "net/dns/mock_host_resolver.h" #include "net/http/failing_http_transaction_factory.h" #include "net/http/http_cache.h" #include "net/test/embedded_test_server/embedded_test_server.h" @@ -185,6 +190,17 @@ TimingFieldBitSet observed_main_frame_fields_; }; +// Due to crbug/705315, paints in subframes are associated with the main frame, +// unless the subframe is cross-origin and Chrome is running with out of process +// cross-origin subframes. As a result, some tests wait for different behavior +// to be observed depending on which mode we are in. TODO(crbug/705315): remove +// this method once the bug is addressed. +static bool AreCrossOriginSubFramesOutOfProcess() { + return base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kSitePerProcess) || + base::FeatureList::IsEnabled(features::kTopDocumentIsolation); +} + using TimingField = PageLoadMetricsWaiter::TimingField; } // namespace @@ -203,6 +219,15 @@ ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL)); } + // TODO(crbug/705315): remove this method once the bug is addressed. + void SetUpOnMainThread() override { + InProcessBrowserTest::SetUpOnMainThread(); + host_resolver()->AddRule("a.com", "127.0.0.1"); + host_resolver()->AddRule("b.com", "127.0.0.1"); + host_resolver()->AddRule("c.com", "127.0.0.1"); + content::SetupCrossSiteRedirector(embedded_test_server()); + } + bool NoPageLoadMetricsRecorded() { // Determine whether any 'public' page load metrics are recorded. We exclude // 'internal' metrics as these may be recorded for debugging purposes. @@ -280,6 +305,109 @@ 0); } +IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, + NoPaintForEmptyDocumentInChildFrame) { + ASSERT_TRUE(embedded_test_server()->Start()); + + // TODO(crbug/705315): remove the a.com domain once the bug is addressed. + GURL a_url(embedded_test_server()->GetURL( + "a.com", "/page_load_metrics/empty_iframe.html")); + + auto waiter = CreatePageLoadMetricsWaiter(); + waiter->AddMainFrameExpectation(TimingField::FIRST_LAYOUT); + waiter->AddMainFrameExpectation(TimingField::LOAD_EVENT); + waiter->AddSubFrameExpectation(TimingField::FIRST_LAYOUT); + waiter->AddSubFrameExpectation(TimingField::LOAD_EVENT); + ui_test_utils::NavigateToURL(browser(), a_url); + waiter->Wait(); + EXPECT_FALSE(waiter->DidObserveInMainFrame(TimingField::FIRST_PAINT)); + + histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 1); + histogram_tester_.ExpectTotalCount(internal::kHistogramLoad, 1); + histogram_tester_.ExpectTotalCount(internal::kHistogramFirstPaint, 0); + histogram_tester_.ExpectTotalCount(internal::kHistogramFirstContentfulPaint, + 0); +} + +IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, PaintInChildFrame) { + ASSERT_TRUE(embedded_test_server()->Start()); + + // TODO(crbug/705315): remove the a.com domain once the bug is addressed. + GURL a_url(embedded_test_server()->GetURL("a.com", + "/page_load_metrics/iframe.html")); + auto waiter = CreatePageLoadMetricsWaiter(); + waiter->AddMainFrameExpectation(TimingField::FIRST_LAYOUT); + waiter->AddMainFrameExpectation(TimingField::LOAD_EVENT); + // TODO(crbug/705315): Once the bug is fixed, remove the else case and make + // the if case the default behavior. + if (AreCrossOriginSubFramesOutOfProcess()) { + waiter->AddSubFrameExpectation(TimingField::FIRST_PAINT); + waiter->AddSubFrameExpectation(TimingField::FIRST_CONTENTFUL_PAINT); + } else { + waiter->AddMainFrameExpectation(TimingField::FIRST_PAINT); + waiter->AddMainFrameExpectation(TimingField::FIRST_CONTENTFUL_PAINT); + } + ui_test_utils::NavigateToURL(browser(), a_url); + waiter->Wait(); + + histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 1); + histogram_tester_.ExpectTotalCount(internal::kHistogramLoad, 1); + histogram_tester_.ExpectTotalCount(internal::kHistogramFirstPaint, 1); +} + +IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, PaintInMultipleChildFrames) { + ASSERT_TRUE(embedded_test_server()->Start()); + + // TODO(crbug/705315): remove the a.com domain once the bug is addressed. + GURL a_url(embedded_test_server()->GetURL("a.com", + "/page_load_metrics/iframes.html")); + + auto waiter = CreatePageLoadMetricsWaiter(); + waiter->AddMainFrameExpectation(TimingField::FIRST_LAYOUT); + waiter->AddMainFrameExpectation(TimingField::LOAD_EVENT); + // TODO(crbug/705315): Once the bug is fixed, remove the else case and make + // the if case the default behavior. + if (AreCrossOriginSubFramesOutOfProcess()) { + waiter->AddSubFrameExpectation(TimingField::FIRST_PAINT); + waiter->AddSubFrameExpectation(TimingField::FIRST_CONTENTFUL_PAINT); + } else { + waiter->AddMainFrameExpectation(TimingField::FIRST_PAINT); + waiter->AddMainFrameExpectation(TimingField::FIRST_CONTENTFUL_PAINT); + } + ui_test_utils::NavigateToURL(browser(), a_url); + waiter->Wait(); + + histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 1); + histogram_tester_.ExpectTotalCount(internal::kHistogramLoad, 1); + histogram_tester_.ExpectTotalCount(internal::kHistogramFirstPaint, 1); +} + +IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, PaintInMainAndChildFrame) { + ASSERT_TRUE(embedded_test_server()->Start()); + + // TODO(crbug/705315): remove the a.com domain once the bug is addressed. + GURL a_url(embedded_test_server()->GetURL( + "a.com", "/page_load_metrics/main_frame_with_iframe.html")); + + auto waiter = CreatePageLoadMetricsWaiter(); + waiter->AddMainFrameExpectation(TimingField::FIRST_LAYOUT); + waiter->AddMainFrameExpectation(TimingField::LOAD_EVENT); + waiter->AddMainFrameExpectation(TimingField::FIRST_PAINT); + waiter->AddMainFrameExpectation(TimingField::FIRST_CONTENTFUL_PAINT); + // TODO(crbug/705315): Once the bug is fixed, make the if case the default + // behavior. + if (AreCrossOriginSubFramesOutOfProcess()) { + waiter->AddSubFrameExpectation(TimingField::FIRST_PAINT); + waiter->AddSubFrameExpectation(TimingField::FIRST_CONTENTFUL_PAINT); + } + ui_test_utils::NavigateToURL(browser(), a_url); + waiter->Wait(); + + histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 1); + histogram_tester_.ExpectTotalCount(internal::kHistogramLoad, 1); + histogram_tester_.ExpectTotalCount(internal::kHistogramFirstPaint, 1); +} + IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, SameDocumentNavigation) { ASSERT_TRUE(embedded_test_server()->Start());
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/page_load_metrics_observer.cc index 997e7f9f..181c30de 100644 --- a/chrome/browser/page_load_metrics/page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/page_load_metrics_observer.cc
@@ -21,7 +21,7 @@ UserInitiatedInfo page_end_user_initiated_info, const base::Optional<base::TimeDelta>& page_end_time, const PageLoadMetadata& main_frame_metadata, - const PageLoadMetadata& child_frame_metadata) + const PageLoadMetadata& subframe_metadata) : navigation_start(navigation_start), first_background_time(first_background_time), first_foreground_time(first_foreground_time), @@ -34,7 +34,7 @@ page_end_user_initiated_info(page_end_user_initiated_info), page_end_time(page_end_time), main_frame_metadata(main_frame_metadata), - child_frame_metadata(child_frame_metadata) {} + subframe_metadata(subframe_metadata) {} PageLoadExtraInfo::PageLoadExtraInfo(const PageLoadExtraInfo& other) = default;
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_observer.h b/chrome/browser/page_load_metrics/page_load_metrics_observer.h index c84787a4..3e12c65 100644 --- a/chrome/browser/page_load_metrics/page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/page_load_metrics_observer.h
@@ -123,7 +123,7 @@ UserInitiatedInfo page_end_user_initiated_info, const base::Optional<base::TimeDelta>& page_end_time, const PageLoadMetadata& main_frame_metadata, - const PageLoadMetadata& child_frame_metadata); + const PageLoadMetadata& subframe_metadata); // Simplified version of the constructor, intended for use in tests. static PageLoadExtraInfo CreateForTesting(const GURL& url, @@ -194,8 +194,8 @@ // renderer for the main frame. const PageLoadMetadata main_frame_metadata; - // PageLoadMetadata for child frames of the current page load. - const PageLoadMetadata child_frame_metadata; + // PageLoadMetadata for subframes of the current page load. + const PageLoadMetadata subframe_metadata; }; // Container for various information about a completed request within a page @@ -270,6 +270,8 @@ STOP_OBSERVING, }; + using FrameTreeNodeId = int; + virtual ~PageLoadMetricsObserver() {} // The page load started, with the given navigation handle. @@ -335,6 +337,7 @@ // loading-dev@chromium.org if you intend to override this method. virtual void OnTimingUpdate(const PageLoadTiming& timing, const PageLoadExtraInfo& extra_info) {} + // OnUserInput is triggered when a new user input is passed in to // web_contents. Contains a TimeDelta from navigation start. virtual void OnUserInput(const blink::WebInputEvent& event) {} @@ -348,23 +351,31 @@ const PageLoadExtraInfo& extra_info) {} virtual void OnFirstLayout(const PageLoadTiming& timing, const PageLoadExtraInfo& extra_info) {} - virtual void OnFirstPaint(const PageLoadTiming& timing, - const PageLoadExtraInfo& extra_info) {} - virtual void OnFirstTextPaint(const PageLoadTiming& timing, - const PageLoadExtraInfo& extra_info) {} - virtual void OnFirstImagePaint(const PageLoadTiming& timing, - const PageLoadExtraInfo& extra_info) {} - virtual void OnFirstContentfulPaint(const PageLoadTiming& timing, - const PageLoadExtraInfo& extra_info) {} - virtual void OnFirstMeaningfulPaint(const PageLoadTiming& timing, - const PageLoadExtraInfo& extra_info) {} virtual void OnParseStart(const PageLoadTiming& timing, const PageLoadExtraInfo& extra_info) {} virtual void OnParseStop(const PageLoadTiming& timing, const PageLoadExtraInfo& extra_info) {} + // On*PaintInPage(...) are invoked when the first relevant paint in the page, + // across all frames, is observed. + virtual void OnFirstPaintInPage(const PageLoadTiming& timing, + const PageLoadExtraInfo& extra_info) {} + virtual void OnFirstTextPaintInPage(const PageLoadTiming& timing, + const PageLoadExtraInfo& extra_info) {} + virtual void OnFirstImagePaintInPage(const PageLoadTiming& timing, + const PageLoadExtraInfo& extra_info) {} + virtual void OnFirstContentfulPaintInPage( + const PageLoadTiming& timing, + const PageLoadExtraInfo& extra_info) {} + + // Unlike other paint callbacks, OnFirstMeaningfulPaintInMainFrameDocument is + // tracked per document, and is reported for the main frame document only. + virtual void OnFirstMeaningfulPaintInMainFrameDocument( + const PageLoadTiming& timing, + const PageLoadExtraInfo& extra_info) {} + // Invoked when there is a change in either the main_frame_metadata or the - // child_frame_metadata's loading behavior_flags. + // subframe_metadata's loading behavior_flags. virtual void OnLoadingBehaviorObserved( const page_load_metrics::PageLoadExtraInfo& extra_info) {}
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_util.cc b/chrome/browser/page_load_metrics/page_load_metrics_util.cc index c46b912d..aba5131a 100644 --- a/chrome/browser/page_load_metrics/page_load_metrics_util.cc +++ b/chrome/browser/page_load_metrics/page_load_metrics_util.cc
@@ -108,7 +108,7 @@ blink::WebLoadingBehaviorFlag behavior) { const int all_frame_loading_behavior_flags = info.main_frame_metadata.behavior_flags | - info.child_frame_metadata.behavior_flags; + info.subframe_metadata.behavior_flags; return (all_frame_loading_behavior_flags & behavior) != 0; }
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_util.h b/chrome/browser/page_load_metrics/page_load_metrics_util.h index b166400..a0af192 100644 --- a/chrome/browser/page_load_metrics/page_load_metrics_util.h +++ b/chrome/browser/page_load_metrics/page_load_metrics_util.h
@@ -126,7 +126,7 @@ const base::Optional<base::TimeDelta>& b); // Whether the given loading behavior was observed in any frame (either the main -// frame or a child frame). +// frame or a subframe). bool DidObserveLoadingBehaviorInAnyFrame( const page_load_metrics::PageLoadExtraInfo& info, blink::WebLoadingBehaviorFlag behavior);
diff --git a/chrome/browser/page_load_metrics/page_load_tracker.cc b/chrome/browser/page_load_metrics/page_load_tracker.cc index 07390cb..77d31f6 100644 --- a/chrome/browser/page_load_metrics/page_load_tracker.cc +++ b/chrome/browser/page_load_metrics/page_load_tracker.cc
@@ -12,12 +12,14 @@ #include "base/logging.h" #include "base/memory/ptr_util.h" #include "base/metrics/histogram_macros.h" +#include "chrome/browser/page_load_metrics/browser_page_track_decider.h" #include "chrome/browser/page_load_metrics/page_load_metrics_embedder_interface.h" #include "chrome/browser/page_load_metrics/page_load_metrics_util.h" #include "chrome/browser/prerender/prerender_contents.h" #include "chrome/common/page_load_metrics/page_load_timing.h" #include "content/public/browser/navigation_details.h" #include "content/public/browser/navigation_handle.h" +#include "content/public/browser/render_frame_host.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_observer.h" #include "content/public/common/browser_side_navigation_policy.h" @@ -277,6 +279,41 @@ return internal::VALID; } +// Updates *|inout_existing_value| with |optional_candidate_new_value|, if +// either *|inout_existing_value| isn't set, or |optional_candidate_new_value| < +// |inout_existing_value|. +void MaybeUpdateTimeDelta( + base::Optional<base::TimeDelta>* inout_existing_value, + base::TimeDelta navigation_start_offset, + const base::Optional<base::TimeDelta>& optional_candidate_new_value) { + // If we don't get a new value, there's nothing to do + if (!optional_candidate_new_value) + return; + + // optional_candidate_new_value is relative to navigation start in its + // frame. We need to adjust it to be relative to navigation start in the main + // frame, so offset it by navigation_start_offset. + base::TimeDelta candidate_new_value = + navigation_start_offset + optional_candidate_new_value.value(); + + if (*inout_existing_value) { + // If we have a new value, but it is after the existing value, then keep the + // existing value. + if (*inout_existing_value <= candidate_new_value) + return; + + // We received a new timing event, but with a timestamp before the timestamp + // of a timing update we had received previously. We expect this to happen + // occasionally, as inter-frame updates can arrive out of order. Record a + // histogram to track how frequently it happens, along with the magnitude + // of the delta. + PAGE_LOAD_HISTOGRAM("PageLoad.Internal.OutOfOrderInterFrameTiming", + inout_existing_value->value() - candidate_new_value); + } + + *inout_existing_value = candidate_new_value; +} + void RecordAppBackgroundPageLoadCompleted(bool completed_after_background) { UMA_HISTOGRAM_BOOLEAN(internal::kPageLoadCompletedAfterAppBackground, completed_after_background); @@ -303,19 +340,19 @@ observer->OnFirstLayout(new_timing, extra_info); if (new_timing.paint_timing.first_paint && !last_timing.paint_timing.first_paint) - observer->OnFirstPaint(new_timing, extra_info); + observer->OnFirstPaintInPage(new_timing, extra_info); if (new_timing.paint_timing.first_text_paint && !last_timing.paint_timing.first_text_paint) - observer->OnFirstTextPaint(new_timing, extra_info); + observer->OnFirstTextPaintInPage(new_timing, extra_info); if (new_timing.paint_timing.first_image_paint && !last_timing.paint_timing.first_image_paint) - observer->OnFirstImagePaint(new_timing, extra_info); + observer->OnFirstImagePaintInPage(new_timing, extra_info); if (new_timing.paint_timing.first_contentful_paint && !last_timing.paint_timing.first_contentful_paint) - observer->OnFirstContentfulPaint(new_timing, extra_info); + observer->OnFirstContentfulPaintInPage(new_timing, extra_info); if (new_timing.paint_timing.first_meaningful_paint && !last_timing.paint_timing.first_meaningful_paint) - observer->OnFirstMeaningfulPaint(new_timing, extra_info); + observer->OnFirstMeaningfulPaintInMainFrameDocument(new_timing, extra_info); if (new_timing.parse_timing.parse_start && !last_timing.parse_timing.parse_start) observer->OnParseStart(new_timing, extra_info); @@ -390,7 +427,7 @@ page_end_reason_ != END_NEW_NAVIGATION) { LogAbortChainHistograms(nullptr); } - } else if (timing_.IsEmpty()) { + } else if (merged_page_timing_.IsEmpty()) { RecordInternalError(ERR_NO_IPCS_RECEIVED); } @@ -399,7 +436,7 @@ if (failed_provisional_load_info_) { observer->OnFailedProvisionalLoad(*failed_provisional_load_info_, info); } else if (did_commit_) { - observer->OnComplete(timing_, info); + observer->OnComplete(merged_page_timing_, info); } } } @@ -466,7 +503,7 @@ ClampBrowserTimestampIfInterProcessTimeTickSkew(&background_time_); } const PageLoadExtraInfo info = ComputePageLoadExtraInfo(); - INVOKE_AND_PRUNE_OBSERVERS(observers_, OnHidden, timing_, info); + INVOKE_AND_PRUNE_OBSERVERS(observers_, OnHidden, merged_page_timing_, info); } void PageLoadTracker::WebContentsShown() { @@ -518,6 +555,29 @@ content::NavigationHandle* navigation_handle) { INVOKE_AND_PRUNE_OBSERVERS(observers_, OnDidFinishSubFrameNavigation, navigation_handle); + + if (!navigation_handle->HasCommitted()) + return; + + // We have a new committed navigation, so discard information about the + // previously committed navigation. + subframe_navigation_start_offset_.erase( + navigation_handle->GetFrameTreeNodeId()); + + BrowserPageTrackDecider decider(embedder_interface_, + navigation_handle->GetWebContents(), + navigation_handle); + if (!decider.ShouldTrack()) + return; + + if (navigation_start_ > navigation_handle->NavigationStart()) { + RecordInternalError(ERR_SUBFRAME_NAVIGATION_START_BEFORE_MAIN_FRAME); + return; + } + base::TimeDelta navigation_delta = + navigation_handle->NavigationStart() - navigation_start_; + subframe_navigation_start_offset_.insert(std::make_pair( + navigation_handle->GetFrameTreeNodeId(), navigation_delta)); } void PageLoadTracker::FailedProvisionalLoad( @@ -549,14 +609,15 @@ const PageLoadExtraInfo info = ComputePageLoadExtraInfo(); INVOKE_AND_PRUNE_OBSERVERS(observers_, FlushMetricsOnAppEnterBackground, - timing_, info); + merged_page_timing_, info); } void PageLoadTracker::NotifyClientRedirectTo( const PageLoadTracker& destination) { - if (timing_.paint_timing.first_paint) { + if (merged_page_timing_.paint_timing.first_paint) { base::TimeTicks first_paint_time = - navigation_start() + timing_.paint_timing.first_paint.value(); + navigation_start() + + merged_page_timing_.paint_timing.first_paint.value(); base::TimeDelta first_paint_to_navigation; if (destination.navigation_start() > first_paint_time) first_paint_to_navigation = @@ -568,14 +629,64 @@ } } -void PageLoadTracker::UpdateChildFrameMetadata( - const PageLoadMetadata& child_metadata) { - // Merge the child loading behavior flags with any we've already observed, - // possibly from other child frames. - const int last_child_loading_behavior_flags = - child_frame_metadata_.behavior_flags; - child_frame_metadata_.behavior_flags |= child_metadata.behavior_flags; - if (last_child_loading_behavior_flags == child_frame_metadata_.behavior_flags) +void PageLoadTracker::UpdateSubFrameTiming( + content::RenderFrameHost* render_frame_host, + const PageLoadTiming& new_timing, + const PageLoadMetadata& new_metadata) { + UpdateSubFrameMetadata(new_metadata); + const auto it = subframe_navigation_start_offset_.find( + render_frame_host->GetFrameTreeNodeId()); + if (it == subframe_navigation_start_offset_.end()) { + // We received timing information for an untracked load. Ignore it. + return; + } + + base::TimeDelta navigation_start_offset = it->second; + const PageLoadTiming last_timing = merged_page_timing_; + MergePaintTiming(navigation_start_offset, new_timing.paint_timing, + false /* is_main_frame */); + + if (last_timing == merged_page_timing_) + return; + + const PageLoadExtraInfo info = ComputePageLoadExtraInfo(); + for (const auto& observer : observers_) { + DispatchObserverTimingCallbacks(observer.get(), last_timing, + merged_page_timing_, main_frame_metadata_, + info); + } +} + +void PageLoadTracker::MergePaintTiming( + base::TimeDelta navigation_start_offset, + const page_load_metrics::PaintTiming& new_paint_timing, + bool is_main_frame) { + MaybeUpdateTimeDelta(&merged_page_timing_.paint_timing.first_paint, + navigation_start_offset, new_paint_timing.first_paint); + MaybeUpdateTimeDelta(&merged_page_timing_.paint_timing.first_text_paint, + navigation_start_offset, + new_paint_timing.first_text_paint); + MaybeUpdateTimeDelta(&merged_page_timing_.paint_timing.first_image_paint, + navigation_start_offset, + new_paint_timing.first_image_paint); + MaybeUpdateTimeDelta(&merged_page_timing_.paint_timing.first_contentful_paint, + navigation_start_offset, + new_paint_timing.first_contentful_paint); + if (is_main_frame) { + // first meaningful paint is only tracked in the main frame. + merged_page_timing_.paint_timing.first_meaningful_paint = + new_paint_timing.first_meaningful_paint; + } +} + +void PageLoadTracker::UpdateSubFrameMetadata( + const PageLoadMetadata& subframe_metadata) { + // Merge the subframe loading behavior flags with any we've already observed, + // possibly from other subframes. + const int last_subframe_loading_behavior_flags = + subframe_metadata_.behavior_flags; + subframe_metadata_.behavior_flags |= subframe_metadata.behavior_flags; + if (last_subframe_loading_behavior_flags == subframe_metadata_.behavior_flags) return; PageLoadExtraInfo extra_info(ComputePageLoadExtraInfo()); @@ -591,8 +702,8 @@ // that a navigation started at different times, so a new timing struct with a // different start time from an earlier struct is considered invalid. const bool valid_timing_descendent = - timing_.navigation_start.is_null() || - timing_.navigation_start == new_timing.navigation_start; + merged_page_timing_.navigation_start.is_null() || + merged_page_timing_.navigation_start == new_timing.navigation_start; if (!valid_timing_descendent) { RecordInternalError(ERR_BAD_TIMING_IPC_INVALID_TIMING_DESCENDENT); return; @@ -622,15 +733,21 @@ // Thus, we make a copy of timing here, update timing_ and // main_frame_metadata_, and then proceed to dispatch the observer timing // callbacks. - const PageLoadTiming last_timing = timing_; - timing_ = new_timing; + const PageLoadTiming last_timing = merged_page_timing_; + + // Update the merged_page_timing_, making sure to merge the previously + // observed |paint_timing|, which is tracked across all frames in the page. + merged_page_timing_ = new_timing; + merged_page_timing_.paint_timing = last_timing.paint_timing; + MergePaintTiming(base::TimeDelta(), new_timing.paint_timing, + true /* is_main_frame */); const PageLoadMetadata last_metadata = main_frame_metadata_; main_frame_metadata_ = new_metadata; const PageLoadExtraInfo info = ComputePageLoadExtraInfo(); for (const auto& observer : observers_) { - DispatchObserverTimingCallbacks(observer.get(), last_timing, new_timing, - last_metadata, info); + DispatchObserverTimingCallbacks(observer.get(), last_timing, + merged_page_timing_, last_metadata, info); } } @@ -717,7 +834,7 @@ navigation_start_, first_background_time, first_foreground_time, started_in_foreground_, user_initiated_info_, url(), start_url_, did_commit_, page_end_reason_, page_end_user_initiated_info_, - page_end_time, main_frame_metadata_, child_frame_metadata_); + page_end_time, main_frame_metadata_, subframe_metadata_); } bool PageLoadTracker::HasMatchingNavigationRequestID(
diff --git a/chrome/browser/page_load_metrics/page_load_tracker.h b/chrome/browser/page_load_metrics/page_load_tracker.h index f6529d1a..c696acb 100644 --- a/chrome/browser/page_load_metrics/page_load_tracker.h +++ b/chrome/browser/page_load_metrics/page_load_tracker.h
@@ -26,6 +26,7 @@ namespace content { class NavigationHandle; +class RenderFrameHost; } // namespace content namespace page_load_metrics { @@ -152,8 +153,8 @@ // No page load end time was recorded for this page load. ERR_NO_PAGE_LOAD_END_TIME, - // Received a timing update from a subframe. - ERR_TIMING_IPC_FROM_SUBFRAME, + // Received a timing update from a subframe (deprecated). + DEPRECATED_ERR_TIMING_IPC_FROM_SUBFRAME, // A timing IPC was sent from the renderer that contained timing data which // was inconsistent with our timing data for the currently committed load. @@ -168,6 +169,10 @@ // (e.g. out of order timings, or other issues). ERR_BAD_TIMING_IPC_INVALID_TIMING, + // We received a navigation start for a child frame that is before the + // navigation start of the main frame. + ERR_SUBFRAME_NAVIGATION_START_BEFORE_MAIN_FRAME, + // Add values before this final count. ERR_LAST_ENTRY, }; @@ -222,10 +227,14 @@ void UpdateTiming(const PageLoadTiming& timing, const PageLoadMetadata& metadata); + void UpdateSubFrameTiming(content::RenderFrameHost* render_frame_host, + const PageLoadTiming& new_timing, + const PageLoadMetadata& new_metadata); + // Update metadata for child frames. Updates for child frames arrive // separately from updates for the main frame, so aren't included in // UpdateTiming. - void UpdateChildFrameMetadata(const PageLoadMetadata& child_metadata); + void UpdateSubFrameMetadata(const PageLoadMetadata& subframe_metadata); void OnStartedResource( const ExtraRequestStartInfo& extra_request_started_info); @@ -306,6 +315,8 @@ base::TimeDelta actual_delay); private: + using FrameTreeNodeId = int; + // This function converts a TimeTicks value taken in the browser process // to navigation_start_ if: // - base::TimeTicks is not comparable across processes because the clock @@ -326,6 +337,12 @@ void MaybeUpdateURL(content::NavigationHandle* navigation_handle); + // Merge values from |new_paint_timing| into |merged_page_timing_|, offsetting + // any new timings by the |navigation_start_offset|. + void MergePaintTiming(base::TimeDelta navigation_start_offset, + const page_load_metrics::PaintTiming& new_paint_timing, + bool is_main_frame); + UserInputTracker input_tracker_; // Whether we stopped tracking this navigation after it was initiated. We may @@ -375,9 +392,13 @@ base::TimeTicks foreground_time_; bool started_in_foreground_; - PageLoadTiming timing_; + // PageLoadTiming for the currently tracked page. The fields in |paint_timing| + // are merged across all frames in the document. All other fields are for the + // main frame document. + PageLoadTiming merged_page_timing_; + PageLoadMetadata main_frame_metadata_; - PageLoadMetadata child_frame_metadata_; + PageLoadMetadata subframe_metadata_; ui::PageTransition page_transition_; @@ -403,6 +424,10 @@ std::vector<std::unique_ptr<PageLoadMetricsObserver>> observers_; + // Navigation start offsets for the most recently committed document in each + // frame. + std::map<FrameTreeNodeId, base::TimeDelta> subframe_navigation_start_offset_; + DISALLOW_COPY_AND_ASSIGN(PageLoadTracker); };
diff --git a/chrome/browser/prerender/prerender_browsertest.cc b/chrome/browser/prerender/prerender_browsertest.cc index 3fb3cf44..24a5fb9 100644 --- a/chrome/browser/prerender/prerender_browsertest.cc +++ b/chrome/browser/prerender/prerender_browsertest.cc
@@ -3440,7 +3440,8 @@ base::TimeDelta::FromMilliseconds(2654); page_load_metrics::PageLoadMetricsObserverTestHarness:: PopulateRequiredTimingFields(&timing); - observer.OnFirstContentfulPaint(timing, GenericPageLoadExtraInfo(dest_url())); + observer.OnFirstContentfulPaintInPage(timing, + GenericPageLoadExtraInfo(dest_url())); histogram_tester().ExpectTotalCount( "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1); @@ -3481,7 +3482,8 @@ base::TimeDelta::FromMilliseconds(2361); page_load_metrics::PageLoadMetricsObserverTestHarness:: PopulateRequiredTimingFields(&timing); - observer.OnFirstContentfulPaint(timing, GenericPageLoadExtraInfo(dest_url())); + observer.OnFirstContentfulPaintInPage(timing, + GenericPageLoadExtraInfo(dest_url())); histogram_tester().ExpectTotalCount( "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1); @@ -3522,7 +3524,8 @@ base::TimeDelta::FromMilliseconds(2361); page_load_metrics::PageLoadMetricsObserverTestHarness:: PopulateRequiredTimingFields(&timing); - observer.OnFirstContentfulPaint(timing, GenericPageLoadExtraInfo(dest_url())); + observer.OnFirstContentfulPaintInPage(timing, + GenericPageLoadExtraInfo(dest_url())); histogram_tester().ExpectTotalCount( "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1); @@ -3565,7 +3568,8 @@ base::TimeDelta::FromMilliseconds(2362); page_load_metrics::PageLoadMetricsObserverTestHarness:: PopulateRequiredTimingFields(&timing); - observer.OnFirstContentfulPaint(timing, GenericPageLoadExtraInfo(dest_url())); + observer.OnFirstContentfulPaintInPage(timing, + GenericPageLoadExtraInfo(dest_url())); histogram_tester().ExpectTotalCount( "Prerender.none_PrefetchTTFCP.Warm.Cacheable.Visible", 0); @@ -3615,7 +3619,8 @@ base::TimeDelta::FromMilliseconds(2654); page_load_metrics::PageLoadMetricsObserverTestHarness:: PopulateRequiredTimingFields(&timing); - observer.OnFirstContentfulPaint(timing, GenericPageLoadExtraInfo(dest_url())); + observer.OnFirstContentfulPaintInPage(timing, + GenericPageLoadExtraInfo(dest_url())); histogram_tester().ExpectTotalCount( "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1); @@ -3646,7 +3651,8 @@ base::TimeDelta::FromMilliseconds(2654); page_load_metrics::PageLoadMetricsObserverTestHarness:: PopulateRequiredTimingFields(&timing); - observer.OnFirstContentfulPaint(timing, GenericPageLoadExtraInfo(dest_url())); + observer.OnFirstContentfulPaintInPage(timing, + GenericPageLoadExtraInfo(dest_url())); histogram_tester().ExpectTotalCount( "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Hidden", 1); @@ -3691,7 +3697,8 @@ base::TimeDelta::FromMilliseconds(2362); page_load_metrics::PageLoadMetricsObserverTestHarness:: PopulateRequiredTimingFields(&timing); - observer.OnFirstContentfulPaint(timing, GenericPageLoadExtraInfo(dest_url())); + observer.OnFirstContentfulPaintInPage(timing, + GenericPageLoadExtraInfo(dest_url())); histogram_tester().ExpectTotalCount( "Prerender.none_PrefetchTTFCP.Warm.Cacheable.Hidden", 0);
diff --git a/chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc b/chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc index c2079d21..340daee6 100644 --- a/chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc +++ b/chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc
@@ -13,6 +13,7 @@ #include "base/message_loop/message_loop.h" #include "base/run_loop.h" #include "base/stl_util.h" +#include "base/task_scheduler/post_task.h" #include "base/threading/thread_task_runner_handle.h" #include "chrome/browser/sync_file_system/drive_backend/callback_helper.h" #include "chrome/browser/sync_file_system/drive_backend/drive_backend_constants.h" @@ -88,18 +89,13 @@ io_task_runner_ = content::BrowserThread::GetTaskRunnerForThread( content::BrowserThread::IO); - scoped_refptr<base::SequencedWorkerPool> worker_pool( - content::BrowserThread::GetBlockingPool()); - worker_task_runner_ = - worker_pool->GetSequencedTaskRunnerWithShutdownBehavior( - worker_pool->GetSequenceToken(), - base::SequencedWorkerPool::SKIP_ON_SHUTDOWN); + worker_task_runner_ = base::CreateSequencedTaskRunnerWithTraits( + {base::MayBlock(), base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN}); file_task_runner_ = content::BrowserThread::GetTaskRunnerForThread( content::BrowserThread::FILE); scoped_refptr<base::SequencedTaskRunner> drive_task_runner = - worker_pool->GetSequencedTaskRunnerWithShutdownBehavior( - worker_pool->GetSequenceToken(), - base::SequencedWorkerPool::SKIP_ON_SHUTDOWN); + base::CreateSequencedTaskRunnerWithTraits( + {base::MayBlock(), base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN}); RegisterSyncableFileSystem(); local_sync_service_ = LocalFileSyncService::CreateForTesting( @@ -118,18 +114,18 @@ drive_service.get(), uploader.get(), kSyncRootFolderTitle)); - remote_sync_service_.reset( - new SyncEngine(base::ThreadTaskRunnerHandle::Get(), // ui_task_runner - worker_task_runner_.get(), drive_task_runner.get(), - worker_pool.get(), base_dir_.GetPath(), - nullptr, // task_logger - nullptr, // notification_manager - nullptr, // extension_service - nullptr, // signin_manager - nullptr, // token_service - nullptr, // request_context - nullptr, // drive_service - in_memory_env_.get())); + remote_sync_service_.reset(new SyncEngine( + base::ThreadTaskRunnerHandle::Get(), // ui_task_runner + worker_task_runner_.get(), drive_task_runner.get(), + content::BrowserThread::GetBlockingPool(), base_dir_.GetPath(), + nullptr, // task_logger + nullptr, // notification_manager + nullptr, // extension_service + nullptr, // signin_manager + nullptr, // token_service + nullptr, // request_context + nullptr, // drive_service + in_memory_env_.get())); remote_sync_service_->AddServiceObserver(this); remote_sync_service_->InitializeForTesting(std::move(drive_service), std::move(uploader),
diff --git a/chrome/browser/sync_file_system/drive_backend/sync_engine_unittest.cc b/chrome/browser/sync_file_system/drive_backend/sync_engine_unittest.cc index 4959849..bd81a9a0 100644 --- a/chrome/browser/sync_file_system/drive_backend/sync_engine_unittest.cc +++ b/chrome/browser/sync_file_system/drive_backend/sync_engine_unittest.cc
@@ -10,6 +10,7 @@ #include "base/files/scoped_temp_dir.h" #include "base/macros.h" #include "base/run_loop.h" +#include "base/task_scheduler/post_task.h" #include "base/test/sequenced_worker_pool_owner.h" #include "base/threading/thread_task_runner_handle.h" #include "chrome/browser/sync_file_system/drive_backend/callback_helper.h" @@ -43,10 +44,8 @@ scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner = base::ThreadTaskRunnerHandle::Get(); - worker_task_runner_ = - worker_pool_owner_.pool()->GetSequencedTaskRunnerWithShutdownBehavior( - worker_pool_owner_.pool()->GetSequenceToken(), - base::SequencedWorkerPool::SKIP_ON_SHUTDOWN); + worker_task_runner_ = base::CreateSequencedTaskRunnerWithTraits( + {base::MayBlock(), base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN}); sync_engine_.reset(new drive_backend::SyncEngine( ui_task_runner.get(), worker_task_runner_.get(),
diff --git a/chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc b/chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc index 926ef21..f9febb9 100644 --- a/chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc +++ b/chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc
@@ -27,9 +27,13 @@ PaymentRequestState* state, PaymentRequestDialogView* dialog, BackNavigationType back_navigation_type, + base::OnceClosure on_edited, + base::OnceCallback<void(const autofill::AutofillProfile&)> on_added, autofill::AutofillProfile* profile) : EditorViewController(spec, state, dialog, back_navigation_type), - profile_to_edit_(profile) {} + profile_to_edit_(profile), + on_edited_(std::move(on_edited)), + on_added_(std::move(on_added)) {} ContactInfoEditorViewController::~ContactInfoEditorViewController() {} @@ -76,12 +80,15 @@ PopulateProfile(profile_to_edit_); state()->GetPersonalDataManager()->UpdateProfile(*profile_to_edit_); state()->profile_comparator()->Invalidate(*profile_to_edit_); + std::move(on_edited_).Run(); + on_added_.Reset(); } else { std::unique_ptr<autofill::AutofillProfile> profile = base::MakeUnique<autofill::AutofillProfile>(); PopulateProfile(profile.get()); state()->GetPersonalDataManager()->AddProfile(*profile); - // TODO(crbug.com/712224): Add to profile cache in state_. + std::move(on_added_).Run(*profile); + on_edited_.Reset(); } return true; }
diff --git a/chrome/browser/ui/views/payments/contact_info_editor_view_controller.h b/chrome/browser/ui/views/payments/contact_info_editor_view_controller.h index b50ec9d..5a0ab4a 100644 --- a/chrome/browser/ui/views/payments/contact_info_editor_view_controller.h +++ b/chrome/browser/ui/views/payments/contact_info_editor_view_controller.h
@@ -25,11 +25,14 @@ // Does not take ownership of the arguments, which should outlive this object. // Passing nullptr as |profile| indicates that we are editing a new profile; // other arguments should never be null. - ContactInfoEditorViewController(PaymentRequestSpec* spec, - PaymentRequestState* state, - PaymentRequestDialogView* dialog, - BackNavigationType back_navigation_type, - autofill::AutofillProfile* profile); + ContactInfoEditorViewController( + PaymentRequestSpec* spec, + PaymentRequestState* state, + PaymentRequestDialogView* dialog, + BackNavigationType back_navigation_type, + base::OnceClosure on_edited, + base::OnceCallback<void(const autofill::AutofillProfile&)> on_added, + autofill::AutofillProfile* profile); ~ContactInfoEditorViewController() override; // EditorViewController: @@ -51,11 +54,16 @@ // Uses the values in the UI fields to populate the corresponding values in // |profile|. void PopulateProfile(autofill::AutofillProfile* profile); - bool GetSheetId(DialogViewID* sheet_id) override; autofill::AutofillProfile* profile_to_edit_; + // Called when |profile_to_edit_| was successfully edited. + base::OnceClosure on_edited_; + // Called when a new profile was added. The const reference is short-lived, + // and the callee should make a copy. + base::OnceCallback<void(const autofill::AutofillProfile&)> on_added_; + class ContactInfoValidationDelegate : public ValidationDelegate { public: ContactInfoValidationDelegate(const EditorField& field,
diff --git a/chrome/browser/ui/views/payments/contact_info_editor_view_controller_browsertest.cc b/chrome/browser/ui/views/payments/contact_info_editor_view_controller_browsertest.cc index e656af7..7c1ee91b 100644 --- a/chrome/browser/ui/views/payments/contact_info_editor_view_controller_browsertest.cc +++ b/chrome/browser/ui/views/payments/contact_info_editor_view_controller_browsertest.cc
@@ -70,6 +70,11 @@ EXPECT_EQ(base::ASCIIToUTF16(kEmailAddress), profile->GetInfo(autofill::AutofillType(autofill::EMAIL_ADDRESS), GetLocale())); + + PaymentRequest* request = GetPaymentRequests(GetActiveWebContents()).front(); + EXPECT_EQ(1U, request->state()->contact_profiles().size()); + EXPECT_EQ(request->state()->contact_profiles().back(), + request->state()->selected_contact_profile()); } IN_PROC_BROWSER_TEST_F(PaymentRequestContactInfoEditorTest, @@ -213,4 +218,66 @@ GetLocale())); } +IN_PROC_BROWSER_TEST_F(PaymentRequestContactInfoEditorTest, + ModifyExistingSelectsIt) { + autofill::PersonalDataManager* personal_data_manager = GetDataManager(); + personal_data_manager->AddObserver(&personal_data_observer_); + + autofill::AutofillProfile incomplete_profile; + incomplete_profile.SetInfo(autofill::AutofillType(autofill::NAME_FULL), + base::ASCIIToUTF16(kNameFull), GetLocale()); + AddAutofillProfile(incomplete_profile); + + autofill::AutofillProfile other_incomplete_profile; + other_incomplete_profile.SetInfo(autofill::AutofillType(autofill::NAME_FULL), + base::ASCIIToUTF16("other"), GetLocale()); + AddAutofillProfile(other_incomplete_profile); + + InvokePaymentRequestUI(); + OpenContactInfoScreen(); + + PaymentRequest* request = GetPaymentRequests(GetActiveWebContents()).front(); + EXPECT_EQ(request->state()->contact_profiles().front(), + request->state()->selected_contact_profile()); + + views::View* list_view = dialog_view()->GetViewByID( + static_cast<int>(DialogViewID::CONTACT_INFO_SHEET_LIST_VIEW)); + DCHECK(list_view); + ClickOnDialogViewAndWait(list_view->child_at(1)); + + // Do not set name: This should have been populated when opening the screen. + EXPECT_EQ(base::ASCIIToUTF16(kNameFull), + GetEditorTextfieldValue(autofill::NAME_FULL)); + SetEditorTextfieldValue(base::ASCIIToUTF16(kPhoneNumber), + autofill::PHONE_HOME_WHOLE_NUMBER); + SetEditorTextfieldValue(base::ASCIIToUTF16(kEmailAddress), + autofill::EMAIL_ADDRESS); + + // Wait until the web database has been updated and the notification sent. + base::RunLoop save_data_loop; + EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged()) + .WillOnce(QuitMessageLoop(&save_data_loop)); + ClickOnDialogViewAndWait(DialogViewID::EDITOR_SAVE_BUTTON); + save_data_loop.Run(); + + ASSERT_EQ(2UL, personal_data_manager->GetProfiles().size()); + autofill::AutofillProfile* profile = personal_data_manager->GetProfiles()[0]; + DCHECK(profile); + + EXPECT_EQ(base::ASCIIToUTF16(kNameFull), + profile->GetInfo(autofill::AutofillType(autofill::NAME_FULL), + GetLocale())); + EXPECT_EQ(base::ASCIIToUTF16(kPhoneNumber), + profile->GetInfo( + autofill::AutofillType(autofill::PHONE_HOME_WHOLE_NUMBER), + GetLocale())); + EXPECT_EQ(base::ASCIIToUTF16(kEmailAddress), + profile->GetInfo(autofill::AutofillType(autofill::EMAIL_ADDRESS), + GetLocale())); + + EXPECT_EQ(2U, request->state()->contact_profiles().size()); + EXPECT_EQ(request->state()->contact_profiles().back(), + request->state()->selected_contact_profile()); +} + } // namespace payments
diff --git a/chrome/browser/ui/views/payments/payment_request_dialog_view.cc b/chrome/browser/ui/views/payments/payment_request_dialog_view.cc index 12c798d..d1f472f 100644 --- a/chrome/browser/ui/views/payments/payment_request_dialog_view.cc +++ b/chrome/browser/ui/views/payments/payment_request_dialog_view.cc
@@ -277,13 +277,16 @@ void PaymentRequestDialogView::ShowContactInfoEditor( BackNavigationType back_navigation_type, + base::OnceClosure on_edited, + base::OnceCallback<void(const autofill::AutofillProfile&)> on_added, autofill::AutofillProfile* profile) { - view_stack_->Push(CreateViewAndInstallController( - base::MakeUnique<ContactInfoEditorViewController>( - request_->spec(), request_->state(), this, - back_navigation_type, profile), - &controller_map_), - /* animate = */ true); + view_stack_->Push( + CreateViewAndInstallController( + base::MakeUnique<ContactInfoEditorViewController>( + request_->spec(), request_->state(), this, back_navigation_type, + std::move(on_edited), std::move(on_added), profile), + &controller_map_), + /* animate = */ true); if (observer_for_testing_) observer_for_testing_->OnContactInfoEditorOpened(); }
diff --git a/chrome/browser/ui/views/payments/payment_request_dialog_view.h b/chrome/browser/ui/views/payments/payment_request_dialog_view.h index 4e18583..f793e84 100644 --- a/chrome/browser/ui/views/payments/payment_request_dialog_view.h +++ b/chrome/browser/ui/views/payments/payment_request_dialog_view.h
@@ -136,10 +136,16 @@ base::OnceCallback<void(const autofill::AutofillProfile&)> on_added, autofill::AutofillProfile* profile); // |profile| is the profile to be edited, or nullptr for adding a profile. + // |on_edited| is called when |profile| was successfully edited, and + // |on_added| is called when a new profile was added (the reference is + // short-lived; callee should make a copy of the profile object). // |back_navigation_type| identifies the type of navigation to execute once // the editor has completed successfully. - void ShowContactInfoEditor(BackNavigationType back_navigation_type, - autofill::AutofillProfile* profile = nullptr); + void ShowContactInfoEditor( + BackNavigationType back_navigation_type, + base::OnceClosure on_edited, + base::OnceCallback<void(const autofill::AutofillProfile&)> on_added, + autofill::AutofillProfile* profile = nullptr); void EditorViewUpdated(); void ShowCvcUnmaskPrompt(
diff --git a/chrome/browser/ui/views/payments/payment_sheet_view_controller.cc b/chrome/browser/ui/views/payments/payment_sheet_view_controller.cc index 6f5a599..3e83ac61 100644 --- a/chrome/browser/ui/views/payments/payment_sheet_view_controller.cc +++ b/chrome/browser/ui/views/payments/payment_sheet_view_controller.cc
@@ -526,7 +526,12 @@ case static_cast<int>( PaymentSheetViewControllerTags::ADD_CONTACT_INFO_BUTTON): - dialog()->ShowContactInfoEditor(BackNavigationType::kPaymentSheet); + dialog()->ShowContactInfoEditor( + BackNavigationType::kPaymentSheet, + /*on_edited=*/base::OnceClosure(), // This is always an add. + /*on_added=*/ + base::BindOnce(&PaymentRequestState::AddAutofillContactProfile, + base::Unretained(state()), /*selected=*/true)); break; case static_cast<int>(
diff --git a/chrome/browser/ui/views/payments/profile_list_view_controller.cc b/chrome/browser/ui/views/payments/profile_list_view_controller.cc index f30561a6..657d5a6 100644 --- a/chrome/browser/ui/views/payments/profile_list_view_controller.cc +++ b/chrome/browser/ui/views/payments/profile_list_view_controller.cc
@@ -245,7 +245,15 @@ } void ShowEditor(autofill::AutofillProfile* profile) override { - dialog()->ShowContactInfoEditor(BackNavigationType::kPaymentSheet, profile); + dialog()->ShowContactInfoEditor( + BackNavigationType::kPaymentSheet, + /*on_edited=*/ + base::BindOnce(&PaymentRequestState::SetSelectedContactProfile, + base::Unretained(state()), profile), + /*on_added=*/ + base::BindOnce(&PaymentRequestState::AddAutofillContactProfile, + base::Unretained(state()), /*selected=*/true), + profile); } autofill::AutofillProfile* GetSelectedProfile() override {
diff --git a/chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc b/chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc index 059653c..10cc4b0a 100644 --- a/chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc +++ b/chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc
@@ -38,9 +38,7 @@ MetricsRenderFrameObserver::~MetricsRenderFrameObserver() {} void MetricsRenderFrameObserver::DidChangePerformanceTiming() { - // Only track timing metrics for main frames. - if (IsMainFrame()) - SendMetrics(); + SendMetrics(); } void MetricsRenderFrameObserver::DidObserveLoadingBehavior( @@ -72,14 +70,8 @@ // non-null, we will send metrics for the current page at some later time, as // those metrics become available. if (ShouldSendMetrics()) { - PageLoadTiming timing; - if (IsMainFrame()) { - // Only populate PageLoadTiming for the main frame. - timing = GetTiming(); - DCHECK(!timing.navigation_start.is_null()); - } - page_timing_metrics_sender_.reset( - new PageTimingMetricsSender(this, routing_id(), CreateTimer(), timing)); + page_timing_metrics_sender_ = base::MakeUnique<PageTimingMetricsSender>( + this, routing_id(), CreateTimer(), GetTiming()); } } @@ -185,8 +177,4 @@ delete this; } -bool MetricsRenderFrameObserver::IsMainFrame() const { - return render_frame()->IsMainFrame(); -} - } // namespace page_load_metrics
diff --git a/chrome/renderer/page_load_metrics/metrics_render_frame_observer.h b/chrome/renderer/page_load_metrics/metrics_render_frame_observer.h index 3b9ad1e..d0441b0 100644 --- a/chrome/renderer/page_load_metrics/metrics_render_frame_observer.h +++ b/chrome/renderer/page_load_metrics/metrics_render_frame_observer.h
@@ -51,7 +51,6 @@ virtual PageLoadTiming GetTiming() const; virtual std::unique_ptr<base::Timer> CreateTimer() const; virtual bool HasNoRenderFrame() const; - virtual bool IsMainFrame() const; DISALLOW_COPY_AND_ASSIGN(MetricsRenderFrameObserver); };
diff --git a/chrome/renderer/page_load_metrics/metrics_render_frame_observer_unittest.cc b/chrome/renderer/page_load_metrics/metrics_render_frame_observer_unittest.cc index 453f15b..22fdad4 100644 --- a/chrome/renderer/page_load_metrics/metrics_render_frame_observer_unittest.cc +++ b/chrome/renderer/page_load_metrics/metrics_render_frame_observer_unittest.cc
@@ -41,8 +41,6 @@ mock_timer_ = std::move(timer); } - void set_is_main_frame(bool is_main_frame) { is_main_frame_ = is_main_frame; } - bool WasFakeTimingConsumed() const { return fake_timing_.IsEmpty(); } void ExpectPageLoadTiming(const PageLoadTiming& timing) { @@ -68,13 +66,11 @@ bool ShouldSendMetrics() const override { return true; } bool HasNoRenderFrame() const override { return false; } - bool IsMainFrame() const override { return is_main_frame_; } private: FakePageTimingMetricsIPCSender fake_timing_ipc_sender_; mutable PageLoadTiming fake_timing_; mutable std::unique_ptr<base::Timer> mock_timer_; - bool is_main_frame_ = true; }; typedef testing::Test MetricsRenderFrameObserverTest; @@ -206,24 +202,4 @@ mock_timer2->Fire(); } -TEST_F(MetricsRenderFrameObserverTest, NoUpdatesFromChildFrames) { - base::Time nav_start = base::Time::FromDoubleT(10); - - TestMetricsRenderFrameObserver observer; - base::MockTimer* mock_timer = new base::MockTimer(false, false); - observer.set_mock_timer(base::WrapUnique(mock_timer)); - observer.set_is_main_frame(false); - - PageLoadTiming timing; - timing.navigation_start = nav_start; - observer.SetFakePageLoadTiming(timing); - observer.DidCommitProvisionalLoad(true, false); - ASSERT_FALSE(observer.WasFakeTimingConsumed()); - ASSERT_FALSE(mock_timer->IsRunning()); - - observer.DidChangePerformanceTiming(); - ASSERT_FALSE(observer.WasFakeTimingConsumed()); - ASSERT_FALSE(mock_timer->IsRunning()); -} - } // namespace page_load_metrics
diff --git a/chrome/renderer/page_load_metrics/page_timing_metrics_sender.cc b/chrome/renderer/page_load_metrics/page_timing_metrics_sender.cc index e016bb3..f4a8855 100644 --- a/chrome/renderer/page_load_metrics/page_timing_metrics_sender.cc +++ b/chrome/renderer/page_load_metrics/page_timing_metrics_sender.cc
@@ -30,8 +30,7 @@ last_timing_(initial_timing), metadata_(PageLoadMetadata()) { if (!initial_timing.IsEmpty()) { - // Send an initial IPC relatively early to help track aborts. - EnsureSendTimer(kInitialTimerDelayMillis); + EnsureSendTimer(); } } @@ -49,7 +48,7 @@ if (behavior & metadata_.behavior_flags) return; metadata_.behavior_flags |= behavior; - EnsureSendTimer(kTimerDelayMillis); + EnsureSendTimer(); } void PageTimingMetricsSender::Send(const PageLoadTiming& timing) { @@ -65,17 +64,23 @@ } last_timing_ = timing; - EnsureSendTimer(kTimerDelayMillis); + EnsureSendTimer(); } -void PageTimingMetricsSender::EnsureSendTimer(int delay) { - if (!timer_->IsRunning()) +void PageTimingMetricsSender::EnsureSendTimer() { + if (!timer_->IsRunning()) { + // Send the first IPC eagerly to make sure the receiving side knows we're + // sending metrics as soon as possible. + int delay_ms = + have_sent_ipc_ ? kTimerDelayMillis : kInitialTimerDelayMillis; timer_->Start( - FROM_HERE, base::TimeDelta::FromMilliseconds(delay), + FROM_HERE, base::TimeDelta::FromMilliseconds(delay_ms), base::Bind(&PageTimingMetricsSender::SendNow, base::Unretained(this))); + } } void PageTimingMetricsSender::SendNow() { + have_sent_ipc_ = true; ipc_sender_->Send(new PageLoadMetricsMsg_TimingUpdated( routing_id_, last_timing_, metadata_)); }
diff --git a/chrome/renderer/page_load_metrics/page_timing_metrics_sender.h b/chrome/renderer/page_load_metrics/page_timing_metrics_sender.h index fc3fba1f..75342f3 100644 --- a/chrome/renderer/page_load_metrics/page_timing_metrics_sender.h +++ b/chrome/renderer/page_load_metrics/page_timing_metrics_sender.h
@@ -39,7 +39,7 @@ base::Timer* timer() const { return timer_.get(); } private: - void EnsureSendTimer(int delay); + void EnsureSendTimer(); void SendNow(); IPC::Sender* const ipc_sender_; @@ -51,6 +51,8 @@ // scoped to a single committed load. PageLoadMetadata metadata_; + bool have_sent_ipc_ = false; + DISALLOW_COPY_AND_ASSIGN(PageTimingMetricsSender); };
diff --git a/chrome/test/data/page_load_metrics/empty_iframe.html b/chrome/test/data/page_load_metrics/empty_iframe.html new file mode 100644 index 0000000..0aab569 --- /dev/null +++ b/chrome/test/data/page_load_metrics/empty_iframe.html
@@ -0,0 +1,7 @@ +<html> + <link rel="stylesheet" href="seamless-iframe.css"> + <body> + <!-- TODO(crbug/705315): remove '/cross-site/b.com' when fixed. --> + <iframe src="/cross-site/b.com/empty.html" class="seamless"></iframe> + </body> +</html>
diff --git a/chrome/test/data/page_load_metrics/iframe.html b/chrome/test/data/page_load_metrics/iframe.html new file mode 100644 index 0000000..dac674b --- /dev/null +++ b/chrome/test/data/page_load_metrics/iframe.html
@@ -0,0 +1,7 @@ +<html> + <link rel="stylesheet" href="seamless-iframe.css"> + <body> + <!-- TODO(crbug/705315): remove '/cross-site/b.com' when fixed. --> + <iframe src="/cross-site/b.com/title1.html" class="seamless"></iframe> + </body> +</html>
diff --git a/chrome/test/data/page_load_metrics/iframes.html b/chrome/test/data/page_load_metrics/iframes.html new file mode 100644 index 0000000..1fde042 --- /dev/null +++ b/chrome/test/data/page_load_metrics/iframes.html
@@ -0,0 +1,8 @@ +<html> + <link rel="stylesheet" href="seamless-iframe.css"> + <body> + <!-- TODO(crbug/705315): remove '/cross-site/?.com' when fixed. --> + <iframe src="/cross-site/b.com/title1.html" class="seamless"></iframe> + <iframe src="/cross-site/c.com/title1.html" class="seamless"></iframe> + </body> +</html>
diff --git a/chrome/test/data/page_load_metrics/main_frame_with_iframe.html b/chrome/test/data/page_load_metrics/main_frame_with_iframe.html new file mode 100644 index 0000000..e1af917e --- /dev/null +++ b/chrome/test/data/page_load_metrics/main_frame_with_iframe.html
@@ -0,0 +1,8 @@ +<html> + <link rel="stylesheet" href="seamless-iframe.css"> + <body> + Here is some paintable content in the main frame. + <!-- TODO(crbug/705315): remove '/cross-site/b.com' when fixed. --> + <iframe src="/cross-site/b.com/title1.html" class="seamless"></iframe> + </body> +</html>
diff --git a/chrome/test/data/page_load_metrics/seamless-iframe.css b/chrome/test/data/page_load_metrics/seamless-iframe.css new file mode 100644 index 0000000..4b9d8ef --- /dev/null +++ b/chrome/test/data/page_load_metrics/seamless-iframe.css
@@ -0,0 +1,12 @@ +/* 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. + */ +/* To prevent an iframe from painting in the document it is hosted in, we use + * the 'seamless' class, which prevents the frame from drawing borders or + * scrollbars. + */ +iframe.seamless { + border: 0px none transparent; + overflow: hidden; +}
diff --git a/components/cronet/android/java/src/org/chromium/net/impl/CronetLibraryLoader.java b/components/cronet/android/java/src/org/chromium/net/impl/CronetLibraryLoader.java index 935240d9..d5461d0 100644 --- a/components/cronet/android/java/src/org/chromium/net/impl/CronetLibraryLoader.java +++ b/components/cronet/android/java/src/org/chromium/net/impl/CronetLibraryLoader.java
@@ -5,6 +5,7 @@ package org.chromium.net.impl; import android.content.Context; +import android.os.ConditionVariable; import android.os.Handler; import android.os.HandlerThread; import android.os.Looper; @@ -33,6 +34,9 @@ private static volatile boolean sLibraryLoaded = false; // Has ensureInitThreadInitialized() completed? private static volatile boolean sInitThreadInitDone = false; + // Block calling native methods until this ConditionVariable opens to indicate loadLibrary() + // is completed and native methods have been registered. + private static final ConditionVariable sWaitForLibLoad = new ConditionVariable(); /** * Ensure that native library is loaded and initialized. Can be called from @@ -41,6 +45,17 @@ public static void ensureInitialized( final Context applicationContext, final CronetEngineBuilderImpl builder) { synchronized (sLoadLock) { + if (!sInitThreadInitDone) { + if (!sInitThread.isAlive()) { + sInitThread.start(); + } + postToInitThread(new Runnable() { + @Override + public void run() { + ensureInitializedOnInitThread(applicationContext); + } + }); + } if (!sLibraryLoaded) { ContextUtils.initApplicationContext(applicationContext); if (builder.libraryLoader() != null) { @@ -58,18 +73,7 @@ Log.i(TAG, "Cronet version: %s, arch: %s", implVersion, System.getProperty("os.arch")); sLibraryLoaded = true; - } - - if (!sInitThreadInitDone) { - if (!sInitThread.isAlive()) { - sInitThread.start(); - } - postToInitThread(new Runnable() { - @Override - public void run() { - ensureInitializedOnInitThread(applicationContext); - } - }); + sWaitForLibLoad.open(); } } } @@ -87,7 +91,6 @@ * init thread native MessageLoop is initialized. */ static void ensureInitializedOnInitThread(Context context) { - assert sLibraryLoaded; assert onInitThread(); if (sInitThreadInitDone) { return; @@ -99,6 +102,9 @@ // observers. Existing observers in the net stack do not // perform expensive work. NetworkChangeNotifier.registerToReceiveNotificationsAlways(); + // Wait for loadLibrary() to complete so JNI is registered. + sWaitForLibLoad.block(); + assert sLibraryLoaded; // registerToReceiveNotificationsAlways() is called before the native // NetworkChangeNotifierAndroid is created, so as to avoid receiving // the undesired initial network change observer notification, which
diff --git a/components/drive/file_system/operation_test_base.cc b/components/drive/file_system/operation_test_base.cc index 5659880..5aa0717f 100644 --- a/components/drive/file_system/operation_test_base.cc +++ b/components/drive/file_system/operation_test_base.cc
@@ -4,7 +4,7 @@ #include "components/drive/file_system/operation_test_base.h" -#include "base/threading/sequenced_worker_pool.h" +#include "base/task_scheduler/post_task.h" #include "components/drive/chromeos/change_list_loader.h" #include "components/drive/chromeos/fake_free_disk_space_getter.h" #include "components/drive/chromeos/file_cache.h" @@ -16,7 +16,6 @@ #include "components/drive/service/fake_drive_service.h" #include "components/drive/service/test_util.h" #include "components/prefs/testing_pref_service.h" -#include "content/public/browser/browser_thread.h" #include "content/public/test/test_utils.h" #include "google_apis/drive/test_util.h" @@ -63,10 +62,8 @@ } void OperationTestBase::SetUp() { - scoped_refptr<base::SequencedWorkerPool> pool = - content::BrowserThread::GetBlockingPool(); blocking_task_runner_ = - pool->GetSequencedTaskRunner(pool->GetSequenceToken()); + base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()}); pref_service_.reset(new TestingPrefServiceSimple); test_util::RegisterDrivePrefs(pref_service_->registry());
diff --git a/components/ntp_snippets/content_suggestions_provider.h b/components/ntp_snippets/content_suggestions_provider.h index 8c485dfd5..3fc37b0 100644 --- a/components/ntp_snippets/content_suggestions_provider.h +++ b/components/ntp_snippets/content_suggestions_provider.h
@@ -97,9 +97,11 @@ // Fetches more suggestions for the given category. The new suggestions // will not include any suggestion of the |known_suggestion_ids| sets. - // The given |callback| is called with these suggestions, along with all - // existing suggestions. It has to be invoked exactly once as the front-end - // might wait for its completion. + // As a result of this call, the provider: + // - should call the |callback| with these additional suggestions (exactly + // once as the front-end might wait for its completion); + // - should *not* notify its Observer by OnNewSuggestions() with these + // additional suggestions. virtual void Fetch(const Category& category, const std::set<std::string>& known_suggestion_ids, const FetchDoneCallback& callback) = 0;
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc b/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc index 2c637e3..09da3bc 100644 --- a/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc +++ b/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
@@ -684,31 +684,12 @@ UpdateCategoryInfo(category, fetched_category.info); SanitizeReceivedSuggestions(existing_content->dismissed, &fetched_category.suggestions); - // We compute the result now before modifying |fetched_category.suggestions|. - // However, we wait with notifying the caller until the end of the method when - // all state is updated. std::vector<ContentSuggestion> result = ConvertToContentSuggestions(category, fetched_category.suggestions); - - // Fill up the newly fetched suggestions with existing ones, store them, and - // notify observers about new data. - while (fetched_category.suggestions.size() < - static_cast<size_t>(kMaxSuggestionCount) && - !existing_content->suggestions.empty()) { - fetched_category.suggestions.emplace( - fetched_category.suggestions.begin(), - std::move(existing_content->suggestions.back())); - existing_content->suggestions.pop_back(); - } - std::vector<std::string> to_dismiss = - *GetSuggestionIDVector(existing_content->suggestions); - for (const auto& id : to_dismiss) { - DismissSuggestionFromCategoryContent(existing_content, id); - } - DCHECK(existing_content->suggestions.empty()); - - IntegrateSuggestions(existing_content, - std::move(fetched_category.suggestions)); + // Store the additional suggestions into the archive to be able to fetch + // images and favicons for them. Note that ArchiveSuggestions clears + // |fetched_category.suggestions|. + ArchiveSuggestions(existing_content, &fetched_category.suggestions); // TODO(tschumann): We should properly honor the existing category state, // e.g. to make sure we don't serve results after the sign-out. Revisit this: @@ -716,7 +697,6 @@ // status? UpdateCategoryStatus(category, CategoryStatus::AVAILABLE); fetching_callback.Run(Status::Success(), std::move(result)); - NotifyNewSuggestions(category, *existing_content); } void RemoteSuggestionsProviderImpl::OnFetchFinished(
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc b/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc index ee61755..47c1737 100644 --- a/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc +++ b/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc
@@ -1012,12 +1012,6 @@ image = FetchImage(service.get(), MakeArticleID("http://second")); EXPECT_FALSE(image.IsEmpty()); EXPECT_EQ(1, image.Width()); - - // Verify that the observer received the update as well. We should see the - // newly-fetched items filled up with existing ones. - EXPECT_THAT(observer().SuggestionsForCategory(articles_category()), - ElementsAre(IdWithinCategoryEq("http://first"), - IdWithinCategoryEq("http://second"))); } // The tests TestMergingFetchedMoreSuggestionsFillup and @@ -1025,14 +1019,14 @@ // story: // 1) fetch suggestions in NTP A // 2) fetch more suggestions in NTP A. -// 3) open new NTP B: See the last 10 results visible in step 2). -// 4) fetch more suggestions in NTP B. Make sure no results from step 1) which -// were superseded in step 2) get merged back in again. +// 3) open new NTP B: See the first 10 results from step 1). +// 4) fetch more suggestions in NTP B. Make sure the results are independent +// from step 2) // TODO(tschumann): Test step 4) on a higher level instead of peeking into the // internal 'dismissed' data. The proper check is to make sure we tell the // backend to exclude these suggestions. TEST_F(RemoteSuggestionsProviderImplTest, - TestMergingFetchedMoreSuggestionsFillup) { + FewMoreFetchedSuggestionsShouldNotInterfere) { auto service = MakeSuggestionsProvider(/*set_empty_response=*/false); LoadFromJSONString(service.get(), GetTestJson({GetSuggestionWithUrl("http://id-1"), @@ -1071,65 +1065,37 @@ "http://id-9", "http://id-10"}, expect_receiving_two_new_suggestions); - // Verify that the observer received the update as well. We should see the - // newly-fetched items filled up with existing ones. The merging is done - // mimicking a scrolling behavior. + // Verify that the observer still has the old set. EXPECT_THAT( observer().SuggestionsForCategory(articles_category()), ElementsAre( + IdWithinCategoryEq("http://id-1"), IdWithinCategoryEq("http://id-2"), IdWithinCategoryEq("http://id-3"), IdWithinCategoryEq("http://id-4"), IdWithinCategoryEq("http://id-5"), IdWithinCategoryEq("http://id-6"), IdWithinCategoryEq("http://id-7"), IdWithinCategoryEq("http://id-8"), - IdWithinCategoryEq("http://id-9"), IdWithinCategoryEq("http://id-10"), - IdWithinCategoryEq("http://more-id-1"), - IdWithinCategoryEq("http://more-id-2"))); - // Verify the superseded suggestions got marked as dismissed. - EXPECT_THAT(service->GetDismissedSuggestionsForTesting(articles_category()), - ElementsAre(IdEq("http://id-1"), IdEq("http://id-2"))); + IdWithinCategoryEq("http://id-9"), + IdWithinCategoryEq("http://id-10"))); + + // No interference from previous Fetch more: we can receive two other ones. + expect_receiving_two_new_suggestions = + base::Bind([](Status status, std::vector<ContentSuggestion> suggestions) { + ASSERT_THAT(suggestions, SizeIs(2)); + EXPECT_THAT(suggestions[0], IdWithinCategoryEq("http://more-id-3")); + EXPECT_THAT(suggestions[1], IdWithinCategoryEq("http://more-id-4")); + }); + LoadMoreFromJSONString( + service.get(), articles_category(), + GetTestJson({GetSuggestionWithUrl("http://more-id-3"), + GetSuggestionWithUrl("http://more-id-4")}), + /*known_ids=*/ + {"http://id-1", "http://id-2", "http://id-3", "http://id-4", + "http://id-5", "http://id-6", "http://id-7", "http://id-8", + "http://id-9", "http://id-10"}, + expect_receiving_two_new_suggestions); } TEST_F(RemoteSuggestionsProviderImplTest, - ClearHistoryShouldDeleteArchivedSuggestions) { - auto service = MakeSuggestionsProvider(/*set_empty_response=*/false); - // First get suggestions into the archived state which happens through - // subsequent fetches. Then we verify the entries are gone from the 'archived' - // state by trying to load their images (and we shouldn't even know the URLs - // anymore). - LoadFromJSONString(service.get(), - GetTestJson({GetSuggestionWithUrl("http://id-1"), - GetSuggestionWithUrl("http://id-2")})); - LoadFromJSONString(service.get(), - GetTestJson({GetSuggestionWithUrl("http://new-id-1"), - GetSuggestionWithUrl("http://new-id-2")})); - // Make sure images of both batches are available. This is to sanity check our - // assumptions for the test are right. - ServeImageCallback cb = - base::Bind(&ServeOneByOneImage, &service->GetImageFetcherForTesting()); - EXPECT_CALL(*image_fetcher(), StartOrQueueNetworkRequest(_, _, _)) - .Times(2) - .WillRepeatedly(WithArgs<0, 2>(Invoke(CreateFunctor(cb)))); - image_decoder()->SetDecodedImage(gfx::test::CreateImage(1, 1)); - gfx::Image image = FetchImage(service.get(), MakeArticleID("http://id-1")); - ASSERT_FALSE(image.IsEmpty()); - ASSERT_EQ(1, image.Width()); - image = FetchImage(service.get(), MakeArticleID("http://new-id-1")); - ASSERT_FALSE(image.IsEmpty()); - ASSERT_EQ(1, image.Width()); - - service->ClearHistory(base::Time::UnixEpoch(), base::Time::Max(), - base::Callback<bool(const GURL& url)>()); - - // Make sure images of both batches are gone. - // Verify we cannot resolve the image of the new suggestions. - image_decoder()->SetDecodedImage(gfx::test::CreateImage(1, 1)); - EXPECT_TRUE( - FetchImage(service.get(), MakeArticleID("http://id-1")).IsEmpty()); - EXPECT_TRUE( - FetchImage(service.get(), MakeArticleID("http://new-id-1")).IsEmpty()); -} - -TEST_F(RemoteSuggestionsProviderImplTest, - TestMergingFetchedMoreSuggestionsReplaceAll) { + TenMoreFetchedSuggestionsShouldNotInterfere) { auto service = MakeSuggestionsProvider(/*set_empty_response=*/false); LoadFromJSONString(service.get(), GetTestJson({GetSuggestionWithUrl("http://id-1"), @@ -1183,24 +1149,88 @@ "http://id-5", "http://id-6", "http://id-7", "http://id-8", "http://id-9", "http://id-10"}, expect_receiving_ten_new_suggestions); - EXPECT_THAT(observer().SuggestionsForCategory(articles_category()), - ElementsAre(IdWithinCategoryEq("http://more-id-1"), - IdWithinCategoryEq("http://more-id-2"), - IdWithinCategoryEq("http://more-id-3"), - IdWithinCategoryEq("http://more-id-4"), - IdWithinCategoryEq("http://more-id-5"), - IdWithinCategoryEq("http://more-id-6"), - IdWithinCategoryEq("http://more-id-7"), - IdWithinCategoryEq("http://more-id-8"), - IdWithinCategoryEq("http://more-id-9"), - IdWithinCategoryEq("http://more-id-10"))); - // Verify the superseded suggestions got marked as dismissed. EXPECT_THAT( - service->GetDismissedSuggestionsForTesting(articles_category()), - ElementsAre(IdEq("http://id-1"), IdEq("http://id-2"), IdEq("http://id-3"), - IdEq("http://id-4"), IdEq("http://id-5"), IdEq("http://id-6"), - IdEq("http://id-7"), IdEq("http://id-8"), IdEq("http://id-9"), - IdEq("http://id-10"))); + observer().SuggestionsForCategory(articles_category()), + ElementsAre( + IdWithinCategoryEq("http://id-1"), IdWithinCategoryEq("http://id-2"), + IdWithinCategoryEq("http://id-3"), IdWithinCategoryEq("http://id-4"), + IdWithinCategoryEq("http://id-5"), IdWithinCategoryEq("http://id-6"), + IdWithinCategoryEq("http://id-7"), IdWithinCategoryEq("http://id-8"), + IdWithinCategoryEq("http://id-9"), + IdWithinCategoryEq("http://id-10"))); + + // This time, test receiving the same set. + expect_receiving_ten_new_suggestions = + base::Bind([](Status status, std::vector<ContentSuggestion> suggestions) { + EXPECT_THAT(suggestions, + ElementsAre(IdWithinCategoryEq("http://more-id-1"), + IdWithinCategoryEq("http://more-id-2"), + IdWithinCategoryEq("http://more-id-3"), + IdWithinCategoryEq("http://more-id-4"), + IdWithinCategoryEq("http://more-id-5"), + IdWithinCategoryEq("http://more-id-6"), + IdWithinCategoryEq("http://more-id-7"), + IdWithinCategoryEq("http://more-id-8"), + IdWithinCategoryEq("http://more-id-9"), + IdWithinCategoryEq("http://more-id-10"))); + }); + LoadMoreFromJSONString( + service.get(), articles_category(), + GetTestJson({GetSuggestionWithUrl("http://more-id-1"), + GetSuggestionWithUrl("http://more-id-2"), + GetSuggestionWithUrl("http://more-id-3"), + GetSuggestionWithUrl("http://more-id-4"), + GetSuggestionWithUrl("http://more-id-5"), + GetSuggestionWithUrl("http://more-id-6"), + GetSuggestionWithUrl("http://more-id-7"), + GetSuggestionWithUrl("http://more-id-8"), + GetSuggestionWithUrl("http://more-id-9"), + GetSuggestionWithUrl("http://more-id-10")}), + /*known_ids=*/ + {"http://id-1", "http://id-2", "http://id-3", "http://id-4", + "http://id-5", "http://id-6", "http://id-7", "http://id-8", + "http://id-9", "http://id-10"}, + expect_receiving_ten_new_suggestions); +} + +TEST_F(RemoteSuggestionsProviderImplTest, + ClearHistoryShouldDeleteArchivedSuggestions) { + auto service = MakeSuggestionsProvider(/*set_empty_response=*/false); + // First get suggestions into the archived state which happens through + // subsequent fetches. Then we verify the entries are gone from the 'archived' + // state by trying to load their images (and we shouldn't even know the URLs + // anymore). + LoadFromJSONString(service.get(), + GetTestJson({GetSuggestionWithUrl("http://id-1"), + GetSuggestionWithUrl("http://id-2")})); + LoadFromJSONString(service.get(), + GetTestJson({GetSuggestionWithUrl("http://new-id-1"), + GetSuggestionWithUrl("http://new-id-2")})); + // Make sure images of both batches are available. This is to sanity check our + // assumptions for the test are right. + ServeImageCallback cb = + base::Bind(&ServeOneByOneImage, &service->GetImageFetcherForTesting()); + EXPECT_CALL(*image_fetcher(), StartOrQueueNetworkRequest(_, _, _)) + .Times(2) + .WillRepeatedly(WithArgs<0, 2>(Invoke(CreateFunctor(cb)))); + image_decoder()->SetDecodedImage(gfx::test::CreateImage(1, 1)); + gfx::Image image = FetchImage(service.get(), MakeArticleID("http://id-1")); + ASSERT_FALSE(image.IsEmpty()); + ASSERT_EQ(1, image.Width()); + image = FetchImage(service.get(), MakeArticleID("http://new-id-1")); + ASSERT_FALSE(image.IsEmpty()); + ASSERT_EQ(1, image.Width()); + + service->ClearHistory(base::Time::UnixEpoch(), base::Time::Max(), + base::Callback<bool(const GURL& url)>()); + + // Make sure images of both batches are gone. + // Verify we cannot resolve the image of the new suggestions. + image_decoder()->SetDecodedImage(gfx::test::CreateImage(1, 1)); + EXPECT_TRUE( + FetchImage(service.get(), MakeArticleID("http://id-1")).IsEmpty()); + EXPECT_TRUE( + FetchImage(service.get(), MakeArticleID("http://new-id-1")).IsEmpty()); } // TODO(tschumann): We don't have test making sure the RemoteSuggestionsFetcher
diff --git a/components/payments/content/payment_request_state.cc b/components/payments/content/payment_request_state.cc index fb728d1..1236b70 100644 --- a/components/payments/content/payment_request_state.cc +++ b/components/payments/content/payment_request_state.cc
@@ -166,6 +166,18 @@ SetSelectedShippingProfile(new_cached_profile); } +void PaymentRequestState::AddAutofillContactProfile( + bool selected, + const autofill::AutofillProfile& profile) { + profile_cache_.push_back( + base::MakeUnique<autofill::AutofillProfile>(profile)); + autofill::AutofillProfile* new_cached_profile = profile_cache_.back().get(); + contact_profiles_.push_back(new_cached_profile); + + if (selected) + SetSelectedContactProfile(new_cached_profile); +} + void PaymentRequestState::SetSelectedShippingOption( const std::string& shipping_option_id) { spec_->StartWaitingForUpdateWith(
diff --git a/components/payments/content/payment_request_state.h b/components/payments/content/payment_request_state.h index 66241bd..72ea70ac5a 100644 --- a/components/payments/content/payment_request_state.h +++ b/components/payments/content/payment_request_state.h
@@ -149,6 +149,12 @@ void AddAutofillShippingProfile(bool selected, const autofill::AutofillProfile& profile); + // Creates and adds an AutofillProfile as a contact profile, which makes a + // copy of |profile|. |selected| indicates if the newly-created shipping + // profile should be selected, after which observers will be notified. + void AddAutofillContactProfile(bool selected, + const autofill::AutofillProfile& profile); + // Setters to change the selected information. Will have the side effect of // recomputing "is ready to pay" and notify observers. void SetSelectedShippingOption(const std::string& shipping_option_id);
diff --git a/content/browser/BUILD.gn b/content/browser/BUILD.gn index cfdac3e5..a6d5d1e8 100644 --- a/content/browser/BUILD.gn +++ b/content/browser/BUILD.gn
@@ -1896,6 +1896,8 @@ "android/java/jni_helper.h", "android/load_url_params.cc", "android/load_url_params.h", + "android/nfc_host.cc", + "android/nfc_host.h", "android/overscroll_controller_android.cc", "android/overscroll_controller_android.h", "android/render_widget_host_connector.cc",
diff --git a/content/browser/android/content_feature_list.cc b/content/browser/android/content_feature_list.cc index 9734477..ee10efb 100644 --- a/content/browser/android/content_feature_list.cc +++ b/content/browser/android/content_feature_list.cc
@@ -22,7 +22,7 @@ // this array may either refer to features defined in the header of this file or // in other locations in the code base (e.g. content_features.h). const base::Feature* kFeaturesExposedToJava[] = { - &kRequestUnbufferedDispatch, &features::kWebNfc, + &kRequestUnbufferedDispatch, }; const base::Feature* FindFeatureExposedToJava(const std::string& feature_name) {
diff --git a/content/browser/android/nfc_host.cc b/content/browser/android/nfc_host.cc new file mode 100644 index 0000000..2e56ee4 --- /dev/null +++ b/content/browser/android/nfc_host.cc
@@ -0,0 +1,49 @@ +// 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 "content/browser/android/nfc_host.h" + +#include <utility> + +#include "base/atomic_sequence_num.h" +#include "content/public/browser/android/content_view_core.h" +#include "content/public/common/service_manager_connection.h" +#include "device/nfc/nfc.mojom.h" +#include "jni/NfcHost_jni.h" +#include "services/device/public/interfaces/constants.mojom.h" +#include "services/service_manager/public/cpp/connector.h" + +namespace content { + +namespace { + +base::StaticAtomicSequenceNumber g_unique_id; + +} // namespace + +NFCHost::NFCHost(WebContents* web_contents) + : id_(g_unique_id.GetNext()), web_contents_(web_contents) { + DCHECK(web_contents_); + + JNIEnv* env = base::android::AttachCurrentThread(); + + java_nfc_host_.Reset( + Java_NfcHost_create(env, web_contents_->GetJavaWebContents().obj(), id_)); + + if (ServiceManagerConnection::GetForProcess()) { + service_manager::Connector* connector = + ServiceManagerConnection::GetForProcess()->GetConnector(); + connector->BindInterface(device::mojom::kServiceName, + mojo::MakeRequest(&nfc_provider_)); + } +} + +void NFCHost::GetNFC(device::nfc::mojom::NFCRequest request) { + // Connect to an NFC object, associating it with |id_|. + nfc_provider_->GetNFCForHost(id_, std::move(request)); +} + +NFCHost::~NFCHost() {} + +} // namespace content
diff --git a/content/browser/android/nfc_host.h b/content/browser/android/nfc_host.h new file mode 100644 index 0000000..b750c5f --- /dev/null +++ b/content/browser/android/nfc_host.h
@@ -0,0 +1,41 @@ +// 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 CONTENT_BROWSER_ANDROID_NFC_HOST_H_ +#define CONTENT_BROWSER_ANDROID_NFC_HOST_H_ + +#include "base/android/jni_android.h" +#include "content/public/browser/web_contents.h" +#include "device/nfc/nfc_provider.mojom.h" + +namespace content { + +// On Android, NFC requires the Activity associated with the context in order to +// access the NFC system APIs. NFCHost provides this functionality by mapping +// NFC context IDs to the WebContents associated with those IDs. +class NFCHost { + public: + explicit NFCHost(WebContents* web_contents); + ~NFCHost(); + + void GetNFC(device::nfc::mojom::NFCRequest request); + + private: + // This instance's ID (passed to the NFC implementation via |nfc_provider_| + // and used from the implementation to map back to this object). + int id_; + + // The WebContents that owns this instance. + WebContents* web_contents_; + + device::nfc::mojom::NFCProviderPtr nfc_provider_; + + base::android::ScopedJavaGlobalRef<jobject> java_nfc_host_; + + DISALLOW_COPY_AND_ASSIGN(NFCHost); +}; + +} // namespace content + +#endif // CONTENT_BROWSER_ANDROID_NFC_HOST_H_
diff --git a/content/browser/frame_host/render_frame_host_delegate.cc b/content/browser/frame_host/render_frame_host_delegate.cc index 370e426..2076f1c 100644 --- a/content/browser/frame_host/render_frame_host_delegate.cc +++ b/content/browser/frame_host/render_frame_host_delegate.cc
@@ -81,6 +81,10 @@ return nullptr; } +#if defined(OS_ANDROID) +void RenderFrameHostDelegate::GetNFC(device::nfc::mojom::NFCRequest request) {} +#endif + bool RenderFrameHostDelegate::ShouldRouteMessageEvent( RenderFrameHost* target_rfh, SiteInstance* source_site_instance) const {
diff --git a/content/browser/frame_host/render_frame_host_delegate.h b/content/browser/frame_host/render_frame_host_delegate.h index 081986b..5079adfb 100644 --- a/content/browser/frame_host/render_frame_host_delegate.h +++ b/content/browser/frame_host/render_frame_host_delegate.h
@@ -29,6 +29,7 @@ #if defined(OS_ANDROID) #include "base/android/scoped_java_ref.h" +#include "device/nfc/nfc.mojom.h" #endif class GURL; @@ -194,6 +195,11 @@ // Gets the WakeLockService that serves wake lock requests from the renderer. virtual device::mojom::WakeLockService* GetRendererWakeLock(); +#if defined(OS_ANDROID) + // Gets an NFC implementation within the context of this delegate. + virtual void GetNFC(device::nfc::mojom::NFCRequest request); +#endif + // Notification that the frame wants to go into fullscreen mode. // |origin| represents the origin of the frame that requests fullscreen. virtual void EnterFullscreenMode(const GURL& origin) {}
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc index 24bdcd1..4b35a18 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -10,6 +10,7 @@ #include "base/bind.h" #include "base/command_line.h" #include "base/containers/hash_tables.h" +#include "base/feature_list.h" #include "base/lazy_instance.h" #include "base/memory/ptr_util.h" #include "base/metrics/histogram_macros.h" @@ -2688,6 +2689,13 @@ base::Bind(&RenderFrameHostImpl::BindWakeLockServiceRequest, base::Unretained(this))); +#if defined(OS_ANDROID) + if (base::FeatureList::IsEnabled(features::kWebNfc)) { + GetInterfaceRegistry()->AddInterface<device::nfc::mojom::NFC>(base::Bind( + &RenderFrameHostImpl::BindNFCRequest, base::Unretained(this))); + } +#endif + if (!permission_service_context_) permission_service_context_.reset(new PermissionServiceContext(this)); @@ -3743,6 +3751,15 @@ renderer_wake_lock->AddClient(std::move(request)); } +#if defined(OS_ANDROID) +void RenderFrameHostImpl::BindNFCRequest( + const service_manager::BindSourceInfo& source_info, + device::nfc::mojom::NFCRequest request) { + if (delegate_) + delegate_->GetNFC(std::move(request)); +} +#endif + void RenderFrameHostImpl::GetInterface( const std::string& interface_name, mojo::ScopedMessagePipeHandle interface_pipe) {
diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h index 7485678..27e48d3e 100644 --- a/content/browser/frame_host/render_frame_host_impl.h +++ b/content/browser/frame_host/render_frame_host_impl.h
@@ -58,6 +58,10 @@ #include "ui/base/mojo/window_open_disposition.mojom.h" #include "ui/base/page_transition_types.h" +#if defined(OS_ANDROID) +#include "device/nfc/nfc.mojom.h" +#endif + class GURL; struct AccessibilityHostMsg_EventParams; struct AccessibilityHostMsg_FindInPageResultParams; @@ -917,6 +921,11 @@ const service_manager::BindSourceInfo& source_info, device::mojom::WakeLockServiceRequest request); +#if defined(OS_ANDROID) + void BindNFCRequest(const service_manager::BindSourceInfo& source_info, + device::nfc::mojom::NFCRequest request); +#endif + // service_manager::mojom::InterfaceProvider: void GetInterface(const std::string& interface_name, mojo::ScopedMessagePipeHandle interface_pipe) override;
diff --git a/content/browser/service_manager/service_manager_context.cc b/content/browser/service_manager/service_manager_context.cc index 4931f96..6bf17dd 100644 --- a/content/browser/service_manager/service_manager_context.cc +++ b/content/browser/service_manager/service_manager_context.cc
@@ -51,6 +51,13 @@ #include "services/service_manager/service_manager.h" #include "services/shape_detection/public/interfaces/constants.mojom.h" +#if defined(OS_ANDROID) +#include "base/android/context_utils.h" +#include "base/android/jni_android.h" +#include "base/android/scoped_java_ref.h" +#include "jni/ContentNfcDelegate_jni.h" +#endif + namespace content { namespace { @@ -291,13 +298,19 @@ ServiceInfo device_info; #if defined(OS_ANDROID) - // See the comments on wake_lock_context_host.h for details on this - // callback. + JNIEnv* env = base::android::AttachCurrentThread(); + base::android::ScopedJavaGlobalRef<jobject> java_nfc_delegate; + java_nfc_delegate.Reset(Java_ContentNfcDelegate_create(env)); + DCHECK(!java_nfc_delegate.is_null()); + + // See the comments on wake_lock_context_host.h and ContentNfcDelegate.java + // respectively for comments on those parameters. device_info.factory = base::Bind(&device::CreateDeviceService, BrowserThread::GetTaskRunnerForThread(BrowserThread::FILE), BrowserThread::GetTaskRunnerForThread(BrowserThread::IO), - base::Bind(&WakeLockContextHost::GetNativeViewForContext)); + base::Bind(&WakeLockContextHost::GetNativeViewForContext), + std::move(java_nfc_delegate)); #else device_info.factory = base::Bind(&device::CreateDeviceService,
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 78c9738..41778bf 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc
@@ -2651,6 +2651,14 @@ return renderer_wake_lock_.get(); } +#if defined(OS_ANDROID) +void WebContentsImpl::GetNFC(device::nfc::mojom::NFCRequest request) { + if (!nfc_host_) + nfc_host_.reset(new NFCHost(this)); + nfc_host_->GetNFC(std::move(request)); +} +#endif + void WebContentsImpl::OnShowValidationMessage( RenderViewHostImpl* source, const gfx::Rect& anchor_in_root_view, @@ -4341,10 +4349,6 @@ render_frame_host->Send( new FrameMsg_EnableViewSourceMode(render_frame_host->GetRoutingID())); } -#if defined(OS_ANDROID) - render_frame_host->GetInterfaceRegistry()->AddInterface( - GetJavaInterfaces()->CreateInterfaceFactory<device::nfc::mojom::NFC>()); -#endif } void WebContentsImpl::RenderFrameDeleted(RenderFrameHost* render_frame_host) {
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h index 21cd93e..6f7d383 100644 --- a/content/browser/web_contents/web_contents_impl.h +++ b/content/browser/web_contents/web_contents_impl.h
@@ -53,6 +53,10 @@ #include "ui/gfx/geometry/rect_f.h" #include "ui/gfx/geometry/size.h" +#if defined(OS_ANDROID) +#include "content/browser/android/nfc_host.h" +#endif + struct ViewHostMsg_DateTimeDialogValue_Params; namespace service_manager { @@ -510,6 +514,9 @@ device::GeolocationServiceContext* GetGeolocationServiceContext() override; device::mojom::WakeLockContext* GetWakeLockContext() override; device::mojom::WakeLockService* GetRendererWakeLock() override; +#if defined(OS_ANDROID) + void GetNFC(device::nfc::mojom::NFCRequest request) override; +#endif void EnterFullscreenMode(const GURL& origin) override; void ExitFullscreenMode(bool will_cause_resize) override; bool ShouldRouteMessageEvent( @@ -1516,6 +1523,10 @@ device::mojom::WakeLockServicePtr renderer_wake_lock_; +#if defined(OS_ANDROID) + std::unique_ptr<NFCHost> nfc_host_; +#endif + std::unique_ptr<ScreenOrientationProvider> screen_orientation_provider_; std::unique_ptr<ManifestManagerHost> manifest_manager_host_;
diff --git a/content/public/android/BUILD.gn b/content/public/android/BUILD.gn index 91aaf8f3..1add51d 100644 --- a/content/public/android/BUILD.gn +++ b/content/public/android/BUILD.gn
@@ -38,7 +38,7 @@ "//device/gamepad:java", "//device/generic_sensor:java", "//device/nfc:mojo_bindings_java", - "//device/nfc/android:java", + "//device/nfc/public/java:nfc_java", "//device/power_save_blocker:java", "//device/sensors:java", "//device/usb:java", @@ -123,6 +123,7 @@ "java/src/org/chromium/content/browser/ChildSpawnData.java", "java/src/org/chromium/content/browser/ContentClassFactory.java", "java/src/org/chromium/content/browser/ContentFeatureList.java", + "java/src/org/chromium/content/browser/ContentNfcDelegate.java", "java/src/org/chromium/content/browser/ContentVideoView.java", "java/src/org/chromium/content/browser/ContentVideoViewEmbedder.java", "java/src/org/chromium/content/browser/ContentView.java", @@ -143,7 +144,7 @@ "java/src/org/chromium/content/browser/MemoryMonitorAndroid.java", "java/src/org/chromium/content/browser/MenuDescriptor.java", "java/src/org/chromium/content/browser/MotionEventSynthesizer.java", - "java/src/org/chromium/content/browser/NfcFactory.java", + "java/src/org/chromium/content/browser/NfcHost.java", "java/src/org/chromium/content/browser/PepperPluginManager.java", "java/src/org/chromium/content/browser/PopupZoomer.java", "java/src/org/chromium/content/browser/PositionObserver.java", @@ -335,6 +336,7 @@ "java/src/org/chromium/content/browser/BrowserStartupController.java", "java/src/org/chromium/content/browser/ChildProcessLauncherHelper.java", "java/src/org/chromium/content/browser/ContentFeatureList.java", + "java/src/org/chromium/content/browser/ContentNfcDelegate.java", "java/src/org/chromium/content/browser/ContentVideoView.java", "java/src/org/chromium/content/browser/ContentViewCore.java", "java/src/org/chromium/content/browser/ContentViewRenderView.java", @@ -347,6 +349,7 @@ "java/src/org/chromium/content/browser/MediaSessionImpl.java", "java/src/org/chromium/content/browser/MemoryMonitorAndroid.java", "java/src/org/chromium/content/browser/MotionEventSynthesizer.java", + "java/src/org/chromium/content/browser/NfcHost.java", "java/src/org/chromium/content/browser/ScreenOrientationProvider.java", "java/src/org/chromium/content/browser/SelectionPopupController.java", "java/src/org/chromium/content/browser/SmartSelectionClient.java",
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentFeatureList.java b/content/public/android/java/src/org/chromium/content/browser/ContentFeatureList.java index 4c81d32..5312887 100644 --- a/content/public/android/java/src/org/chromium/content/browser/ContentFeatureList.java +++ b/content/public/android/java/src/org/chromium/content/browser/ContentFeatureList.java
@@ -31,7 +31,6 @@ // Alphabetical: public static final String REQUEST_UNBUFFERED_DISPATCH = "RequestUnbufferedDispatch"; - public static final String WEB_NFC = "WebNFC"; private static native boolean nativeIsEnabled(String featureName); }
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentNfcDelegate.java b/content/public/android/java/src/org/chromium/content/browser/ContentNfcDelegate.java new file mode 100644 index 0000000..2fea57f --- /dev/null +++ b/content/public/android/java/src/org/chromium/content/browser/ContentNfcDelegate.java
@@ -0,0 +1,38 @@ +// 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.content.browser; + +import android.app.Activity; + +import org.chromium.base.Callback; +import org.chromium.base.annotations.CalledByNative; +import org.chromium.device.nfc.NfcDelegate; + +/** A //content-specific implementation of the NfcDelegate interface. Maps NFC host ideas to their + * corresponding NfcHost objects, allowing the NFC implementation to access the Activity of the + * WebContents with which its requesting frame is associated. + */ +public class ContentNfcDelegate implements NfcDelegate { + @CalledByNative + private static ContentNfcDelegate create() { + return new ContentNfcDelegate(); + } + + @Override + public void trackActivityForHost(int hostId, Callback<Activity> callback) { + NfcHost host = NfcHost.fromContextId(hostId); + if (host == null) return; + host.trackActivityChanges(callback); + } + + @Override + public void stopTrackingActivityForHost(int hostId) { + NfcHost host = NfcHost.fromContextId(hostId); + if (host == null) { + return; + } + host.stopTrackingActivityChanges(); + } +}
diff --git a/content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrarImpl.java b/content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrarImpl.java index 8135734..9bd21e3 100644 --- a/content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrarImpl.java +++ b/content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrarImpl.java
@@ -12,7 +12,6 @@ import org.chromium.content_public.browser.InterfaceRegistrar; import org.chromium.content_public.browser.RenderFrameHost; import org.chromium.content_public.browser.WebContents; -import org.chromium.device.nfc.mojom.Nfc; import org.chromium.media.mojom.AndroidOverlayProvider; import org.chromium.mojo.system.impl.CoreImpl; import org.chromium.services.service_manager.InterfaceRegistry; @@ -54,8 +53,6 @@ if (sHasRegisteredRegistrars) return; sHasRegisteredRegistrars = true; InterfaceRegistrar.Registry.addContextRegistrar(new ContentContextInterfaceRegistrar()); - InterfaceRegistrar.Registry.addWebContentsRegistrar( - new ContentWebContentsInterfaceRegistrar()); } private static class ContentContextInterfaceRegistrar implements InterfaceRegistrar<Context> { @@ -67,14 +64,4 @@ // TODO(avayvod): Register the PresentationService implementation here. } } - - private static class ContentWebContentsInterfaceRegistrar - implements InterfaceRegistrar<WebContents> { - @Override - public void registerInterfaces(InterfaceRegistry registry, final WebContents webContents) { - if (ContentFeatureList.isEnabled(ContentFeatureList.WEB_NFC)) { - registry.addInterface(Nfc.MANAGER, new NfcFactory(webContents)); - } - } - } }
diff --git a/content/public/android/java/src/org/chromium/content/browser/NfcFactory.java b/content/public/android/java/src/org/chromium/content/browser/NfcFactory.java deleted file mode 100644 index 35268c1..0000000 --- a/content/public/android/java/src/org/chromium/content/browser/NfcFactory.java +++ /dev/null
@@ -1,61 +0,0 @@ -// Copyright 2016 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.content.browser; - -import android.app.Activity; - -import org.chromium.content_public.browser.WebContents; -import org.chromium.device.nfc.NfcImpl; -import org.chromium.device.nfc.mojom.Nfc; -import org.chromium.services.service_manager.InterfaceFactory; -import org.chromium.ui.base.WindowAndroid; - -class NfcFactory implements InterfaceFactory<Nfc> { - private final WebContents mContents; - - NfcFactory(WebContents contents) { - mContents = contents; - } - - private static class ContextAwareNfcImpl - extends NfcImpl implements WindowAndroidChangedObserver { - private final ContentViewCore mContentViewCore; - - ContextAwareNfcImpl(ContentViewCore contentViewCore) { - super(contentViewCore.getContext().getApplicationContext()); - mContentViewCore = contentViewCore; - mContentViewCore.addWindowAndroidChangedObserver(this); - if (mContentViewCore.getWindowAndroid() != null) { - setActivity(mContentViewCore.getWindowAndroid().getActivity().get()); - } - } - - @Override - public void close() { - super.close(); - if (mContentViewCore != null) { - mContentViewCore.removeWindowAndroidChangedObserver(this); - } - } - - @Override - public void onWindowAndroidChanged(WindowAndroid newWindowAndroid) { - Activity activity = null; - if (newWindowAndroid != null) { - activity = newWindowAndroid.getActivity().get(); - } - setActivity(activity); - } - } - - @Override - public Nfc createImpl() { - ContentViewCore contentViewCore = ContentViewCore.fromWebContents(mContents); - if (contentViewCore == null) { - return null; - } - return new ContextAwareNfcImpl(contentViewCore); - } -}
diff --git a/content/public/android/java/src/org/chromium/content/browser/NfcHost.java b/content/public/android/java/src/org/chromium/content/browser/NfcHost.java new file mode 100644 index 0000000..32813fd --- /dev/null +++ b/content/public/android/java/src/org/chromium/content/browser/NfcHost.java
@@ -0,0 +1,112 @@ +// Copyright 2016 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.content.browser; + +import android.app.Activity; +import android.util.SparseArray; + +import org.chromium.base.Callback; +import org.chromium.base.annotations.CalledByNative; +import org.chromium.content_public.browser.WebContents; +import org.chromium.content_public.browser.WebContentsObserver; +import org.chromium.ui.base.WindowAndroid; + +/** Tracks the Activiy for a given WebContents on behalf of a NFC instance that cannot talk + * directly to WebContents. + */ +class NfcHost extends WebContentsObserver implements WindowAndroidChangedObserver { + private static final SparseArray<NfcHost> sContextHostsMap = new SparseArray<NfcHost>(); + + // The WebContents with which this host is associated. + private final WebContents mWebContents; + private final ContentViewCore mContentViewCore; + + // The context ID with which this host is associated. + private final int mContextId; + + // The callback that the NFC instance has registered for being notified when the Activity + // changes. + private Callback<Activity> mCallback; + + /** + * Provides access to NfcHost via context ID. + */ + public static NfcHost fromContextId(int contextId) { + return sContextHostsMap.get(contextId); + } + + @CalledByNative + private static NfcHost create(WebContents webContents, int contextId) { + return new NfcHost(webContents, contextId); + } + + NfcHost(WebContents webContents, int contextId) { + super(webContents); + + mWebContents = webContents; + mContentViewCore = ContentViewCore.fromWebContents(mWebContents); + + // NFC will not work if there is no CVC associated with the WebContents, and it will only + // be requested in contexts where there is a CVC associated with the WebContents as far as + // we are aware. If the latter ever proves false, then we will need to simply drop any NFC + // request that comes in if there is no CVC available. + assert mContentViewCore != null; + mContextId = contextId; + sContextHostsMap.put(mContextId, this); + } + + /** Called by the NFC implementation (via ContentNfcDelegate) to allow that implementation to + * track changes to the Activity associated with its context ID (i.e., the activity associated + * with |mWebContents|). + */ + + public void trackActivityChanges(Callback<Activity> callback) { + // Only the main frame is allowed to access NFC + // (https://w3c.github.io/web-nfc/#security-policies). The renderer enforces this by + // dropping connection requests from nested frames. Therefore, this class should never see + // a request to track activity changes while there is already such a request. + assert mCallback == null : "Unexpected request to track activity changes"; + mCallback = callback; + Activity activity = null; + + mContentViewCore.addWindowAndroidChangedObserver(this); + if (mContentViewCore.getWindowAndroid() != null) { + activity = mContentViewCore.getWindowAndroid().getActivity().get(); + } + + mCallback.onResult(activity); + } + + /** + * @see org.chromium.device.nfc.NfcDelegate#stopTrackingActivityForHost(int) + */ + public void stopTrackingActivityChanges() { + mCallback = null; + mContentViewCore.removeWindowAndroidChangedObserver(this); + } + + /** + * Tears down current and future Activity tracking. + */ + @Override + public void destroy() { + stopTrackingActivityChanges(); + sContextHostsMap.remove(mContextId); + super.destroy(); + } + + /** + * Updates the Activity associated with this instance. + */ + @Override + public void onWindowAndroidChanged(WindowAndroid newWindowAndroid) { + Activity activity = null; + if (newWindowAndroid != null) { + activity = newWindowAndroid.getActivity().get(); + } + assert mCallback != null : "should have callback"; + mCallback.onResult(activity); + } +}
diff --git a/content/public/app/mojo/content_browser_manifest.json b/content/public/app/mojo/content_browser_manifest.json index a89ba20..fbce377 100644 --- a/content/public/app/mojo/content_browser_manifest.json +++ b/content/public/app/mojo/content_browser_manifest.json
@@ -59,7 +59,7 @@ "content_renderer": [ "browser" ], "content_utility": [ "browser" ], "data_decoder": [ "image_decoder" ], - "device": [ "device:wake_lock" ], + "device": [ "device:nfc", "device:wake_lock" ], "file": [ "file:filesystem", "file:leveldb" ], "media": [ "media:media" ], "network": [
diff --git a/device/BUILD.gn b/device/BUILD.gn index fd30fa3..9ce68f2 100644 --- a/device/BUILD.gn +++ b/device/BUILD.gn
@@ -306,6 +306,7 @@ "//device/geolocation:geolocation_java_test_support", "//device/nfc:mojo_bindings_java", "//device/nfc/android:java", + "//device/nfc/public/java:nfc_java", "//mojo/public/java:bindings_java", "//third_party/android_tools:android_support_annotations_java", ]
diff --git a/device/nfc/BUILD.gn b/device/nfc/BUILD.gn index 45dd3691..8c810f29 100644 --- a/device/nfc/BUILD.gn +++ b/device/nfc/BUILD.gn
@@ -8,5 +8,6 @@ mojom("mojo_bindings") { sources = [ "nfc.mojom", + "nfc_provider.mojom", ] }
diff --git a/device/nfc/android/BUILD.gn b/device/nfc/android/BUILD.gn index ed1f51d..8dc6042 100644 --- a/device/nfc/android/BUILD.gn +++ b/device/nfc/android/BUILD.gn
@@ -8,6 +8,7 @@ android_library("java") { java_files = [ "java/src/org/chromium/device/nfc/InvalidNfcMessageException.java", + "java/src/org/chromium/device/nfc/NfcProviderImpl.java", "java/src/org/chromium/device/nfc/NfcImpl.java", "java/src/org/chromium/device/nfc/NfcMessageValidator.java", "java/src/org/chromium/device/nfc/NfcTagHandler.java", @@ -17,7 +18,10 @@ deps = [ "//base:base_java", "//device/nfc:mojo_bindings_java", + "//device/nfc/public/java:nfc_java", "//mojo/public/java:bindings_java", "//mojo/public/java:system_java", + "//services/service_manager/public/interfaces:interfaces_java", + "//services/service_manager/public/java:service_manager_java", ] }
diff --git a/device/nfc/android/java/src/org/chromium/device/nfc/NfcImpl.java b/device/nfc/android/java/src/org/chromium/device/nfc/NfcImpl.java index a2c5380..e4efd51 100644 --- a/device/nfc/android/java/src/org/chromium/device/nfc/NfcImpl.java +++ b/device/nfc/android/java/src/org/chromium/device/nfc/NfcImpl.java
@@ -21,6 +21,7 @@ import android.os.Process; import android.util.SparseArray; +import org.chromium.base.Callback; import org.chromium.base.Log; import org.chromium.device.nfc.mojom.Nfc; import org.chromium.device.nfc.mojom.NfcClient; @@ -39,13 +40,15 @@ import java.util.ArrayList; import java.util.List; -/** - * Android implementation of the NFC mojo service defined in - * device/nfc/nfc.mojom. +/** Android implementation of the NFC mojo service defined in device/nfc/nfc.mojom. */ public class NfcImpl implements Nfc { private static final String TAG = "NfcImpl"; + private final int mHostId; + + private final NfcDelegate mDelegate; + /** * Used to get instance of NFC adapter, @see android.nfc.NfcManager */ @@ -114,10 +117,20 @@ */ private Runnable mPushTimeoutRunnable; - public NfcImpl(Context context) { + public NfcImpl(Context context, int hostId, NfcDelegate delegate) { + mHostId = hostId; + mDelegate = delegate; int permission = context.checkPermission(Manifest.permission.NFC, Process.myPid(), Process.myUid()); mHasPermission = permission == PackageManager.PERMISSION_GRANTED; + Callback<Activity> onActivityUpdatedCallback = new Callback<Activity>() { + @Override + public void onResult(Activity activity) { + setActivity(activity); + } + }; + + mDelegate.trackActivityForHost(mHostId, onActivityUpdatedCallback); if (!mHasPermission || Build.VERSION.SDK_INT < Build.VERSION_CODES.KITKAT) { Log.w(TAG, "NFC operations are not permitted."); @@ -293,6 +306,7 @@ @Override public void close() { + mDelegate.stopTrackingActivityForHost(mHostId); disableReaderMode(); }
diff --git a/device/nfc/android/java/src/org/chromium/device/nfc/NfcProviderImpl.java b/device/nfc/android/java/src/org/chromium/device/nfc/NfcProviderImpl.java new file mode 100644 index 0000000..04f19f6 --- /dev/null +++ b/device/nfc/android/java/src/org/chromium/device/nfc/NfcProviderImpl.java
@@ -0,0 +1,56 @@ +// 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.device.nfc; + +import android.content.Context; + +import org.chromium.device.nfc.mojom.Nfc; +import org.chromium.device.nfc.mojom.NfcProvider; +import org.chromium.mojo.bindings.InterfaceRequest; +import org.chromium.mojo.system.MojoException; +import org.chromium.services.service_manager.InterfaceFactory; + +/** + * Android implementation of the NfcProvider Mojo interface. + */ +public class NfcProviderImpl implements NfcProvider { + private static final String TAG = "NfcProviderImpl"; + private Context mContext; + private NfcDelegate mDelegate; + + public NfcProviderImpl(Context context, NfcDelegate delegate) { + mContext = context; + mDelegate = delegate; + } + + @Override + public void close() {} + + @Override + public void onConnectionError(MojoException e) {} + + @Override + public void getNfcForHost(int hostId, InterfaceRequest<Nfc> request) { + Nfc.MANAGER.bind(new NfcImpl(mContext, hostId, mDelegate), request); + } + + /** + * A factory for implementations of the NfcProvider interface. + */ + public static class Factory implements InterfaceFactory<NfcProvider> { + private Context mContext; + private NfcDelegate mDelegate; + + public Factory(Context context, NfcDelegate delegate) { + mContext = context; + mDelegate = delegate; + } + + @Override + public NfcProvider createImpl() { + return new NfcProviderImpl(mContext, mDelegate); + } + } +}
diff --git a/device/nfc/android/java/src/org/chromium/device/nfc/NfcTagHandler.java b/device/nfc/android/java/src/org/chromium/device/nfc/NfcTagHandler.java index b4029da..9e8b9c46 100644 --- a/device/nfc/android/java/src/org/chromium/device/nfc/NfcTagHandler.java +++ b/device/nfc/android/java/src/org/chromium/device/nfc/NfcTagHandler.java
@@ -154,4 +154,4 @@ } return false; } -} \ No newline at end of file +}
diff --git a/device/nfc/android/junit/src/org/chromium/device/nfc/NFCTest.java b/device/nfc/android/junit/src/org/chromium/device/nfc/NFCTest.java index 00daefd..dc9858801 100644 --- a/device/nfc/android/junit/src/org/chromium/device/nfc/NFCTest.java +++ b/device/nfc/android/junit/src/org/chromium/device/nfc/NFCTest.java
@@ -40,6 +40,7 @@ import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowLooper; +import org.chromium.base.Callback; import org.chromium.base.test.util.Feature; import org.chromium.device.nfc.mojom.Nfc.CancelAllWatchesResponse; import org.chromium.device.nfc.mojom.Nfc.CancelPushResponse; @@ -68,6 +69,7 @@ @RunWith(LocalRobolectricTestRunner.class) @Config(manifest = Config.NONE) public class NFCTest { + private TestNfcDelegate mDelegate; @Mock private Context mContext; @Mock @@ -103,12 +105,8 @@ * Class that is used test NfcImpl implementation */ private static class TestNfcImpl extends NfcImpl { - public TestNfcImpl(Context context) { - super(context); - } - - public void setActivityForTesting(Activity activity) { - super.setActivity(activity); + public TestNfcImpl(Context context, NfcDelegate delegate) { + super(context, 0, delegate); } public void processPendingOperationsForTesting(NfcTagHandler handler) { @@ -116,9 +114,28 @@ } } + private static class TestNfcDelegate implements NfcDelegate { + Activity mActivity; + Callback<Activity> mCallback; + + public TestNfcDelegate(Activity activity) { + mActivity = activity; + } + public void trackActivityForHost(int hostId, Callback<Activity> callback) { + mCallback = callback; + } + + public void invokeCallback() { + mCallback.onResult(mActivity); + } + + public void stopTrackingActivityForHost(int hostId) {} + } + @Before public void setUp() { MockitoAnnotations.initMocks(this); + mDelegate = new TestNfcDelegate(mActivity); doReturn(mNfcManager).when(mContext).getSystemService(Context.NFC_SERVICE); doReturn(mNfcAdapter).when(mNfcManager).getDefaultAdapter(); doReturn(true).when(mNfcAdapter).isEnabled(); @@ -149,8 +166,8 @@ @Feature({"NFCTest"}) public void testNFCNotSupported() { doReturn(null).when(mNfcManager).getDefaultAdapter(); - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); CancelAllWatchesResponse mockCallback = mock(CancelAllWatchesResponse.class); nfc.cancelAllWatches(mockCallback); verify(mockCallback).call(mErrorCaptor.capture()); @@ -166,7 +183,7 @@ doReturn(PackageManager.PERMISSION_DENIED) .when(mContext) .checkPermission(anyString(), anyInt(), anyInt()); - TestNfcImpl nfc = new TestNfcImpl(mContext); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); CancelAllWatchesResponse mockCallback = mock(CancelAllWatchesResponse.class); nfc.cancelAllWatches(mockCallback); verify(mockCallback).call(mErrorCaptor.capture()); @@ -179,8 +196,8 @@ @Test @Feature({"NFCTest"}) public void testNFCIsSupported() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); WatchResponse mockCallback = mock(WatchResponse.class); nfc.watch(createNfcWatchOptions(), mockCallback); verify(mockCallback).call(anyInt(), mErrorCaptor.capture()); @@ -315,8 +332,8 @@ @Test @Feature({"NFCTest"}) public void testInvalidNfcMessage() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); PushResponse mockCallback = mock(PushResponse.class); nfc.push(new NfcMessage(), createNfcPushOptions(), mockCallback); nfc.processPendingOperationsForTesting(mNfcTagHandler); @@ -330,12 +347,12 @@ @Test @Feature({"NFCTest"}) public void testResumeSuspend() { - TestNfcImpl nfc = new TestNfcImpl(mContext); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); // No activity / client or active pending operations nfc.suspendNfcOperations(); nfc.resumeNfcOperations(); - nfc.setActivityForTesting(mActivity); + mDelegate.invokeCallback(); nfc.setClient(mNfcClient); WatchResponse mockCallback = mock(WatchResponse.class); nfc.watch(createNfcWatchOptions(), mockCallback); @@ -364,8 +381,8 @@ @Test @Feature({"NFCTest"}) public void testPush() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); PushResponse mockCallback = mock(PushResponse.class); nfc.push(createNfcMessage(), createNfcPushOptions(), mockCallback); nfc.processPendingOperationsForTesting(mNfcTagHandler); @@ -379,8 +396,8 @@ @Test @Feature({"NFCTest"}) public void testCancelPush() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); PushResponse mockPushCallback = mock(PushResponse.class); CancelPushResponse mockCancelPushCallback = mock(CancelPushResponse.class); nfc.push(createNfcMessage(), createNfcPushOptions(), mockPushCallback); @@ -401,8 +418,8 @@ @Test @Feature({"NFCTest"}) public void testWatch() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); nfc.setClient(mNfcClient); WatchResponse mockWatchCallback1 = mock(WatchResponse.class); nfc.watch(createNfcWatchOptions(), mockWatchCallback1); @@ -436,8 +453,8 @@ @Test @Feature({"NFCTest"}) public void testlWatchMatching() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); nfc.setClient(mNfcClient); // Should match by WebNFC Id. @@ -499,8 +516,8 @@ @Test @Feature({"NFCTest"}) public void testCancelWatch() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); WatchResponse mockWatchCallback = mock(WatchResponse.class); nfc.watch(createNfcWatchOptions(), mockWatchCallback); @@ -525,8 +542,8 @@ @Test @Feature({"NFCTest"}) public void testCancelAllWatches() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); WatchResponse mockWatchCallback1 = mock(WatchResponse.class); WatchResponse mockWatchCallback2 = mock(WatchResponse.class); nfc.watch(createNfcWatchOptions(), mockWatchCallback1); @@ -551,8 +568,8 @@ @Test @Feature({"NFCTest"}) public void testCancelWatchInvalidId() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); WatchResponse mockWatchCallback = mock(WatchResponse.class); nfc.watch(createNfcWatchOptions(), mockWatchCallback); @@ -573,8 +590,8 @@ @Test @Feature({"NFCTest"}) public void testCancelAllWatchesWithNoWathcers() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); CancelAllWatchesResponse mockCallback = mock(CancelAllWatchesResponse.class); nfc.cancelAllWatches(mockCallback); @@ -588,8 +605,8 @@ @Test @Feature({"NFCTest"}) public void testTagDisconnectedDuringRead() throws IOException, FormatException { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); nfc.setClient(mNfcClient); WatchResponse mockWatchCallback = mock(WatchResponse.class); nfc.watch(createNfcWatchOptions(), mockWatchCallback); @@ -611,8 +628,8 @@ @Test @Feature({"NFCTest"}) public void testTagDisconnectedDuringWrite() throws IOException, FormatException { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); PushResponse mockCallback = mock(PushResponse.class); // Force write operation to fail @@ -632,8 +649,8 @@ @Test @Feature({"NFCTest"}) public void testPushTimeout() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); PushResponse mockCallback = mock(PushResponse.class); // Set 1 millisecond timeout. @@ -654,8 +671,8 @@ @Test @Feature({"NFCTest"}) public void testPushMultipleInvocations() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); PushResponse mockCallback1 = mock(PushResponse.class); PushResponse mockCallback2 = mock(PushResponse.class); @@ -678,8 +695,8 @@ @Test @Feature({"NFCTest"}) public void testReaderModeIsDisabledAfterTimeout() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); PushResponse mockCallback = mock(PushResponse.class); // Set 1 millisecond timeout. @@ -708,8 +725,8 @@ @Test @Feature({"NFCTest"}) public void testTwoPushInvocationsWithCancel() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); PushResponse mockCallback1 = mock(PushResponse.class); @@ -754,8 +771,8 @@ @Test @Feature({"NFCTest"}) public void testTwoPushInvocationsWithTimeout() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); PushResponse mockCallback1 = mock(PushResponse.class); @@ -797,8 +814,8 @@ @Test @Feature({"NFCTest"}) public void testTimeoutDontDisableReaderMode() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); WatchResponse mockWatchCallback = mock(WatchResponse.class); nfc.watch(createNfcWatchOptions(), mockWatchCallback); @@ -837,8 +854,8 @@ @Test @Feature({"NFCTest"}) public void testInvalidPushOptions() { - TestNfcImpl nfc = new TestNfcImpl(mContext); - nfc.setActivityForTesting(mActivity); + TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate); + mDelegate.invokeCallback(); PushResponse mockCallback = mock(PushResponse.class); // Long overflow
diff --git a/device/nfc/nfc.mojom b/device/nfc/nfc.mojom index 3f344a1..eb40e40 100644 --- a/device/nfc/nfc.mojom +++ b/device/nfc/nfc.mojom
@@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// TODO(rockot): This should probably be moved to nfc.mojom. +// TODO(blundell): This should be moved to device.mojom. module device.nfc.mojom; enum NFCErrorType {
diff --git a/device/nfc/nfc_provider.mojom b/device/nfc/nfc_provider.mojom new file mode 100644 index 0000000..030ad6f --- /dev/null +++ b/device/nfc/nfc_provider.mojom
@@ -0,0 +1,15 @@ +// 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. + +// TODO(blundell): This should be moved to device.mojom. +module device.nfc.mojom; + +import "device/nfc/nfc.mojom"; + +interface NFCProvider { + // Gets an NFC that is associated with |host_id|. |host_id| + // is used to obtain the Activity that this NFC instance should use on + // Android (see NFCDelegate.java). + GetNFCForHost(int32 host_id, NFC& nfc); +};
diff --git a/device/nfc/public/java/BUILD.gn b/device/nfc/public/java/BUILD.gn new file mode 100644 index 0000000..f062c6c --- /dev/null +++ b/device/nfc/public/java/BUILD.gn
@@ -0,0 +1,12 @@ +# 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. + +import("//build/config/android/rules.gni") + +android_library("nfc_java") { + java_files = [ "src/org/chromium/device/nfc/NfcDelegate.java" ] + deps = [ + "//base:base_java", + ] +}
diff --git a/device/nfc/public/java/src/org/chromium/device/nfc/NfcDelegate.java b/device/nfc/public/java/src/org/chromium/device/nfc/NfcDelegate.java new file mode 100644 index 0000000..9b144dc3 --- /dev/null +++ b/device/nfc/public/java/src/org/chromium/device/nfc/NfcDelegate.java
@@ -0,0 +1,24 @@ +// 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.device.nfc; + +import android.app.Activity; + +import org.chromium.base.Callback; + +/** Interface that allows the NFC implementation to access the Activity associated with a given + * client. |hostId| is the same ID passed in NFCProvider::GetNFCForHost(). + */ +public interface NfcDelegate { + /** Calls |callback| with the Activity associated with |hostId|, and subsequently calls + * |callback| again whenever the Activity associated with |hostId| changes. + */ + void trackActivityForHost(int hostId, Callback<Activity> callback); + + /** Called when the NFC implementation no longer needs to track the Activity associated with + * |hostId|. + */ + void stopTrackingActivityForHost(int hostId); +}
diff --git a/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc b/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc index bc5e16f..189f51dc 100644 --- a/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc +++ b/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc
@@ -9,6 +9,7 @@ #include "base/json/json_reader.h" #include "base/memory/ptr_util.h" #include "base/memory/singleton.h" +#include "base/task_scheduler/post_task.h" #include "base/threading/sequenced_worker_pool.h" #include "base/threading/thread_task_runner_handle.h" #include "base/time/default_clock.h" @@ -172,10 +173,9 @@ base::FilePath database_dir( browser_state->GetStatePath().Append(ntp_snippets::kDatabaseFolder)); scoped_refptr<base::SequencedTaskRunner> task_runner = - web::WebThread::GetBlockingPool() - ->GetSequencedTaskRunnerWithShutdownBehavior( - base::SequencedWorkerPool::GetSequenceToken(), - base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN); + base::CreateSequencedTaskRunnerWithTraits( + {base::MayBlock(), base::TaskPriority::BACKGROUND, + base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN}); std::string api_key; // This API needs whitelisted API keys. Get the key only if it is not a
diff --git a/ios/chrome/browser/snapshots/snapshots_util.mm b/ios/chrome/browser/snapshots/snapshots_util.mm index 8ab62fe..83fea37 100644 --- a/ios/chrome/browser/snapshots/snapshots_util.mm +++ b/ios/chrome/browser/snapshots/snapshots_util.mm
@@ -13,7 +13,7 @@ #include "base/macros.h" #include "base/path_service.h" #include "base/strings/stringprintf.h" -#include "ios/web/public/web_thread.h" +#include "base/task_scheduler/post_task.h" namespace { const char* kOrientationDescriptions[] = { @@ -30,9 +30,10 @@ std::vector<base::FilePath> snapshotsPaths; GetSnapshotsPaths(&snapshotsPaths); for (base::FilePath snapshotPath : snapshotsPaths) { - web::WebThread::PostBlockingPoolTask( - FROM_HERE, - base::Bind(base::IgnoreResult(&base::DeleteFile), snapshotPath, false)); + base::PostTaskWithTraits( + FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND}, + base::BindOnce(base::IgnoreResult(&base::DeleteFile), snapshotPath, + false)); } }
diff --git a/ios/chrome/browser/tabs/BUILD.gn b/ios/chrome/browser/tabs/BUILD.gn index 55f50de..bc1da78 100644 --- a/ios/chrome/browser/tabs/BUILD.gn +++ b/ios/chrome/browser/tabs/BUILD.gn
@@ -35,96 +35,11 @@ source_set("tabs_internal") { sources = [ - "tab_model.mm", - ] - deps = [ - ":tabs", - ":tabs_internal_arc", - "//base", - "//components/content_settings/core/browser", - "//components/favicon/core", - "//components/favicon/ios", - "//components/google/core/browser", - "//components/history/core/browser", - "//components/history/ios/browser", - "//components/infobars/core", - "//components/keyed_service/core", - "//components/metrics_services_manager", - "//components/navigation_metrics", - "//components/prefs", - "//components/reading_list/core", - "//components/search_engines", - "//components/sessions", - "//components/signin/core/browser", - "//components/signin/ios/browser", - "//components/strings", - "//components/url_formatter", - "//ios/chrome/app/strings", - "//ios/chrome/browser", - "//ios/chrome/browser/autofill", - "//ios/chrome/browser/autofill:autofill_internal", - "//ios/chrome/browser/bookmarks", - "//ios/chrome/browser/browser_state", - "//ios/chrome/browser/content_settings", - "//ios/chrome/browser/favicon", - "//ios/chrome/browser/find_in_page", - "//ios/chrome/browser/geolocation:geolocation_internal", - "//ios/chrome/browser/history", - "//ios/chrome/browser/infobars", - "//ios/chrome/browser/metrics", - "//ios/chrome/browser/metrics:metrics_internal", - "//ios/chrome/browser/native_app_launcher:native_app_launcher_internal", - "//ios/chrome/browser/passwords", - "//ios/chrome/browser/passwords:passwords_internal", - "//ios/chrome/browser/reading_list", - "//ios/chrome/browser/search_engines", - "//ios/chrome/browser/sessions", - "//ios/chrome/browser/sessions:serialisation", - "//ios/chrome/browser/signin", - "//ios/chrome/browser/signin:signin_internal", - "//ios/chrome/browser/snapshots", - "//ios/chrome/browser/snapshots:snapshots_internal", - "//ios/chrome/browser/ssl", - "//ios/chrome/browser/store_kit", - "//ios/chrome/browser/sync", - "//ios/chrome/browser/translate", - "//ios/chrome/browser/u2f", - "//ios/chrome/browser/ui", - "//ios/chrome/browser/ui:ui_internal", - "//ios/chrome/browser/ui/alert_coordinator", - "//ios/chrome/browser/ui/commands", - "//ios/chrome/browser/ui/downloads", - "//ios/chrome/browser/ui/overscroll_actions", - "//ios/chrome/browser/ui/reader_mode", - "//ios/chrome/browser/ui/sad_tab", - "//ios/chrome/browser/ui/toolbar", - "//ios/chrome/browser/ui/util", - "//ios/chrome/browser/web", - "//ios/chrome/browser/web:sad_tab_tab_helper_delegate", - "//ios/chrome/browser/web:web_internal", - "//ios/chrome/browser/web_state_list", - "//ios/net", - "//ios/public/provider/chrome/browser", - "//ios/public/provider/chrome/browser/native_app_launcher", - "//ios/web", - "//ios/web:user_agent", - "//net", - "//ui/base", - "//url", - ] - allow_circular_includes_from = [ ":tabs_internal_arc" ] - libs = [ - "CoreLocation.framework", - "UIKit.framework", - ] -} - -source_set("tabs_internal_arc") { - sources = [ "legacy_tab_helper.mm", "tab.h", "tab.mm", "tab_helper_util.mm", + "tab_model.mm", "tab_model_closing_web_state_observer.h", "tab_model_closing_web_state_observer.mm", "tab_model_list.mm",
diff --git a/ios/chrome/browser/tabs/tab_model.mm b/ios/chrome/browser/tabs/tab_model.mm index 278d1394..b3d60d2c 100644 --- a/ios/chrome/browser/tabs/tab_model.mm +++ b/ios/chrome/browser/tabs/tab_model.mm
@@ -11,7 +11,6 @@ #include "base/bind.h" #include "base/logging.h" #import "base/mac/foundation_util.h" -#import "base/mac/scoped_nsobject.h" #include "base/metrics/histogram_macros.h" #include "base/metrics/user_metrics_action.h" #include "base/strings/sys_string_conversions.h" @@ -60,6 +59,10 @@ #import "ios/web/web_state/web_state_impl.h" #include "url/gurl.h" +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + NSString* const kTabModelTabWillStartLoadingNotification = @"kTabModelTabWillStartLoadingNotification"; NSString* const kTabModelTabDidStartLoadingNotification = @@ -147,8 +150,7 @@ // Strong references to id<WebStateListObserving> wrapped by non-owning // WebStateListObserverBridges. - base::scoped_nsobject<NSArray<id<WebStateListObserving>>> - _retainedWebStateListObservers; + NSArray<id<WebStateListObserving>>* _retainedWebStateListObservers; // The delegate for sync. std::unique_ptr<TabModelSyncedWindowDelegate> _syncedWindowDelegate; @@ -161,9 +163,9 @@ // Backs up property with the same name. const SessionID _sessionID; // Saves session's state. - base::scoped_nsobject<SessionServiceIOS> _sessionService; + SessionServiceIOS* _sessionService; // List of TabModelObservers. - base::scoped_nsobject<TabModelObservers> _observers; + TabModelObservers* _observers; // Used to ensure thread-safety of the certificate policy management code. base::CancelableTaskTracker _clearPoliciesTaskTracker; @@ -172,10 +174,6 @@ // Session window for the contents of the tab model. @property(nonatomic, readonly) SessionIOS* sessionForSaving; -// Helper method that posts a notification with the given name with |tab| -// in the userInfo dictionary under the kTabModelTabKey. -- (void)postNotificationName:(NSString*)notificationName withTab:(Tab*)tab; - // Helper method to restore a saved session and control if the state should // be persisted or not. Used to implement the public -restoreSessionWindow: // method and restoring session in the initialiser. @@ -220,11 +218,9 @@ for (const auto& webStateListObserver : _webStateListObservers) _webStateList->RemoveObserver(webStateListObserver.get()); _webStateListObservers.clear(); - _retainedWebStateListObservers.reset(); + _retainedWebStateListObservers = nil; _clearPoliciesTaskTracker.TryCancelAll(); - - [super dealloc]; } #pragma mark - Public methods @@ -269,7 +265,7 @@ sessionService:(SessionServiceIOS*)service browserState:(ios::ChromeBrowserState*)browserState { if ((self = [super init])) { - _observers.reset([[TabModelObservers observers] retain]); + _observers = [TabModelObservers observers]; _webStateListDelegate = base::MakeUnique<TabModelWebStateListDelegate>(self); @@ -294,16 +290,16 @@ // There must be a valid session service defined to consume session windows. DCHECK(service); - _sessionService.reset([service retain]); + _sessionService = service; - base::scoped_nsobject<NSMutableArray<id<WebStateListObserving>>> - retainedWebStateListObservers([[NSMutableArray alloc] init]); + NSMutableArray<id<WebStateListObserving>>* retainedWebStateListObservers = + [[NSMutableArray alloc] init]; - base::scoped_nsobject<TabModelClosingWebStateObserver> - tabModelClosingWebStateObserver([[TabModelClosingWebStateObserver alloc] - initWithTabModel:self - restoreService:IOSChromeTabRestoreServiceFactory:: - GetForBrowserState(_browserState)]); + TabModelClosingWebStateObserver* tabModelClosingWebStateObserver = [ + [TabModelClosingWebStateObserver alloc] + initWithTabModel:self + restoreService:IOSChromeTabRestoreServiceFactory::GetForBrowserState( + _browserState)]; [retainedWebStateListObservers addObject:tabModelClosingWebStateObserver]; _webStateListObservers.push_back( @@ -320,17 +316,16 @@ } _webStateListObservers.push_back(base::MakeUnique<TabParentingObserver>()); - base::scoped_nsobject<TabModelSelectedTabObserver> - tabModelSelectedTabObserver( - [[TabModelSelectedTabObserver alloc] initWithTabModel:self]); + TabModelSelectedTabObserver* tabModelSelectedTabObserver = + [[TabModelSelectedTabObserver alloc] initWithTabModel:self]; [retainedWebStateListObservers addObject:tabModelSelectedTabObserver]; _webStateListObservers.push_back( base::MakeUnique<WebStateListObserverBridge>( tabModelSelectedTabObserver)); - base::scoped_nsobject<TabModelObserversBridge> tabModelObserversBridge( + TabModelObserversBridge* tabModelObserversBridge = [[TabModelObserversBridge alloc] initWithTabModel:self - tabModelObservers:_observers.get()]); + tabModelObservers:_observers]; [retainedWebStateListObservers addObject:tabModelObserversBridge]; _webStateListObservers.push_back( base::MakeUnique<WebStateListObserverBridge>(tabModelObserversBridge)); @@ -342,7 +337,7 @@ for (const auto& webStateListObserver : _webStateListObservers) _webStateList->AddObserver(webStateListObserver.get()); - _retainedWebStateListObservers.reset([retainedWebStateListObservers copy]); + _retainedWebStateListObservers = [retainedWebStateListObservers copy]; if (window) { DCHECK([_observers empty]); @@ -670,7 +665,7 @@ #pragma mark - NSFastEnumeration - (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState*)state - objects:(id*)objects + objects:(id __unsafe_unretained*)objects count:(NSUInteger)count { return [_fastEnumerationHelper->GetFastEnumeration() countByEnumeratingWithState:state @@ -700,18 +695,6 @@ initWithWindows:@[ SerializeWebStateList(_webStateList.get()) ]]; } -- (void)postNotificationName:(NSString*)notificationName withTab:(Tab*)tab { - // A scoped_nsobject is used rather than an NSDictionary with static - // initializer dictionaryWithObject, because that approach adds the dictionary - // to the autorelease pool, which in turn holds Tab alive longer than - // necessary. - base::scoped_nsobject<NSDictionary> userInfo( - [[NSDictionary alloc] initWithObjectsAndKeys:tab, kTabModelTabKey, nil]); - [[NSNotificationCenter defaultCenter] postNotificationName:notificationName - object:self - userInfo:userInfo]; -} - - (BOOL)restoreSessionWindow:(SessionWindowIOS*)window persistState:(BOOL)persistState { DCHECK(_browserState); @@ -736,8 +719,8 @@ scoped_refptr<web::CertificatePolicyCache> policyCache = web::BrowserState::GetCertificatePolicyCache(_browserState); - base::scoped_nsobject<NSMutableArray<Tab*>> restoredTabs( - [[NSMutableArray alloc] initWithCapacity:window.sessions.count]); + NSMutableArray<Tab*>* restoredTabs = + [[NSMutableArray alloc] initWithCapacity:window.sessions.count]; for (int index = oldCount; index < _webStateList->count(); ++index) { web::WebState* webState = _webStateList->GetWebStateAt(index); @@ -763,7 +746,7 @@ } } if (_tabUsageRecorder) - _tabUsageRecorder->InitialRestoredTabs(self.currentTab, restoredTabs.get()); + _tabUsageRecorder->InitialRestoredTabs(self.currentTab, restoredTabs); return closedNTPTab; }
diff --git a/net/cookies/parsed_cookie.cc b/net/cookies/parsed_cookie.cc index bf95a12..d2fc37ad 100644 --- a/net/cookies/parsed_cookie.cc +++ b/net/cookies/parsed_cookie.cc
@@ -353,7 +353,7 @@ return; } - for (int pair_num = 0; pair_num < kMaxPairs && it != end; ++pair_num) { + for (int pair_num = 0; it != end; ++pair_num) { TokenValuePair pair; std::string::const_iterator token_start, token_end;
diff --git a/net/cookies/parsed_cookie.h b/net/cookies/parsed_cookie.h index ccb30d8e..5e6f615 100644 --- a/net/cookies/parsed_cookie.h +++ b/net/cookies/parsed_cookie.h
@@ -23,8 +23,6 @@ // The maximum length of a cookie string we will try to parse static const size_t kMaxCookieSize = 4096; - // The maximum number of Token/Value pairs. Shouldn't have more than 8. - static const int kMaxPairs = 16; // Construct from a cookie string like "BLAH=1; path=/; domain=.google.com" // Format is according to RFC 6265. Cookies with both name and value empty
diff --git a/net/cookies/parsed_cookie_unittest.cc b/net/cookies/parsed_cookie_unittest.cc index 61a7d04..562bb10 100644 --- a/net/cookies/parsed_cookie_unittest.cc +++ b/net/cookies/parsed_cookie_unittest.cc
@@ -217,17 +217,17 @@ EXPECT_EQ(2U, pc.NumberOfAttributes()); } -TEST(ParsedCookieTest, TooManyPairs) { - std::string blankpairs; - blankpairs.resize(ParsedCookie::kMaxPairs - 2, ';'); +TEST(ParsedCookieTest, LotsOfPairs) { + for (int i = 1; i < 100; i++) { + std::string blankpairs; + blankpairs.resize(i, ';'); - ParsedCookie pc1("a=b;" + blankpairs + "secure"); - EXPECT_TRUE(pc1.IsValid()); - EXPECT_TRUE(pc1.IsSecure()); - - ParsedCookie pc2("a=b;" + blankpairs + ";secure"); - EXPECT_TRUE(pc2.IsValid()); - EXPECT_FALSE(pc2.IsSecure()); + ParsedCookie c("a=b;" + blankpairs + "secure"); + EXPECT_EQ("a", c.Name()); + EXPECT_EQ("b", c.Value()); + EXPECT_TRUE(c.IsValid()); + EXPECT_TRUE(c.IsSecure()); + } } // TODO(erikwright): some better test cases for invalid cookies.
diff --git a/services/device/BUILD.gn b/services/device/BUILD.gn index 7681092..8d5d4ee7 100644 --- a/services/device/BUILD.gn +++ b/services/device/BUILD.gn
@@ -26,6 +26,7 @@ deps = [ "//base", "//device/generic_sensor", + "//device/nfc:mojo_bindings", "//device/sensors", "//device/sensors/public/interfaces", "//device/wake_lock", @@ -163,6 +164,9 @@ java_files = [ "android/java/src/org/chromium/services/device/InterfaceRegistrar.java" ] deps = [ "//base:base_java", + "//device/nfc:mojo_bindings_java", + "//device/nfc/android:java", + "//device/nfc/public/java:nfc_java", "//mojo/android:system_java", "//mojo/public/java:bindings_java", "//mojo/public/java:system_java",
diff --git a/services/device/android/java/src/org/chromium/services/device/InterfaceRegistrar.java b/services/device/android/java/src/org/chromium/services/device/InterfaceRegistrar.java index b9e230b..62347c5 100644 --- a/services/device/android/java/src/org/chromium/services/device/InterfaceRegistrar.java +++ b/services/device/android/java/src/org/chromium/services/device/InterfaceRegistrar.java
@@ -11,6 +11,9 @@ import org.chromium.device.battery.BatteryMonitorFactory; import org.chromium.device.mojom.BatteryMonitor; import org.chromium.device.mojom.VibrationManager; +import org.chromium.device.nfc.NfcDelegate; +import org.chromium.device.nfc.NfcProviderImpl; +import org.chromium.device.nfc.mojom.NfcProvider; import org.chromium.device.vibration.VibrationManagerImpl; import org.chromium.mojo.system.impl.CoreImpl; import org.chromium.services.service_manager.InterfaceRegistry; @@ -18,7 +21,8 @@ @JNINamespace("device") class InterfaceRegistrar { @CalledByNative - static void createInterfaceRegistryForContext(int nativeHandle, Context applicationContext) { + static void createInterfaceRegistryForContext( + int nativeHandle, Context applicationContext, NfcDelegate nfcDelegate) { // Note: The bindings code manages the lifetime of this object, so it // is not necessary to hold on to a reference to it explicitly. InterfaceRegistry registry = InterfaceRegistry.create( @@ -26,6 +30,8 @@ registry.addInterface( BatteryMonitor.MANAGER, new BatteryMonitorFactory(applicationContext)); registry.addInterface( + NfcProvider.MANAGER, new NfcProviderImpl.Factory(applicationContext, nfcDelegate)); + registry.addInterface( VibrationManager.MANAGER, new VibrationManagerImpl.Factory(applicationContext)); } }
diff --git a/services/device/device_service.cc b/services/device/device_service.cc index 9b18f35d1..a70a078 100644 --- a/services/device/device_service.cc +++ b/services/device/device_service.cc
@@ -27,6 +27,7 @@ #if defined(OS_ANDROID) #include "base/android/context_utils.h" #include "base/android/jni_android.h" +#include "device/nfc/nfc_provider.mojom.h" #include "jni/InterfaceRegistrar_jni.h" #include "services/device/android/register_jni.h" #include "services/device/screen_orientation/screen_orientation_listener_android.h" @@ -42,15 +43,16 @@ std::unique_ptr<service_manager::Service> CreateDeviceService( scoped_refptr<base::SingleThreadTaskRunner> file_task_runner, scoped_refptr<base::SingleThreadTaskRunner> io_task_runner, - const WakeLockContextCallback& wake_lock_context_callback) { + const WakeLockContextCallback& wake_lock_context_callback, + const base::android::JavaRef<jobject>& java_nfc_delegate) { if (!EnsureJniRegistered()) { DLOG(ERROR) << "Failed to register JNI for Device Service"; return nullptr; } - return base::MakeUnique<DeviceService>(std::move(file_task_runner), - std::move(io_task_runner), - wake_lock_context_callback); + return base::MakeUnique<DeviceService>( + std::move(file_task_runner), std::move(io_task_runner), + wake_lock_context_callback, java_nfc_delegate); } #else std::unique_ptr<service_manager::Service> CreateDeviceService( @@ -65,11 +67,14 @@ DeviceService::DeviceService( scoped_refptr<base::SingleThreadTaskRunner> file_task_runner, scoped_refptr<base::SingleThreadTaskRunner> io_task_runner, - const WakeLockContextCallback& wake_lock_context_callback) - : java_interface_provider_initialized_(false), - file_task_runner_(std::move(file_task_runner)), + const WakeLockContextCallback& wake_lock_context_callback, + const base::android::JavaRef<jobject>& java_nfc_delegate) + : file_task_runner_(std::move(file_task_runner)), io_task_runner_(std::move(io_task_runner)), - wake_lock_context_callback_(wake_lock_context_callback) {} + wake_lock_context_callback_(wake_lock_context_callback), + java_interface_provider_initialized_(false) { + java_nfc_delegate_.Reset(java_nfc_delegate); +} #else DeviceService::DeviceService( scoped_refptr<base::SingleThreadTaskRunner> file_task_runner, @@ -113,10 +118,15 @@ ->CreateInterfaceFactory<mojom::BatteryMonitor>()); registry_.AddInterface( GetJavaInterfaceProvider() + ->CreateInterfaceFactory<nfc::mojom::NFCProvider>()); + registry_.AddInterface( + GetJavaInterfaceProvider() ->CreateInterfaceFactory<mojom::VibrationManager>()); #else registry_.AddInterface<mojom::BatteryMonitor>(base::Bind( &DeviceService::BindBatteryMonitorRequest, base::Unretained(this))); + registry_.AddInterface<nfc::mojom::NFCProvider>(base::Bind( + &DeviceService::BindNFCProviderRequest, base::Unretained(this))); registry_.AddInterface<mojom::VibrationManager>(base::Bind( &DeviceService::BindVibrationManagerRequest, base::Unretained(this))); #endif @@ -137,6 +147,13 @@ BatteryMonitorImpl::Create(std::move(request)); } +void DeviceService::BindNFCProviderRequest( + const service_manager::BindSourceInfo& source_info, + nfc::mojom::NFCProviderRequest request) { + LOG(ERROR) << "NFC is only supported on Android"; + NOTREACHED(); +} + void DeviceService::BindVibrationManagerRequest( const service_manager::BindSourceInfo& source_info, mojom::VibrationManagerRequest request) { @@ -257,7 +274,7 @@ JNIEnv* env = base::android::AttachCurrentThread(); Java_InterfaceRegistrar_createInterfaceRegistryForContext( env, mojo::MakeRequest(&provider).PassMessagePipe().release().value(), - base::android::GetApplicationContext()); + base::android::GetApplicationContext(), java_nfc_delegate_); java_interface_provider_.Bind(std::move(provider)); java_interface_provider_initialized_ = true;
diff --git a/services/device/device_service.h b/services/device/device_service.h index 66a6c77..127fa905 100644 --- a/services/device/device_service.h +++ b/services/device/device_service.h
@@ -7,6 +7,7 @@ #include "base/memory/ref_counted.h" #include "device/generic_sensor/public/interfaces/sensor_provider.mojom.h" +#include "device/nfc/nfc_provider.mojom.h" #include "device/screen_orientation/public/interfaces/screen_orientation.mojom.h" #include "device/sensors/public/interfaces/motion.mojom.h" #include "device/sensors/public/interfaces/orientation.mojom.h" @@ -22,6 +23,10 @@ #include "services/service_manager/public/cpp/interface_provider.h" #include "services/service_manager/public/cpp/service.h" +#if defined(OS_ANDROID) +#include "base/android/scoped_java_ref.h" +#endif + namespace base { class SingleThreadTaskRunner; } @@ -32,10 +37,14 @@ class TimeZoneMonitor; #if defined(OS_ANDROID) +// NOTE: See the comments on the definitions of |WakeLockContextCallback| +// and NFCDelegate.java to understand the semantics and usage of these +// parameters. std::unique_ptr<service_manager::Service> CreateDeviceService( scoped_refptr<base::SingleThreadTaskRunner> file_task_runner, scoped_refptr<base::SingleThreadTaskRunner> io_task_runner, - const WakeLockContextCallback& wake_lock_context_callback); + const WakeLockContextCallback& wake_lock_context_callback, + const base::android::JavaRef<jobject>& java_nfc_delegate); #else std::unique_ptr<service_manager::Service> CreateDeviceService( scoped_refptr<base::SingleThreadTaskRunner> file_task_runner, @@ -47,7 +56,8 @@ #if defined(OS_ANDROID) DeviceService(scoped_refptr<base::SingleThreadTaskRunner> file_task_runner, scoped_refptr<base::SingleThreadTaskRunner> io_task_runner, - const WakeLockContextCallback& wake_lock_context_callback); + const WakeLockContextCallback& wake_lock_context_callback, + const base::android::JavaRef<jobject>& java_nfc_delegate); #else DeviceService(scoped_refptr<base::SingleThreadTaskRunner> file_task_runner, scoped_refptr<base::SingleThreadTaskRunner> io_task_runner); @@ -81,6 +91,9 @@ void BindBatteryMonitorRequest( const service_manager::BindSourceInfo& source_info, mojom::BatteryMonitorRequest request); + void BindNFCProviderRequest( + const service_manager::BindSourceInfo& source_info, + nfc::mojom::NFCProviderRequest request); void BindVibrationManagerRequest( const service_manager::BindSourceInfo& source_info, mojom::VibrationManagerRequest request); @@ -109,6 +122,11 @@ std::unique_ptr<PowerMonitorMessageBroadcaster> power_monitor_message_broadcaster_; std::unique_ptr<TimeZoneMonitor> time_zone_monitor_; + scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_; + scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_; + + WakeLockContextCallback wake_lock_context_callback_; + #if defined(OS_ANDROID) // Binds |java_interface_provider_| to an interface registry that exposes // factories for the interfaces that are provided via Java on Android. @@ -118,13 +136,10 @@ service_manager::InterfaceProvider java_interface_provider_; bool java_interface_provider_initialized_; + + base::android::ScopedJavaGlobalRef<jobject> java_nfc_delegate_; #endif - scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_; - scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_; - - WakeLockContextCallback wake_lock_context_callback_; - service_manager::BinderRegistry registry_; DISALLOW_COPY_AND_ASSIGN(DeviceService);
diff --git a/services/device/device_service_test_base.cc b/services/device/device_service_test_base.cc index 1ff7380..db3a2fc 100644 --- a/services/device/device_service_test_base.cc +++ b/services/device/device_service_test_base.cc
@@ -51,7 +51,7 @@ device_service_context_.reset(new service_manager::ServiceContext( CreateDeviceService(file_thread_.task_runner(), io_thread_.task_runner(), - wake_lock_context_callback_), + wake_lock_context_callback_, nullptr), std::move(request))); #else device_service_context_.reset(new service_manager::ServiceContext(
diff --git a/services/device/manifest.json b/services/device/manifest.json index 83401c26..516c384b 100644 --- a/services/device/manifest.json +++ b/services/device/manifest.json
@@ -7,6 +7,7 @@ "device:battery_monitor": [ "device::mojom::BatteryMonitor" ], "device:fingerprint": [ "device::mojom::Fingerprint" ], "device:generic_sensor": [ "device::mojom::SensorProvider" ], + "device:nfc": [ "device::nfc::mojom::NFCProvider" ], "device:power_monitor": [ "device::mojom::PowerMonitor" ], "device:screen_orientation": [ "device::mojom::ScreenOrientationListener" ], "device:sensors": [
diff --git a/third_party/WebKit/LayoutTests/NeverFixTests b/third_party/WebKit/LayoutTests/NeverFixTests index 14700d6..56b5e400 100644 --- a/third_party/WebKit/LayoutTests/NeverFixTests +++ b/third_party/WebKit/LayoutTests/NeverFixTests
@@ -58,6 +58,7 @@ [ Mac ] editing/spelling/spelling-on-context-menu-key.html [ WontFix ] [ Mac ] fast/events/context-menu-key-shift-f10-modifiers.html [ WontFix ] [ Mac ] fast/events/context-menu-key-shift-f10-prevent-default.html [ WontFix ] +[ Mac ] fast/events/contextmenu-follows-focus.html [ WontFix ] [ Mac ] fast/events/menu-key-context-menu-document.html [ WontFix ] [ Mac ] fast/events/menu-key-context-menu.html [ WontFix ] [ Mac ] fast/events/menu-key-context-menu-position.html [ WontFix ]
diff --git a/third_party/WebKit/LayoutTests/fast/events/contextmenu-follows-focus.html b/third_party/WebKit/LayoutTests/fast/events/contextmenu-follows-focus.html new file mode 100644 index 0000000..2ae04bc --- /dev/null +++ b/third_party/WebKit/LayoutTests/fast/events/contextmenu-follows-focus.html
@@ -0,0 +1,32 @@ +<!doctype html> +<script src="../../resources/testharness.js"></script> +<script src="../../resources/testharnessreport.js"></script> +<script> +var contextForMenu; +function catchContextMenu(event) { + contextForMenu = event.currentTarget.tagName; +} +</script> + +<input oncontextmenu="catchContextMenu(event);"> +<a href="www" oncontextmenu="catchContextMenu(event);">A link</a> + +<script> +test(function() { + assert_exists(window, 'eventSender', 'This test requires eventSender.'); + + document.querySelector('INPUT').focus(); + eventSender.keyDown('ContextMenu'); + assert_equals(contextForMenu, 'INPUT', + 'ContextMenu should use the focused input field as context.'); + + // Hide INPUT's context menu before we display A's context menu. + eventSender.keyDown('Escape'); + + document.querySelector('A').focus(); + eventSender.keyDown('ContextMenu'); + assert_equals(contextForMenu, 'A', + 'ContextMenu should use the focused link as context.'); + +}, 'ContextMenu should always follow focused element.'); +</script>
diff --git a/third_party/WebKit/LayoutTests/sensor/resources/generic-sensor-tests.js b/third_party/WebKit/LayoutTests/sensor/resources/generic-sensor-tests.js index ff03398..69fc7d2 100644 --- a/third_party/WebKit/LayoutTests/sensor/resources/generic-sensor-tests.js +++ b/third_party/WebKit/LayoutTests/sensor/resources/generic-sensor-tests.js
@@ -345,11 +345,8 @@ // By the moment slow sensor (9 Hz) is notified for the // next time, the fast sensor (30 Hz) has been notified // for int(30/9) = 3 times. - // In actual implementation updates are bound to rAF, - // (not to a timer) so fluctuations are possible, so we - // reference to the actual elapsed updates count. let elapsedUpdates = mockSensor.reading_updates_count() - readingUpdatesCounter; - assert_approx_equals(fastSensorNotifiedCounter, elapsedUpdates, 1); + assert_equals(fastSensorNotifiedCounter, elapsedUpdates); fastSensor.stop(); slowSensor.stop(); resolve(mockSensor);
diff --git a/third_party/WebKit/Source/core/events/BUILD.gn b/third_party/WebKit/Source/core/events/BUILD.gn index 656c053..a474c98 100644 --- a/third_party/WebKit/Source/core/events/BUILD.gn +++ b/third_party/WebKit/Source/core/events/BUILD.gn
@@ -111,6 +111,8 @@ "VisualViewportResizeEvent.h", "VisualViewportScrollEvent.cpp", "VisualViewportScrollEvent.h", + "WebInputEventConversion.cpp", + "WebInputEventConversion.h", "WheelEvent.cpp", "WheelEvent.h", "WindowEventContext.cpp",
diff --git a/third_party/WebKit/Source/web/WebInputEventConversion.cpp b/third_party/WebKit/Source/core/events/WebInputEventConversion.cpp similarity index 99% rename from third_party/WebKit/Source/web/WebInputEventConversion.cpp rename to third_party/WebKit/Source/core/events/WebInputEventConversion.cpp index 76b7fa5..a21eb86 100644 --- a/third_party/WebKit/Source/web/WebInputEventConversion.cpp +++ b/third_party/WebKit/Source/core/events/WebInputEventConversion.cpp
@@ -28,7 +28,7 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "web/WebInputEventConversion.h" +#include "core/events/WebInputEventConversion.h" #include "core/dom/Touch.h" #include "core/dom/TouchList.h"
diff --git a/third_party/WebKit/Source/web/WebInputEventConversion.h b/third_party/WebKit/Source/core/events/WebInputEventConversion.h similarity index 84% rename from third_party/WebKit/Source/web/WebInputEventConversion.h rename to third_party/WebKit/Source/core/events/WebInputEventConversion.h index 7a778b3..cea87e35 100644 --- a/third_party/WebKit/Source/web/WebInputEventConversion.h +++ b/third_party/WebKit/Source/core/events/WebInputEventConversion.h
@@ -32,13 +32,13 @@ #define WebInputEventConversion_h #include <vector> +#include "core/CoreExport.h" #include "platform/scroll/ScrollTypes.h" #include "platform/wtf/Compiler.h" #include "public/platform/WebInputEvent.h" #include "public/platform/WebKeyboardEvent.h" #include "public/platform/WebMouseWheelEvent.h" #include "public/platform/WebTouchEvent.h" -#include "web/WebExport.h" namespace blink { @@ -53,8 +53,7 @@ // These classes are used to convert from WebInputEvent subclasses to // corresponding WebCore events. - -class WEB_EXPORT WebMouseEventBuilder +class CORE_EXPORT WebMouseEventBuilder : NON_EXPORTED_BASE(public WebMouseEvent) { public: // Converts a MouseEvent to a corresponding WebMouseEvent. @@ -69,7 +68,7 @@ // NOTE: For KeyboardEvent, this is only implemented for keydown, // keyup, and keypress. If the event mapping fails, the event type will be set // to Undefined. -class WEB_EXPORT WebKeyboardEventBuilder +class CORE_EXPORT WebKeyboardEventBuilder : NON_EXPORTED_BASE(public WebKeyboardEvent) { public: WebKeyboardEventBuilder(const KeyboardEvent&); @@ -78,7 +77,7 @@ // Converts a TouchEvent to a corresponding WebTouchEvent. // NOTE: WebTouchEvents have a cap on the number of WebTouchPoints. Any points // exceeding that cap will be dropped. -class WEB_EXPORT WebTouchEventBuilder +class CORE_EXPORT WebTouchEventBuilder : NON_EXPORTED_BASE(public WebTouchEvent) { public: WebTouchEventBuilder(const LayoutItem, const TouchEvent&); @@ -86,21 +85,21 @@ // Return a new transformed WebGestureEvent by applying the Widget's scale // and translation. -WEB_EXPORT WebGestureEvent TransformWebGestureEvent(FrameView*, - const WebGestureEvent&); -WEB_EXPORT WebMouseEvent TransformWebMouseEvent(FrameView*, - const WebMouseEvent&); +CORE_EXPORT WebGestureEvent TransformWebGestureEvent(FrameView*, + const WebGestureEvent&); +CORE_EXPORT WebMouseEvent TransformWebMouseEvent(FrameView*, + const WebMouseEvent&); -WEB_EXPORT WebMouseWheelEvent +CORE_EXPORT WebMouseWheelEvent TransformWebMouseWheelEvent(FrameView*, const WebMouseWheelEvent&); -WEB_EXPORT WebTouchEvent TransformWebTouchEvent(FrameView*, - const WebTouchEvent&); +CORE_EXPORT WebTouchEvent TransformWebTouchEvent(FrameView*, + const WebTouchEvent&); -Vector<WebMouseEvent> WEB_EXPORT +Vector<WebMouseEvent> CORE_EXPORT TransformWebMouseEventVector(FrameView*, const std::vector<const WebInputEvent*>&); -Vector<WebTouchEvent> WEB_EXPORT +Vector<WebTouchEvent> CORE_EXPORT TransformWebTouchEventVector(FrameView*, const std::vector<const WebInputEvent*>&);
diff --git a/third_party/WebKit/Source/core/input/EventHandler.cpp b/third_party/WebKit/Source/core/input/EventHandler.cpp index cf17e1d..b370359 100644 --- a/third_party/WebKit/Source/core/input/EventHandler.cpp +++ b/third_party/WebKit/Source/core/input/EventHandler.cpp
@@ -1826,7 +1826,7 @@ selection.ComputeVisibleSelectionInDOMTreeDeprecated().Start(); VisualViewport& visual_viewport = frame_->GetPage()->GetVisualViewport(); - if (!override_target_element && start.AnchorNode() && + if (!override_target_element && start.AnchorNode() && !selection.IsHidden() && (selection.ComputeVisibleSelectionInDOMTreeDeprecated() .RootEditableElement() || selection.ComputeVisibleSelectionInDOMTreeDeprecated().IsRange())) {
diff --git a/third_party/WebKit/Source/modules/sensor/BUILD.gn b/third_party/WebKit/Source/modules/sensor/BUILD.gn index 9660eb03..7fd28fa6 100644 --- a/third_party/WebKit/Source/modules/sensor/BUILD.gn +++ b/third_party/WebKit/Source/modules/sensor/BUILD.gn
@@ -26,8 +26,6 @@ "SensorProviderProxy.h", "SensorProxy.cpp", "SensorProxy.h", - "SensorReadingUpdater.cpp", - "SensorReadingUpdater.h", ] deps = [
diff --git a/third_party/WebKit/Source/modules/sensor/OrientationSensor.cpp b/third_party/WebKit/Source/modules/sensor/OrientationSensor.cpp index ed5454c..09f02be 100644 --- a/third_party/WebKit/Source/modules/sensor/OrientationSensor.cpp +++ b/third_party/WebKit/Source/modules/sensor/OrientationSensor.cpp
@@ -127,8 +127,9 @@ : Sensor(execution_context, options, exception_state, type), reading_dirty_(true) {} -void OrientationSensor::OnSensorReadingChanged() { +void OrientationSensor::OnSensorReadingChanged(double timestamp) { reading_dirty_ = true; + Sensor::OnSensorReadingChanged(timestamp); } DEFINE_TRACE(OrientationSensor) {
diff --git a/third_party/WebKit/Source/modules/sensor/OrientationSensor.h b/third_party/WebKit/Source/modules/sensor/OrientationSensor.h index 6402c79..35f3bdf 100644 --- a/third_party/WebKit/Source/modules/sensor/OrientationSensor.h +++ b/third_party/WebKit/Source/modules/sensor/OrientationSensor.h
@@ -30,7 +30,7 @@ private: // SensorProxy override. - void OnSensorReadingChanged() override; + void OnSensorReadingChanged(double timestamp) override; template <typename Matrix> void PopulateMatrixInternal(Matrix*, ExceptionState&);
diff --git a/third_party/WebKit/Source/modules/sensor/Sensor.cpp b/third_party/WebKit/Source/modules/sensor/Sensor.cpp index 7581a9e..130162e 100644 --- a/third_party/WebKit/Source/modules/sensor/Sensor.cpp +++ b/third_party/WebKit/Source/modules/sensor/Sensor.cpp
@@ -18,6 +18,13 @@ namespace blink { +namespace { + +constexpr double kMinWaitingInterval = + 1 / device::mojom::blink::SensorConfiguration::kMaxAllowedFrequency; + +} // namespace + Sensor::Sensor(ExecutionContext* execution_context, const SensorOptions& sensor_options, ExceptionState& exception_state, @@ -26,7 +33,8 @@ sensor_options_(sensor_options), type_(type), state_(SensorState::kIdle), - last_update_timestamp_(0.0) { + last_update_timestamp_(0.0), + pending_reading_update_(false) { // Check secure context. String error_message; if (!execution_context->IsSecureContext(error_message)) { @@ -89,7 +97,7 @@ is_null = false; return performance->MonotonicTimeToDOMHighResTimeStamp( - sensor_proxy_->Reading().timestamp); + sensor_proxy_->reading().timestamp); } DEFINE_TRACE(Sensor) { @@ -133,7 +141,7 @@ double Sensor::ReadingValueUnchecked(int index) const { DCHECK(sensor_proxy_); DCHECK(index >= 0 && index < device::SensorReading::kValuesCount); - return sensor_proxy_->Reading().values[index]; + return sensor_proxy_->reading().values[index]; } void Sensor::InitSensorProxyIfNeeded() { @@ -162,16 +170,38 @@ RequestAddConfiguration(); } -void Sensor::NotifySensorChanged(double timestamp) { +void Sensor::OnSensorReadingChanged(double timestamp) { if (state_ != Sensor::SensorState::kActivated) return; - DCHECK_GT(configuration_->frequency, 0.0); - double period = 1 / configuration_->frequency; + // Return if reading update is already scheduled or the cached + // reading is up-to-date. + if (pending_reading_update_ || + sensor_proxy_->reading().timestamp == reading_.timestamp) + return; - if (timestamp - last_update_timestamp_ >= period) { - last_update_timestamp_ = timestamp; - NotifySensorReadingChanged(); + pending_reading_update_ = true; + + double elapsedTime = timestamp - last_update_timestamp_; + + DCHECK_GT(configuration_->frequency, 0.0); + double waitingTime = 1 / configuration_->frequency - elapsedTime; + + // Negative or zero 'waitingTime' means that polling period has elapsed. + // We also avoid scheduling if the elapsed time is slightly behind the + // polling period. + auto sensor_reading_changed = + WTF::Bind(&Sensor::UpdateReading, WrapWeakPersistent(this)); + if (waitingTime < kMinWaitingInterval) { + // Invoke JS callbacks in a different callchain to obviate + // possible modifications of SensorProxy::observers_ container + // while it is being iterated through. + TaskRunnerHelper::Get(TaskType::kSensor, GetExecutionContext()) + ->PostTask(BLINK_FROM_HERE, std::move(sensor_reading_changed)); + } else { + TaskRunnerHelper::Get(TaskType::kSensor, GetExecutionContext()) + ->PostDelayedTask(BLINK_FROM_HERE, std::move(sensor_reading_changed), + WTF::TimeDelta::FromSecondsD(waitingTime)); } } @@ -273,13 +303,11 @@ } } -void Sensor::NotifySensorReadingChanged() { - DCHECK(sensor_proxy_); - - if (sensor_proxy_->Reading().timestamp != stored_data_.timestamp) { - stored_data_ = sensor_proxy_->Reading(); - DispatchEvent(Event::Create(EventTypeNames::change)); - } +void Sensor::UpdateReading() { + last_update_timestamp_ = WTF::MonotonicallyIncreasingTime(); + reading_ = sensor_proxy_->reading(); + pending_reading_update_ = false; + DispatchEvent(Event::Create(EventTypeNames::change)); } void Sensor::NotifyOnActivate() { @@ -295,7 +323,7 @@ if (!IsActivated()) return false; DCHECK(sensor_proxy_); - return sensor_proxy_->Reading().timestamp != 0.0; + return sensor_proxy_->reading().timestamp != 0.0; } } // namespace blink
diff --git a/third_party/WebKit/Source/modules/sensor/Sensor.h b/third_party/WebKit/Source/modules/sensor/Sensor.h index a6370711..00e4179 100644 --- a/third_party/WebKit/Source/modules/sensor/Sensor.h +++ b/third_party/WebKit/Source/modules/sensor/Sensor.h
@@ -79,19 +79,19 @@ bool CanReturnReadings() const; bool IsActivated() const { return state_ == SensorState::kActivated; } + // SensorProxy::Observer overrides. + void OnSensorInitialized() override; + void OnSensorReadingChanged(double timestamp) override; + void OnSensorError(ExceptionCode, + const String& sanitized_message, + const String& unsanitized_message) override; + private: void InitSensorProxyIfNeeded(); // ContextLifecycleObserver overrides. void ContextDestroyed(ExecutionContext*) override; - // SensorProxy::Observer overrides. - void OnSensorInitialized() override; - void NotifySensorChanged(double timestamp) override; - void OnSensorError(ExceptionCode, - const String& sanitized_message, - const String& unsanitized_message) override; - void OnAddConfigurationRequestCompleted(bool); void StartListening(); @@ -104,7 +104,7 @@ const String& sanitized_message = String(), const String& unsanitized_message = String()); - void NotifySensorReadingChanged(); + void UpdateReading(); void NotifyOnActivate(); void NotifyError(DOMException* error); @@ -113,9 +113,10 @@ device::mojom::blink::SensorType type_; SensorState state_; Member<SensorProxy> sensor_proxy_; - device::SensorReading stored_data_; + device::SensorReading reading_; SensorConfigurationPtr configuration_; double last_update_timestamp_; + bool pending_reading_update_; }; } // namespace blink
diff --git a/third_party/WebKit/Source/modules/sensor/SensorProviderProxy.cpp b/third_party/WebKit/Source/modules/sensor/SensorProviderProxy.cpp index 8597e12d..408e833 100644 --- a/third_party/WebKit/Source/modules/sensor/SensorProviderProxy.cpp +++ b/third_party/WebKit/Source/modules/sensor/SensorProviderProxy.cpp
@@ -66,7 +66,7 @@ device::mojom::blink::SensorType type) { for (SensorProxy* sensor : sensor_proxies_) { // TODO(Mikhail) : Hash sensors by type for efficiency. - if (sensor->GetType() == type) + if (sensor->type() == type) return sensor; }
diff --git a/third_party/WebKit/Source/modules/sensor/SensorProxy.cpp b/third_party/WebKit/Source/modules/sensor/SensorProxy.cpp index d7f8d93..a4087b3 100644 --- a/third_party/WebKit/Source/modules/sensor/SensorProxy.cpp +++ b/third_party/WebKit/Source/modules/sensor/SensorProxy.cpp
@@ -4,10 +4,9 @@ #include "modules/sensor/SensorProxy.h" -#include "core/dom/Document.h" +#include "core/dom/TaskRunnerHelper.h" #include "core/frame/LocalFrame.h" #include "modules/sensor/SensorProviderProxy.h" -#include "modules/sensor/SensorReadingUpdater.h" #include "platform/mojo/MojoHelper.h" #include "public/platform/Platform.h" @@ -24,7 +23,11 @@ provider_(provider), client_binding_(this), state_(SensorProxy::kUninitialized), - suspended_(false) {} + suspended_(false), + polling_timer_(TaskRunnerHelper::Get(TaskType::kSensor, + provider->GetSupplementable()), + this, + &SensorProxy::OnPollingTimer) {} SensorProxy::~SensorProxy() {} @@ -33,7 +36,6 @@ } DEFINE_TRACE(SensorProxy) { - visitor->Trace(reading_updater_); visitor->Trace(observers_); visitor->Trace(provider_); PageVisibilityObserver::Trace(visitor); @@ -64,10 +66,6 @@ callback); } -bool SensorProxy::IsActive() const { - return IsInitialized() && !suspended_ && !frequencies_used_.IsEmpty(); -} - void SensorProxy::AddConfiguration( SensorConfigurationPtr configuration, std::unique_ptr<Function<void(bool)>> callback) { @@ -94,6 +92,7 @@ sensor_->Suspend(); suspended_ = true; + UpdatePollingStatus(); } void SensorProxy::Resume() { @@ -103,9 +102,7 @@ sensor_->Resume(); suspended_ = false; - - if (IsActive()) - reading_updater_->Start(); + UpdatePollingStatus(); } const SensorConfiguration* SensorProxy::DefaultConfig() const { @@ -113,10 +110,6 @@ return default_config_.get(); } -Document* SensorProxy::GetDocument() const { - return provider_->GetSupplementable()->GetDocument(); -} - void SensorProxy::UpdateSensorReading() { DCHECK(IsInitialized()); int read_attempts = 0; @@ -131,27 +124,19 @@ if (reading_.timestamp != reading_data.timestamp) { reading_ = reading_data; + double now = WTF::MonotonicallyIncreasingTime(); for (Observer* observer : observers_) - observer->OnSensorReadingChanged(); + observer->OnSensorReadingChanged(now); } } -void SensorProxy::NotifySensorChanged(double timestamp) { - // This notification leads to sync 'onchange' event sending, so - // we must cache m_observers as it can be modified within event handlers. - auto copy = observers_; - for (Observer* observer : copy) - observer->NotifySensorChanged(timestamp); -} - void SensorProxy::RaiseError() { HandleSensorError(); } void SensorProxy::SensorReadingChanged() { DCHECK_EQ(ReportingMode::ON_CHANGE, mode_); - if (IsActive()) - reading_updater_->Start(); + UpdateSensorReading(); } void SensorProxy::PageVisibilityChanged() { @@ -169,6 +154,7 @@ state_ = kUninitialized; frequencies_used_.clear(); reading_ = device::SensorReading(); + UpdatePollingStatus(); // The m_sensor.reset() will release all callbacks and its bound parameters, // therefore, handleSensorError accepts messages by value. @@ -229,8 +215,6 @@ sensor_.set_connection_error_handler( ConvertToBaseCallback(std::move(error_callback))); - reading_updater_ = SensorReadingUpdater::Create(this, mode_); - state_ = kInitialized; for (Observer* observer : observers_) @@ -244,8 +228,7 @@ if (result) { frequencies_used_.push_back(frequency); std::sort(frequencies_used_.begin(), frequencies_used_.end()); - if (IsActive()) - reading_updater_->Start(); + UpdatePollingStatus(); } (*callback)(result); @@ -278,4 +261,22 @@ return true; } +void SensorProxy::OnPollingTimer(TimerBase*) { + UpdateSensorReading(); +} + +void SensorProxy::UpdatePollingStatus() { + bool start_polling = (mode_ == ReportingMode::CONTINUOUS) && + IsInitialized() && !suspended_ && + !frequencies_used_.IsEmpty(); + if (start_polling) { + // TODO(crbug/721297) : We need to find out an algorithm for resulting + // polling frequency. + polling_timer_.StartRepeating(1 / frequencies_used_.back(), + BLINK_FROM_HERE); + } else { + polling_timer_.Stop(); + } +} + } // namespace blink
diff --git a/third_party/WebKit/Source/modules/sensor/SensorProxy.h b/third_party/WebKit/Source/modules/sensor/SensorProxy.h index b21ac49..51fd139 100644 --- a/third_party/WebKit/Source/modules/sensor/SensorProxy.h +++ b/third_party/WebKit/Source/modules/sensor/SensorProxy.h
@@ -12,6 +12,7 @@ #include "device/generic_sensor/public/interfaces/sensor_provider.mojom-blink.h" #include "mojo/public/cpp/bindings/binding.h" #include "platform/Supplementable.h" +#include "platform/Timer.h" #include "platform/heap/Handle.h" #include "platform/wtf/Vector.h" @@ -19,7 +20,6 @@ class SensorProviderProxy; class SensorReading; -class SensorReadingUpdater; // This class wraps 'Sensor' mojo interface and used by multiple // JS sensor instances of the same type (within a single frame). @@ -36,20 +36,14 @@ // Has valid 'Sensor' binding, {add, remove}Configuration() // methods can be called. virtual void OnSensorInitialized() {} - // Platfrom sensor reading has changed. - virtual void OnSensorReadingChanged() {} - // Observer should send 'onchange' event if needed. - // The 'notifySensorChanged' calls are in sync with rAF. - // Currently, we decide whether to send 'onchange' event based on the - // time elapsed from the previous notification. - // TODO: Reconsider this after https://github.com/w3c/sensors/issues/152 - // is resolved. + // Observer should update its cached reading and send 'onchange' + // event if needed. // |timestamp| Reference timestamp in seconds of the moment when // sensor reading was updated from the buffer. // Note: |timestamp| values are only used to calculate elapsed time // between shared buffer readings. These values *do not* correspond // to sensor reading timestamps which are obtained on platform side. - virtual void NotifySensorChanged(double timestamp) {} + virtual void OnSensorReadingChanged(double timestamp) {} // An error has occurred. virtual void OnSensorError(ExceptionCode, const String& sanitized_message, @@ -68,10 +62,6 @@ bool IsInitializing() const { return state_ == kInitializing; } bool IsInitialized() const { return state_ == kInitialized; } - // Is watching new reading data (initialized, not suspended and has - // configurations added). - bool IsActive() const; - void AddConfiguration(device::mojom::blink::SensorConfigurationPtr, std::unique_ptr<Function<void(bool)>>); @@ -80,11 +70,10 @@ void Suspend(); void Resume(); - device::mojom::blink::SensorType GetType() const { return type_; } - device::mojom::blink::ReportingMode GetReportingMode() const { return mode_; } + device::mojom::blink::SensorType type() const { return type_; } // Note: the returned value is reset after updateSensorReading() call. - const device::SensorReading& Reading() const { return reading_; } + const device::SensorReading& reading() const { return reading_; } const device::mojom::blink::SensorConfiguration* DefaultConfig() const; @@ -92,11 +81,6 @@ return frequency_limits_; } - Document* GetDocument() const; - const WTF::Vector<double>& FrequenciesUsed() const { - return frequencies_used_; - } - DECLARE_VIRTUAL_TRACE(); private: @@ -129,7 +113,11 @@ void OnRemoveConfigurationCompleted(double frequency, bool result); bool TryReadFromBuffer(device::SensorReading& result); - void OnAnimationFrame(double timestamp); + + void OnPollingTimer(TimerBase*); + // Starts polling timer if needed (continuous reporting, initialized, not + // suspended and has configurations added). + void UpdatePollingStatus(); device::mojom::blink::SensorType type_; device::mojom::blink::ReportingMode mode_; @@ -149,9 +137,8 @@ device::SensorReading reading_; std::pair<double, double> frequency_limits_; - Member<SensorReadingUpdater> reading_updater_; WTF::Vector<double> frequencies_used_; - double last_raf_timestamp_; + TaskRunnerTimer<SensorProxy> polling_timer_; using ReadingBuffer = device::SensorReadingSharedBuffer; static_assert(
diff --git a/third_party/WebKit/Source/modules/sensor/SensorReadingUpdater.cpp b/third_party/WebKit/Source/modules/sensor/SensorReadingUpdater.cpp deleted file mode 100644 index 8904912..0000000 --- a/third_party/WebKit/Source/modules/sensor/SensorReadingUpdater.cpp +++ /dev/null
@@ -1,124 +0,0 @@ -// Copyright 2016 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 "modules/sensor/SensorReadingUpdater.h" - -#include "core/dom/Document.h" -#include "device/generic_sensor/public/interfaces/sensor.mojom-blink.h" -#include "modules/sensor/SensorProxy.h" -#include "platform/wtf/CurrentTime.h" - -using device::mojom::blink::ReportingMode; - -namespace blink { - -SensorReadingUpdater::SensorReadingUpdater(SensorProxy* sensor_proxy) - : sensor_proxy_(sensor_proxy), - document_(sensor_proxy_->GetDocument()), - has_pending_animation_frame_task_(false) {} - -void SensorReadingUpdater::EnqueueAnimationFrameTask() { - if (!document_ || document_->IsDetached()) { - // If the document has detached the scheduled callbacks - // will never be called. - has_pending_animation_frame_task_ = false; - document_ = sensor_proxy_->GetDocument(); - if (!document_ || document_->IsDetached()) - return; - } - - if (has_pending_animation_frame_task_) - return; - - auto callback = WTF::Bind(&SensorReadingUpdater::OnAnimationFrame, - WrapWeakPersistent(this)); - document_->EnqueueAnimationFrameTask(std::move(callback)); - has_pending_animation_frame_task_ = true; -} - -void SensorReadingUpdater::Start() { - EnqueueAnimationFrameTask(); -} - -void SensorReadingUpdater::OnAnimationFrame() { - has_pending_animation_frame_task_ = false; - OnAnimationFrameInternal(); -} - -DEFINE_TRACE(SensorReadingUpdater) { - visitor->Trace(document_); - visitor->Trace(sensor_proxy_); -} - -class SensorReadingUpdaterContinuous : public SensorReadingUpdater { - public: - explicit SensorReadingUpdaterContinuous(SensorProxy* sensor_proxy) - : SensorReadingUpdater(sensor_proxy) {} - - DEFINE_INLINE_VIRTUAL_TRACE() { SensorReadingUpdater::Trace(visitor); } - - protected: - void OnAnimationFrameInternal() override { - if (!sensor_proxy_->IsActive()) - return; - - sensor_proxy_->UpdateSensorReading(); - sensor_proxy_->NotifySensorChanged(WTF::MonotonicallyIncreasingTime()); - EnqueueAnimationFrameTask(); - } -}; - -// New data is fetched from shared buffer only once after 'start()' -// call. Further, notification is send until every client is updated -// (i.e. until longest notification period elapses) rAF stops after that. -class SensorReadingUpdaterOnChange : public SensorReadingUpdater { - public: - explicit SensorReadingUpdaterOnChange(SensorProxy* sensor_proxy) - : SensorReadingUpdater(sensor_proxy), - new_data_arrived_time_(0.0), - new_data_arrived_(false) {} - - DEFINE_INLINE_VIRTUAL_TRACE() { SensorReadingUpdater::Trace(visitor); } - - void Start() override { - new_data_arrived_ = true; - SensorReadingUpdater::Start(); - } - - protected: - void OnAnimationFrameInternal() override { - if (!sensor_proxy_->IsActive()) - return; - - double timestamp = WTF::MonotonicallyIncreasingTime(); - - if (new_data_arrived_) { - new_data_arrived_ = false; - sensor_proxy_->UpdateSensorReading(); - new_data_arrived_time_ = timestamp; - } - sensor_proxy_->NotifySensorChanged(timestamp); - - DCHECK_GT(sensor_proxy_->FrequenciesUsed().front(), 0.0); - double longest_notification_period = - 1 / sensor_proxy_->FrequenciesUsed().front(); - - if (timestamp - new_data_arrived_time_ <= longest_notification_period) - EnqueueAnimationFrameTask(); - } - - private: - double new_data_arrived_time_; - bool new_data_arrived_; -}; - -// static -SensorReadingUpdater* SensorReadingUpdater::Create(SensorProxy* proxy, - ReportingMode mode) { - if (mode == ReportingMode::CONTINUOUS) - return new SensorReadingUpdaterContinuous(proxy); - return new SensorReadingUpdaterOnChange(proxy); -} - -} // namespace blink
diff --git a/third_party/WebKit/Source/modules/sensor/SensorReadingUpdater.h b/third_party/WebKit/Source/modules/sensor/SensorReadingUpdater.h deleted file mode 100644 index be9ac9c..0000000 --- a/third_party/WebKit/Source/modules/sensor/SensorReadingUpdater.h +++ /dev/null
@@ -1,41 +0,0 @@ -// Copyright 2016 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 SensorReadingUpdater_h -#define SensorReadingUpdater_h - -#include "device/generic_sensor/public/interfaces/sensor_provider.mojom-blink.h" -#include "platform/heap/Handle.h" - -namespace blink { - -class Document; -class SensorProxy; - -// This class encapsulates sensor reading update notification logic. -class SensorReadingUpdater : public GarbageCollected<SensorReadingUpdater> { - public: - static SensorReadingUpdater* Create(SensorProxy*, - device::mojom::blink::ReportingMode); - - virtual void Start(); - - DECLARE_VIRTUAL_TRACE(); - - protected: - explicit SensorReadingUpdater(SensorProxy*); - void EnqueueAnimationFrameTask(); - virtual void OnAnimationFrameInternal() = 0; - - Member<SensorProxy> sensor_proxy_; - WeakMember<Document> document_; - bool has_pending_animation_frame_task_; - - private: - void OnAnimationFrame(); -}; - -} // namespace blink - -#endif // SensorReadingUpdater_h
diff --git a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp b/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp index 4ca1989f..a9aa01e 100644 --- a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp +++ b/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
@@ -589,9 +589,8 @@ // An URL with the "cid" scheme can only be handled by an MHTML Archive. // Abort the request when there is none. - if (resource_request.Url().ProtocolIs(kContentIdScheme) && !archive_) { + if (!archive_ && resource_request.Url().ProtocolIs(kContentIdScheme)) return nullptr; - } bool is_data_url = resource_request.Url().ProtocolIsData(); bool is_static_data = is_data_url || substitute_data.IsValid() || archive_;
diff --git a/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc b/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc index 168aca3..09f5c3dd 100644 --- a/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc +++ b/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
@@ -469,13 +469,14 @@ void RendererSchedulerImpl::BeginMainFrameNotExpectedUntil( base::TimeTicks time) { - TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("renderer.scheduler"), - "RendererSchedulerImpl::BeginMainFrametime"); helper_.CheckOnValidThread(); if (helper_.IsShutdown()) return; base::TimeTicks now(helper_.scheduler_tqm_delegate()->NowTicks()); + TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("renderer.scheduler"), + "RendererSchedulerImpl::BeginMainFrameNotExpectedUntil", + "time_remaining", (time - now).InMillisecondsF()); if (now < time) { // End any previous idle period.
diff --git a/third_party/WebKit/Source/web/BUILD.gn b/third_party/WebKit/Source/web/BUILD.gn index a600e85..602d8e5 100644 --- a/third_party/WebKit/Source/web/BUILD.gn +++ b/third_party/WebKit/Source/web/BUILD.gn
@@ -152,8 +152,6 @@ "WebIDBKeyRange.cpp", "WebInputElement.cpp", "WebInputEvent.cpp", - "WebInputEventConversion.cpp", - "WebInputEventConversion.h", "WebInputMethodControllerImpl.cpp", "WebInputMethodControllerImpl.h", "WebKit.cpp",
diff --git a/third_party/WebKit/Source/web/ChromeClientImpl.cpp b/third_party/WebKit/Source/web/ChromeClientImpl.cpp index 4eae5205..98ae6fc1 100644 --- a/third_party/WebKit/Source/web/ChromeClientImpl.cpp +++ b/third_party/WebKit/Source/web/ChromeClientImpl.cpp
@@ -39,6 +39,7 @@ #include "core/dom/Fullscreen.h" #include "core/dom/Node.h" #include "core/events/UIEventWithKeyState.h" +#include "core/events/WebInputEventConversion.h" #include "core/exported/WebFileChooserCompletionImpl.h" #include "core/exported/WebViewBase.h" #include "core/frame/FrameView.h" @@ -120,7 +121,6 @@ #include "web/NavigatorContentUtilsClientImpl.h" #include "web/PopupMenuImpl.h" #include "web/WebFrameWidgetImpl.h" -#include "web/WebInputEventConversion.h" #include "web/WebLocalFrameImpl.h" #include "web/WebPluginContainerImpl.h" #include "web/WebRemoteFrameImpl.h"
diff --git a/third_party/WebKit/Source/web/DevToolsEmulator.cpp b/third_party/WebKit/Source/web/DevToolsEmulator.cpp index e9e2995..3e4229e4 100644 --- a/third_party/WebKit/Source/web/DevToolsEmulator.cpp +++ b/third_party/WebKit/Source/web/DevToolsEmulator.cpp
@@ -4,6 +4,7 @@ #include "web/DevToolsEmulator.h" +#include "core/events/WebInputEventConversion.h" #include "core/exported/WebViewBase.h" #include "core/frame/FrameView.h" #include "core/frame/Settings.h" @@ -20,7 +21,6 @@ #include "platform/loader/fetch/MemoryCache.h" #include "platform/wtf/PtrUtil.h" #include "public/platform/WebLayerTreeView.h" -#include "web/WebInputEventConversion.h" #include "web/WebLocalFrameImpl.h" #include "web/WebSettingsImpl.h"
diff --git a/third_party/WebKit/Source/web/InspectorOverlayAgent.cpp b/third_party/WebKit/Source/web/InspectorOverlayAgent.cpp index ad3a919..f42b7cc3 100644 --- a/third_party/WebKit/Source/web/InspectorOverlayAgent.cpp +++ b/third_party/WebKit/Source/web/InspectorOverlayAgent.cpp
@@ -38,6 +38,7 @@ #include "core/dom/Node.h" #include "core/dom/StaticNodeList.h" #include "core/dom/TaskRunnerHelper.h" +#include "core/events/WebInputEventConversion.h" #include "core/exported/WebViewBase.h" #include "core/frame/FrameView.h" #include "core/frame/LocalFrame.h" @@ -65,7 +66,6 @@ #include "v8/include/v8.h" #include "web/ChromeClientImpl.h" #include "web/PageOverlay.h" -#include "web/WebInputEventConversion.h" #include "web/WebLocalFrameImpl.h" namespace blink {
diff --git a/third_party/WebKit/Source/web/LinkHighlightImplTest.cpp b/third_party/WebKit/Source/web/LinkHighlightImplTest.cpp index d6f5e80..ac2d009 100644 --- a/third_party/WebKit/Source/web/LinkHighlightImplTest.cpp +++ b/third_party/WebKit/Source/web/LinkHighlightImplTest.cpp
@@ -28,6 +28,7 @@ #include <memory> #include "bindings/core/v8/ExceptionState.h" #include "core/dom/Node.h" +#include "core/events/WebInputEventConversion.h" #include "core/exported/WebViewBase.h" #include "core/frame/FrameView.h" #include "core/input/EventHandler.h" @@ -48,7 +49,6 @@ #include "public/web/WebFrameClient.h" #include "public/web/WebViewClient.h" #include "testing/gtest/include/gtest/gtest.h" -#include "web/WebInputEventConversion.h" #include "web/WebLocalFrameImpl.h" #include "web/tests/FrameTestHelpers.h"
diff --git a/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp b/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp index 95487a64..8d910a7 100644 --- a/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp +++ b/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
@@ -124,6 +124,31 @@ return frame ? WebFrame::ToCoreFrame(*frame) : nullptr; } +// Return the parent of |frame| as a LocalFrame, nullptr when there is no +// parent or when the parent is a remote frame. +LocalFrame* GetLocalParentFrame(WebLocalFrameImpl* frame) { + WebFrame* parent = frame->Parent(); + if (!parent || !parent->IsWebLocalFrame()) + return nullptr; + + return ToWebLocalFrameImpl(parent)->GetFrame(); +} + +// Returns whether the |local_frame| has been loaded using an MHTMLArchive. When +// it is the case, each subframe must use it for loading. +bool IsLoadedAsMHTMLArchive(LocalFrame* local_frame) { + return local_frame && local_frame->GetDocument()->Fetcher()->Archive(); +} + +// Returns whether the |local_frame| is in a middle of a back/forward +// navigation. +bool IsBackForwardNavigationInProgress(LocalFrame* local_frame) { + return local_frame && + IsBackForwardLoadType( + local_frame->Loader().GetDocumentLoader()->LoadType()) && + !local_frame->GetDocument()->LoadEventFinished(); +} + } // namespace LocalFrameClientImpl::LocalFrameClientImpl(WebLocalFrameImpl* frame) @@ -483,23 +508,6 @@ WebDataSourceImpl* ds = WebDataSourceImpl::FromDocumentLoader(loader); - // Newly created child frames may need to be navigated to a history item - // during a back/forward navigation. This will only happen when the parent - // is a LocalFrame doing a back/forward navigation that has not completed. - // (If the load has completed and the parent later adds a frame with script, - // we do not want to use a history item for it.) - bool is_history_navigation_in_new_child_frame = - web_frame_->Parent() && web_frame_->Parent()->IsWebLocalFrame() && - IsBackForwardLoadType(ToWebLocalFrameImpl(web_frame_->Parent()) - ->GetFrame() - ->Loader() - .GetDocumentLoader() - ->LoadType()) && - !ToWebLocalFrameImpl(web_frame_->Parent()) - ->GetFrame() - ->GetDocument() - ->LoadEventFinished(); - WrappedResourceRequest wrapped_resource_request(request); WebFrameClient::NavigationPolicyInfo navigation_info( wrapped_resource_request); @@ -507,8 +515,6 @@ navigation_info.default_policy = static_cast<WebNavigationPolicy>(policy); navigation_info.extra_data = ds ? ds->GetExtraData() : nullptr; navigation_info.replaces_current_history_item = replaces_current_history_item; - navigation_info.is_history_navigation_in_new_child_frame = - is_history_navigation_in_new_child_frame; navigation_info.is_client_redirect = is_client_redirect; navigation_info.should_check_main_world_content_security_policy = should_check_main_world_content_security_policy == @@ -516,10 +522,25 @@ ? kWebContentSecurityPolicyDispositionCheck : kWebContentSecurityPolicyDispositionDoNotCheck; - if (IsLoadedAsMHTMLArchive()) { - navigation_info.archive_status = - WebFrameClient::NavigationPolicyInfo::ArchiveStatus::Present; - } + // Can be null. + LocalFrame* local_parent_frame = GetLocalParentFrame(web_frame_); + + // Newly created child frames may need to be navigated to a history item + // during a back/forward navigation. This will only happen when the parent + // is a LocalFrame doing a back/forward navigation that has not completed. + // (If the load has completed and the parent later adds a frame with script, + // we do not want to use a history item for it.) + navigation_info.is_history_navigation_in_new_child_frame = + IsBackForwardNavigationInProgress(local_parent_frame); + + // TODO(nasko): How should this work with OOPIF? + // The MHTMLArchive is parsed as a whole, but can be constructed from frames + // in multiple processes. In that case, which process should parse it and how + // should the output be spread back across multiple processes? + navigation_info.archive_status = + IsLoadedAsMHTMLArchive(local_parent_frame) + ? WebFrameClient::NavigationPolicyInfo::ArchiveStatus::Present + : WebFrameClient::NavigationPolicyInfo::ArchiveStatus::Absent; // Caching could be disabled for requests initiated by DevTools. // TODO(ananta) @@ -987,26 +1008,6 @@ ->DevToolsAgentImpl(); } -bool LocalFrameClientImpl::IsLoadedAsMHTMLArchive() const { - WebFrame* parent_frame = web_frame_->Parent(); - if (!parent_frame) - return false; - - // TODO(nasko): How should this work with OOPIF? - // The MHTMLArchive is parsed as a whole, but can be constructed from frames - // in multiple processes. In that case, which process should parse it and how - // should the output be spread back across multiple processes? - if (!parent_frame->IsWebLocalFrame()) - return false; - - return ToWebLocalFrameImpl(parent_frame) - ->GetFrame() - ->Loader() - .GetDocumentLoader() - ->Fetcher() - ->Archive(); -} - KURL LocalFrameClientImpl::OverrideFlashEmbedWithHTML(const KURL& url) { return web_frame_->Client()->OverrideFlashEmbedWithHTML(WebURL(url)); }
diff --git a/third_party/WebKit/Source/web/LocalFrameClientImpl.h b/third_party/WebKit/Source/web/LocalFrameClientImpl.h index bb578a49..9e8a861 100644 --- a/third_party/WebKit/Source/web/LocalFrameClientImpl.h +++ b/third_party/WebKit/Source/web/LocalFrameClientImpl.h
@@ -232,11 +232,6 @@ bool IsLocalFrameClientImpl() const override { return true; } WebDevToolsAgentImpl* DevToolsAgent(); - // An MHTML Archive is a set of resources, including iframes, that is parsed - // as a whole. This function tells whether of not an archive exists and should - // be used for loading a subframe. - bool IsLoadedAsMHTMLArchive() const; - // The WebFrame that owns this object and manages its lifetime. Therefore, // the web frame object is guaranteed to exist. Member<WebLocalFrameImpl> web_frame_;
diff --git a/third_party/WebKit/Source/web/PageWidgetDelegate.cpp b/third_party/WebKit/Source/web/PageWidgetDelegate.cpp index fef987a1..d004c45c 100644 --- a/third_party/WebKit/Source/web/PageWidgetDelegate.cpp +++ b/third_party/WebKit/Source/web/PageWidgetDelegate.cpp
@@ -30,6 +30,7 @@ #include "web/PageWidgetDelegate.h" +#include "core/events/WebInputEventConversion.h" #include "core/frame/FrameView.h" #include "core/frame/LocalFrame.h" #include "core/input/EventHandler.h" @@ -45,7 +46,6 @@ #include "platform/transforms/AffineTransform.h" #include "platform/wtf/CurrentTime.h" #include "public/platform/WebInputEvent.h" -#include "web/WebInputEventConversion.h" namespace blink {
diff --git a/third_party/WebKit/Source/web/RemoteFrameClientImpl.cpp b/third_party/WebKit/Source/web/RemoteFrameClientImpl.cpp index a681c4a..842ec78 100644 --- a/third_party/WebKit/Source/web/RemoteFrameClientImpl.cpp +++ b/third_party/WebKit/Source/web/RemoteFrameClientImpl.cpp
@@ -7,6 +7,7 @@ #include <memory> #include "core/events/KeyboardEvent.h" #include "core/events/MouseEvent.h" +#include "core/events/WebInputEventConversion.h" #include "core/events/WheelEvent.h" #include "core/frame/RemoteFrame.h" #include "core/frame/RemoteFrameView.h" @@ -19,7 +20,6 @@ #include "platform/weborigin/SecurityPolicy.h" #include "platform/wtf/PtrUtil.h" #include "public/web/WebRemoteFrameClient.h" -#include "web/WebInputEventConversion.h" #include "web/WebRemoteFrameImpl.h" namespace blink {
diff --git a/third_party/WebKit/Source/web/WebDevToolsAgentImpl.cpp b/third_party/WebKit/Source/web/WebDevToolsAgentImpl.cpp index a6c67164..775a47b 100644 --- a/third_party/WebKit/Source/web/WebDevToolsAgentImpl.cpp +++ b/third_party/WebKit/Source/web/WebDevToolsAgentImpl.cpp
@@ -36,6 +36,7 @@ #include "bindings/core/v8/ScriptController.h" #include "bindings/core/v8/V8BindingForCore.h" #include "core/CoreProbeSink.h" +#include "core/events/WebInputEventConversion.h" #include "core/exported/WebViewBase.h" #include "core/frame/FrameView.h" #include "core/frame/LocalFrame.h" @@ -87,7 +88,6 @@ #include "web/InspectorEmulationAgent.h" #include "web/InspectorOverlayAgent.h" #include "web/WebFrameWidgetImpl.h" -#include "web/WebInputEventConversion.h" #include "web/WebLocalFrameImpl.h" #include "web/WebSettingsImpl.h"
diff --git a/third_party/WebKit/Source/web/WebFrameWidgetBase.cpp b/third_party/WebKit/Source/web/WebFrameWidgetBase.cpp index 20da5c42..3f462d3d 100644 --- a/third_party/WebKit/Source/web/WebFrameWidgetBase.cpp +++ b/third_party/WebKit/Source/web/WebFrameWidgetBase.cpp
@@ -5,6 +5,7 @@ #include "web/WebFrameWidgetBase.h" #include "core/dom/DocumentUserGestureToken.h" +#include "core/events/WebInputEventConversion.h" #include "core/exported/WebViewBase.h" #include "core/frame/FrameView.h" #include "core/frame/VisualViewport.h" @@ -17,10 +18,7 @@ #include "core/page/Page.h" #include "core/page/PointerLockController.h" #include "platform/UserGestureIndicator.h" -#include "public/web/WebAutofillClient.h" -#include "public/web/WebDocument.h" #include "public/web/WebWidgetClient.h" -#include "web/WebInputEventConversion.h" namespace blink {
diff --git a/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp b/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp index c85a3996..edc60fd 100644 --- a/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp +++ b/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp
@@ -39,6 +39,7 @@ #include "core/editing/FrameSelection.h" #include "core/editing/InputMethodController.h" #include "core/editing/PlainTextRange.h" +#include "core/events/WebInputEventConversion.h" #include "core/exported/WebViewBase.h" #include "core/frame/FrameView.h" #include "core/frame/RemoteFrame.h" @@ -69,7 +70,6 @@ #include "web/CompositorWorkerProxyClientImpl.h" #include "web/ContextMenuAllowedScope.h" #include "web/WebDevToolsAgentImpl.h" -#include "web/WebInputEventConversion.h" #include "web/WebInputMethodControllerImpl.h" #include "web/WebLocalFrameImpl.h" #include "web/WebPagePopupImpl.h"
diff --git a/third_party/WebKit/Source/web/WebPagePopupImpl.cpp b/third_party/WebKit/Source/web/WebPagePopupImpl.cpp index bba0f17b..7f86f04 100644 --- a/third_party/WebKit/Source/web/WebPagePopupImpl.cpp +++ b/third_party/WebKit/Source/web/WebPagePopupImpl.cpp
@@ -32,6 +32,7 @@ #include "core/dom/ContextFeatures.h" #include "core/events/MessageEvent.h" +#include "core/events/WebInputEventConversion.h" #include "core/exported/WebViewBase.h" #include "core/frame/FrameView.h" #include "core/frame/LocalFrame.h" @@ -64,7 +65,6 @@ #include "public/web/WebFrameClient.h" #include "public/web/WebViewClient.h" #include "public/web/WebWidgetClient.h" -#include "web/WebInputEventConversion.h" #include "web/WebLocalFrameImpl.h" #include "web/WebSettingsImpl.h"
diff --git a/third_party/WebKit/Source/web/WebPluginContainerImpl.cpp b/third_party/WebKit/Source/web/WebPluginContainerImpl.cpp index 263d6d3..95bd2924 100644 --- a/third_party/WebKit/Source/web/WebPluginContainerImpl.cpp +++ b/third_party/WebKit/Source/web/WebPluginContainerImpl.cpp
@@ -49,6 +49,7 @@ #include "core/events/ProgressEvent.h" #include "core/events/ResourceProgressEvent.h" #include "core/events/TouchEvent.h" +#include "core/events/WebInputEventConversion.h" #include "core/events/WheelEvent.h" #include "core/exported/WebDataSourceImpl.h" #include "core/exported/WebViewBase.h" @@ -104,7 +105,6 @@ #include "public/web/WebPrintPresetOptions.h" #include "public/web/WebViewClient.h" #include "web/ChromeClientImpl.h" -#include "web/WebInputEventConversion.h" namespace blink {
diff --git a/third_party/WebKit/Source/web/WebViewImpl.cpp b/third_party/WebKit/Source/web/WebViewImpl.cpp index 1ce8583e..97202d4 100644 --- a/third_party/WebKit/Source/web/WebViewImpl.cpp +++ b/third_party/WebKit/Source/web/WebViewImpl.cpp
@@ -49,6 +49,7 @@ #include "core/editing/serializers/Serialization.h" #include "core/events/KeyboardEvent.h" #include "core/events/UIEventWithKeyState.h" +#include "core/events/WebInputEventConversion.h" #include "core/events/WheelEvent.h" #include "core/frame/BrowserControls.h" #include "core/frame/EventHandlerRegistry.h" @@ -168,7 +169,6 @@ #include "web/StorageQuotaClientImpl.h" #include "web/ValidationMessageClientImpl.h" #include "web/WebDevToolsAgentImpl.h" -#include "web/WebInputEventConversion.h" #include "web/WebInputMethodControllerImpl.h" #include "web/WebLocalFrameImpl.h" #include "web/WebPluginContainerImpl.h"
diff --git a/third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp b/third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp index 0fb4d61..0acd896 100644 --- a/third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp +++ b/third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp
@@ -28,7 +28,7 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "web/WebInputEventConversion.h" +#include "core/events/WebInputEventConversion.h" #include "core/dom/Touch.h" #include "core/dom/TouchList.h"
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index c8a6a73..0eb13d53 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml
@@ -19463,10 +19463,12 @@ <int value="8" label="Inter process TimeTicks skew"/> <int value="9" label="No commit or failed provisional load received"/> <int value="10" label="No page load end time recorded"/> - <int value="11" label="Timing IPC received from subframe"/> + <int value="11" label="Timing IPC received from subframe (deprecated)"/> <int value="12" label="Invalid timing IPC (invalid timing descendent)"/> <int value="13" label="Invalid timing IPC (invalid behavior descendent)"/> <int value="14" label="Invalid timing IPC (invalid timing)"/> + <int value="15" + label="Subframe navigation start before main frame navigation start"/> </enum> <enum name="InterruptReason" type="int">
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index b3f6f5e..441ddd975 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -48254,6 +48254,17 @@ <summary>Whether a navigation started in the foreground.</summary> </histogram> +<histogram name="PageLoad.Internal.OutOfOrderInterFrameTiming" units="ms"> + <owner>bmcquade@chromium.org</owner> + <summary> + The difference in magnitude between existing and updated timing, for + inter-frame timings that are received out of order. Page load metrics + observes timing updates in all frames on a page. It's possible for timings + in different frames to arrive out of order. This metric tracks how often + this happens, along with the magnitude of the difference. + </summary> +</histogram> + <histogram name="PageLoad.Internal.PageLoadCompleted.AfterAppBackground" enum="BooleanStartedCompleted"> <owner>bmcquade@chromium.org</owner>