diff --git a/DEPS b/DEPS index d1956b6f..09ff504 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': '4c672f28bb3980d31dd916a426a5e1138775f8d1', + 'v8_revision': 'f6aefd1d65e1ce2bef3c0d4928c56cc738051e0f', # 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. @@ -228,7 +228,7 @@ Var('chromium_git') + '/native_client/src/third_party/scons-2.0.1.git' + '@' + '1c1550e17fc26355d08627fbdec13d8291227067', 'src/third_party/webrtc': - Var('chromium_git') + '/external/webrtc/trunk/webrtc.git' + '@' + 'df3c7ed816f08cfed8a033881fb5e2bc5ce29ee3', # commit position 16123 + Var('chromium_git') + '/external/webrtc/trunk/webrtc.git' + '@' + 'd68fcc42256f0f6483d562aa69531091560ff9f2', # commit position 16134 'src/third_party/openmax_dl': Var('chromium_git') + '/external/webrtc/deps/third_party/openmax.git' + '@' + Var('openmax_dl_revision'),
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/ClientManager.java b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/ClientManager.java index 285980ef..a47814ff 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/ClientManager.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/ClientManager.java
@@ -36,6 +36,13 @@ /** Manages the clients' state for Custom Tabs. This class is threadsafe. */ @SuppressFBWarnings("CHROMIUM_SYNCHRONIZED_METHOD") class ClientManager { + // Values for the "CustomTabs.MayLaunchUrlType" UMA histogram. Append-only. + @VisibleForTesting static final int NO_MAY_LAUNCH_URL = 0; + @VisibleForTesting static final int LOW_CONFIDENCE = 1; + @VisibleForTesting static final int HIGH_CONFIDENCE = 2; + @VisibleForTesting static final int BOTH = 3; // LOW + HIGH. + private static final int MAY_LAUNCH_URL_TYPE_COUNT = 4; + // Values for the "CustomTabs.PredictionStatus" UMA histogram. Append-only. @VisibleForTesting static final int NO_PREDICTION = 0; @VisibleForTesting static final int GOOD_PREDICTION = 1; @@ -59,6 +66,8 @@ public final String packageName; public final PostMessageHandler postMessageHandler; public boolean mIgnoreFragments; + public boolean lowConfidencePrediction; + public boolean highConfidencePrediction; private boolean mShouldHideDomain; private boolean mShouldPrerenderOnCellular; private boolean mShouldSendNavigationInfo; @@ -89,9 +98,23 @@ mKeepAliveConnection = serviceConnection; } - public void setPredictionMetrics(String predictedUrl, long lastMayLaunchUrlTimestamp) { + public void setPredictionMetrics( + String predictedUrl, long lastMayLaunchUrlTimestamp, boolean lowConfidence) { mPredictedUrl = predictedUrl; mLastMayLaunchUrlTimestamp = lastMayLaunchUrlTimestamp; + highConfidencePrediction |= !TextUtils.isEmpty(predictedUrl); + lowConfidencePrediction |= lowConfidence; + } + + /** + * Resets the prediction metrics. This clears the predicted URL, last prediction time, + * and whether a low and/or high confidence prediction has been done. + */ + public void resetPredictionMetrics() { + mPredictedUrl = null; + mLastMayLaunchUrlTimestamp = 0; + highConfidencePrediction = false; + lowConfidencePrediction = false; } public String getPredictedUrl() { @@ -155,16 +178,22 @@ /** Updates the client behavior stats and returns whether speculation is allowed. * + * The first call to the "low priority" mode is not throttled. Subsequent ones are. + * * @param session Client session. * @param uid As returned by Binder.getCallingUid(). * @param url Predicted URL. + * @param lowConfidence whether the request contains some "low confidence" URLs. * @return true if speculation is allowed. */ public synchronized boolean updateStatsAndReturnWhetherAllowed( - CustomTabsSessionToken session, int uid, String url) { + CustomTabsSessionToken session, int uid, String url, boolean lowConfidence) { SessionParams params = mSessionParams.get(session); if (params == null || params.uid != uid) return false; - params.setPredictionMetrics(url, SystemClock.elapsedRealtime()); + boolean firstLowConfidencePrediction = + TextUtils.isEmpty(url) && lowConfidence && !params.lowConfidencePrediction; + params.setPredictionMetrics(url, SystemClock.elapsedRealtime(), lowConfidence); + if (firstLowConfidencePrediction) return true; RequestThrottler throttler = RequestThrottler.getForUid(mContext, uid); return throttler.updateStatsAndReturnWhetherAllowed(); } @@ -186,6 +215,9 @@ return result; } + /** + * @return the prediction outcome. NO_PREDICTION if mSessionParams.get(session) returns null. + */ @VisibleForTesting synchronized int getPredictionOutcome(CustomTabsSessionToken session, String url) { SessionParams params = mSessionParams.get(session); @@ -219,7 +251,14 @@ } RecordHistogram.recordEnumeratedHistogram( "CustomTabs.WarmupStateOnLaunch", getWarmupState(session), SESSION_WARMUP_COUNT); - if (params != null) params.setPredictionMetrics(null, 0); + + if (params == null) return; + + int value = (params.lowConfidencePrediction ? LOW_CONFIDENCE : 0) + + (params.highConfidencePrediction ? HIGH_CONFIDENCE : 0); + RecordHistogram.recordEnumeratedHistogram( + "CustomTabs.MayLaunchUrlType", value, MAY_LAUNCH_URL_TYPE_COUNT); + params.resetPredictionMetrics(); } /**
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java index d51df57..750ce1c 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
@@ -401,9 +401,8 @@ if (!warmupInternal(false)) return false; // Also does the foreground check. final int uid = Binder.getCallingUid(); - // TODO(lizeb): Also throttle low-confidence mode. - if (!lowConfidence - && !mClientManager.updateStatsAndReturnWhetherAllowed(session, uid, urlString)) { + if (!mClientManager.updateStatsAndReturnWhetherAllowed( + session, uid, urlString, otherLikelyBundles != null)) { return false; } ThreadUtils.postOnUiThread(new Runnable() {
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAAccountChangeListener.java b/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAAccountChangeListener.java index 5cc600f3..67bf96e5 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAAccountChangeListener.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAAccountChangeListener.java
@@ -142,6 +142,7 @@ } private void notifyGsaBroadcastsAccountChanges() { + if (mClient == null) return; mClient.disconnect(); mClient = null; }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java b/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java index 0809a4a..f8fa33d 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java
@@ -82,7 +82,13 @@ MediaSessionTabHelper.convertMediaActionSourceToUMA(actionSource)); if (mMediaSessionObserver.getMediaSession() != null) { - mMediaSessionObserver.getMediaSession().resume(); + if (mMediaSessionActions != null + && mMediaSessionActions.contains(MediaSessionAction.PLAY)) { + mMediaSessionObserver.getMediaSession() + .didReceiveAction(MediaSessionAction.PLAY); + } else { + mMediaSessionObserver.getMediaSession().resume(); + } } } @@ -94,7 +100,13 @@ MediaSessionTabHelper.convertMediaActionSourceToUMA(actionSource)); if (mMediaSessionObserver.getMediaSession() != null) { - mMediaSessionObserver.getMediaSession().suspend(); + if (mMediaSessionActions != null + && mMediaSessionActions.contains(MediaSessionAction.PAUSE)) { + mMediaSessionObserver.getMediaSession() + .didReceiveAction(MediaSessionAction.PAUSE); + } else { + mMediaSessionObserver.getMediaSession().suspend(); + } } }
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/ClientManagerTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/ClientManagerTest.java index e9b983e7..8a47e45 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/ClientManagerTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/ClientManagerTest.java
@@ -9,6 +9,8 @@ import android.support.customtabs.CustomTabsSessionToken; import android.support.test.filters.SmallTest; +import org.chromium.base.metrics.RecordHistogram; +import org.chromium.base.test.util.MetricsUtils; import org.chromium.base.test.util.RetryOnFailure; import org.chromium.content.browser.test.NativeLibraryTestBase; @@ -27,6 +29,7 @@ loadNativeLibraryNoBrowserProcess(); RequestThrottler.purgeAllEntriesForTesting(context); mClientManager = new ClientManager(context); + RecordHistogram.initialize(); } @SmallTest @@ -88,7 +91,7 @@ @RetryOnFailure public void testPredictionOutcomeSuccess() { assertTrue(mClientManager.newSession(mSession, mUid, null, null)); - assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, URL)); + assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, URL, false)); assertEquals( ClientManager.GOOD_PREDICTION, mClientManager.getPredictionOutcome(mSession, URL)); } @@ -104,7 +107,7 @@ @SmallTest public void testPredictionOutcomeBadPrediction() { assertTrue(mClientManager.newSession(mSession, mUid, null, null)); - assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, URL)); + assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, URL, false)); assertEquals( ClientManager.BAD_PREDICTION, mClientManager.getPredictionOutcome(mSession, URL + "#fragment")); @@ -113,10 +116,80 @@ @SmallTest public void testPredictionOutcomeIgnoreFragment() { assertTrue(mClientManager.newSession(mSession, mUid, null, null)); - assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, URL)); + assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, URL, false)); mClientManager.setIgnoreFragmentsForSession(mSession, true); assertEquals( ClientManager.GOOD_PREDICTION, mClientManager.getPredictionOutcome(mSession, URL + "#fragment")); } + + @SmallTest + public void testFirstLowConfidencePredictionIsNotThrottled() { + Context context = getInstrumentation().getTargetContext().getApplicationContext(); + assertTrue(mClientManager.newSession(mSession, mUid, null, null)); + + // Two low confidence in a row is OK. + assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, null, true)); + assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, null, true)); + mClientManager.registerLaunch(mSession, URL); + + // Low -> High as well. + RequestThrottler.purgeAllEntriesForTesting(context); + assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, null, true)); + assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, URL, false)); + mClientManager.registerLaunch(mSession, URL); + + // High -> Low as well. + RequestThrottler.purgeAllEntriesForTesting(context); + assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, URL, false)); + assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, null, true)); + mClientManager.registerLaunch(mSession, URL); + } + + @SmallTest + public void testMayLaunchUrlAccounting() { + Context context = getInstrumentation().getTargetContext().getApplicationContext(); + + String name = "CustomTabs.MayLaunchUrlType"; + MetricsUtils.HistogramDelta noMayLaunchUrlDelta = + new MetricsUtils.HistogramDelta(name, ClientManager.NO_MAY_LAUNCH_URL); + MetricsUtils.HistogramDelta lowConfidenceDelta = + new MetricsUtils.HistogramDelta(name, ClientManager.LOW_CONFIDENCE); + MetricsUtils.HistogramDelta highConfidenceDelta = + new MetricsUtils.HistogramDelta(name, ClientManager.HIGH_CONFIDENCE); + MetricsUtils.HistogramDelta bothDelta = + new MetricsUtils.HistogramDelta(name, ClientManager.BOTH); + + assertTrue(mClientManager.newSession(mSession, mUid, null, null)); + + // No prediction; + mClientManager.registerLaunch(mSession, URL); + assertEquals(1, noMayLaunchUrlDelta.getDelta()); + + // Low confidence. + RequestThrottler.purgeAllEntriesForTesting(context); + assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, null, true)); + mClientManager.registerLaunch(mSession, URL); + assertEquals(1, lowConfidenceDelta.getDelta()); + + // High confidence. + RequestThrottler.purgeAllEntriesForTesting(context); + assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, URL, false)); + mClientManager.registerLaunch(mSession, URL); + assertEquals(1, highConfidenceDelta.getDelta()); + + // Low and High confidence. + RequestThrottler.purgeAllEntriesForTesting(context); + assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, URL, false)); + assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, null, true)); + mClientManager.registerLaunch(mSession, URL); + assertEquals(1, bothDelta.getDelta()); + + // Low and High confidence, same call. + RequestThrottler.purgeAllEntriesForTesting(context); + bothDelta = new MetricsUtils.HistogramDelta(name, ClientManager.BOTH); + assertTrue(mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, URL, true)); + mClientManager.registerLaunch(mSession, URL); + assertEquals(1, bothDelta.getDelta()); + } }
diff --git a/chrome/browser/browsing_data/cache_counter.cc b/chrome/browser/browsing_data/cache_counter.cc index 12373dc3..1c2f5e80 100644 --- a/chrome/browser/browsing_data/cache_counter.cc +++ b/chrome/browser/browsing_data/cache_counter.cc
@@ -30,18 +30,17 @@ } void CacheCounter::Count() { - bool is_upper_limit = !GetPeriodStart().is_null(); base::WeakPtr<browsing_data::ConditionalCacheCountingHelper> counter = browsing_data::ConditionalCacheCountingHelper::CreateForRange( content::BrowserContext::GetDefaultStoragePartition(profile_), - base::Time(), base::Time::Max()) + GetPeriodStart(), base::Time::Max()) ->CountAndDestroySelfWhenFinished( base::Bind(&CacheCounter::OnCacheSizeCalculated, - weak_ptr_factory_.GetWeakPtr(), is_upper_limit)); + weak_ptr_factory_.GetWeakPtr())); } -void CacheCounter::OnCacheSizeCalculated(bool is_upper_limit, - int64_t result_bytes) { +void CacheCounter::OnCacheSizeCalculated(int64_t result_bytes, + bool is_upper_limit) { // A value less than 0 means a net error code. if (result_bytes < 0) return;
diff --git a/chrome/browser/browsing_data/cache_counter.h b/chrome/browser/browsing_data/cache_counter.h index 76902247..3b4d4d2 100644 --- a/chrome/browser/browsing_data/cache_counter.h +++ b/chrome/browser/browsing_data/cache_counter.h
@@ -42,7 +42,7 @@ private: void Count() override; - void OnCacheSizeCalculated(bool is_upper_limit, int64_t bytes); + void OnCacheSizeCalculated(int64_t bytes, bool is_upper_limit); void FetchEstimate( base::WeakPtr<browsing_data::ConditionalCacheCountingHelper>);
diff --git a/chrome/browser/browsing_data/cache_counter_browsertest.cc b/chrome/browser/browsing_data/cache_counter_browsertest.cc index 646cf37..a70e67d 100644 --- a/chrome/browser/browsing_data/cache_counter_browsertest.cc +++ b/chrome/browser/browsing_data/cache_counter_browsertest.cc
@@ -243,10 +243,7 @@ EXPECT_EQ(0u, GetResult()); } -// Tests that the counting is restarted when the time period changes. Currently, -// the results should be the same for every period. This is because the counter -// always counts the size of the entire cache, and it is up to the UI -// to interpret it as exact value or upper bound. +// Tests that the counting is restarted when the time period changes. IN_PROC_BROWSER_TEST_F(CacheCounterTest, PeriodChanged) { CreateCacheEntry(); @@ -259,22 +256,18 @@ SetDeletionPeriodPref(browsing_data::LAST_HOUR); WaitForIOThread(); browsing_data::BrowsingDataCounter::ResultInt result = GetResult(); - EXPECT_TRUE(IsUpperLimit()); SetDeletionPeriodPref(browsing_data::LAST_DAY); WaitForIOThread(); EXPECT_EQ(result, GetResult()); - EXPECT_TRUE(IsUpperLimit()); SetDeletionPeriodPref(browsing_data::LAST_WEEK); WaitForIOThread(); EXPECT_EQ(result, GetResult()); - EXPECT_TRUE(IsUpperLimit()); SetDeletionPeriodPref(browsing_data::FOUR_WEEKS); WaitForIOThread(); EXPECT_EQ(result, GetResult()); - EXPECT_TRUE(IsUpperLimit()); SetDeletionPeriodPref(browsing_data::ALL_TIME); WaitForIOThread();
diff --git a/chrome/browser/browsing_data/conditional_cache_counting_helper_browsertest.cc b/chrome/browser/browsing_data/conditional_cache_counting_helper_browsertest.cc index 9dc94e4..24eb59b 100644 --- a/chrome/browser/browsing_data/conditional_cache_counting_helper_browsertest.cc +++ b/chrome/browser/browsing_data/conditional_cache_counting_helper_browsertest.cc
@@ -21,7 +21,7 @@ class ConditionalCacheCountingHelperBrowserTest : public InProcessBrowserTest { public: - const int64_t kTimeoutMs = 10; + const int64_t kTimeoutMs = 1000; void SetUpOnMainThread() override { count_callback_ = @@ -35,11 +35,12 @@ void TearDownOnMainThread() override { cache_util_.reset(); } - void CountCallback(int64_t size) { + void CountCallback(int64_t size, bool is_upper_limit) { // Negative values represent an unexpected error. DCHECK(size >= 0 || size == net::ERR_ABORTED); DCHECK_CURRENTLY_ON(BrowserThread::UI); last_size_ = size; + last_is_upper_limit_ = is_upper_limit; if (run_loop_) run_loop_->Quit(); @@ -64,6 +65,8 @@ return last_size_; } + int64_t IsUpperLimit() { return last_is_upper_limit_; } + int64_t GetResultOrError() { return last_size_; } CacheTestUtil* GetCacheTestUtil() { return cache_util_.get(); } @@ -74,6 +77,7 @@ std::unique_ptr<CacheTestUtil> cache_util_; int64_t last_size_; + bool last_is_upper_limit_; }; // Tests that ConditionalCacheCountingHelper only counts those cache entries @@ -86,7 +90,8 @@ GetCacheTestUtil()->CreateCacheEntries(keys1); base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(kTimeoutMs)); - // base::Time t2 = base::Time::Now(); + base::Time t2 = base::Time::Now(); + base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(kTimeoutMs)); std::set<std::string> keys2 = {"6", "7"}; GetCacheTestUtil()->CreateCacheEntries(keys2); @@ -94,8 +99,6 @@ base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(kTimeoutMs)); base::Time t3 = base::Time::Now(); - // TODO(dullweber): Add test for time ranges when GetEntrySize() is done. - // Count all entries. CountEntries(t1, t3); WaitForTasksOnIOThread(); @@ -105,4 +108,25 @@ CountEntries(base::Time(), base::Time::Max()); WaitForTasksOnIOThread(); EXPECT_EQ(size_1_3, GetResult()); + + // Count the size of the first set of entries. + CountEntries(t1, t2); + WaitForTasksOnIOThread(); + int64_t size_1_2 = GetResult(); + + // Count the size of the second set of entries. + CountEntries(t2, t3); + WaitForTasksOnIOThread(); + int64_t size_2_3 = GetResult(); + + if (IsUpperLimit()) { + EXPECT_EQ(size_1_2, size_1_3); + EXPECT_EQ(size_2_3, size_1_3); + } else { + EXPECT_GT(size_1_2, 0); + EXPECT_GT(size_2_3, 0); + EXPECT_LT(size_1_2, size_1_3); + EXPECT_LT(size_2_3, size_1_3); + EXPECT_EQ(size_1_2 + size_2_3, size_1_3); + } }
diff --git a/chrome/browser/ui/webui/options/chromeos/storage_manager_handler.cc b/chrome/browser/ui/webui/options/chromeos/storage_manager_handler.cc index a6f21af5..ba7c607 100644 --- a/chrome/browser/ui/webui/options/chromeos/storage_manager_handler.cc +++ b/chrome/browser/ui/webui/options/chromeos/storage_manager_handler.cc
@@ -305,8 +305,8 @@ content::BrowserContext::GetDefaultStoragePartition(profile), base::Time(), base::Time::Max()) ->CountAndDestroySelfWhenFinished( - base::Bind(&StorageManagerHandler::OnGetBrowsingDataSize, - weak_ptr_factory_.GetWeakPtr(), false)); + base::Bind(&StorageManagerHandler::OnGetCacheSize, + weak_ptr_factory_.GetWeakPtr())); // Fetch the size of site data in browsing data. if (!site_data_size_collector_.get()) { @@ -334,6 +334,11 @@ weak_ptr_factory_.GetWeakPtr(), true)); } +void StorageManagerHandler::OnGetCacheSize(int64_t size, bool is_upper_limit) { + DCHECK(!is_upper_limit); + OnGetBrowsingDataSize(false, size); +} + void StorageManagerHandler::OnGetBrowsingDataSize(bool is_site_data, int64_t size) { if (is_site_data) {
diff --git a/chrome/browser/ui/webui/options/chromeos/storage_manager_handler.h b/chrome/browser/ui/webui/options/chromeos/storage_manager_handler.h index a02dd36..f1785b5 100644 --- a/chrome/browser/ui/webui/options/chromeos/storage_manager_handler.h +++ b/chrome/browser/ui/webui/options/chromeos/storage_manager_handler.h
@@ -69,6 +69,9 @@ // Requests updating the size of browsing data. void UpdateBrowsingDataSize(); + // Callback to receive the cache size. + void OnGetCacheSize(int64_t size, bool is_upper_limit); + // Callback to update the UI about the size of browsing data. void OnGetBrowsingDataSize(bool is_site_data, int64_t size);
diff --git a/chrome/browser/ui/webui/settings/chromeos/device_storage_handler.cc b/chrome/browser/ui/webui/settings/chromeos/device_storage_handler.cc index b1d991a3..345cbe19 100644 --- a/chrome/browser/ui/webui/settings/chromeos/device_storage_handler.cc +++ b/chrome/browser/ui/webui/settings/chromeos/device_storage_handler.cc
@@ -237,8 +237,7 @@ content::BrowserContext::GetDefaultStoragePartition(profile), base::Time(), base::Time::Max()) ->CountAndDestroySelfWhenFinished( - base::Bind(&StorageHandler::OnGetBrowsingDataSize, - base::Unretained(this), false)); + base::Bind(&StorageHandler::OnGetCacheSize, base::Unretained(this))); // Fetch the size of site data in browsing data. if (!site_data_size_collector_.get()) { @@ -266,6 +265,11 @@ base::Unretained(this), true)); } +void StorageHandler::OnGetCacheSize(int64_t size, bool is_upper_limit) { + DCHECK(!is_upper_limit); + OnGetBrowsingDataSize(false, size); +} + void StorageHandler::OnGetBrowsingDataSize(bool is_site_data, int64_t size) { if (is_site_data) { has_browser_site_data_size_ = true;
diff --git a/chrome/browser/ui/webui/settings/chromeos/device_storage_handler.h b/chrome/browser/ui/webui/settings/chromeos/device_storage_handler.h index 8a0dabbb..e99db4a7 100644 --- a/chrome/browser/ui/webui/settings/chromeos/device_storage_handler.h +++ b/chrome/browser/ui/webui/settings/chromeos/device_storage_handler.h
@@ -66,6 +66,9 @@ // Requests updating the size of browsing data. void UpdateBrowsingDataSize(); + // Callback to receive the cache size. + void OnGetCacheSize(int64_t size, bool is_upper_limit); + // Callback to update the UI about the size of browsing data. void OnGetBrowsingDataSize(bool is_site_data, int64_t size);
diff --git a/components/autofill/content/renderer/password_form_conversion_utils.cc b/components/autofill/content/renderer/password_form_conversion_utils.cc index d41ac97f..3465657 100644 --- a/components/autofill/content/renderer/password_form_conversion_utils.cc +++ b/components/autofill/content/renderer/password_form_conversion_utils.cc
@@ -154,12 +154,16 @@ } // Helper to determine which password is the main (current) one, and which is -// the new password (e.g., on a sign-up or change password form), if any. +// the new password (e.g., on a sign-up or change password form), if any. If the +// new password is found and there is another password field with the same user +// input, the function also sets |confirmation_password| to this field. bool LocateSpecificPasswords(std::vector<WebInputElement> passwords, WebInputElement* current_password, - WebInputElement* new_password) { + WebInputElement* new_password, + WebInputElement* confirmation_password) { DCHECK(current_password && current_password->isNull()); DCHECK(new_password && new_password->isNull()); + DCHECK(confirmation_password && confirmation_password->isNull()); // First, look for elements marked with either autocomplete='current-password' // or 'new-password' -- if we find any, take the hint, and treat the first of @@ -171,6 +175,9 @@ } else if (HasAutocompleteAttributeValue(it, kAutocompleteNewPassword) && new_password->isNull()) { *new_password = it; + } else if (!new_password->isNull() && + (new_password->value() == it.value())) { + *confirmation_password = it; } } @@ -197,6 +204,7 @@ // password with a confirmation. This can be either a sign-up form or a // password change form that does not ask for the old password. *new_password = passwords[0]; + *confirmation_password = passwords[1]; } else { // Assume first is old password, second is new (no choice but to guess). // This case also includes empty passwords in order to allow filling of @@ -218,12 +226,14 @@ // with 3 password fields, in which case we will assume this layout. *current_password = passwords[0]; *new_password = passwords[1]; + *confirmation_password = passwords[2]; } else if (passwords[0].value() == passwords[1].value()) { // It is strange that the new password comes first, but trust more which // fields are duplicated than the ordering of fields. Assume that // any password fields after the new password contain sensitive // information that isn't actually a password (security hint, SSN, etc.) *new_password = passwords[0]; + *confirmation_password = passwords[1]; } else { // Three different passwords, or first and last match with middle // different. No idea which is which, so no luck. @@ -505,7 +515,9 @@ WebInputElement password; WebInputElement new_password; - if (!LocateSpecificPasswords(passwords, &password, &new_password)) + WebInputElement confirmation_password; + if (!LocateSpecificPasswords(passwords, &password, &new_password, + &confirmation_password)) return false; DCHECK_EQ(passwords.size(), last_text_input_before_password.size()); @@ -584,6 +596,10 @@ new_password.getAttribute("value") == new_password.value(); if (HasAutocompleteAttributeValue(new_password, kAutocompleteNewPassword)) password_form->new_password_marked_by_site = true; + if (!confirmation_password.isNull()) { + password_form->confirmation_password_element = + FieldName(confirmation_password, "anonymous_confirmation_password"); + } } if (username_element.isNull()) {
diff --git a/components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc b/components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc index ef86fa0..08b1988 100644 --- a/components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc +++ b/components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc
@@ -383,19 +383,20 @@ const char* expected_password_value; const char* expected_new_password_element; const char* expected_new_password_value; + const char* expected_confirmation_element; } cases[] = { // Two non-empty fields with the same value should be treated as a new // password field plus a confirmation field for the new password. - {{"alpha", "alpha"}, "", "", "password1", "alpha"}, + {{"alpha", "alpha"}, "", "", "password1", "alpha", "password2"}, // The same goes if the fields are yet empty: we speculate that we will // identify them as new password fields once they are filled out, and we // want to keep our abstract interpretation of the form less flaky. - {{"", ""}, "password1", "", "password2", ""}, + {{"", ""}, "password1", "", "password2", "", ""}, // Two different values should be treated as a password change form, one // that also asks for the current password, but only once for the new. - {{"alpha", ""}, "password1", "alpha", "password2", ""}, - {{"", "beta"}, "password1", "", "password2", "beta"}, - {{"alpha", "beta"}, "password1", "alpha", "password2", "beta"}}; + {{"alpha", ""}, "password1", "alpha", "password2", "", ""}, + {{"", "beta"}, "password1", "", "password2", "beta", ""}, + {{"alpha", "beta"}, "password1", "alpha", "password2", "beta", ""}}; for (size_t i = 0; i < arraysize(cases); ++i) { SCOPED_TRACE(testing::Message() << "Iteration " << i); @@ -420,6 +421,8 @@ password_form->new_password_element); EXPECT_EQ(base::UTF8ToUTF16(cases[i].expected_new_password_value), password_form->new_password_value); + EXPECT_EQ(base::UTF8ToUTF16(cases[i].expected_confirmation_element), + password_form->confirmation_password_element); // Do a basic sanity check that we are still selecting the right username. EXPECT_EQ(base::UTF8ToUTF16("username1"), password_form->username_element); @@ -438,24 +441,30 @@ const char* expected_password_value; const char* expected_new_password_element; const char* expected_new_password_value; + const char* expected_confirmation_element; } cases[] = { // Two fields with the same value, and one different: we should treat this // as a password change form with confirmation for the new password. Note // that we only recognize (current + new + new) and (new + new + current) // without autocomplete attributes. - {{"alpha", "", ""}, "password1", "alpha", "password2", ""}, - {{"", "beta", "beta"}, "password1", "", "password2", "beta"}, - {{"alpha", "beta", "beta"}, "password1", "alpha", "password2", "beta"}, + {{"alpha", "", ""}, "password1", "alpha", "password2", "", "password3"}, + {{"", "beta", "beta"}, "password1", "", "password2", "beta", "password3"}, + {{"alpha", "beta", "beta"}, + "password1", + "alpha", + "password2", + "beta", + "password3"}, // If confirmed password comes first, assume that the third password // field is related to security question, SSN, or credit card and ignore // it. - {{"beta", "beta", "alpha"}, "", "", "password1", "beta"}, + {{"beta", "beta", "alpha"}, "", "", "password1", "beta", "password2"}, // If the fields are yet empty, we speculate that we will identify them as // (current + new + new) once they are filled out, so we should classify // them the same for now to keep our abstract interpretation less flaky. - {{"", "", ""}, "password1", "", "password2", ""}}; - // Note: In all other cases, we give up and consider the form invalid. - // This is tested in InvalidFormDueToConfusingPasswordFields. + {{"", "", ""}, "password1", "", "password2", "", "password3"}}; + // Note: In all other cases, we give up and consider the form invalid. + // This is tested in InvalidFormDueToConfusingPasswordFields. for (size_t i = 0; i < arraysize(cases); ++i) { SCOPED_TRACE(testing::Message() << "Iteration " << i); @@ -481,6 +490,8 @@ password_form->new_password_element); EXPECT_EQ(base::UTF8ToUTF16(cases[i].expected_new_password_value), password_form->new_password_value); + EXPECT_EQ(base::UTF8ToUTF16(cases[i].expected_confirmation_element), + password_form->confirmation_password_element); // Do a basic sanity check that we are still selecting the right username. EXPECT_EQ(base::UTF8ToUTF16("username1"), password_form->username_element);
diff --git a/components/autofill/core/browser/autofill_type.cc b/components/autofill/core/browser/autofill_type.cc index 4708199..8dc9dbc 100644 --- a/components/autofill/core/browser/autofill_type.cc +++ b/components/autofill/core/browser/autofill_type.cc
@@ -126,6 +126,7 @@ case PROBABLY_NEW_PASSWORD: case NOT_NEW_PASSWORD: case PROBABLY_ACCOUNT_CREATION_PASSWORD: + case CONFIRMATION_PASSWORD: return PASSWORD_FIELD; case NO_SERVER_DATA: @@ -757,6 +758,8 @@ return "NOT_NEW_PASSWORD"; case PROBABLY_ACCOUNT_CREATION_PASSWORD: return "PROBABLY_ACCOUNT_CREATION_PASSWORD"; + case CONFIRMATION_PASSWORD: + return "CONFIRMATION_PASSWORD"; case MAX_VALID_FIELD_TYPE: return std::string();
diff --git a/components/autofill/core/browser/field_types.h b/components/autofill/core/browser/field_types.h index 7dee427..6c2aa807 100644 --- a/components/autofill/core/browser/field_types.h +++ b/components/autofill/core/browser/field_types.h
@@ -159,9 +159,13 @@ // for local heuristics. PROBABLY_ACCOUNT_CREATION_PASSWORD = 94, + // The confirmation password field in account creation or change password + // forms. + CONFIRMATION_PASSWORD = 95, + // No new types can be added without a corresponding change to the Autofill // server. - MAX_VALID_FIELD_TYPE = 95, + MAX_VALID_FIELD_TYPE = 96, }; // The list of all HTML autocomplete field type hints supported by Chrome.
diff --git a/components/autofill/core/common/password_form.h b/components/autofill/core/common/password_form.h index be74b85..74716230 100644 --- a/components/autofill/core/common/password_form.h +++ b/components/autofill/core/common/password_form.h
@@ -165,6 +165,10 @@ // element corresponding to the new password. Optional, and not persisted. base::string16 new_password_element; + // The confirmation password element. Optional, only set on form parsing, and + // not persisted. + base::string16 confirmation_password_element; + // The new password. Optional, and not persisted. base::string16 new_password_value;
diff --git a/components/browsing_data/content/conditional_cache_counting_helper.cc b/components/browsing_data/content/conditional_cache_counting_helper.cc index 94b772f14..05f4f17 100644 --- a/components/browsing_data/content/conditional_cache_counting_helper.cc +++ b/components/browsing_data/content/conditional_cache_counting_helper.cc
@@ -57,6 +57,7 @@ DCHECK(!result_callback.is_null()); result_callback_ = result_callback; calculation_result_ = 0; + is_upper_limit_ = false; BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, @@ -73,7 +74,7 @@ DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK(!is_finished_); is_finished_ = true; - result_callback_.Run(calculation_result_); + result_callback_.Run(calculation_result_, is_upper_limit_); base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this); } @@ -141,11 +142,16 @@ base::Bind(&ConditionalCacheCountingHelper::DoCountCache, base::Unretained(this))); } else { - // TODO(dullweber): Readd code for counting with timeout. - // TODO(dullweber): Implement faster counting for SimpleBackendImpl. - rv = cache_->CalculateSizeOfAllEntries( + rv = cache_->CalculateSizeOfEntriesBetween( + begin_time_, end_time_, base::Bind(&ConditionalCacheCountingHelper::DoCountCache, base::Unretained(this))); + if (rv == net::ERR_NOT_IMPLEMENTED) { + is_upper_limit_ = true; + rv = cache_->CalculateSizeOfAllEntries( + base::Bind(&ConditionalCacheCountingHelper::DoCountCache, + base::Unretained(this))); + } } cache_ = NULL; }
diff --git a/components/browsing_data/content/conditional_cache_counting_helper.h b/components/browsing_data/content/conditional_cache_counting_helper.h index d40fba6..dff721d2 100644 --- a/components/browsing_data/content/conditional_cache_counting_helper.h +++ b/components/browsing_data/content/conditional_cache_counting_helper.h
@@ -24,8 +24,9 @@ // Helper to count the size of the http cache data from a StoragePartition. class ConditionalCacheCountingHelper { public: - // Returns the number bytes in the selected range. - typedef base::Callback<void(int64_t)> CacheCountCallback; + // Returns the number bytes in the selected range and if this value is an + // upper estimate. + typedef base::Callback<void(int64_t, bool)> CacheCountCallback; static ConditionalCacheCountingHelper* CreateForRange( content::StoragePartition* storage_partition, @@ -70,6 +71,7 @@ // via a callback. This is either the sum of size of the the two cache // backends, or an error code if the calculation failed. int64_t calculation_result_; + bool is_upper_limit_; CacheCountCallback result_callback_; const base::Time begin_time_;
diff --git a/components/ntp_snippets/BUILD.gn b/components/ntp_snippets/BUILD.gn index 1bac0cf..652704b 100644 --- a/components/ntp_snippets/BUILD.gn +++ b/components/ntp_snippets/BUILD.gn
@@ -133,6 +133,7 @@ "category_rankers/click_based_category_ranker_unittest.cc", "category_rankers/constant_category_ranker_unittest.cc", "category_unittest.cc", + "content_suggestions_metrics_unittest.cc", "content_suggestions_service_unittest.cc", "offline_pages/recent_tab_suggestions_provider_unittest.cc", "physical_web_pages/physical_web_page_suggestions_provider_unittest.cc",
diff --git a/components/ntp_snippets/content_suggestions_metrics.cc b/components/ntp_snippets/content_suggestions_metrics.cc index 4f61eed..493f8c0 100644 --- a/components/ntp_snippets/content_suggestions_metrics.cc +++ b/components/ntp_snippets/content_suggestions_metrics.cc
@@ -4,10 +4,12 @@ #include "components/ntp_snippets/content_suggestions_metrics.h" +#include <cmath> #include <string> #include <type_traits> #include "base/metrics/histogram.h" +#include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_macros.h" #include "base/metrics/user_metrics.h" #include "base/strings/stringprintf.h" @@ -25,18 +27,19 @@ "NewTabPage.ContentSuggestions.CountOnNtpOpened"; const char kHistogramShown[] = "NewTabPage.ContentSuggestions.Shown"; const char kHistogramShownAge[] = "NewTabPage.ContentSuggestions.ShownAge"; -const char kHistogramShownScore[] = "NewTabPage.ContentSuggestions.ShownScore"; +const char kHistogramShownScore[] = + "NewTabPage.ContentSuggestions.ShownScoreNormalized"; const char kHistogramOpened[] = "NewTabPage.ContentSuggestions.Opened"; const char kHistogramOpenedAge[] = "NewTabPage.ContentSuggestions.OpenedAge"; const char kHistogramOpenedScore[] = - "NewTabPage.ContentSuggestions.OpenedScore"; + "NewTabPage.ContentSuggestions.OpenedScoreNormalized"; const char kHistogramOpenDisposition[] = "NewTabPage.ContentSuggestions.OpenDisposition"; const char kHistogramMenuOpened[] = "NewTabPage.ContentSuggestions.MenuOpened"; const char kHistogramMenuOpenedAge[] = "NewTabPage.ContentSuggestions.MenuOpenedAge"; const char kHistogramMenuOpenedScore[] = - "NewTabPage.ContentSuggestions.MenuOpenedScore"; + "NewTabPage.ContentSuggestions.MenuOpenedScoreNormalized"; const char kHistogramDismissedUnvisited[] = "NewTabPage.ContentSuggestions.DismissedUnvisited"; const char kHistogramDismissedVisited[] = @@ -60,7 +63,7 @@ // and contains exactly the values to be recorded in UMA. Don't remove or // reorder elements, only add new ones at the end (before COUNT), and keep in // sync with ContentSuggestionsCategory in histograms.xml. -enum class HistogramCategories { +enum HistogramCategories { EXPERIMENTAL, RECENT_TABS, DOWNLOADS, @@ -135,27 +138,6 @@ GetCategorySuffix(category).c_str()); } -// This corresponds to UMA_HISTOGRAM_ENUMERATION, for use with dynamic histogram -// names. -void UmaHistogramEnumeration(const std::string& name, - int value, - int boundary_value) { - base::LinearHistogram::FactoryGet( - name, 1, boundary_value, boundary_value + 1, - base::HistogramBase::kUmaTargetedHistogramFlag) - ->Add(value); -} - -// This corresponds to UMA_HISTOGRAM_LONG_TIMES for use with dynamic histogram -// names. -void UmaHistogramLongTimes(const std::string& name, - const base::TimeDelta& value) { - base::Histogram::FactoryTimeGet( - name, base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromHours(1), - 50, base::HistogramBase::kUmaTargetedHistogramFlag) - ->AddTime(value); -} - // This corresponds to UMA_HISTOGRAM_CUSTOM_TIMES (with min/max appropriate // for the age of suggestions) for use with dynamic histogram names. void UmaHistogramAge(const std::string& name, const base::TimeDelta& value) { @@ -165,29 +147,13 @@ ->AddTime(value); } -// This corresponds to UMA_HISTOGRAM_CUSTOM_COUNTS (with min/max appropriate -// for the score of suggestions) for use with dynamic histogram names. -void UmaHistogramScore(const std::string& name, float value) { - base::Histogram::FactoryGet(name, 1, 100000, 50, - base::HistogramBase::kUmaTargetedHistogramFlag) - ->Add(value); -} - -void LogCategoryHistogramEnumeration(const char* base_name, - Category category, - int value, - int boundary_value) { +void LogCategoryHistogramPosition(const char* base_name, + Category category, + int position, + int max_position) { std::string name = GetCategoryHistogramName(base_name, category); // Since the histogram name is dynamic, we can't use the regular macro. - UmaHistogramEnumeration(name, value, boundary_value); -} - -void LogCategoryHistogramLongTimes(const char* base_name, - Category category, - const base::TimeDelta& value) { - std::string name = GetCategoryHistogramName(base_name, category); - // Since the histogram name is dynamic, we can't use the regular macro. - UmaHistogramLongTimes(name, value); + base::UmaHistogramExactLinear(name, position, max_position); } void LogCategoryHistogramAge(const char* base_name, @@ -202,8 +168,12 @@ Category category, float score) { std::string name = GetCategoryHistogramName(base_name, category); - // Since the histogram name is dynamic, we can't use the regular macro. - UmaHistogramScore(name, score); + // Scores are typically reported in a range of (0,1]. As UMA does not support + // floats, we put them on a discrete scale of [1,10]. We keep the extra bucket + // 11 for unexpected over-flows as we want to distinguish them from scores + // close to 1. For instance, the discrete value 1 represents score values + // within (0.0, 0.1]. + base::UmaHistogramExactLinear(name, ceil(score * 10), 11); } // Records ContentSuggestions usage. Therefore the day is sliced into 20min @@ -222,10 +192,9 @@ std::string histogram_name( base::StringPrintf("%s.%s", kHistogramArticlesUsageTimeLocal, kWeekdayNames[now_exploded.day_of_week])); - UmaHistogramEnumeration(histogram_name, bucket, kNumBuckets); - - UMA_HISTOGRAM_ENUMERATION(kHistogramArticlesUsageTimeLocal, bucket, - kNumBuckets); + base::UmaHistogramExactLinear(histogram_name, bucket, kNumBuckets); + UMA_HISTOGRAM_EXACT_LINEAR(kHistogramArticlesUsageTimeLocal, bucket, + kNumBuckets); base::RecordAction( base::UserMetricsAction("NewTabPage_ContentSuggestions_ArticlesUsage")); @@ -237,13 +206,12 @@ const std::vector<std::pair<Category, int>>& suggestions_per_category) { int suggestions_total = 0; for (const std::pair<Category, int>& item : suggestions_per_category) { - LogCategoryHistogramEnumeration(kHistogramCountOnNtpOpened, item.first, - item.second, kMaxSuggestionsPerCategory); + LogCategoryHistogramPosition(kHistogramCountOnNtpOpened, item.first, + item.second, kMaxSuggestionsPerCategory); suggestions_total += item.second; } - - UMA_HISTOGRAM_ENUMERATION(kHistogramCountOnNtpOpened, suggestions_total, - kMaxSuggestionsTotal); + UMA_HISTOGRAM_EXACT_LINEAR(kHistogramCountOnNtpOpened, suggestions_total, + kMaxSuggestionsTotal); } void OnSuggestionShown(int global_position, @@ -252,10 +220,10 @@ base::Time publish_date, base::Time last_background_fetch_time, float score) { - UMA_HISTOGRAM_ENUMERATION(kHistogramShown, global_position, - kMaxSuggestionsTotal); - LogCategoryHistogramEnumeration(kHistogramShown, category, category_position, - kMaxSuggestionsPerCategory); + UMA_HISTOGRAM_EXACT_LINEAR(kHistogramShown, global_position, + kMaxSuggestionsTotal); + LogCategoryHistogramPosition(kHistogramShown, category, category_position, + kMaxSuggestionsPerCategory); base::TimeDelta age = base::Time::Now() - publish_date; LogCategoryHistogramAge(kHistogramShownAge, category, age); @@ -286,21 +254,24 @@ base::Time publish_date, float score, WindowOpenDisposition disposition) { - UMA_HISTOGRAM_ENUMERATION(kHistogramOpened, global_position, - kMaxSuggestionsTotal); - LogCategoryHistogramEnumeration(kHistogramOpened, category, category_position, - kMaxSuggestionsPerCategory); + UMA_HISTOGRAM_EXACT_LINEAR(kHistogramOpened, global_position, + kMaxSuggestionsTotal); + LogCategoryHistogramPosition(kHistogramOpened, category, category_position, + kMaxSuggestionsPerCategory); base::TimeDelta age = base::Time::Now() - publish_date; LogCategoryHistogramAge(kHistogramOpenedAge, category, age); LogCategoryHistogramScore(kHistogramOpenedScore, category, score); - UMA_HISTOGRAM_ENUMERATION( + // We use WindowOpenDisposition::MAX_VALUE + 1 for |value_max| since MAX_VALUE + // itself is a valid (and used) enum value. + UMA_HISTOGRAM_EXACT_LINEAR( kHistogramOpenDisposition, static_cast<int>(disposition), static_cast<int>(WindowOpenDisposition::MAX_VALUE) + 1); - LogCategoryHistogramEnumeration( - kHistogramOpenDisposition, category, static_cast<int>(disposition), + base::UmaHistogramExactLinear( + GetCategoryHistogramName(kHistogramOpenDisposition, category), + static_cast<int>(disposition), static_cast<int>(WindowOpenDisposition::MAX_VALUE) + 1); if (category.IsKnownCategory(KnownCategories::ARTICLES)) { @@ -313,11 +284,10 @@ int category_position, base::Time publish_date, float score) { - UMA_HISTOGRAM_ENUMERATION(kHistogramMenuOpened, global_position, - kMaxSuggestionsTotal); - LogCategoryHistogramEnumeration(kHistogramMenuOpened, category, - category_position, - kMaxSuggestionsPerCategory); + UMA_HISTOGRAM_EXACT_LINEAR(kHistogramMenuOpened, global_position, + kMaxSuggestionsTotal); + LogCategoryHistogramPosition(kHistogramMenuOpened, category, + category_position, kMaxSuggestionsPerCategory); base::TimeDelta age = base::Time::Now() - publish_date; LogCategoryHistogramAge(kHistogramMenuOpenedAge, category, age); @@ -330,42 +300,42 @@ int category_position, bool visited) { if (visited) { - UMA_HISTOGRAM_ENUMERATION(kHistogramDismissedVisited, global_position, - kMaxSuggestionsTotal); - LogCategoryHistogramEnumeration(kHistogramDismissedVisited, category, - category_position, - kMaxSuggestionsPerCategory); + UMA_HISTOGRAM_EXACT_LINEAR(kHistogramDismissedVisited, global_position, + kMaxSuggestionsTotal); + LogCategoryHistogramPosition(kHistogramDismissedVisited, category, + category_position, kMaxSuggestionsPerCategory); } else { - UMA_HISTOGRAM_ENUMERATION(kHistogramDismissedUnvisited, global_position, - kMaxSuggestionsTotal); - LogCategoryHistogramEnumeration(kHistogramDismissedUnvisited, category, - category_position, - kMaxSuggestionsPerCategory); + UMA_HISTOGRAM_EXACT_LINEAR(kHistogramDismissedUnvisited, global_position, + kMaxSuggestionsTotal); + LogCategoryHistogramPosition(kHistogramDismissedUnvisited, category, + category_position, kMaxSuggestionsPerCategory); } } void OnSuggestionTargetVisited(Category category, base::TimeDelta visit_time) { - LogCategoryHistogramLongTimes(kHistogramVisitDuration, category, visit_time); + std::string name = + GetCategoryHistogramName(kHistogramVisitDuration, category); + base::UmaHistogramLongTimes(name, visit_time); } void OnMoreButtonShown(Category category, int position) { // The "more" card can appear in addition to the actual suggestions, so add // one extra bucket to this histogram. - LogCategoryHistogramEnumeration(kHistogramMoreButtonShown, category, position, - kMaxSuggestionsPerCategory + 1); + LogCategoryHistogramPosition(kHistogramMoreButtonShown, category, position, + kMaxSuggestionsPerCategory + 1); } void OnMoreButtonClicked(Category category, int position) { // The "more" card can appear in addition to the actual suggestions, so add // one extra bucket to this histogram. - LogCategoryHistogramEnumeration(kHistogramMoreButtonClicked, category, - position, kMaxSuggestionsPerCategory + 1); + LogCategoryHistogramPosition(kHistogramMoreButtonClicked, category, position, + kMaxSuggestionsPerCategory + 1); } void OnCategoryDismissed(Category category) { UMA_HISTOGRAM_ENUMERATION(kHistogramCategoryDismissed, - static_cast<int>(GetHistogramCategory(category)), - static_cast<int>(HistogramCategories::COUNT)); + GetHistogramCategory(category), + HistogramCategories::COUNT); } } // namespace metrics
diff --git a/components/ntp_snippets/content_suggestions_metrics_unittest.cc b/components/ntp_snippets/content_suggestions_metrics_unittest.cc new file mode 100644 index 0000000..2424d50 --- /dev/null +++ b/components/ntp_snippets/content_suggestions_metrics_unittest.cc
@@ -0,0 +1,52 @@ +// 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 "components/ntp_snippets/content_suggestions_metrics.h" + +#include "base/test/histogram_tester.h" +#include "base/time/time.h" +#include "components/ntp_snippets/category.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace ntp_snippets { +namespace metrics { +namespace { + +using testing::ElementsAre; + +TEST(ContentSuggestionsMetricsTest, ShouldLogOnSuggestionsShown) { + base::HistogramTester histogram_tester; + OnSuggestionShown(/*global_position=*/1, + Category::FromKnownCategory(KnownCategories::ARTICLES), + /*category_position=*/3, + base::Time::Now(), + base::Time::Now() - base::TimeDelta::FromHours(2), + 0.01f); + // Test corner cases for score. + OnSuggestionShown(/*global_position=*/1, + Category::FromKnownCategory(KnownCategories::ARTICLES), + /*category_position=*/3, base::Time::Now(), + base::Time::Now() - base::TimeDelta::FromHours(2), 0.0f); + OnSuggestionShown(/*global_position=*/1, + Category::FromKnownCategory(KnownCategories::ARTICLES), + /*category_position=*/3, base::Time::Now(), + base::Time::Now() - base::TimeDelta::FromHours(2), 1.0f); + OnSuggestionShown(/*global_position=*/1, + Category::FromKnownCategory(KnownCategories::ARTICLES), + /*category_position=*/3, base::Time::Now(), + base::Time::Now() - base::TimeDelta::FromHours(2), 8.0f); + + EXPECT_THAT( + histogram_tester.GetAllSamples( + "NewTabPage.ContentSuggestions.ShownScoreNormalized.Articles"), + ElementsAre(base::Bucket(/*min=*/0, /*count=*/1), + base::Bucket(/*min=*/1, /*count=*/1), + base::Bucket(/*min=*/10, /*count=*/1), + base::Bucket(/*min=*/11, /*count=*/1))); +} + +} // namespace +} // namespace metrics +} // namespace ntp_snippets
diff --git a/components/password_manager/core/browser/password_form_manager.cc b/components/password_manager/core/browser/password_form_manager.cc index 2278bff..51e3f1e 100644 --- a/components/password_manager/core/browser/password_form_manager.cc +++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -133,6 +133,67 @@ } } +// Sets autofill types of password and new password fields in |field_types|. +// |password_type| (the autofill type of new password field) should be equal to +// NEW_PASSWORD, PROBABLY_NEW_PASSWORD or NOT_NEW_PASSWORD. These values +// correspond to cases when the user confirmed password update, did nothing or +// declined to update password respectively. +void SetFieldLabelsOnUpdate(const autofill::ServerFieldType password_type, + const autofill::PasswordForm& submitted_form, + FieldTypeMap* field_types) { + DCHECK(password_type == autofill::NEW_PASSWORD || + password_type == autofill::PROBABLY_NEW_PASSWORD || + password_type == autofill::NOT_NEW_PASSWORD) + << password_type; + DCHECK(!submitted_form.new_password_element.empty()); + + (*field_types)[submitted_form.password_element] = autofill::PASSWORD; + (*field_types)[submitted_form.new_password_element] = password_type; +} + +// Sets the autofill type of the password field stored in |submitted_form| to +// |password_type| in |field_types| map. +void SetFieldLabelsOnSave(const autofill::ServerFieldType password_type, + const autofill::PasswordForm& submitted_form, + FieldTypeMap* field_types) { + DCHECK(password_type == autofill::PASSWORD || + password_type == autofill::PROBABLY_ACCOUNT_CREATION_PASSWORD || + password_type == autofill::ACCOUNT_CREATION_PASSWORD || + password_type == autofill::NOT_ACCOUNT_CREATION_PASSWORD) + << password_type; + + if (!submitted_form.new_password_element.empty()) { + (*field_types)[submitted_form.new_password_element] = password_type; + } else { + DCHECK(!submitted_form.password_element.empty()); + (*field_types)[submitted_form.password_element] = password_type; + } +} + +// Label username and password fields with autofill types in |form_structure| +// based on |field_types|. The function also adds the types to +// |available_field_types|. +void LabelFields(const FieldTypeMap& field_types, + FormStructure* form_structure, + autofill::ServerFieldTypeSet* available_field_types) { + for (size_t i = 0; i < form_structure->field_count(); ++i) { + autofill::AutofillField* field = form_structure->field(i); + if (field->name.empty()) + continue; + + autofill::ServerFieldType type = autofill::UNKNOWN_TYPE; + auto iter = field_types.find(field->name); + if (iter != field_types.end()) { + type = iter->second; + available_field_types->insert(type); + } + + autofill::ServerFieldTypeSet types; + types.insert(type); + field->set_possible_types(types); + } +} + } // namespace PasswordFormManager::PasswordFormManager( @@ -354,7 +415,7 @@ const autofill::PasswordForm& credentials_to_update) { if (observed_form_.IsPossibleChangePasswordForm()) { FormStructure form_structure(credentials_to_update.form_data); - UploadPasswordVote(base::string16(), autofill::NEW_PASSWORD, + UploadPasswordVote(autofill::NEW_PASSWORD, form_structure.FormSignatureAsStr()); } base::string16 password_to_save = pending_credentials_.password_value; @@ -627,8 +688,7 @@ // in cases where we currently save the wrong username isn't great. // TODO(gcasto): Determine if generation should be offered in this case. if (pending->times_used == 1 && selected_username_.empty()) { - if (UploadPasswordVote(pending->username_element, - autofill::ACCOUNT_CREATION_PASSWORD, + if (UploadPasswordVote(autofill::ACCOUNT_CREATION_PASSWORD, observed_structure.FormSignatureAsStr())) { pending->generation_upload_status = autofill::PasswordForm::POSITIVE_SIGNAL_SENT; @@ -639,8 +699,7 @@ // A signal was sent that this was an account creation form, but the // credential is now being used on the same form again. This cancels out // the previous vote. - if (UploadPasswordVote(base::string16(), - autofill::NOT_ACCOUNT_CREATION_PASSWORD, + if (UploadPasswordVote(autofill::NOT_ACCOUNT_CREATION_PASSWORD, std::string())) { pending->generation_upload_status = autofill::PasswordForm::NEGATIVE_SIGNAL_SENT; @@ -648,12 +707,11 @@ } else if (generation_popup_was_shown_) { // Even if there is no autofill vote to be sent, send the vote about the // usage of the generation popup. - UploadPasswordVote(base::string16(), autofill::UNKNOWN_TYPE, std::string()); + UploadPasswordVote(autofill::UNKNOWN_TYPE, std::string()); } } bool PasswordFormManager::UploadPasswordVote( - const base::string16& username_field, const autofill::ServerFieldType& password_type, const std::string& login_form_signature) { // Check if there is any vote to be sent. @@ -683,16 +741,22 @@ autofill::ServerFieldTypeSet available_field_types; if (has_autofill_vote) { + // A map from field names to field types. + FieldTypeMap field_types; + DCHECK(submitted_form_); if (is_update) { - if (!submitted_form_ || submitted_form_->new_password_element.empty()) + if (submitted_form_->new_password_element.empty()) return false; - SetAutofillTypesOnUpdate(password_type, &form_structure, - &available_field_types); - + SetFieldLabelsOnUpdate(password_type, *submitted_form_, &field_types); } else { - SetAutofillTypesOnSave(username_field, password_type, &form_structure, - &available_field_types); + SetFieldLabelsOnSave(password_type, *submitted_form_, &field_types); + if (password_type == autofill::ACCOUNT_CREATION_PASSWORD) { + field_types[pending_credentials_.username_element] = autofill::USERNAME; + } } + field_types[submitted_form_->confirmation_password_element] = + autofill::CONFIRMATION_PASSWORD; + LabelFields(field_types, &form_structure, &available_field_types); } if (generation_popup_was_shown_) @@ -717,98 +781,6 @@ return success; } -void PasswordFormManager::SetAutofillTypesOnUpdate( - const autofill::ServerFieldType password_type, - FormStructure* form_structure, - autofill::ServerFieldTypeSet* available_field_types) { - DCHECK(password_type == autofill::NEW_PASSWORD || - password_type == autofill::PROBABLY_NEW_PASSWORD || - password_type == autofill::NOT_NEW_PASSWORD) - << password_type; - DCHECK(!submitted_form_->new_password_element.empty()); - - // Create a map from field names to field types. - std::map<base::string16, autofill::ServerFieldType> field_types; - if (!submitted_form_->username_element.empty()) { - field_types[submitted_form_->username_element] = autofill::USERNAME; - } - if (!submitted_form_->password_element.empty()) { - field_types[submitted_form_->password_element] = autofill::PASSWORD; - } - field_types[submitted_form_->new_password_element] = password_type; - - // Find all password fields after |new_password_element| and set their type to - // |password_type|. They are considered to be confirmation fields. - const autofill::FormData& form_data = observed_form_.form_data; - bool is_new_password_field_found = false; - for (size_t i = 0; i < form_data.fields.size(); ++i) { - const autofill::FormFieldData& field = form_data.fields[i]; - if (field.form_control_type != "password") - continue; - if (is_new_password_field_found) { - field_types[field.name] = password_type; - // We don't care about password fields after a confirmation field. - break; - } else if (field.name == submitted_form_->new_password_element) { - is_new_password_field_found = true; - } - } - DCHECK(is_new_password_field_found); - - for (size_t i = 0; i < form_structure->field_count(); ++i) { - autofill::AutofillField* field = form_structure->field(i); - autofill::ServerFieldType type = autofill::UNKNOWN_TYPE; - auto iter = field_types.find(field->name); - if (iter != field_types.end()) { - type = iter->second; - available_field_types->insert(type); - } - - autofill::ServerFieldTypeSet types; - types.insert(type); - field->set_possible_types(types); - } -} - -void PasswordFormManager::SetAutofillTypesOnSave( - const base::string16& username_field, - const autofill::ServerFieldType password_type, - FormStructure* form_structure, - autofill::ServerFieldTypeSet* available_field_types) { - DCHECK(password_type == autofill::PASSWORD || - password_type == autofill::PROBABLY_ACCOUNT_CREATION_PASSWORD || - password_type == autofill::ACCOUNT_CREATION_PASSWORD || - password_type == autofill::NOT_ACCOUNT_CREATION_PASSWORD) - << password_type; - - // Find the first password field to label. If the provided username field - // name is not empty, then also find the first field with that name to label. - // We don't try to label anything else. - bool found_password_field = false; - bool should_find_username_field = !username_field.empty(); - for (size_t i = 0; i < form_structure->field_count(); ++i) { - autofill::AutofillField* field = form_structure->field(i); - - autofill::ServerFieldType type = autofill::UNKNOWN_TYPE; - if (!found_password_field && field->form_control_type == "password") { - type = password_type; - found_password_field = true; - } else if (should_find_username_field && field->name == username_field) { - type = autofill::USERNAME; - should_find_username_field = false; - } - - autofill::ServerFieldTypeSet types; - types.insert(type); - field->set_possible_types(types); - } - DCHECK(found_password_field); - DCHECK(!should_find_username_field); - - available_field_types->insert(password_type); - available_field_types->insert(autofill::USERNAME); -} - void PasswordFormManager::AddGeneratedVote( autofill::FormStructure* form_structure) { DCHECK(form_structure); @@ -1164,23 +1136,19 @@ } void PasswordFormManager::OnNopeUpdateClicked() { - UploadPasswordVote(base::string16(), autofill::NOT_NEW_PASSWORD, - std::string()); + UploadPasswordVote(autofill::NOT_NEW_PASSWORD, std::string()); } void PasswordFormManager::OnNeverClicked() { - UploadPasswordVote(pending_credentials_.username_element, - autofill::UNKNOWN_TYPE, std::string()); + UploadPasswordVote(autofill::UNKNOWN_TYPE, std::string()); PermanentlyBlacklist(); } void PasswordFormManager::OnNoInteraction(bool is_update) { if (is_update) - UploadPasswordVote(base::string16(), autofill::PROBABLY_NEW_PASSWORD, - std::string()); + UploadPasswordVote(autofill::PROBABLY_NEW_PASSWORD, std::string()); else { - UploadPasswordVote(pending_credentials_.username_element, - autofill::UNKNOWN_TYPE, std::string()); + UploadPasswordVote(autofill::UNKNOWN_TYPE, std::string()); } } @@ -1235,13 +1203,10 @@ // Credentials that have been previously used (e.g., PSL matches) are checked // to see if they are valid account creation forms. if (pending_credentials_.times_used == 0) { - base::string16 username_field; autofill::ServerFieldType password_type = autofill::PASSWORD; - if (does_look_like_signup_form_) { - username_field = pending_credentials_.username_element; + if (does_look_like_signup_form_) password_type = autofill::PROBABLY_ACCOUNT_CREATION_PASSWORD; - } - UploadPasswordVote(username_field, password_type, std::string()); + UploadPasswordVote(password_type, std::string()); } else SendAutofillVotes(observed_form_, &pending_credentials_); }
diff --git a/components/password_manager/core/browser/password_form_manager.h b/components/password_manager/core/browser/password_form_manager.h index 6f599b6..7b312eb 100644 --- a/components/password_manager/core/browser/password_form_manager.h +++ b/components/password_manager/core/browser/password_form_manager.h
@@ -33,6 +33,9 @@ class PasswordManager; class PasswordManagerClient; +// A map from field names to field types. +using FieldTypeMap = std::map<base::string16, autofill::ServerFieldType>; + // This class helps with filling the observed form (both HTML and from HTTP // auth) and with saving/updating the stored information about it. class PasswordFormManager : public FormFetcher::Consumer { @@ -369,8 +372,7 @@ // Tries to set all votes (e.g. autofill field types, generation vote) to // a |FormStructure| and upload it to the server. Returns true on success. - bool UploadPasswordVote(const base::string16& username_field, - const autofill::ServerFieldType& password_type, + bool UploadPasswordVote(const autofill::ServerFieldType& password_type, const std::string& login_form_signature); // Adds a vote on password generation usage to |form_structure|. @@ -422,26 +424,6 @@ base::Optional<autofill::PasswordForm> UpdatePendingAndGetOldKey( std::vector<autofill::PasswordForm>* credentials_to_update); - // Sets autofill types of username, password and new password fields in - // |form_structure|. |password_type| (the autofill type of new password field) - // should be equal to NEW_PASSWORD, PROBABLY_NEW_PASSWORD or NOT_NEW_PASSWORD. - // These values correspond to cases when the user confirmed password update, - // did nothing or declined to update password respectively. The function also - // add the types to |available_field_types|. - void SetAutofillTypesOnUpdate( - const autofill::ServerFieldType password_type, - FormStructure* form_structure, - autofill::ServerFieldTypeSet* available_field_types); - - // Sets autofill types of username and password fields in |form_structure|. - // |username_field| determines the name of the username field. The function - // also add the types to |available_field_types|. - void SetAutofillTypesOnSave( - const base::string16& username_field, - const autofill::ServerFieldType password_type, - FormStructure* form_structure, - autofill::ServerFieldTypeSet* available_field_types); - // Set of nonblacklisted PasswordForms from the DB that best match the form // being managed by |this|, indexed by username. They are owned by // |form_fetcher_|.
diff --git a/components/password_manager/core/browser/password_form_manager_unittest.cc b/components/password_manager/core/browser/password_form_manager_unittest.cc index 4db36b9..a91c1822 100644 --- a/components/password_manager/core/browser/password_form_manager_unittest.cc +++ b/components/password_manager/core/browser/password_form_manager_unittest.cc
@@ -112,33 +112,28 @@ expected_types, "Unexpected autofill types or form signature") { if (form_signature != arg.FormSignatureAsStr()) { - // An unexpected form is uploaded. + // Unexpected form's signature. + ADD_FAILURE() << "Expected form signature is " << form_signature + << ", but found " << arg.FormSignatureAsStr(); return false; } for (const auto& field : arg) { - if (expected_types.find(field->name) == expected_types.end()) { - if (!field->possible_types().empty()) { - // Unexpected types are uploaded. - return false; - } - } else { - if (field->possible_types().size() > 1) { - // Currently we expect not more than 1 type per field. - return false; - } + if (field->possible_types().size() > 1) { + ADD_FAILURE() << field->name << " field has several possible types"; + return false; + } - if (field->possible_types().empty()) { - if (expected_types.find(field->name) != expected_types.end()) { - // A vote is expected but not found. - return false; - } - } else { - if (expected_types.find(field->name)->second != - *field->possible_types().begin()) { - // An unexpected field type is found. - return false; - } - } + autofill::ServerFieldType expected_vote = + expected_types.find(field->name) == expected_types.end() + ? autofill::NO_SERVER_DATA + : expected_types.find(field->name)->second; + autofill::ServerFieldType actual_vote = + field->possible_types().empty() ? autofill::NO_SERVER_DATA + : *field->possible_types().begin(); + if (expected_vote != actual_vote) { + ADD_FAILURE() << field->name << " field: expected vote " << expected_vote + << ", but found " << actual_vote; + return false; } } return true; @@ -395,13 +390,6 @@ form_to_save.username_value = match.username_value; form_to_save.password_value = match.password_value; - // When we're voting for an account creation form, we should also vote - // for its username field. - base::string16 username_vote = - (field_type && *field_type == autofill::ACCOUNT_CREATION_PASSWORD) - ? match.username_element - : base::string16(); - fetcher.SetNonFederated({&match}, 0u); std::string expected_login_signature; autofill::FormStructure observed_structure(observed_form_data); @@ -412,11 +400,18 @@ expected_login_signature = observed_structure.FormSignatureAsStr(); } autofill::ServerFieldTypeSet expected_available_field_types; - expected_available_field_types.insert(autofill::USERNAME); - std::map<base::string16, autofill::ServerFieldType> expected_types; + FieldTypeMap expected_types; expected_types[ASCIIToUTF16("full_name")] = autofill::UNKNOWN_TYPE; - expected_types[saved_match()->username_element] = - username_vote.empty() ? autofill::UNKNOWN_TYPE : autofill::USERNAME; + + // When we're voting for an account creation form, we should also vote + // for its username field. + if (field_type && *field_type == autofill::ACCOUNT_CREATION_PASSWORD) { + expected_types[match.username_element] = autofill::USERNAME; + expected_available_field_types.insert(autofill::USERNAME); + } else { + expected_types[match.username_element] = autofill::UNKNOWN_TYPE; + } + if (field_type) { expected_available_field_types.insert(*field_type); expected_types[saved_match()->password_element] = *field_type; @@ -442,20 +437,34 @@ form_manager.ProvisionallySave( form_to_save, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); form_manager.Save(); + Mock::VerifyAndClearExpectations( + client()->mock_driver()->mock_autofill_download_manager()); } // Test upload votes on change password forms. |field_type| is a vote that we // expect to be uploaded. - void ChangePasswordUploadTest(autofill::ServerFieldType field_type) { + void ChangePasswordUploadTest(autofill::ServerFieldType field_type, + bool has_confirmation_field) { + SCOPED_TRACE(testing::Message() + << "field_type=" << field_type + << " has_confirmation_field=" << has_confirmation_field); + // |observed_form_| should have |form_data| in order to be uploaded. observed_form()->form_data = saved_match()->form_data; // Turn |observed_form_| and into change password form. observed_form()->new_password_element = ASCIIToUTF16("NewPasswd"); + observed_form()->confirmation_password_element = ASCIIToUTF16("ConfPwd"); autofill::FormFieldData field; field.label = ASCIIToUTF16("NewPasswd"); field.name = ASCIIToUTF16("NewPasswd"); field.form_control_type = "password"; observed_form()->form_data.fields.push_back(field); + if (has_confirmation_field) { + field.label = ASCIIToUTF16("ConfPwd"); + field.name = ASCIIToUTF16("ConfPwd"); + field.form_control_type = "password"; + observed_form()->form_data.fields.push_back(field); + } FakeFormFetcher fetcher; fetcher.Fetch(); @@ -489,14 +498,18 @@ std::map<base::string16, autofill::ServerFieldType> expected_types; expected_types[ASCIIToUTF16("full_name")] = autofill::UNKNOWN_TYPE; - expected_types[observed_form_.username_element] = autofill::USERNAME; + expected_types[observed_form_.username_element] = autofill::UNKNOWN_TYPE; expected_types[observed_form_.password_element] = autofill::PASSWORD; expected_types[observed_form_.new_password_element] = field_type; autofill::ServerFieldTypeSet expected_available_field_types; - expected_available_field_types.insert(autofill::USERNAME); expected_available_field_types.insert(autofill::PASSWORD); expected_available_field_types.insert(field_type); + if (has_confirmation_field) { + expected_types[observed_form_.confirmation_password_element] = + autofill::CONFIRMATION_PASSWORD; + expected_available_field_types.insert(autofill::CONFIRMATION_PASSWORD); + } std::string observed_form_signature = autofill::FormStructure(observed_form()->form_data) @@ -526,6 +539,8 @@ default: NOTREACHED(); } + Mock::VerifyAndClearExpectations( + client()->mock_driver()->mock_autofill_download_manager()); } autofill::AutofillUploadContents::Field::PasswordGenerationType @@ -602,10 +617,8 @@ autofill::ServerFieldTypeSet expected_available_field_types; // Don't send autofill votes if the user didn't press "Save" button. - if (interaction == SAVE) { - expected_available_field_types.insert(autofill::USERNAME); + if (interaction == SAVE) expected_available_field_types.insert(autofill::PASSWORD); - } form_manager.set_is_manual_generation(is_manual_generation); base::string16 generation_element = is_change_password_form @@ -647,6 +660,8 @@ form_manager.OnNoInteraction(false /* not an update prompt*/); break; } + Mock::VerifyAndClearExpectations( + client()->mock_driver()->mock_autofill_download_manager()); } PasswordForm* observed_form() { return &observed_form_; } @@ -1747,7 +1762,6 @@ form_to_save.password_value = ASCIIToUTF16("1234"); autofill::ServerFieldTypeSet expected_available_field_types; - expected_available_field_types.insert(autofill::USERNAME); expected_available_field_types.insert(autofill::PASSWORD); EXPECT_CALL( *client()->mock_driver()->mock_autofill_download_manager(), @@ -1776,27 +1790,27 @@ } TEST_F(PasswordFormManagerTest, UploadPasswordForm) { - autofill::FormData form_data; + autofill::FormData observed_form_data; autofill::FormFieldData field; field.label = ASCIIToUTF16("Email"); - field.name = ASCIIToUTF16("Email"); + field.name = ASCIIToUTF16("observed-username-field"); field.form_control_type = "text"; - form_data.fields.push_back(field); + observed_form_data.fields.push_back(field); field.label = ASCIIToUTF16("password"); field.name = ASCIIToUTF16("password"); field.form_control_type = "password"; - form_data.fields.push_back(field); + observed_form_data.fields.push_back(field); // Form data is different than saved form data, account creation signal should // be sent. autofill::ServerFieldType field_type = autofill::ACCOUNT_CREATION_PASSWORD; - AccountCreationUploadTest(form_data, 0, PasswordForm::NO_SIGNAL_SENT, + AccountCreationUploadTest(observed_form_data, 0, PasswordForm::NO_SIGNAL_SENT, &field_type); // Non-zero times used will not upload since we only upload a positive signal // at most once. - AccountCreationUploadTest(form_data, 1, PasswordForm::NO_SIGNAL_SENT, + AccountCreationUploadTest(observed_form_data, 1, PasswordForm::NO_SIGNAL_SENT, nullptr); // Same form data as saved match and POSITIVE_SIGNAL_SENT means there should @@ -2213,17 +2227,15 @@ // when PasswordFormManager::Save is called, then PasswordFormManager also // calls PasswordManager::UpdateFormManagers. -TEST_F(PasswordFormManagerTest, UploadChangePasswordForm_NEW_PASSWORD) { - ChangePasswordUploadTest(autofill::NEW_PASSWORD); -} - -TEST_F(PasswordFormManagerTest, - UploadChangePasswordForm_PROBABLY_NEW_PASSWORD) { - ChangePasswordUploadTest(autofill::PROBABLY_NEW_PASSWORD); -} - -TEST_F(PasswordFormManagerTest, UploadChangePasswordForm_NOT_NEW_PASSWORD) { - ChangePasswordUploadTest(autofill::NOT_NEW_PASSWORD); +TEST_F(PasswordFormManagerTest, UploadChangePasswordForm) { + autofill::ServerFieldType kChangePasswordVotes[] = { + autofill::NEW_PASSWORD, autofill::PROBABLY_NEW_PASSWORD, + autofill::NOT_NEW_PASSWORD}; + bool kFalseTrue[] = {false, true}; + for (autofill::ServerFieldType vote : kChangePasswordVotes) { + for (bool has_confirmation_field : kFalseTrue) + ChangePasswordUploadTest(vote, has_confirmation_field); + } } TEST_F(PasswordFormManagerTest, TestUpdatePSLMatchedCredentials) { @@ -2755,8 +2767,7 @@ autofill::ServerFieldTypeSet expected_available_field_types; std::map<base::string16, autofill::ServerFieldType> expected_types; expected_types[ASCIIToUTF16("full_name")] = autofill::UNKNOWN_TYPE; - expected_available_field_types.insert(autofill::USERNAME); - expected_types[saved_match()->username_element] = autofill::USERNAME; + expected_types[saved_match()->username_element] = autofill::UNKNOWN_TYPE; expected_available_field_types.insert( autofill::PROBABLY_ACCOUNT_CREATION_PASSWORD); expected_types[saved_match()->password_element] =
diff --git a/ios/chrome/browser/browser_state/test_chrome_browser_state.mm b/ios/chrome/browser/browser_state/test_chrome_browser_state.mm index c38e532e..4c4e4d8 100644 --- a/ios/chrome/browser/browser_state/test_chrome_browser_state.mm +++ b/ios/chrome/browser/browser_state/test_chrome_browser_state.mm
@@ -178,8 +178,6 @@ // Normally this would happen during browser startup, but for tests we need to // trigger creation of BrowserState-related services. EnsureBrowserStateKeyedServiceFactoriesBuilt(); - if (ios::GetChromeBrowserProvider()) - ios::GetChromeBrowserProvider()->AssertBrowserContextKeyedFactoriesBuilt(); if (prefs_) { // If user passed a custom PrefServiceSyncable, then leave |testing_prefs_|
diff --git a/ios/chrome/browser/ios_chrome_main_parts.mm b/ios/chrome/browser/ios_chrome_main_parts.mm index 4ba0f50..1bc03fd 100644 --- a/ios/chrome/browser/ios_chrome_main_parts.mm +++ b/ios/chrome/browser/ios_chrome_main_parts.mm
@@ -156,7 +156,6 @@ // Ensure that the browser state is initialized. EnsureBrowserStateKeyedServiceFactoriesBuilt(); - ios::GetChromeBrowserProvider()->AssertBrowserContextKeyedFactoriesBuilt(); ios::ChromeBrowserStateManager* browser_state_manager = application_context_->GetChromeBrowserStateManager(); ios::ChromeBrowserState* last_used_browser_state =
diff --git a/ios/public/provider/chrome/browser/chrome_browser_provider.h b/ios/public/provider/chrome/browser/chrome_browser_provider.h index 3e98961..dc7b9992 100644 --- a/ios/public/provider/chrome/browser/chrome_browser_provider.h +++ b/ios/public/provider/chrome/browser/chrome_browser_provider.h
@@ -67,8 +67,6 @@ // This is called after web startup. virtual void Initialize() const; - // Asserts all iOS-specific |BrowserContextKeyedServiceFactory| are built. - virtual void AssertBrowserContextKeyedFactoriesBuilt(); // Returns an instance of a signing error provider. virtual SigninErrorProvider* GetSigninErrorProvider(); // Returns an instance of a signin resources provider.
diff --git a/ios/public/provider/chrome/browser/chrome_browser_provider.mm b/ios/public/provider/chrome/browser/chrome_browser_provider.mm index 7cb146b..4b04c6a 100644 --- a/ios/public/provider/chrome/browser/chrome_browser_provider.mm +++ b/ios/public/provider/chrome/browser/chrome_browser_provider.mm
@@ -31,8 +31,6 @@ void ChromeBrowserProvider::Initialize() const {} -void ChromeBrowserProvider::AssertBrowserContextKeyedFactoriesBuilt() {} - SigninErrorProvider* ChromeBrowserProvider::GetSigninErrorProvider() { return nullptr; }
diff --git a/ios/web/BUILD.gn b/ios/web/BUILD.gn index 99f2e59..f1bbde1 100644 --- a/ios/web/BUILD.gn +++ b/ios/web/BUILD.gn
@@ -72,14 +72,6 @@ "net/cert_host_pair.h", "net/cert_policy.cc", "net/certificate_policy_cache.cc", - "net/clients/crw_js_injection_network_client.h", - "net/clients/crw_js_injection_network_client.mm", - "net/clients/crw_js_injection_network_client_factory.h", - "net/clients/crw_js_injection_network_client_factory.mm", - "net/clients/crw_redirect_network_client.h", - "net/clients/crw_redirect_network_client.mm", - "net/clients/crw_redirect_network_client_factory.h", - "net/clients/crw_redirect_network_client_factory.mm", "net/cookie_notification_bridge.h", "net/cookie_notification_bridge.mm", "net/crw_cert_verification_controller.h", @@ -178,6 +170,8 @@ "web_state/blocked_popup_info.h", "web_state/blocked_popup_info.mm", "web_state/context_menu_params.mm", + "web_state/context_menu_params_utils.h", + "web_state/context_menu_params_utils.mm", "web_state/credential.cc", "web_state/crw_pass_kit_downloader.h", "web_state/crw_pass_kit_downloader.mm", @@ -202,6 +196,8 @@ "web_state/js/page_script_util.mm", "web_state/page_viewport_state.h", "web_state/page_viewport_state.mm", + "web_state/ui/crw_context_menu_controller.h", + "web_state/ui/crw_context_menu_controller.mm", "web_state/ui/crw_generic_content_view.mm", "web_state/ui/crw_swipe_recognizer_provider.h", "web_state/ui/crw_touch_tracking_recognizer.h", @@ -270,8 +266,6 @@ ] sources = [ - "web_state/ui/crw_context_menu_controller.h", - "web_state/ui/crw_context_menu_controller.mm", "web_state/ui/crw_web_controller.h", "web_state/ui/crw_web_controller.mm", "web_state/ui/crw_web_controller_container_view.h", @@ -498,6 +492,7 @@ ":web", "//base", "//base/test:test_support", + "//components/url_formatter", "//ios/net", "//ios/testing:ocmock_support", "//ios/web/public/image_fetcher:unit_tests", @@ -521,7 +516,6 @@ "navigation/nscoder_util_unittest.mm", "net/cert_host_pair_unittest.cc", "net/cert_policy_unittest.cc", - "net/clients/crw_js_injection_network_client_unittest.mm", "net/crw_cert_verification_controller_unittest.mm", "net/crw_ssl_status_updater_unittest.mm", "net/request_group_util_unittest.mm", @@ -533,6 +527,7 @@ "public/web_state/page_viewport_state_unittest.mm", "url_scheme_util_unittest.mm", "url_util_unittest.cc", + "web_state/context_menu_params_utils_unittest.mm", "web_state/crw_pass_kit_downloader_unittest.mm", "web_state/crw_web_view_scroll_view_proxy_unittest.mm", "web_state/error_translation_util_unittest.mm",
diff --git a/ios/web/net/clients/DEPS b/ios/web/net/clients/DEPS deleted file mode 100644 index d47151bf..0000000 --- a/ios/web/net/clients/DEPS +++ /dev/null
@@ -1,3 +0,0 @@ -include_rules = [ - "+ios/third_party/blink", -]
diff --git a/ios/web/net/clients/crw_js_injection_network_client.h b/ios/web/net/clients/crw_js_injection_network_client.h deleted file mode 100644 index f241cafc..0000000 --- a/ios/web/net/clients/crw_js_injection_network_client.h +++ /dev/null
@@ -1,33 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef IOS_WEB_NET_CLIENTS_CRW_JS_INJECTION_NETWORK_CLIENT_H_ -#define IOS_WEB_NET_CLIENTS_CRW_JS_INJECTION_NETWORK_CLIENT_H_ - -#import "ios/net/clients/crn_forwarding_network_client.h" - -namespace web { -// Used for UMA histogram and must be kept in sync with the histograms.xml file. -enum class InjectionResult : int { - SUCCESS_INJECTED, - FAIL_FIND_INJECTION_LOCATION, - FAIL_INSUFFICIENT_CONTENT_LENGTH, - FAIL_SELF_REFERRER, - - INJECTION_RESULT_COUNT - // INJECTION_RESULT_COUNT must always be the last element in this enum -}; -} // namespace web - -// Network client that injects a script tag into HTML and XHTML documents. -@interface CRWJSInjectionNetworkClient : CRNForwardingNetworkClient - -// Returns YES if |response| has a "Content-Type" header approriate for -// injection. At this time this means if the "Content-Type" header is HTML -// or XHTML. -+ (BOOL)canHandleResponse:(NSURLResponse*)response; - -@end - -#endif // IOS_WEB_NET_CLIENTS_CRW_JS_INJECTION_NETWORK_CLIENT_H_
diff --git a/ios/web/net/clients/crw_js_injection_network_client.mm b/ios/web/net/clients/crw_js_injection_network_client.mm deleted file mode 100644 index ebb635d..0000000 --- a/ios/web/net/clients/crw_js_injection_network_client.mm +++ /dev/null
@@ -1,639 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "ios/web/net/clients/crw_js_injection_network_client.h" - -#include <stddef.h> -#include <stdint.h> - -#include "base/logging.h" -#import "base/mac/scoped_nsobject.h" -#include "base/metrics/histogram.h" -#import "ios/net/crn_http_url_response.h" -#import "ios/third_party/blink/src/html_tokenizer.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -// CRWJSInjectionNetworkClient injects an external script tag reference for -// crweb.js into HTML and XHTML documents. To do this correctly, three data -// points are needed: where to inject the script tag, what encoding the content -// is in (ASCII compatible, UTF32, UTF16 and big or little endian) and the byte -// length of the injection tag itself. -// -// The content encoding is handled first. As data is received, -// CRWJSInjectionNetworkClient will look at the beginning few bytes of the -// content for either a byte-order mark or an XML declaration. If present, they -// will be matched to a known set of patterns to determine the encoding. If not -// present, by definition, the content is in an ASCII compatible encoding (at -// least with regard to the markup tags themselves, which is all -// CRWJSInjectionNetworkClient cares about). -// -// Next, CRWJSInjectionNetworkClient will look for the byte offset at which to -// inject. For HTML and XHTML documents to work correctly, the script tag must -// be injected after the "<html>" tag (right after the '>' character). To -// determine the byte offset of this character, blink's HTMLTokenizer is used. -// Specifically, as data is received from the network, -// CRWJSInjectionNetworkClient will buffer the data until a sufficient amount of -// data is collected to scan for the "<html>" tag. This scan is repeated, as -// data is received, until one of three things happens: 1) an "<html>" tag is -// found, 2) some tag OTHER than an "<html>" tag is found, or 3) no tag at all -// is found within the first 1024 bytes of the content. -// -// For case 3) above, nothing is injected and the NSURLHTTPResponse as well as -// all associated data are immediately dispatched in unaltered form to their -// intended recipient (WebKit). All subsequently received data is simply passed -// along to its destination unaltered. -// -// For cases 1) & 2) above, the "Content-Length" header of the NSURLHTTPResponse -// object will be updated as appropriate (since NSURLHTTPResponse is unmutable, -// this means a copy is made) and then dispatched. Immediately after this, the -// data is dispatched in three chunks: -// - Everything before the injection byte offset. -// - The appropriately content encoded <script> tag string. -// - All remaining data. -// -// All subsequently received data is simply passed along to its destination -// unaltered. -// - -namespace { - -// When looking for the meta-charset tag, blink/webkit -// limits itself to the first 1024 bytes. -const size_t kLimitOfBytesToCheckForHeadTag = 1024; -const size_t kMinimumBytesNeededForHTMLTag = 7; -const size_t kMinimumBytesNeededForBOM = 4; -const size_t kMinimumBytesNeededForXMLDecl = 8; - -NSString* const kContentLength = @"Content-Length"; - -NSString* const kJSContentTemplate = - @"<script src=\"%@_crweb.js\" charset=\"utf-8\"></script>"; - -// Compares a value pointed to by |bytes| to a single value |byte|. Returns -// true if they are equal, false otherwise. This is a base case for the -// variadic template version of the method of the same name below. -template <typename T1, typename T2> -bool BytesEqual(const T1* bytes, T2 byte) { - DCHECK(bytes); - return (*bytes == byte); -} - -// Compares an array of values |bytes| to a value |byte| and a variable number -// of other values specified by |args|. Returns true if the values pointed to -// by |bytes| are equal to values |byte| through |args| in positional order, -// false otherwise. This means that if true is returned, values bytes[0...N] and -// byte...args are all equal. -template <typename T1, typename T2, typename... Arguments> -bool BytesEqual(const T1* bytes, T2 byte, Arguments... args) { - DCHECK(bytes); - if (*bytes == byte) - return BytesEqual(++bytes, args...); - - return false; -} - -// Gets the value corresponding to the key "Content-Length" in the passed -// |allHeaders| dictionary. If no such key is present in |allHeaders| returns -// -1. -long long GetContentLengthFromAllHeaders(NSDictionary* all_headers) { - NSObject* content_length_object = [all_headers objectForKey:kContentLength]; - - // This handles both the case when the object is an NSNumber and the case - // when the object is an NSString. - if ([content_length_object respondsToSelector:@selector(longLongValue)]) - return [static_cast<id>(content_length_object) longLongValue]; - - return -1; -} - -// Returns an CRNHTTPURLResponse instance with the "Content-Length" -// increased to include the passed |additionContentSize|. If -// |additionContentSize| is zero, the passed response is returned. -CRNHTTPURLResponse* ResponseWithUpdatedContentSize( - CRNHTTPURLResponse* response, - NSUInteger addition_content_size) { - if (!response) - return nil; - - if (!addition_content_size) - return response; - - if (![response isKindOfClass:[CRNHTTPURLResponse class]]) { - NOTREACHED(); - return response; - } - - // NSURLResponse uses a long long return type for expectedContentLength. - NSDictionary* all_headers = [response allHeaderFields]; - long long content_length = GetContentLengthFromAllHeaders(all_headers); - if (content_length < 0) { - return response; - } - - // Create a new content length value. - content_length += addition_content_size; - NSString* content_length_value = - [[NSNumber numberWithLongLong:content_length] stringValue]; - - base::scoped_nsobject<NSMutableDictionary> all_headers_mutable; - all_headers_mutable.reset([all_headers mutableCopy]); - [all_headers_mutable setObject:content_length_value forKey:kContentLength]; - - CRNHTTPURLResponse* update_response = - [[CRNHTTPURLResponse alloc] initWithURL:[response URL] - statusCode:[response statusCode] - HTTPVersion:[response cr_HTTPVersion] - headerFields:all_headers_mutable]; - - return update_response; -} -} // namespace - -@interface CRWJSInjectionNetworkClient () { - // The CRNHTTPURLResponse that is held until it is determined if the content - // will have JavaScript injected. - base::scoped_nsobject<CRNHTTPURLResponse> _pendingResponse; - - // An array of data that is buffered until a determination is made about - // injecting JavaScript. - base::scoped_nsobject<NSMutableArray> _pendingData; - - // The content that will be injected in NSData form. - base::scoped_nsobject<NSData> _jsInjectionContent; - - BOOL _completedByteOrderMarkCheck; - BOOL _completedXMLDeclarationCheck; - BOOL _completedCheckForWhereToInject; - - NSStringEncoding _contentEncoding; - NSUInteger _headerLength; - NSUInteger _pendingDataLength; - - NSUInteger _byteOffsetAtWhichToInject; - BOOL _proceedWithInjection; -} - -// Returns YES if all checks (BOM, XML declaration, injection location) are -// complete. -- (BOOL)completedAllChecks; - -// Records a UMA histogram indicating the injection result -// (see enum InjectionResult). -- (void)recordHistogramResult; - -// Returns an NSData containing the correctly encoded script tag referencing -// the appropriate crweb.js for this web view. -- (NSData*)jsInjectionContent; - -// If injection is appropriate for this content, dispatches buffered data in -// the form: -// 1) [everything PRIOR to injection byte offset] -// 2) [self jsInjectionContent] -// 3) [everything AFTER the injection byte offset] -- (void)sendInjectedResponseIfNeeded; - -// Dispatches the buffered and appropriately "Content-Length"-header-updated -// NSURLHTTPResponse to the next level network client (almost certainly -// WebKit/CFNetwork itself). -- (void)sendPendingResponse; - -// Calls [self sendInjectedResponseIfNeeded] and then sends any remaining -// buffered data to the next level network client. -- (void)sendPendingData; - -// Coalesces data contained in self.pendingData until a single NSData object of -// at least length |lengthNeeded| has been created and inserted into -// self.pendingData as the first element. -- (void)coalesceDataIfNeeded:(NSUInteger)lengthNeeded; - -// Looks for a byte order mark in the content to indicate character encoding. -// Sets _completedByteOrderMarkCheck to YES once complete and updates -// _contentEncoding if an encoding has been determined. -- (void)checkForByteOrderMark; - -// Looks for an XML declaration in the content to indicate character encoding. -// Sets _completedXMLDeclarationCheck to YES once complete and updates -// _contentEncoding if an encoding has been determined. -- (void)checkForXMLDeclaration; - -// Checks whether the hard byte limit specified by -// kLimitOfBytesToCheckForHeadTag has been hit or exceeded during the check -// for an appropriate injection location. If the limit is hit or exceeded, -// _completedCheckForWhereToInject will be set to YES and -// -[CRWJSInjectionNetworkClient checkForWhereToInject will stop. -- (void)checkIfByteLimitPassed:(const WebCore::CharacterProvider&)provider; - -// Checks for an appropriate byte offset at which to inject the external -// script tag. Sets _completedCheckForWhereToInject to YES once complete and -// updates _byteOffsetAtWhichToInject to the location at which to inject. Since -// this may be set to zero, _proceedWithInjection is set to YES to indicate that -// a location at which to inject has been found. Uses blink's HTMLTokenizer. -- (void)checkForWhereToInject; -@end - -@implementation CRWJSInjectionNetworkClient - -+ (BOOL)canHandleResponse:(NSURLResponse*)response { - NSString* scheme = [[response URL] scheme]; - if (![scheme isEqualToString:@"http"] && ![scheme isEqualToString:@"https"]) - return NO; - - NSString* mimeType = [response MIMEType]; - - if ([mimeType isEqualToString:@"text/html"]) - return YES; - - if ([mimeType isEqualToString:@"application/xhtml+xml"]) - return YES; - - return NO; -} - -#pragma mark CRNNetworkClientProtocol implementation - -- (void)didFailWithNSErrorCode:(NSInteger)nsErrorCode - netErrorCode:(int)netErrorCode { - _proceedWithInjection = NO; - [self sendPendingResponse]; - [self sendPendingData]; - [super didFailWithNSErrorCode:nsErrorCode netErrorCode:netErrorCode]; -} - -- (void)didLoadData:(NSData*)data { - if ([self completedAllChecks]) { - [super didLoadData:data]; - return; - } - - if (!_pendingData) - _pendingData.reset([[NSMutableArray alloc] init]); - - [_pendingData addObject:data]; - _pendingDataLength += [data length]; - - [self checkForByteOrderMark]; - [self checkForXMLDeclaration]; - [self checkForWhereToInject]; - - if ([self completedAllChecks]) { - if (!_contentEncoding) - _contentEncoding = NSASCIIStringEncoding; - [self sendPendingResponse]; - [self sendPendingData]; - } -} - -- (void)didReceiveResponse:(NSURLResponse*)response { - DCHECK([response isKindOfClass:[CRNHTTPURLResponse class]]); - DCHECK([response expectedContentLength] == - GetContentLengthFromAllHeaders( - [static_cast<NSHTTPURLResponse*>(response) allHeaderFields])); - DCHECK([CRWJSInjectionNetworkClient canHandleResponse:response]); - - // If response does not come with Content-Length header field (i.e. the - // Transfer-Encoding header field has value 'chunked'), response does not have - // to be updated. - if ([response expectedContentLength] == -1) { - [super didReceiveResponse:response]; - } else { - // Client calls [super didReceiveResponse:] in sendPendingResponse. - _pendingResponse.reset(static_cast<CRNHTTPURLResponse*>(response)); - } -} - -- (void)didFinishLoading { - [self recordHistogramResult]; - [self sendPendingResponse]; - [self sendPendingData]; - [super didFinishLoading]; -} - -#pragma mark Internal methods - -- (BOOL)completedAllChecks { - if (!_completedByteOrderMarkCheck) - return NO; - - if (!_completedXMLDeclarationCheck) - return NO; - - if (!_completedCheckForWhereToInject) - return NO; - - return YES; -} - -- (void)recordHistogramResult { - web::InjectionResult result = web::InjectionResult::INJECTION_RESULT_COUNT; - if (_proceedWithInjection) - result = web::InjectionResult::SUCCESS_INJECTED; - else if (_pendingDataLength < kMinimumBytesNeededForHTMLTag) - result = web::InjectionResult::FAIL_INSUFFICIENT_CONTENT_LENGTH; - else - result = web::InjectionResult::FAIL_FIND_INJECTION_LOCATION; - - if (result < web::InjectionResult::INJECTION_RESULT_COUNT) { - UMA_HISTOGRAM_ENUMERATION( - "NetworkLayerJSInjection.Result", static_cast<int>(result), - static_cast<int>(web::InjectionResult::INJECTION_RESULT_COUNT)); - } else { - NOTREACHED(); - } -} - -- (NSData*)jsInjectionContent { - DCHECK(_proceedWithInjection); - if (_jsInjectionContent) - return _jsInjectionContent; - - NSString* jsContentString = [NSString - stringWithFormat:kJSContentTemplate, [[NSUUID UUID] UUIDString]]; - _jsInjectionContent.reset( - [jsContentString dataUsingEncoding:_contentEncoding]); - - return _jsInjectionContent; -} - -- (void)sendInjectedResponseIfNeeded { - if (!_proceedWithInjection) - return; - - NSData* firstData = [_pendingData firstObject]; - NSUInteger dataLength = [firstData length]; - if (!dataLength) - return; - - const uint8_t* bytes = reinterpret_cast<const uint8_t*>([firstData bytes]); - - // Construct one data in which to send the content + injected script tag. - base::scoped_nsobject<NSMutableData> combined([[NSMutableData alloc] init]); - if (_byteOffsetAtWhichToInject) - [combined appendBytes:static_cast<const void*>(bytes) - length:_byteOffsetAtWhichToInject]; - - // Send back the JavaScript content to inject. - [combined appendData:[self jsInjectionContent]]; - [combined - appendBytes:static_cast<const void*>(bytes + _byteOffsetAtWhichToInject) - length:(dataLength - _byteOffsetAtWhichToInject)]; - - [super didLoadData:combined]; - - // The first data, into which the JS was injected, is no longer needed. - [_pendingData.get() removeObjectAtIndex:0]; - _jsInjectionContent.reset(); -} - -- (void)sendPendingResponse { - if (!_pendingResponse) - return; - - if (_proceedWithInjection) { - NSUInteger additionalLength = [[self jsInjectionContent] length]; - CRNHTTPURLResponse* responseToSend = - ResponseWithUpdatedContentSize(_pendingResponse, additionalLength); - _pendingResponse.reset(responseToSend); - } - - [super didReceiveResponse:_pendingResponse]; - _pendingResponse.reset(); -} - -- (void)sendPendingData { - if (![_pendingData count]) - return; - - [self sendInjectedResponseIfNeeded]; - - for (NSData* data in _pendingData.get()) - [super didLoadData:data]; - - _pendingData.reset(); -} - -- (void)coalesceDataIfNeeded:(NSUInteger)lengthNeeded { - NSData* firstData = [_pendingData firstObject]; - - // Obviously if the first data object has enough data, - // nothing needs to be done. - if ([firstData length] >= lengthNeeded) - return; - - // Make sure we have something that can be coalesced. - if ([_pendingData count] < 2) - return; - - // Start with a mutable copy of the first data item. - base::scoped_nsobject<NSMutableData> coalescedData([firstData mutableCopy]); - - // Replace the first item in the array. - [_pendingData removeObjectAtIndex:0]; - - // While not enough data has been coalesced and there are items - // that can be coalesced, do so. - while ([coalescedData length] < lengthNeeded && [_pendingData count]) { - [coalescedData appendData:_pendingData[0]]; - [_pendingData removeObjectAtIndex:0]; - } - - // Finally, put the coalesced object at the front of the pending data array. - [_pendingData insertObject:coalescedData atIndex:0]; -} - -- (void)checkForByteOrderMark { - // Bail if the check has already been completed. - if (_completedByteOrderMarkCheck) - return; - - // Bail if there is not yet enough data to check. - if (_pendingDataLength < kMinimumBytesNeededForBOM) - return; - - // It's possible that the server returned data in small chunks, so coalesce - // the data into one buffer if needed. - [self coalesceDataIfNeeded:kMinimumBytesNeededForBOM]; - - // Get some data to investigate. - NSData* firstData = [_pendingData firstObject]; - if (!firstData && [firstData length] < kMinimumBytesNeededForBOM) { - NOTREACHED(); - return; - } - - // Do the same check that WebKit does for the byte order mark (BOM), which - // must be right at the beginning of the content to be accepted. - // Info on byte order mark: http://en.wikipedia.org/wiki/Byte_order_mark - const uint8_t* bytes = reinterpret_cast<const uint8_t*>([firstData bytes]); - if (BytesEqual(bytes, 0xFF, 0xFE)) { - bytes += 2; - - if (!BytesEqual(bytes, 0x00, 0x00)) { - _contentEncoding = NSUTF16LittleEndianStringEncoding; - _headerLength += 2; - } else { - _contentEncoding = NSUTF32LittleEndianStringEncoding; - _headerLength += 4; - } - } else if (BytesEqual(bytes, 0xEF, 0xBB, 0xBF)) { - _contentEncoding = NSUTF8StringEncoding; - _headerLength += 3; - } else if (BytesEqual(bytes, 0xFE, 0xFF)) { - _contentEncoding = NSUTF16BigEndianStringEncoding; - _headerLength += 2; - } else if (BytesEqual(bytes, 0x00, 0x00, 0xFE, 0xFF)) { - _contentEncoding = NSUTF32BigEndianStringEncoding; - _headerLength += 4; - } - - _completedByteOrderMarkCheck = YES; -} - -- (void)checkForXMLDeclaration { - // WebKit interprets the byte order mark (BOM) as the truth and ignores and - // subsequent encodings. Thus if a BOM has already been found, there is no - // need to check for an XML declaration. - if (_headerLength) - _completedXMLDeclarationCheck = YES; - - // Bail if we have completed this. - if (_completedXMLDeclarationCheck) - return; - - // Do we already know the encoding? - if (_contentEncoding) { - _completedXMLDeclarationCheck = YES; - return; - } - - // Bail if there is not yet enough data to check. - if (_pendingDataLength < kMinimumBytesNeededForXMLDecl) - return; - - // It's possible that the server returned data in small chunks, so coalesce - // the data into one buffer if needed. - [self coalesceDataIfNeeded:kMinimumBytesNeededForXMLDecl]; - - // Get some data to investigate. - NSData* firstData = [_pendingData firstObject]; - if (!firstData && [firstData length] < kMinimumBytesNeededForXMLDecl) { - NOTREACHED(); - return; - } - - // Do the same check that WebKit does for an XML declaration. The XML spec - // is not exactly clear about what, if anything, can appear before an XML - // declaration. Can there be white space? Can there be comments? WebKit only - // accepts XML declarations if they are right at the beginning of the content. - const uint8_t* bytes = reinterpret_cast<const uint8_t*>([firstData bytes]); - if (BytesEqual(bytes, '<', '?', 'x', 'm', 'l')) { - _contentEncoding = NSISOLatin1StringEncoding; - } else if (BytesEqual(bytes, '<', 0, '?', 0, 'x', 0)) { - _contentEncoding = NSUTF16LittleEndianStringEncoding; - } else if (BytesEqual(bytes, 0, '<', 0, '?', 0, 'x')) { - _contentEncoding = NSUTF16BigEndianStringEncoding; - } else if (BytesEqual(bytes, '<', 0, 0, 0, '?', 0, 0, 0)) { - _contentEncoding = NSUTF32LittleEndianStringEncoding; - } else if (BytesEqual(bytes, 0, 0, 0, '<', 0, 0, 0, '?')) { - _contentEncoding = NSUTF32BigEndianStringEncoding; - } - - _completedXMLDeclarationCheck = YES; -} - -- (void)checkIfByteLimitPassed:(const WebCore::CharacterProvider&)provider { - // Put a hard limit on the number of characters we check before doing - // an injection. - if (provider.bytesProvided() >= kLimitOfBytesToCheckForHeadTag) - _completedCheckForWhereToInject = YES; -} - -- (void)checkForWhereToInject { - // Bail if the check has already been completed. - if (_completedCheckForWhereToInject) - return; - - // Bail if there is not yet enough data to check. We need a complete "<html " - // or a complete "<html>" - if (_pendingDataLength < kMinimumBytesNeededForHTMLTag) - return; - - // It's possible that the server returned data in small chunks, so coalesce - // the data into one buffer if needed. Try pooling everything into one chunk - // that meets the limit of what we look at. - [self coalesceDataIfNeeded:kLimitOfBytesToCheckForHeadTag]; - - // Get some data to investigate. Of course we need at least - // kMinimumBytesNeededForHTMLTag bytes to do the investigation. - NSData* firstData = [_pendingData firstObject]; - DCHECK([firstData length] >= kMinimumBytesNeededForHTMLTag); - - const uint8_t* bytes8 = reinterpret_cast<const uint8_t*>([firstData bytes]); - - WebCore::CharacterProvider provider; - switch (_contentEncoding) { - case NSUTF16BigEndianStringEncoding: - case NSUTF16LittleEndianStringEncoding: - case NSUTF32BigEndianStringEncoding: - case NSUTF32LittleEndianStringEncoding: { - const uint16_t* bytes16 = - reinterpret_cast<const uint16_t*>(bytes8 + _headerLength); - - provider.setContents(bytes16, [firstData length] - _headerLength); - - if (_contentEncoding == NSUTF16LittleEndianStringEncoding || - _contentEncoding == NSUTF32LittleEndianStringEncoding) - provider.setLittleEndian(); - break; - } - - default: { - provider.setContents(bytes8 + _headerLength, - [firstData length] - _headerLength); - break; - } - } - - WebCore::HTMLTokenizer tokenizer; - WebCore::HTMLToken token; - while(!_completedCheckForWhereToInject && - tokenizer.nextToken(provider, token)) { - WebCore::HTMLToken::Type tokenType = token.type(); - switch (tokenType) { - case WebCore::HTMLToken::StartTag: { - DEFINE_STATIC_LOCAL_STRING(htmlTag, "html"); - - // If any start tag is seen, the check is complete. - _proceedWithInjection = YES; - _completedCheckForWhereToInject = YES; - - // We always have to account for any BOM header when injecting. - _byteOffsetAtWhichToInject = _headerLength; - - if (token.nameEquals(htmlTag, htmlTagLength)) { - // There is an html tag, so the insertion needs - // to happen after this start tag. - _byteOffsetAtWhichToInject += provider.bytesProvided(); - } - - break; - } - - default: { - [self checkIfByteLimitPassed:provider]; - token.clear(); - break; - } - } - } - - // There is an early exit case right at the end of the start-tag from the - // WebCore::HTMLTokenizer::nextToken(), so double check to see if we hit - // the limit. - [self checkIfByteLimitPassed:provider]; -} - -@end
diff --git a/ios/web/net/clients/crw_js_injection_network_client_factory.h b/ios/web/net/clients/crw_js_injection_network_client_factory.h deleted file mode 100644 index 6452625..0000000 --- a/ios/web/net/clients/crw_js_injection_network_client_factory.h +++ /dev/null
@@ -1,16 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef IOS_WEB_NET_CLIENTS_CRW_JS_INJECTION_NETWORK_CLIENT_FACTORY_H_ -#define IOS_WEB_NET_CLIENTS_CRW_JS_INJECTION_NETWORK_CLIENT_FACTORY_H_ - -#import "ios/net/clients/crn_forwarding_network_client_factory.h" - -// Factory that creates CRWJSInjectionNetworkClient instances -@interface CRWJSInjectionNetworkClientFactory - : CRNForwardingNetworkClientFactory - -@end - -#endif // IOS_WEB_NET_CLIENTS_CRW_JS_INJECTION_NETWORK_CLIENT_FACTORY_H_
diff --git a/ios/web/net/clients/crw_js_injection_network_client_factory.mm b/ios/web/net/clients/crw_js_injection_network_client_factory.mm deleted file mode 100644 index afb89ef2..0000000 --- a/ios/web/net/clients/crw_js_injection_network_client_factory.mm +++ /dev/null
@@ -1,45 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "ios/web/net/clients/crw_js_injection_network_client_factory.h" - -#include "base/metrics/histogram.h" -#import "ios/web/net/clients/crw_js_injection_network_client.h" -#include "net/url_request/url_request.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -@implementation CRWJSInjectionNetworkClientFactory - -- (CRWJSInjectionNetworkClient*) - clientHandlingResponse:(NSURLResponse*)response - request:(const net::URLRequest&)request { - // Only add the JS Injection client to requests if response can be handled. - // Only injects if the the request URL is the main frame's URL to weed out - // subframes and resource requests. - if ([CRWJSInjectionNetworkClient canHandleResponse:response] && - (request.url() == request.first_party_for_cookies())) { - // Heuristic to avoid injection on XHR to the origin url. This will cause - // some false negatives, so record UMA to monitor how many pages this. - // JS injection for these false negatives will be handled by polling-based - // injection. - if (GURL(request.referrer()) == request.url()) { - UMA_HISTOGRAM_ENUMERATION( - "NetworkLayerJSInjection.Result", - static_cast<int>(web::InjectionResult::FAIL_SELF_REFERRER), - static_cast<int>(web::InjectionResult::INJECTION_RESULT_COUNT)); - return nil; - } - return [[CRWJSInjectionNetworkClient alloc] init]; - } - return nil; -} - -- (Class)clientClass { - return [CRWJSInjectionNetworkClient class]; -} - -@end
diff --git a/ios/web/net/clients/crw_js_injection_network_client_unittest.mm b/ios/web/net/clients/crw_js_injection_network_client_unittest.mm deleted file mode 100644 index 8fe2424..0000000 --- a/ios/web/net/clients/crw_js_injection_network_client_unittest.mm +++ /dev/null
@@ -1,288 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "ios/web/net/clients/crw_js_injection_network_client.h" - -#import <Foundation/Foundation.h> - -#include "base/files/file_path.h" -#include "base/files/file_util.h" -#import "base/mac/scoped_nsobject.h" -#include "base/path_service.h" -#include "base/strings/sys_string_conversions.h" -#import "ios/net/clients/crn_network_client_protocol.h" -#import "ios/net/crn_http_url_response.h" -#include "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" -#import "third_party/ocmock/OCMock/OCMock.h" -#import "third_party/ocmock/gtest_support.h" - -namespace { - -// Returns an NSString filled with the char 'a' of length |length|. -NSString* GetLongString(NSUInteger length) { - base::scoped_nsobject<NSMutableData> data( - [[NSMutableData alloc] initWithLength:length]); - memset([data mutableBytes], 'a', length); - NSString* long_string = - [[NSString alloc] initWithData:data - encoding:NSASCIIStringEncoding]; - return [long_string autorelease]; -} - -} - -// Class to serve as underlying client for JS injection client to expose -// data and responses that are passed on from the JS injection client. -@interface UnderlyingClient : CRNForwardingNetworkClient { - base::scoped_nsobject<NSMutableData> _loadedData; - base::scoped_nsobject<NSURLResponse> _receivedResponse; -} -// Returns all data loaded by the client. -- (NSData*)loadedData; -// Returns response received by the client. -- (NSURLResponse*)receivedResponse; -@end - -@implementation UnderlyingClient - -- (instancetype)init { - if ((self = [super init])) { - _loadedData.reset([[NSMutableData alloc] init]); - } - return self; -} - -- (NSData*)loadedData { - return _loadedData.get(); -} - -- (NSURLResponse*)receivedResponse { - return _receivedResponse.get(); -} - -- (void)didLoadData:(NSData*)data { - [_loadedData appendData:data]; - [super didLoadData:data]; -} - -- (void)didReceiveResponse:(NSURLResponse*)response { - _receivedResponse.reset([response copy]); - [super didReceiveResponse:response]; -} - -@end - -namespace { - -const char kTestFile[] = "ios/web/test/data/chrome.html"; - -class CRWJSInjectionNetworkClientTest : public testing::Test { - public: - CRWJSInjectionNetworkClientTest() {} - - void SetUp() override { - // Set up mock original network client proxy. - mock_web_proxy_.reset([[OCMockObject - niceMockForProtocol:@protocol(CRNNetworkClientProtocol)] retain]); - - // Set up underlying client to inspect data and responses passed on by - // the JS injection client. - underlying_client_.reset([[UnderlyingClient alloc] init]); - [underlying_client_ setUnderlyingClient: - static_cast<id<CRNNetworkClientProtocol>>(mock_web_proxy_)]; - - // Link mock proxy into the JSInjectionNetworkClient. - js_injection_client_.reset([[CRWJSInjectionNetworkClient alloc] init]); - [js_injection_client_ setUnderlyingClient:underlying_client_]; - - // Load data for testing - base::FilePath file_path; - ASSERT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &file_path)); - file_path = file_path.AppendASCII(kTestFile); - test_data_.reset([[NSData dataWithContentsOfFile: - base::SysUTF8ToNSString(file_path.value())] retain]); - ASSERT_TRUE(test_data_); - } - - void TearDown() override { EXPECT_OCMOCK_VERIFY(mock_web_proxy_); } - - protected: - // Returns a CRNHTTPURLResponse. If |include_content_length|, header includes - // Content-Length set to the length of test_data_. - CRNHTTPURLResponse* CreateTestResponse(BOOL include_content_length); - - // Returns number of times an injected cr_web script tag is found in the - // underlying client's loaded data. Script tag should immediately follow - // the html start tag, if it exists, or should be injected before any header, - // if the first tag is something other than an html start tag. - NSUInteger GetScriptTagCount() const; - - // Checks that if response forwarded to the underlying client has header field - // Content-Length, the value matches the length of the data. - void ExpectConsistentContentLength(); - - base::scoped_nsobject<CRWJSInjectionNetworkClient> js_injection_client_; - base::scoped_nsobject<UnderlyingClient> underlying_client_; - base::scoped_nsobject<OCMockObject> mock_web_proxy_; - base::scoped_nsobject<NSData> test_data_; -}; - -CRNHTTPURLResponse* CRWJSInjectionNetworkClientTest::CreateTestResponse( - BOOL include_content_length) { - NSMutableDictionary *headers = [NSMutableDictionary - dictionaryWithDictionary:@{ @"Content-Type" : @"text/html" }]; - if (include_content_length) { - headers[@"Content-Length"] = @([test_data_ length]).stringValue; - } - return [[CRNHTTPURLResponse alloc] - initWithURL:[NSURL URLWithString:@"http://testjsinjection.html"] - statusCode:200 - HTTPVersion:@"HTTP/1.1" - headerFields:headers]; -}; - -NSUInteger CRWJSInjectionNetworkClientTest::GetScriptTagCount() const { - base::scoped_nsobject<NSString> data_string( - [[NSString alloc] initWithData:[underlying_client_ loadedData] - encoding:NSUTF8StringEncoding]); - NSRegularExpression* script_tag_reg_exp = [NSRegularExpression - regularExpressionWithPattern:@"(^|<html>)<script src=" - "\"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-" - "[0-9a-f]{4}-[0-9a-f]{12}+_crweb\\.js\" " - "charset=\"utf-8\"></script>" - options:NSRegularExpressionCaseInsensitive - error:nil]; - return [script_tag_reg_exp - numberOfMatchesInString:data_string - options:0 - range:NSMakeRange(0, [data_string length])]; -} - -void CRWJSInjectionNetworkClientTest::ExpectConsistentContentLength() { - NSData* forwarded_data = [underlying_client_ loadedData]; - NSDictionary* output_headers = - [static_cast<NSHTTPURLResponse*>([underlying_client_ receivedResponse]) - allHeaderFields]; - ASSERT_TRUE(output_headers); - NSInteger content_length = [output_headers[@"Content-Length"] integerValue]; - if (content_length) { - EXPECT_EQ(static_cast<NSInteger>([forwarded_data length]), - (content_length)); - } -} - -} // namespace - -#pragma mark - Tests - -// Tests injection where response header has Content-Length. Checks that -// Content-Length is updated to match new size of data. -TEST_F(CRWJSInjectionNetworkClientTest, InjectionWithContentLength) { - base::scoped_nsobject<CRNHTTPURLResponse> test_response( - CreateTestResponse(YES)); - [js_injection_client_ didReceiveResponse:test_response]; - [js_injection_client_ didLoadData:test_data_]; - [js_injection_client_ didFinishLoading]; - - EXPECT_EQ(1u, GetScriptTagCount()); - ExpectConsistentContentLength(); -} - -// Tests injection where response header does not have Content-Length. -TEST_F(CRWJSInjectionNetworkClientTest, InjectionWithoutContentLength) { - base::scoped_nsobject<CRNHTTPURLResponse> test_response( - CreateTestResponse(NO)); - [js_injection_client_ didReceiveResponse:test_response]; - [js_injection_client_ didLoadData:test_data_]; - [js_injection_client_ didFinishLoading]; - - EXPECT_EQ(1u, GetScriptTagCount()); - ExpectConsistentContentLength(); -} - -// Tests that injection occurs at the beginning of the file if data has no html -// start tag but does have other tags within the first 1KB. -TEST_F(CRWJSInjectionNetworkClientTest, MissingHTMLTag) { - base::scoped_nsobject<NSString> test_string( - [[NSString alloc] initWithData:test_data_ - encoding:NSUTF8StringEncoding]); - NSData* truncated_data = - [[test_string stringByReplacingOccurrencesOfString:@"<html>" - withString:@""] - dataUsingEncoding:NSUTF8StringEncoding]; - test_data_.reset([truncated_data retain]); - ASSERT_TRUE(test_data_); - - base::scoped_nsobject<CRNHTTPURLResponse> test_response( - CreateTestResponse(YES)); - [js_injection_client_ didReceiveResponse:test_response]; - [js_injection_client_ didLoadData:test_data_]; - [js_injection_client_ didFinishLoading]; - - EXPECT_EQ(1u, GetScriptTagCount()); - ExpectConsistentContentLength(); -} - -// Tests that injection occurs just following the html tag when there is < 1KB -// of padding preceding the html start tag. -TEST_F(CRWJSInjectionNetworkClientTest, LessThan1KBBeforeHTMLTag) { - base::scoped_nsobject<NSString> test_string( - [[NSString alloc] initWithData:test_data_ - encoding:NSUTF8StringEncoding]); - NSData* padded_data = [[GetLongString(900u) - stringByAppendingString:test_string] - dataUsingEncoding:NSUTF8StringEncoding]; - test_data_.reset([padded_data retain]); - ASSERT_TRUE(test_data_); - - base::scoped_nsobject<CRNHTTPURLResponse> test_response( - CreateTestResponse(YES)); - [js_injection_client_ didReceiveResponse:test_response]; - [js_injection_client_ didLoadData:test_data_]; - [js_injection_client_ didFinishLoading]; - - EXPECT_EQ(1u, GetScriptTagCount()); - ExpectConsistentContentLength(); -} - -TEST_F(CRWJSInjectionNetworkClientTest, PaddedMissingHTMLTag) { - base::scoped_nsobject<NSString> test_string( - [[NSString alloc] initWithData:test_data_ - encoding:NSUTF8StringEncoding]); - test_string.reset( - [[test_string stringByReplacingOccurrencesOfString:@"<html>" - withString:@""] retain]); - NSData* padded_data = [[GetLongString(900u) - stringByAppendingString:test_string] - dataUsingEncoding:NSUTF8StringEncoding]; - test_data_.reset([padded_data retain]); - ASSERT_TRUE(test_data_); - - base::scoped_nsobject<CRNHTTPURLResponse> test_response( - CreateTestResponse(YES)); - [js_injection_client_ didReceiveResponse:test_response]; - [js_injection_client_ didLoadData:test_data_]; - [js_injection_client_ didFinishLoading]; - - EXPECT_EQ(1u, GetScriptTagCount()); - ExpectConsistentContentLength(); -} - -// Tests scenario in which data is loaded one byte at a time, as might occur -// under a slow connection. -TEST_F(CRWJSInjectionNetworkClientTest, FragmentedDataLoad) { - base::scoped_nsobject<CRNHTTPURLResponse> test_response( - CreateTestResponse(YES)); - [js_injection_client_ didReceiveResponse:test_response]; - // Load data one byte at a time. - for (NSUInteger i = 0; i < [test_data_ length]; i++) { - [js_injection_client_ didLoadData: - [test_data_ subdataWithRange:NSMakeRange(i, 1u)]]; - } - [js_injection_client_ didFinishLoading]; - - EXPECT_EQ(1u, GetScriptTagCount()); - ExpectConsistentContentLength(); -}
diff --git a/ios/web/net/clients/crw_redirect_network_client.h b/ios/web/net/clients/crw_redirect_network_client.h deleted file mode 100644 index 5e37ca6..0000000 --- a/ios/web/net/clients/crw_redirect_network_client.h +++ /dev/null
@@ -1,33 +0,0 @@ -// Copyright 2015 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 IOS_WEB_NET_CLIENTS_CRW_REDIRECT_NETWORK_CLIENT_H_ -#define IOS_WEB_NET_CLIENTS_CRW_REDIRECT_NETWORK_CLIENT_H_ - -#import <Foundation/Foundation.h> - -#import "ios/net/clients/crn_forwarding_network_client.h" - -@protocol CRWRedirectClientDelegate; - -// The CRWRedirectNetworkClient observes redirects and notifies its delegate -// when they occur. -@interface CRWRedirectNetworkClient : CRNForwardingNetworkClient - -// Designated initializer. -- (instancetype)initWithDelegate:(id<CRWRedirectClientDelegate>)delegate; - -@end - -// Delegate for CRWRedirectNetworkClients. -@protocol CRWRedirectClientDelegate<NSObject> - -// Notifies the delegate that the current http request recieved |response| and -// was redirected to |request|. -- (void)wasRedirectedToRequest:(NSURLRequest*)request - redirectResponse:(NSURLResponse*)response; - -@end - -#endif // IOS_WEB_NET_CLIENTS_CRW_REDIRECT_NETWORK_CLIENT_H_
diff --git a/ios/web/net/clients/crw_redirect_network_client.mm b/ios/web/net/clients/crw_redirect_network_client.mm deleted file mode 100644 index ab4194b..0000000 --- a/ios/web/net/clients/crw_redirect_network_client.mm +++ /dev/null
@@ -1,51 +0,0 @@ -// Copyright 2015 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 "ios/web/net/clients/crw_redirect_network_client.h" - -#include "base/location.h" -#import "base/mac/bind_objc_block.h" -#include "ios/web/public/web_thread.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -@interface CRWRedirectNetworkClient () { - // The delegate. - __weak id<CRWRedirectClientDelegate> _delegate; -} - -@end - -@implementation CRWRedirectNetworkClient - -- (instancetype)initWithDelegate:(id<CRWRedirectClientDelegate>)delegate { - self = [super init]; - if (self) { - _delegate = delegate; - } - return self; -} - -- (instancetype)init { - NOTREACHED(); - return nil; -} - -- (void)wasRedirectedToRequest:(NSURLRequest*)request - nativeRequest:(net::URLRequest*)nativeRequest - redirectResponse:(NSURLResponse*)redirectResponse { - DCHECK_CURRENTLY_ON(web::WebThread::IO); - web::WebThread::PostTask(web::WebThread::UI, FROM_HERE, base::BindBlockArc(^{ - [_delegate - wasRedirectedToRequest:request - redirectResponse:redirectResponse]; - })); - [super wasRedirectedToRequest:request - nativeRequest:nativeRequest - redirectResponse:redirectResponse]; -} - -@end
diff --git a/ios/web/net/clients/crw_redirect_network_client_factory.h b/ios/web/net/clients/crw_redirect_network_client_factory.h deleted file mode 100644 index 68e3c49..0000000 --- a/ios/web/net/clients/crw_redirect_network_client_factory.h +++ /dev/null
@@ -1,21 +0,0 @@ -// Copyright 2015 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 IOS_WEB_NET_CLIENTS_CRW_REDIRECT_NETWORK_CLIENT_FACTORY_H_ -#define IOS_WEB_NET_CLIENTS_CRW_REDIRECT_NETWORK_CLIENT_FACTORY_H_ - -#import <Foundation/Foundation.h> - -#import "ios/web/net/clients/crw_redirect_network_client.h" -#import "ios/net/clients/crn_forwarding_network_client_factory.h" - -// Factory that creates CRWRedirectNetworkClient instances. -@interface CRWRedirectNetworkClientFactory : CRNForwardingNetworkClientFactory - -// Designated initializer. |delegate| is expected to be non-nil. -- (instancetype)initWithDelegate:(id<CRWRedirectClientDelegate>)delegate; - -@end - -#endif // IOS_WEB_NET_CLIENTS_CRW_REDIRECT_NETWORK_CLIENT_FACTORY_H_
diff --git a/ios/web/net/clients/crw_redirect_network_client_factory.mm b/ios/web/net/clients/crw_redirect_network_client_factory.mm deleted file mode 100644 index 34a5ce07..0000000 --- a/ios/web/net/clients/crw_redirect_network_client_factory.mm +++ /dev/null
@@ -1,55 +0,0 @@ -// Copyright 2015 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 "ios/web/net/clients/crw_redirect_network_client_factory.h" - -#include "base/location.h" -#include "ios/web/public/web_thread.h" -#include "net/http/http_response_headers.h" -#include "net/url_request/url_request.h" -#include "url/gurl.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -@interface CRWRedirectNetworkClientFactory () { - // Delegate passed to each vended CRWRedirectClient. - __weak id<CRWRedirectClientDelegate> _client_delegate; -} - -@end - -@implementation CRWRedirectNetworkClientFactory - -- (instancetype)initWithDelegate:(id<CRWRedirectClientDelegate>)delegate { - self = [super init]; - if (self) { - DCHECK_CURRENTLY_ON(web::WebThread::UI); - DCHECK(delegate); - _client_delegate = delegate; - } - return self; -} - -- (instancetype)init { - NOTREACHED(); - return nil; -} - -#pragma mark - CRNForwardingNetworkClientFactory - -- (Class)clientClass { - return [CRWRedirectNetworkClient class]; -} - -- (CRNForwardingNetworkClient*)clientHandlingRedirect: - (const net::URLRequest&)request - url:(const GURL&)url - response:(NSURLResponse*)response { - DCHECK_CURRENTLY_ON(web::WebThread::IO); - return [[CRWRedirectNetworkClient alloc] initWithDelegate:_client_delegate]; -} - -@end
diff --git a/ios/web/web_state/context_menu_params_utils.h b/ios/web/web_state/context_menu_params_utils.h new file mode 100644 index 0000000..f3f22762 --- /dev/null +++ b/ios/web/web_state/context_menu_params_utils.h
@@ -0,0 +1,22 @@ +// 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 IOS_WEB_WEB_STATE_CONTEXT_MENU_PARAMS_UTILS_H_ +#define IOS_WEB_WEB_STATE_CONTEXT_MENU_PARAMS_UTILS_H_ + +#import <UIKit/UIKit.h> + +#import "ios/web/public/web_state/context_menu_params.h" + +namespace web { +// creates a ContextMenuParams from a NSDictionary representing an HTML element. +// The fields "href", "src", "title", "referrerPolicy" and "innerText" will +// be used (if present) to generate the ContextMenuParams. +// All these fields must be NSString*. +// This constructor does not set fields relative to the touch event (view and +// location). +ContextMenuParams ContextMenuParamsFromElementDictionary(NSDictionary* element); +} // namespace web + +#endif // IOS_WEB_WEB_STATE_CONTEXT_MENU_PARAMS_UTILS_H_
diff --git a/ios/web/web_state/context_menu_params_utils.mm b/ios/web/web_state/context_menu_params_utils.mm new file mode 100644 index 0000000..3acabb5e6 --- /dev/null +++ b/ios/web/web_state/context_menu_params_utils.mm
@@ -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. + +#import "ios/web/web_state/context_menu_params_utils.h" + +#include "base/strings/sys_string_conversions.h" +#include "components/url_formatter/url_formatter.h" +#include "ios/web/public/referrer_util.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +namespace web { +ContextMenuParams ContextMenuParamsFromElementDictionary( + NSDictionary* element) { + ContextMenuParams params; + NSString* title = nil; + NSString* href = element[@"href"]; + if (href) { + params.link_url = GURL(base::SysNSStringToUTF8(href)); + if (params.link_url.SchemeIs(url::kJavaScriptScheme)) { + title = @"JavaScript"; + } else { + base::string16 URLText = url_formatter::FormatUrl(params.link_url); + title = base::SysUTF16ToNSString(URLText); + } + } + NSString* src = element[@"src"]; + if (src) { + params.src_url = GURL(base::SysNSStringToUTF8(src)); + if (!title) + title = [src copy]; + if ([title hasPrefix:base::SysUTF8ToNSString(url::kDataScheme)]) + title = nil; + } + NSString* titleAttribute = element[@"title"]; + if (titleAttribute) + title = titleAttribute; + if (title) { + params.menu_title.reset([title copy]); + } + NSString* referrerPolicy = element[@"referrerPolicy"]; + if (referrerPolicy) { + params.referrer_policy = + web::ReferrerPolicyFromString(base::SysNSStringToUTF8(referrerPolicy)); + } + NSString* innerText = element[@"innerText"]; + if ([innerText length] > 0) { + params.link_text.reset([innerText copy]); + } + return params; +} + +} // namespace web
diff --git a/ios/web/web_state/context_menu_params_utils_unittest.mm b/ios/web/web_state/context_menu_params_utils_unittest.mm new file mode 100644 index 0000000..d43a4b3 --- /dev/null +++ b/ios/web/web_state/context_menu_params_utils_unittest.mm
@@ -0,0 +1,99 @@ +// 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 "ios/web/web_state/context_menu_params_utils.h" + +#include "base/strings/sys_string_conversions.h" +#include "components/url_formatter/url_formatter.h" +#include "ios/web/public/referrer_util.h" +#import "ios/web/public/web_state/context_menu_params.h" +#import "net/base/mac/url_conversions.h" +#include "testing/gtest/include/gtest/gtest.h" +#import "testing/gtest_mac.h" +#include "testing/platform_test.h" + +namespace { +// Text values for the tapped element triggering the context menu. +const char* kLinkUrl = "http://link.url/"; +const char* kSrcUrl = "http://src.url/"; +const char* kTitle = "title"; +const char* kReferrerPolicy = "always"; +const char* kLinkText = "link text"; +const char* kJavaScriptLinkUrl = "javascript://src.url/"; +const char* kDataUrl = "data://foo.bar/"; +} + +// Test fixture for error translation testing. +typedef PlatformTest ContextMenuParamsUtilsTest; + +// Tests the empty contructor. +TEST_F(ContextMenuParamsUtilsTest, EmptyParams) { + web::ContextMenuParams params; + EXPECT_EQ(params.menu_title.get(), nil); + EXPECT_FALSE(params.link_url.is_valid()); + EXPECT_FALSE(params.src_url.is_valid()); + EXPECT_EQ(params.referrer_policy, web::ReferrerPolicyDefault); + EXPECT_EQ(params.view.get(), nil); + EXPECT_TRUE(CGPointEqualToPoint(params.location, CGPointZero)); + EXPECT_EQ(params.link_text.get(), nil); +} + +// Tests the the parsing of the element NSDictionary. +TEST_F(ContextMenuParamsUtilsTest, DictionaryConstructorTest) { + web::ContextMenuParams params = web::ContextMenuParamsFromElementDictionary(@{ + @"href" : @(kLinkUrl), + @"src" : @(kSrcUrl), + @"title" : @(kTitle), + @"referrerPolicy" : @(kReferrerPolicy), + @"innerText" : @(kLinkText), + }); + + EXPECT_NSEQ(params.menu_title.get(), @(kTitle)); + EXPECT_EQ(params.link_url, GURL(kLinkUrl)); + EXPECT_EQ(params.src_url, GURL(kSrcUrl)); + EXPECT_NSEQ(params.link_text.get(), @(kLinkText)); + EXPECT_EQ(params.referrer_policy, + web::ReferrerPolicyFromString(kReferrerPolicy)); + + EXPECT_EQ(params.view.get(), nil); + EXPECT_TRUE(CGPointEqualToPoint(params.location, CGPointZero)); +} + +// Tests title is set as the formatted URL there is no title. +TEST_F(ContextMenuParamsUtilsTest, DictionaryConstructorTestNoTitle) { + web::ContextMenuParams params = web::ContextMenuParamsFromElementDictionary(@{ + @"href" : @(kLinkUrl), + }); + base::string16 urlText = url_formatter::FormatUrl(GURL(kLinkUrl)); + NSString* title = base::SysUTF16ToNSString(urlText); + + EXPECT_NSEQ(params.menu_title.get(), title); +} + +// Tests title is set to "JavaScript" if there is no title and "href" links to +// JavaScript URL. +TEST_F(ContextMenuParamsUtilsTest, DictionaryConstructorTestJavascriptTitle) { + web::ContextMenuParams params = web::ContextMenuParamsFromElementDictionary(@{ + @"href" : @(kJavaScriptLinkUrl), + }); + EXPECT_NSEQ(params.menu_title.get(), @"JavaScript"); +} + +// Tests title is set to |src_url| if there is no title. +TEST_F(ContextMenuParamsUtilsTest, DictionaryConstructorTestSrcTitle) { + web::ContextMenuParams params = web::ContextMenuParamsFromElementDictionary(@{ + @"src" : @(kSrcUrl), + }); + EXPECT_EQ(params.src_url, GURL(kSrcUrl)); + EXPECT_NSEQ(params.menu_title.get(), @(kSrcUrl)); +} + +// Tests title is set to nil if there is no title and src is a data URL. +TEST_F(ContextMenuParamsUtilsTest, DictionaryConstructorTestDataTitle) { + web::ContextMenuParams params = web::ContextMenuParamsFromElementDictionary(@{ + @"src" : @(kDataUrl), + }); + EXPECT_EQ(params.src_url, GURL(kDataUrl)); + EXPECT_NSEQ(params.menu_title.get(), nil); +}
diff --git a/ios/web/web_state/ui/crw_context_menu_controller.mm b/ios/web/web_state/ui/crw_context_menu_controller.mm index 707c129f..0abc9d2 100644 --- a/ios/web/web_state/ui/crw_context_menu_controller.mm +++ b/ios/web/web_state/ui/crw_context_menu_controller.mm
@@ -7,16 +7,18 @@ #import <objc/runtime.h> #include <stddef.h> -#import "base/ios/weak_nsobject.h" #include "base/logging.h" #include "base/mac/foundation_util.h" #include "base/metrics/histogram.h" #include "base/strings/sys_string_conversions.h" -#include "components/url_formatter/url_formatter.h" -#include "ios/web/public/referrer_util.h" #import "ios/web/public/web_state/context_menu_params.h" #import "ios/web/public/web_state/js/crw_js_injection_evaluator.h" #import "ios/web/public/web_state/ui/crw_context_menu_delegate.h" +#import "ios/web/web_state/context_menu_params_utils.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif namespace { // The long press detection duration must be shorter than the WKWebView's @@ -41,26 +43,19 @@ @interface CRWContextMenuController ()<UIGestureRecognizerDelegate> -// Sets the specified recognizer to take priority over any recognizers in the -// view that have a description containing the specified text fragment. -+ (void)requireGestureRecognizerToFail:(UIGestureRecognizer*)recognizer - inView:(UIView*)view - containingDescription:(NSString*)fragment; - // The |webView|. -@property(nonatomic, readonly) WKWebView* webView; +@property(nonatomic, readonly, weak) WKWebView* webView; // The delegate that allow execute javascript. -@property(nonatomic, readonly) id<CRWJSInjectionEvaluator> injectionEvaluator; +@property(nonatomic, readonly, weak) id<CRWJSInjectionEvaluator> + injectionEvaluator; // The scroll view of |webView|. -@property(nonatomic, readonly) id<CRWContextMenuDelegate> delegate; +@property(nonatomic, readonly, weak) id<CRWContextMenuDelegate> delegate; // Returns the x, y offset the content has been scrolled. @property(nonatomic, readonly) CGPoint scrollPosition; // Called when the window has determined there was a long-press and context menu // must be shown. - (void)showContextMenu:(UIGestureRecognizer*)gestureRecognizer; -// Extracts context menu information from the given DOM element. -- (web::ContextMenuParams)contextMenuParamsForElement:(NSDictionary*)element; // Cancels all touch events in the web view (long presses, tapping, scrolling). - (void)cancelAllTouches; // Asynchronously fetches full width of the rendered web page. @@ -79,76 +74,44 @@ @end @implementation CRWContextMenuController { - base::WeakNSProtocol<id<CRWContextMenuDelegate>> _delegate; - base::WeakNSProtocol<id<CRWJSInjectionEvaluator>> _injectionEvaluator; - base::WeakNSObject<WKWebView> _webView; - // Long press recognizer that allows showing context menus. - base::scoped_nsobject<UILongPressGestureRecognizer> _contextMenuRecognizer; + UILongPressGestureRecognizer* _contextMenuRecognizer; // DOM element information for the point where the user made the last touch. // Can be nil if has not been calculated yet. Precalculation is necessary // because retreiving DOM element relies on async API so element info can not // be built on demand. May contain the following keys: @"href", @"src", // @"title", @"referrerPolicy". All values are strings. - base::scoped_nsobject<NSDictionary> _DOMElementForLastTouch; + NSDictionary* _DOMElementForLastTouch; } +@synthesize delegate = _delegate; +@synthesize injectionEvaluator = _injectionEvaluator; +@synthesize webView = _webView; + - (instancetype)initWithWebView:(WKWebView*)webView injectionEvaluator:(id<CRWJSInjectionEvaluator>)injectionEvaluator delegate:(id<CRWContextMenuDelegate>)delegate { DCHECK(webView); self = [super init]; if (self) { - _webView.reset(webView); - _delegate.reset(delegate); - _injectionEvaluator.reset(injectionEvaluator); + _webView = webView; + _delegate = delegate; + _injectionEvaluator = injectionEvaluator; // The system context menu triggers after 0.55 second. Add a gesture // recognizer with a shorter delay to be able to cancel the system menu if // needed. - _contextMenuRecognizer.reset([[UILongPressGestureRecognizer alloc] + _contextMenuRecognizer = [[UILongPressGestureRecognizer alloc] initWithTarget:self - action:@selector(showContextMenu:)]); + action:@selector(showContextMenu:)]; [_contextMenuRecognizer setMinimumPressDuration:kLongPressDurationSeconds]; [_contextMenuRecognizer setAllowableMovement:kLongPressMoveDeltaPixels]; [_contextMenuRecognizer setDelegate:self]; [_webView addGestureRecognizer:_contextMenuRecognizer]; - // Certain system gesture handlers are known to conflict with our context - // menu handler, causing extra events to fire when the context menu is - // active. - - // A number of solutions have been investigated. The lowest-risk solution - // appears to be to recurse through the web controller's recognizers, - // looking - // for fingerprints of the recognizers known to cause problems, which are - // then - // de-prioritized (below our own long click handler). - // Hunting for description fragments of system recognizers is undeniably - // brittle for future versions of iOS. If it does break the context menu - // events may leak (regressing b/5310177), but the app will otherwise work. - // TODO(crbug.com/680930): This code is not needed anymore in iOS9+ and has - // to be removed. - [CRWContextMenuController - requireGestureRecognizerToFail:_contextMenuRecognizer - inView:_webView - containingDescription: - @"action=_highlightLongPressRecognized:"]; } return self; } -- (WKWebView*)webView { - return _webView.get(); -} - -- (id<CRWContextMenuDelegate>)delegate { - return _delegate.get(); -} - -- (id<CRWJSInjectionEvaluator>)injectionEvaluator { - return _injectionEvaluator.get(); -} - - (UIScrollView*)webScrollView { return [_webView scrollView]; } @@ -177,8 +140,8 @@ return; web::ContextMenuParams params = - [self contextMenuParamsForElement:_DOMElementForLastTouch.get()]; - params.view.reset([_webView retain]); + web::ContextMenuParamsFromElementDictionary(_DOMElementForLastTouch); + params.view.reset(_webView); params.location = [gestureRecognizer locationInView:_webView]; if ([_delegate webView:_webView handleContextMenu:params]) { // Cancelling all touches has the intended side effect of suppressing the @@ -187,66 +150,6 @@ } } -+ (void)requireGestureRecognizerToFail:(UIGestureRecognizer*)recognizer - inView:(UIView*)view - containingDescription:(NSString*)fragment { - for (UIGestureRecognizer* iRecognizer in [view gestureRecognizers]) { - if (iRecognizer != recognizer) { - NSString* description = [iRecognizer description]; - if ([description rangeOfString:fragment].length) { - [iRecognizer requireGestureRecognizerToFail:recognizer]; - // requireGestureRecognizerToFail: doesn't retain the recognizer, so it - // is possible for |iRecognizer| to outlive |recognizer| and end up with - // a dangling pointer. Add a retaining associative reference to ensure - // that the lifetimes work out. - // Note that normally using the value as the key wouldn't make any - // sense, but here it's fine since nothing needs to look up the value. - objc_setAssociatedObject(view, recognizer, recognizer, - OBJC_ASSOCIATION_RETAIN_NONATOMIC); - } - } - } -} - -- (web::ContextMenuParams)contextMenuParamsForElement:(NSDictionary*)element { - web::ContextMenuParams params; - NSString* title = nil; - NSString* href = element[@"href"]; - if (href) { - params.link_url = GURL(base::SysNSStringToUTF8(href)); - if (params.link_url.SchemeIs(url::kJavaScriptScheme)) { - title = @"JavaScript"; - } else { - base::string16 URLText = url_formatter::FormatUrl(params.link_url); - title = base::SysUTF16ToNSString(URLText); - } - } - NSString* src = element[@"src"]; - if (src) { - params.src_url = GURL(base::SysNSStringToUTF8(src)); - if (!title) - title = [[src copy] autorelease]; - if ([title hasPrefix:base::SysUTF8ToNSString(url::kDataScheme)]) - title = nil; - } - NSString* titleAttribute = element[@"title"]; - if (titleAttribute) - title = titleAttribute; - if (title) { - params.menu_title.reset([title copy]); - } - NSString* referrerPolicy = element[@"referrerPolicy"]; - if (referrerPolicy) { - params.referrer_policy = - web::ReferrerPolicyFromString(base::SysNSStringToUTF8(referrerPolicy)); - } - NSString* innerText = element[@"innerText"]; - if ([innerText length] > 0) { - params.link_text.reset([innerText copy]); - } - return params; -} - - (void)cancelAllTouches { // Disable web view scrolling. CancelTouches(self.webView.scrollView.panGestureRecognizer); @@ -267,7 +170,7 @@ } - (void)setDOMElementForLastTouch:(NSDictionary*)element { - _DOMElementForLastTouch.reset([element copy]); + _DOMElementForLastTouch = [element copy]; } #pragma mark - @@ -292,7 +195,7 @@ // menu and show another one. If for some reason context menu info is not // fetched - system context menu will be shown. [self setDOMElementForLastTouch:nil]; - base::WeakNSObject<CRWContextMenuController> weakSelf(self); + __weak CRWContextMenuController* weakSelf = self; [self fetchDOMElementAtPoint:[touch locationInView:_webView] completionHandler:^(NSDictionary* element) { [weakSelf setDOMElementForLastTouch:element]; @@ -336,7 +239,7 @@ // scrolled). CGPoint scrollOffset = self.scrollPosition; CGFloat webViewContentWidth = self.webScrollView.contentSize.width; - base::WeakNSObject<CRWContextMenuController> weakSelf(self); + CRWContextMenuController* weakSelf = self; [self fetchWebPageWidthWithCompletionHandler:^(CGFloat pageWidth) { CGFloat scale = pageWidth / webViewContentWidth; CGPoint localPoint = CGPointMake((point.x + scrollOffset.x) * scale, @@ -344,10 +247,10 @@ NSString* const kGetElementScript = [NSString stringWithFormat:@"__gCrWeb.getElementFromPoint(%g, %g);", localPoint.x, localPoint.y]; - [self executeJavaScript:kGetElementScript - completionHandler:^(id element, NSError*) { - handler(base::mac::ObjCCastStrict<NSDictionary>(element)); - }]; + [weakSelf executeJavaScript:kGetElementScript + completionHandler:^(id element, NSError*) { + handler(base::mac::ObjCCastStrict<NSDictionary>(element)); + }]; }]; }
diff --git a/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/access-control-origin-header-in-isolated-world.html b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/access-control-origin-header-in-isolated-world.html new file mode 100644 index 0000000..6590472 --- /dev/null +++ b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/access-control-origin-header-in-isolated-world.html
@@ -0,0 +1,33 @@ +<!DOCTYPE html> +<script src="/js-test-resources/testharness.js"></script> +<script src="/js-test-resources/testharnessreport.js"></script> +<script> +if (window.testRunner) { + // Testing with "http" scheme than "chrome-extension://" since content_shell + // doesn't register the extension scheme as a web safe isolated scheme. + testRunner.setIsolatedWorldSecurityOrigin(1, 'http://example.com'); +} + +function runTest() { + const xhr = new XMLHttpRequest(); + xhr.open('GET', 'http://localhost:8000/xmlhttprequest/resources/access-control-origin-header-in-isolated-world.php'); + xhr.setRequestHeader('X-Custom-Header', 'PASS'); + xhr.onerror = () => { + postMessage('FAIL: XHR errored', '*'); + }; + xhr.onload = () => { + postMessage(xhr.responseText, '*'); + }; + xhr.send(); +} + +async_test(t => { + addEventListener('message', t.step_func(event => { + assert_equals(event.data, 'PASS'); + t.done(); + })); + testRunner.evaluateScriptInIsolatedWorld(1, String(eval('runTest')) + '\nrunTest();'); +}, 'The Origin header contains the origin of the isolated world where the script is running'); + +done(); +</script>
diff --git a/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/resources/access-control-origin-header-in-isolated-world.php b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/resources/access-control-origin-header-in-isolated-world.php new file mode 100644 index 0000000..0556700 --- /dev/null +++ b/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/resources/access-control-origin-header-in-isolated-world.php
@@ -0,0 +1,13 @@ +<?php +if ($_SERVER["HTTP_ORIGIN"] == "http://example.com") { + header("Access-Control-Allow-Origin: http://example.com"); + + if ($_SERVER["REQUEST_METHOD"] == "OPTIONS") { + header("Access-Control-Allow-Headers: X-Custom-Header"); + } else { + print $_SERVER["HTTP_X_CUSTOM_HEADER"]; + } +} else { + header("HTTP/1.1 400"); +} +?>
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptPromisePropertyBase.cpp b/third_party/WebKit/Source/bindings/core/v8/ScriptPromisePropertyBase.cpp index 815a3ba..37c2aed 100644 --- a/third_party/WebKit/Source/bindings/core/v8/ScriptPromisePropertyBase.cpp +++ b/third_party/WebKit/Source/bindings/core/v8/ScriptPromisePropertyBase.cpp
@@ -38,7 +38,7 @@ ScriptState::Scope scope(scriptState); v8::Local<v8::Object> wrapper = ensureHolderWrapper(scriptState); - ASSERT(wrapper->CreationContext() == context); + DCHECK(wrapper->CreationContext() == context); v8::Local<v8::Value> cachedPromise = V8HiddenValue::getHiddenValue(scriptState, wrapper, promiseName()); @@ -93,6 +93,7 @@ v8::Local<v8::Promise::Resolver> resolver = V8HiddenValue::getHiddenValue(scriptState, wrapper, resolverName()) .As<v8::Promise::Resolver>(); + DCHECK(!resolver.IsEmpty()); V8HiddenValue::deleteHiddenValue(scriptState, wrapper, resolverName()); resolveOrRejectInternal(resolver); @@ -148,6 +149,7 @@ weakPersistent->set(m_isolate, wrapper); weakPersistent->setPhantom(); m_wrappers.push_back(std::move(weakPersistent)); + DCHECK(wrapper->CreationContext() == context); return wrapper; }
diff --git a/third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp b/third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp index 347c27bc8..c6fb0b8b 100644 --- a/third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp +++ b/third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp
@@ -394,8 +394,9 @@ // NOTE: Some threads (namely utility threads) don't have a scheduler. WebScheduler* scheduler = Platform::current()->currentThread()->scheduler(); + // TODO(altimin): Consider switching to timerTaskRunner here. v8::Isolate* isolate = V8PerIsolateData::initialize( - scheduler ? scheduler->timerTaskRunner() + scheduler ? scheduler->loadingTaskRunner() : Platform::current()->currentThread()->getWebTaskRunner()); initializeV8Common(isolate);
diff --git a/third_party/WebKit/Source/bindings/scripts/v8_types.py b/third_party/WebKit/Source/bindings/scripts/v8_types.py index c89004f..b9c4750e 100644 --- a/third_party/WebKit/Source/bindings/scripts/v8_types.py +++ b/third_party/WebKit/Source/bindings/scripts/v8_types.py
@@ -26,6 +26,8 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# pylint: disable=relative-import + """Functions for type handling and type conversion (Blink/C++ <-> V8/JS). Extends IdlType and IdlUnionType with V8-specific properties, methods, and @@ -42,6 +44,7 @@ from idl_types import IdlTypeBase, IdlType, IdlUnionType, IdlArrayOrSequenceType, IdlNullableType import v8_attributes # for IdlType.constructor_type_name from v8_globals import includes +from v8_utilities import extended_attribute_value_contains ################################################################################ @@ -810,6 +813,21 @@ 'DOMWrapperForMainWorld': 'v8SetReturnValueForMainWorld(info, {cpp_value})', 'DOMWrapperFast': 'v8SetReturnValueFast(info, {cpp_value}, {script_wrappable})', 'DOMWrapperDefault': 'v8SetReturnValue(info, {cpp_value})', + # If [CheckSecurity=ReturnValue] is specified, the returned object must be + # wrapped in its own realm, which can be different from the realm of the + # receiver object. + # + # [CheckSecurity=ReturnValue] is used only for contentDocument and + # getSVGDocument attributes of HTML{IFrame,Frame,Object,Embed}Element, + # and Window.frameElement. Except for Window.frameElement, all interfaces + # support contentWindow(), so we create a new wrapper in the realm of + # contentWindow(). Note that DOMWindow* has its own realm and there is no + # need to pass |creationContext| in for ToV8(DOMWindow*). + # Window.frameElement is implemented with [Custom]. + 'DOMWrapperAcrossContext': ( + 'v8SetReturnValue(info, ToV8({cpp_value}, ' + + 'ToV8(impl->contentWindow(), v8::Local<v8::Object>(), ' + + 'info.GetIsolate()).As<v8::Object>(), info.GetIsolate()))'), # Note that static attributes and operations do not check whether |this| is # an instance of the interface nor |this|'s creation context is the same as # the current context. So we must always use the current context as the @@ -832,6 +850,10 @@ """ def dom_wrapper_conversion_type(): + if ('CheckSecurity' in extended_attributes and + extended_attribute_value_contains( + extended_attributes['CheckSecurity'], 'ReturnValue')): + return 'DOMWrapperAcrossContext' if is_static: return 'DOMWrapperStatic' if not script_wrappable:
diff --git a/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp b/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp index af019af..61e99e2 100644 --- a/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp +++ b/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp
@@ -2551,7 +2551,7 @@ return; } - v8SetReturnValueFast(info, WTF::getPtr(impl->checkSecurityForNodeReadonlyDocumentAttribute()), impl); + v8SetReturnValue(info, ToV8(WTF::getPtr(impl->checkSecurityForNodeReadonlyDocumentAttribute()), ToV8(impl->contentWindow(), v8::Local<v8::Object>(), info.GetIsolate()).As<v8::Object>(), info.GetIsolate())); } void checkSecurityForNodeReadonlyDocumentAttributeAttributeGetterCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {
diff --git a/third_party/WebKit/Source/core/editing/BUILD.gn b/third_party/WebKit/Source/core/editing/BUILD.gn index b236f57..23e2892 100644 --- a/third_party/WebKit/Source/core/editing/BUILD.gn +++ b/third_party/WebKit/Source/core/editing/BUILD.gn
@@ -237,6 +237,7 @@ "VisibleUnitsTest.cpp", "commands/ApplyBlockElementCommandTest.cpp", "commands/ApplyStyleCommandTest.cpp", + "commands/CompositeEditCommandTest.cpp", "commands/DeleteSelectionCommandTest.cpp", "commands/InsertListCommandTest.cpp", "commands/ReplaceSelectionCommandTest.cpp",
diff --git a/third_party/WebKit/Source/core/editing/CaretBase.cpp b/third_party/WebKit/Source/core/editing/CaretBase.cpp index 144b86c..f9b6a3e 100644 --- a/third_party/WebKit/Source/core/editing/CaretBase.cpp +++ b/third_party/WebKit/Source/core/editing/CaretBase.cpp
@@ -113,18 +113,6 @@ caretLocalRect); } -IntRect CaretBase::absoluteBoundsForLocalRect(Node* node, - const LayoutRect& rect) { - LayoutBlock* caretPainter = caretLayoutObject(node); - if (!caretPainter) - return IntRect(); - - LayoutRect localRect(rect); - caretPainter->flipForWritingMode(localRect); - return caretPainter->localToAbsoluteQuad(FloatRect(localRect)) - .enclosingBoundingBox(); -} - // TODO(yoichio): |node| is FrameSelection::m_previousCaretNode and this is bad // design. We should use only previous layoutObject or Rectangle to invalidate // old caret.
diff --git a/third_party/WebKit/Source/core/editing/CaretBase.h b/third_party/WebKit/Source/core/editing/CaretBase.h index 3984329..efad461 100644 --- a/third_party/WebKit/Source/core/editing/CaretBase.h +++ b/third_party/WebKit/Source/core/editing/CaretBase.h
@@ -50,7 +50,6 @@ // PositionWithAffinity version if possible. // A position in HTMLTextFromControlElement is a typical example. static LayoutRect computeCaretRect(const PositionWithAffinity& caretPosition); - static IntRect absoluteBoundsForLocalRect(Node*, const LayoutRect&); void paintCaret(Node*, GraphicsContext&,
diff --git a/third_party/WebKit/Source/core/editing/Editor.cpp b/third_party/WebKit/Source/core/editing/Editor.cpp index d5cbfa2..0efff2a 100644 --- a/third_party/WebKit/Source/core/editing/Editor.cpp +++ b/third_party/WebKit/Source/core/editing/Editor.cpp
@@ -836,15 +836,14 @@ // Request spell checking before any further DOM change. spellChecker().markMisspellingsAfterApplyingCommand(*cmd); - UndoStep* composition = cmd->composition(); - DCHECK(composition); - dispatchEditableContentChangedEvents( - composition->startingRootEditableElement(), - composition->endingRootEditableElement()); + UndoStep* undoStep = cmd->undoStep(); + DCHECK(undoStep); + dispatchEditableContentChangedEvents(undoStep->startingRootEditableElement(), + undoStep->endingRootEditableElement()); // TODO(chongz): Filter empty InputType after spec is finalized. dispatchInputEventEditableContentChanged( - composition->startingRootEditableElement(), - composition->endingRootEditableElement(), cmd->inputType(), + undoStep->startingRootEditableElement(), + undoStep->endingRootEditableElement(), cmd->inputType(), cmd->textDataForInputEvent(), isComposingFromCommand(cmd)); // TODO(editing-dev): The use of updateStyleAndLayoutIgnorePendingStylesheets @@ -870,14 +869,14 @@ (cmd->inputType() == InputEvent::InputType::DeleteByDrag || cmd->inputType() == InputEvent::InputType::InsertFromDrop)) { // Only register undo entry when combined with other commands. - if (!m_lastEditCommand->composition()) - m_undoStack->registerUndoStep(m_lastEditCommand->ensureComposition()); - m_lastEditCommand->appendCommandToComposite(cmd); + if (!m_lastEditCommand->undoStep()) + m_undoStack->registerUndoStep(m_lastEditCommand->ensureUndoStep()); + m_lastEditCommand->appendCommandToUndoStep(cmd); } else { // Only register a new undo command if the command passed in is // different from the last command m_lastEditCommand = cmd; - m_undoStack->registerUndoStep(m_lastEditCommand->ensureComposition()); + m_undoStack->registerUndoStep(m_lastEditCommand->ensureUndoStep()); } respondToChangedContents(newSelection);
diff --git a/third_party/WebKit/Source/core/editing/FrameCaret.cpp b/third_party/WebKit/Source/core/editing/FrameCaret.cpp index e00c0a8..c2337b5 100644 --- a/third_party/WebKit/Source/core/editing/FrameCaret.cpp +++ b/third_party/WebKit/Source/core/editing/FrameCaret.cpp
@@ -35,6 +35,7 @@ #include "core/frame/LocalFrame.h" #include "core/frame/Settings.h" #include "core/html/TextControlElement.h" +#include "core/layout/LayoutBlock.h" #include "core/layout/LayoutTheme.h" #include "core/layout/api/LayoutPartItem.h" #include "core/page/Page.h" @@ -223,6 +224,17 @@ m_previousCaretVisibility = m_caretVisibility; } +static IntRect absoluteBoundsForLocalRect(Node* node, const LayoutRect& rect) { + LayoutBlock* caretPainter = CaretBase::caretLayoutObject(node); + if (!caretPainter) + return IntRect(); + + LayoutRect localRect(rect); + caretPainter->flipForWritingMode(localRect); + return caretPainter->localToAbsoluteQuad(FloatRect(localRect)) + .enclosingBoundingBox(); +} + // TDOO(yosin): We should mark |FrameCaret::absoluteCaretBounds()| to |const|. IntRect FrameCaret::absoluteCaretBounds() { DCHECK_NE(m_frame->document()->lifecycle().state(), @@ -233,15 +245,15 @@ Node* const caretNode = caretPosition().anchorNode(); if (!isActive()) - return CaretBase::absoluteBoundsForLocalRect(caretNode, LayoutRect()); + return absoluteBoundsForLocalRect(caretNode, LayoutRect()); // TODO(yosin): We should get rid of text control short path since layout // tree is clean. if (enclosingTextControl(caretPosition().position()) && isVisuallyEquivalentCandidate(caretPosition().position())) { - return CaretBase::absoluteBoundsForLocalRect( + return absoluteBoundsForLocalRect( caretNode, CaretBase::computeCaretRect(caretPosition())); } - return CaretBase::absoluteBoundsForLocalRect( + return absoluteBoundsForLocalRect( caretNode, CaretBase::computeCaretRect( createVisiblePosition(caretPosition()).toPositionWithAffinity()));
diff --git a/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp b/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp index ce3d8711..65b544a 100644 --- a/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp +++ b/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp
@@ -80,10 +80,13 @@ using namespace HTMLNames; CompositeEditCommand::CompositeEditCommand(Document& document) - : EditCommand(document) {} + : EditCommand(document) { + setStartingSelection(document.frame()->selection().selection()); + setEndingVisibleSelection(m_startingSelection); +} CompositeEditCommand::~CompositeEditCommand() { - DCHECK(isTopLevelCommand() || !m_composition); + DCHECK(isTopLevelCommand() || !m_undoStep); } bool CompositeEditCommand::apply() { @@ -113,7 +116,7 @@ return false; } } - ensureComposition(); + ensureUndoStep(); // Changes to the document may have been made since the last editing operation // that require a layout, as in <rdar://problem/5658603>. Low level @@ -138,15 +141,15 @@ return !editingState.isAborted(); } -UndoStep* CompositeEditCommand::ensureComposition() { +UndoStep* CompositeEditCommand::ensureUndoStep() { CompositeEditCommand* command = this; while (command && command->parent()) command = command->parent(); - if (!command->m_composition) { - command->m_composition = UndoStep::create(&document(), startingSelection(), - endingSelection(), inputType()); + if (!command->m_undoStep) { + command->m_undoStep = UndoStep::create(&document(), startingSelection(), + endingSelection(), inputType()); } - return command->m_composition.get(); + return command->m_undoStep.get(); } bool CompositeEditCommand::preservesTypingStyle() const { @@ -185,7 +188,7 @@ } if (command->isSimpleEditCommand()) { command->setParent(0); - ensureComposition()->append(toSimpleEditCommand(command)); + ensureUndoStep()->append(toSimpleEditCommand(command)); } m_commands.push_back(command); } @@ -204,10 +207,10 @@ m_commands.push_back(command); } -void CompositeEditCommand::appendCommandToComposite( +void CompositeEditCommand::appendCommandToUndoStep( CompositeEditCommand* command) { - ensureComposition()->append(command->ensureComposition()); - command->m_composition = nullptr; + ensureUndoStep()->append(command->ensureUndoStep()); + command->m_undoStep = nullptr; command->setParent(this); m_commands.push_back(command); } @@ -271,6 +274,9 @@ EditingState* editingState, ShouldAssumeContentIsAlwaysEditable shouldAssumeContentIsAlwaysEditable) { DCHECK_NE(document().body(), refChild); + // TODO(editing-dev): Use of updateStyleAndLayoutIgnorePendingStylesheets + // needs to be audited. See http://crbug.com/590369 for more details. + document().updateStyleAndLayoutIgnorePendingStylesheets(); ABORT_EDITING_COMMAND_IF(!hasEditableStyle(*refChild->parentNode()) && refChild->parentNode()->inActiveDocument()); applyCommandToComposite( @@ -1932,7 +1938,9 @@ DEFINE_TRACE(CompositeEditCommand) { visitor->trace(m_commands); - visitor->trace(m_composition); + visitor->trace(m_startingSelection); + visitor->trace(m_endingSelection); + visitor->trace(m_undoStep); EditCommand::trace(visitor); }
diff --git a/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h b/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h index f9a287c7..2a26f4b 100644 --- a/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h +++ b/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h
@@ -44,7 +44,6 @@ enum class EditCommandSource { kMenuOrKeyBinding, kDOM }; -// TODO(xiaochengh): Cleanup the names with term "composition". class CORE_EXPORT CompositeEditCommand : public EditCommand { public: enum ShouldPreserveSelection { PreserveSelection, DoNotPreserveSelection }; @@ -52,15 +51,28 @@ ~CompositeEditCommand() override; + const VisibleSelection& startingSelection() const { + return m_startingSelection; + } + const VisibleSelection& endingSelection() const { return m_endingSelection; } + + void setStartingSelection(const VisibleSelection&); + void setEndingSelection(const SelectionInDOMTree&); + // TODO(yosin): |setEndingVisibleSelection()| will take |SelectionInUndoStep| + // You should not use this function other than copying existing selection. + void setEndingVisibleSelection(const VisibleSelection&); + + void setParent(CompositeEditCommand*) override; + // Returns |false| if the command failed. e.g. It's aborted. bool apply(); bool isFirstCommand(EditCommand* command) { return !m_commands.isEmpty() && m_commands.front() == command; } - UndoStep* composition() { return m_composition.get(); } - UndoStep* ensureComposition(); - // Append composition from an already applied command. - void appendCommandToComposite(CompositeEditCommand*); + UndoStep* undoStep() { return m_undoStep.get(); } + UndoStep* ensureUndoStep(); + // Append undo step from an already applied command. + void appendCommandToUndoStep(CompositeEditCommand*); virtual bool isReplaceSelectionCommand() const; virtual bool isTypingCommand() const; @@ -224,7 +236,9 @@ private: bool isCompositeEditCommand() const final { return true; } - Member<UndoStep> m_composition; + VisibleSelection m_startingSelection; + VisibleSelection m_endingSelection; + Member<UndoStep> m_undoStep; }; DEFINE_TYPE_CASTS(CompositeEditCommand,
diff --git a/third_party/WebKit/Source/core/editing/commands/CompositeEditCommandTest.cpp b/third_party/WebKit/Source/core/editing/commands/CompositeEditCommandTest.cpp new file mode 100644 index 0000000..9980ae5 --- /dev/null +++ b/third_party/WebKit/Source/core/editing/commands/CompositeEditCommandTest.cpp
@@ -0,0 +1,103 @@ +// 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 "core/editing/commands/CompositeEditCommand.h" + +#include "core/css/StylePropertySet.h" +#include "core/dom/Document.h" +#include "core/editing/EditingTestBase.h" +#include "core/editing/FrameSelection.h" +#include "core/frame/LocalFrame.h" + +namespace blink { + +namespace { + +class SampleCommand final : public CompositeEditCommand { + public: + SampleCommand(Document&); + + void insertNodeBefore( + Node*, + Node* refChild, + EditingState*, + ShouldAssumeContentIsAlwaysEditable = DoNotAssumeContentIsAlwaysEditable); + + // CompositeEditCommand member implementations + void doApply(EditingState*) final {} + InputEvent::InputType inputType() const final { + return InputEvent::InputType::None; + } +}; + +SampleCommand::SampleCommand(Document& document) + : CompositeEditCommand(document) {} + +void SampleCommand::insertNodeBefore( + Node* insertChild, + Node* refChild, + EditingState* editingState, + ShouldAssumeContentIsAlwaysEditable shouldAssumeContentIsAlwaysEditable) { + CompositeEditCommand::insertNodeBefore(insertChild, refChild, editingState, + shouldAssumeContentIsAlwaysEditable); +} + +} // namespace + +class CompositeEditCommandTest : public EditingTestBase {}; + +TEST_F(CompositeEditCommandTest, insertNodeBefore) { + setBodyContent("<div contenteditable><b></b></div>"); + SampleCommand& sample = *new SampleCommand(document()); + Node* insertChild = document().createTextNode("foo"); + Element* refChild = document().querySelector("b"); + Element* div = document().querySelector("div"); + + EditingState editingState; + sample.insertNodeBefore(insertChild, refChild, &editingState); + EXPECT_FALSE(editingState.isAborted()); + EXPECT_EQ("foo<b></b>", div->innerHTML()); +} + +TEST_F(CompositeEditCommandTest, insertNodeBeforeInUneditable) { + setBodyContent("<div><b></b></div>"); + SampleCommand& sample = *new SampleCommand(document()); + Node* insertChild = document().createTextNode("foo"); + Element* refChild = document().querySelector("b"); + + EditingState editingState; + sample.insertNodeBefore(insertChild, refChild, &editingState); + EXPECT_TRUE(editingState.isAborted()); +} + +TEST_F(CompositeEditCommandTest, insertNodeBeforeDisconnectedNode) { + setBodyContent("<div><b></b></div>"); + SampleCommand& sample = *new SampleCommand(document()); + Node* insertChild = document().createTextNode("foo"); + Element* refChild = document().querySelector("b"); + Element* div = document().querySelector("div"); + div->remove(); + + EditingState editingState; + sample.insertNodeBefore(insertChild, refChild, &editingState); + EXPECT_FALSE(editingState.isAborted()); + EXPECT_EQ("<b></b>", div->innerHTML()) + << "InsertNodeBeforeCommand does nothing for disconnected node"; +} + +TEST_F(CompositeEditCommandTest, insertNodeBeforeWithDirtyLayoutTree) { + setBodyContent("<div><b></b></div>"); + SampleCommand& sample = *new SampleCommand(document()); + Node* insertChild = document().createTextNode("foo"); + Element* refChild = document().querySelector("b"); + Element* div = document().querySelector("div"); + div->setAttribute(HTMLNames::contenteditableAttr, "true"); + + EditingState editingState; + sample.insertNodeBefore(insertChild, refChild, &editingState); + EXPECT_FALSE(editingState.isAborted()); + EXPECT_EQ("foo<b></b>", div->innerHTML()); +} + +} // namespace blink
diff --git a/third_party/WebKit/Source/core/editing/commands/EditCommand.cpp b/third_party/WebKit/Source/core/editing/commands/EditCommand.cpp index 4835d1b..a8e51ac 100644 --- a/third_party/WebKit/Source/core/editing/commands/EditCommand.cpp +++ b/third_party/WebKit/Source/core/editing/commands/EditCommand.cpp
@@ -38,8 +38,6 @@ : m_document(&document), m_parent(nullptr) { DCHECK(m_document); DCHECK(m_document->frame()); - setStartingSelection(m_document->frame()->selection().selection()); - setEndingVisibleSelection(m_startingSelection); } EditCommand::~EditCommand() {} @@ -52,28 +50,26 @@ return nullAtom; } -static inline UndoStep* compositionIfPossible(EditCommand* command) { - if (!command->isCompositeEditCommand()) - return 0; - return toCompositeEditCommand(command)->composition(); -} - -void EditCommand::setStartingSelection(const VisibleSelection& selection) { - for (EditCommand* command = this;; command = command->m_parent) { - if (UndoStep* composition = compositionIfPossible(command)) { +// TODO(xiaochengh): Move it to CompositeEditCommand.cpp +void CompositeEditCommand::setStartingSelection( + const VisibleSelection& selection) { + for (CompositeEditCommand* command = this;; command = command->parent()) { + if (UndoStep* undoStep = command->undoStep()) { DCHECK(command->isTopLevelCommand()); - composition->setStartingSelection(selection); + undoStep->setStartingSelection(selection); } command->m_startingSelection = selection; - if (!command->m_parent || command->m_parent->isFirstCommand(command)) + if (!command->parent() || command->parent()->isFirstCommand(command)) break; } } +// TODO(xiaochengh): Move it to CompositeEditCommand.cpp // TODO(yosin): We will make |SelectionInDOMTree| version of // |setEndingSelection()| as primary function instead of wrapper, once // |EditCommand| holds other than |VisibleSelection|. -void EditCommand::setEndingSelection(const SelectionInDOMTree& selection) { +void CompositeEditCommand::setEndingSelection( + const SelectionInDOMTree& selection) { // TODO(editing-dev): The use of // updateStyleAndLayoutIgnorePendingStylesheets // needs to be audited. See http://crbug.com/590369 for more details. @@ -81,13 +77,16 @@ setEndingVisibleSelection(createVisibleSelection(selection)); } +// TODO(xiaochengh): Move it to CompositeEditCommand.cpp // TODO(yosin): We will make |SelectionInDOMTree| version of // |setEndingSelection()| as primary function instead of wrapper. -void EditCommand::setEndingVisibleSelection(const VisibleSelection& selection) { - for (EditCommand* command = this; command; command = command->m_parent) { - if (UndoStep* composition = compositionIfPossible(command)) { +void CompositeEditCommand::setEndingVisibleSelection( + const VisibleSelection& selection) { + for (CompositeEditCommand* command = this; command; + command = command->parent()) { + if (UndoStep* undoStep = command->undoStep()) { DCHECK(command->isTopLevelCommand()); - composition->setEndingSelection(selection); + undoStep->setEndingSelection(selection); } command->m_endingSelection = selection; } @@ -111,8 +110,13 @@ void EditCommand::setParent(CompositeEditCommand* parent) { DCHECK((parent && !m_parent) || (!parent && m_parent)); DCHECK(!parent || !isCompositeEditCommand() || - !toCompositeEditCommand(this)->composition()); + !toCompositeEditCommand(this)->undoStep()); m_parent = parent; +} + +// TODO(xiaochengh): Move it to CompositeEditCommand.cpp +void CompositeEditCommand::setParent(CompositeEditCommand* parent) { + EditCommand::setParent(parent); if (parent) { m_startingSelection = parent->m_endingSelection; m_endingSelection = parent->m_endingSelection; @@ -126,8 +130,6 @@ DEFINE_TRACE(EditCommand) { visitor->trace(m_document); - visitor->trace(m_startingSelection); - visitor->trace(m_endingSelection); visitor->trace(m_parent); }
diff --git a/third_party/WebKit/Source/core/editing/commands/EditCommand.h b/third_party/WebKit/Source/core/editing/commands/EditCommand.h index 34939bc1a..668b52e6 100644 --- a/third_party/WebKit/Source/core/editing/commands/EditCommand.h +++ b/third_party/WebKit/Source/core/editing/commands/EditCommand.h
@@ -41,15 +41,10 @@ public: virtual ~EditCommand(); - void setParent(CompositeEditCommand*); + virtual void setParent(CompositeEditCommand*); virtual InputEvent::InputType inputType() const; - const VisibleSelection& startingSelection() const { - return m_startingSelection; - } - const VisibleSelection& endingSelection() const { return m_endingSelection; } - virtual bool isSimpleEditCommand() const { return false; } virtual bool isCompositeEditCommand() const { return false; } bool isTopLevelCommand() const { return !m_parent; } @@ -67,11 +62,6 @@ Document& document() const { return *m_document.get(); } CompositeEditCommand* parent() const { return m_parent; } - void setStartingSelection(const VisibleSelection&); - void setEndingSelection(const SelectionInDOMTree&); - // TODO(yosin): |setEndingVisibleSelection()| will take |SelectionInUndoStep| - // You should not use this function other than copying existing selection. - void setEndingVisibleSelection(const VisibleSelection&); // TODO(yosin) |isRenderedCharacter()| should be removed, and we should use // |VisiblePosition::characterAfter()| and @@ -80,8 +70,6 @@ private: Member<Document> m_document; - VisibleSelection m_startingSelection; - VisibleSelection m_endingSelection; Member<CompositeEditCommand> m_parent; };
diff --git a/third_party/WebKit/Source/core/editing/commands/EditingState.h b/third_party/WebKit/Source/core/editing/commands/EditingState.h index 608408df..d5feab48 100644 --- a/third_party/WebKit/Source/core/editing/commands/EditingState.h +++ b/third_party/WebKit/Source/core/editing/commands/EditingState.h
@@ -5,6 +5,7 @@ #ifndef EditingState_h #define EditingState_h +#include "core/CoreExport.h" #include "wtf/Allocator.h" #include "wtf/Assertions.h" #include "wtf/Noncopyable.h" @@ -21,7 +22,7 @@ // if (editingState.isAborted()) // return; // -class EditingState final { +class CORE_EXPORT EditingState final { STACK_ALLOCATED(); WTF_MAKE_NONCOPYABLE(EditingState);
diff --git a/third_party/WebKit/Source/core/editing/commands/UndoStep.cpp b/third_party/WebKit/Source/core/editing/commands/UndoStep.cpp index d033b43..bbc47d2 100644 --- a/third_party/WebKit/Source/core/editing/commands/UndoStep.cpp +++ b/third_party/WebKit/Source/core/editing/commands/UndoStep.cpp
@@ -76,8 +76,8 @@ m_commands.push_back(command); } -void UndoStep::append(UndoStep* composition) { - m_commands.appendVector(composition->m_commands); +void UndoStep::append(UndoStep* undoStep) { + m_commands.appendVector(undoStep->m_commands); } void UndoStep::setStartingSelection(const VisibleSelection& selection) {
diff --git a/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.cpp b/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.cpp index ff43d881..a135fbd 100644 --- a/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.cpp +++ b/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.cpp
@@ -87,8 +87,7 @@ } ResourceRequest createAccessControlPreflightRequest( - const ResourceRequest& request, - const SecurityOrigin* securityOrigin) { + const ResourceRequest& request) { const KURL& requestURL = request.url(); DCHECK(requestURL.user().isEmpty());
diff --git a/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.h b/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.h index 354e01c..a53696b 100644 --- a/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.h +++ b/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.h
@@ -142,8 +142,7 @@ CORE_EXPORT bool isOnAccessControlResponseHeaderWhitelist(const String&); CORE_EXPORT ResourceRequest -createAccessControlPreflightRequest(const ResourceRequest&, - const SecurityOrigin*); +createAccessControlPreflightRequest(const ResourceRequest&); CORE_EXPORT void parseAccessControlExposeHeadersAllowList( const String& headerValue,
diff --git a/third_party/WebKit/Source/core/fetch/CrossOriginAccessControlTest.cpp b/third_party/WebKit/Source/core/fetch/CrossOriginAccessControlTest.cpp index 9ce09006..aaf03c3 100644 --- a/third_party/WebKit/Source/core/fetch/CrossOriginAccessControlTest.cpp +++ b/third_party/WebKit/Source/core/fetch/CrossOriginAccessControlTest.cpp
@@ -14,16 +14,7 @@ namespace { -class CreateAccessControlPreflightRequestTest : public ::testing::Test { - protected: - virtual void SetUp() { - m_securityOrigin = SecurityOrigin::createFromString("http://example.com"); - } - - RefPtr<SecurityOrigin> m_securityOrigin; -}; - -TEST_F(CreateAccessControlPreflightRequestTest, LexicographicalOrder) { +TEST(CreateAccessControlPreflightRequestTest, LexicographicalOrder) { ResourceRequest request; request.addHTTPHeaderField("Orange", "Orange"); request.addHTTPHeaderField("Apple", "Red"); @@ -31,22 +22,20 @@ request.addHTTPHeaderField("Content-Type", "application/octet-stream"); request.addHTTPHeaderField("Strawberry", "Red"); - ResourceRequest preflight = - createAccessControlPreflightRequest(request, m_securityOrigin.get()); + ResourceRequest preflight = createAccessControlPreflightRequest(request); EXPECT_EQ("apple,content-type,kiwifruit,orange,strawberry", preflight.httpHeaderField("Access-Control-Request-Headers")); } -TEST_F(CreateAccessControlPreflightRequestTest, ExcludeSimpleHeaders) { +TEST(CreateAccessControlPreflightRequestTest, ExcludeSimpleHeaders) { ResourceRequest request; request.addHTTPHeaderField("Accept", "everything"); request.addHTTPHeaderField("Accept-Language", "everything"); request.addHTTPHeaderField("Content-Language", "everything"); request.addHTTPHeaderField("Save-Data", "on"); - ResourceRequest preflight = - createAccessControlPreflightRequest(request, m_securityOrigin.get()); + ResourceRequest preflight = createAccessControlPreflightRequest(request); // Do not emit empty-valued headers; an empty list of non-"CORS safelisted" // request headers should cause "Access-Control-Request-Headers:" to be @@ -55,37 +44,33 @@ preflight.httpHeaderField("Access-Control-Request-Headers")); } -TEST_F(CreateAccessControlPreflightRequestTest, - ExcludeSimpleContentTypeHeader) { +TEST(CreateAccessControlPreflightRequestTest, ExcludeSimpleContentTypeHeader) { ResourceRequest request; request.addHTTPHeaderField("Content-Type", "text/plain"); - ResourceRequest preflight = - createAccessControlPreflightRequest(request, m_securityOrigin.get()); + ResourceRequest preflight = createAccessControlPreflightRequest(request); // Empty list also; see comment in test above. EXPECT_EQ(nullAtom, preflight.httpHeaderField("Access-Control-Request-Headers")); } -TEST_F(CreateAccessControlPreflightRequestTest, IncludeNonSimpleHeader) { +TEST(CreateAccessControlPreflightRequestTest, IncludeNonSimpleHeader) { ResourceRequest request; request.addHTTPHeaderField("X-Custom-Header", "foobar"); - ResourceRequest preflight = - createAccessControlPreflightRequest(request, m_securityOrigin.get()); + ResourceRequest preflight = createAccessControlPreflightRequest(request); EXPECT_EQ("x-custom-header", preflight.httpHeaderField("Access-Control-Request-Headers")); } -TEST_F(CreateAccessControlPreflightRequestTest, - IncludeNonSimpleContentTypeHeader) { +TEST(CreateAccessControlPreflightRequestTest, + IncludeNonSimpleContentTypeHeader) { ResourceRequest request; request.addHTTPHeaderField("Content-Type", "application/octet-stream"); - ResourceRequest preflight = - createAccessControlPreflightRequest(request, m_securityOrigin.get()); + ResourceRequest preflight = createAccessControlPreflightRequest(request); EXPECT_EQ("content-type", preflight.httpHeaderField("Access-Control-Request-Headers"));
diff --git a/third_party/WebKit/Source/core/loader/DocumentLoader.cpp b/third_party/WebKit/Source/core/loader/DocumentLoader.cpp index a0fa9d4..245fd644 100644 --- a/third_party/WebKit/Source/core/loader/DocumentLoader.cpp +++ b/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
@@ -117,16 +117,28 @@ m_state(NotStarted), m_inDataReceived(false), m_dataBuffer(SharedBuffer::create()) { + DCHECK(m_frame); + // The document URL needs to be added to the head of the list as that is // where the redirects originated. if (m_isClientRedirect) appendRedirect(m_frame->document()->url()); } -FrameLoader* DocumentLoader::frameLoader() const { - if (!m_frame) - return nullptr; - return &m_frame->loader(); +FrameLoader& DocumentLoader::frameLoader() const { + DCHECK(m_frame); + return m_frame->loader(); +} + +FrameLoaderClient& DocumentLoader::frameLoaderClient() const { + DCHECK(m_frame); + FrameLoaderClient* client = m_frame->client(); + // LocalFrame clears its |m_client| only after detaching all DocumentLoaders + // (i.e. calls detachFromFrame() which clears |m_frame|) owned by the + // LocalFrame's FrameLoader. So, if |m_frame| is non nullptr, |client| is + // also non nullptr. + DCHECK(client); + return *client; } DocumentLoader::~DocumentLoader() { @@ -222,8 +234,7 @@ // If a redirection happens during a back/forward navigation, don't restore // any state from the old HistoryItem. There is a provisional history item for // back/forward navigation only. In the other case, clearing it is a no-op. - DCHECK(frameLoader()); - frameLoader()->clearProvisionalHistoryItem(); + frameLoader().clearProvisionalHistoryItem(); } void DocumentLoader::dispatchLinkHeaderPreloads( @@ -236,16 +247,16 @@ } void DocumentLoader::didChangePerformanceTiming() { - if (frame() && frame()->isMainFrame() && m_state >= Committed) { - frameLoader()->client()->didChangePerformanceTiming(); + if (m_frame && m_frame->isMainFrame() && m_state >= Committed) { + frameLoaderClient().didChangePerformanceTiming(); } } void DocumentLoader::didObserveLoadingBehavior( WebLoadingBehaviorFlag behavior) { - if (frame() && frame()->isMainFrame()) { + if (m_frame && m_frame->isMainFrame()) { DCHECK_GE(m_state, Committed); - frameLoader()->client()->didObserveLoadingBehavior(behavior); + frameLoaderClient().didObserveLoadingBehavior(behavior); } } @@ -272,7 +283,7 @@ void DocumentLoader::commitIfReady() { if (m_state < Committed) { m_state = Committed; - frameLoader()->commitProvisionalLoad(); + frameLoader().commitProvisionalLoad(); } } @@ -294,7 +305,7 @@ m_mainResource.get()); } - frameLoader()->loadFailed(this, m_mainResource->resourceError()); + frameLoader().loadFailed(this, m_mainResource->resourceError()); clearMainResourceHandle(); } @@ -311,7 +322,7 @@ timing().setResponseEnd(responseEndTime); commitIfReady(); - if (!frameLoader()) + if (!m_frame) return; if (!maybeCreateArchive()) { @@ -347,6 +358,7 @@ Resource* resource, const ResourceRequest& request, const ResourceResponse& redirectResponse) { + DCHECK(m_frame); DCHECK_EQ(resource, m_mainResource); DCHECK(!redirectResponse.isNull()); m_request = request; @@ -361,7 +373,7 @@ m_fetcher->stopFetching(); return false; } - if (!frameLoader()->shouldContinueForNavigationPolicy( + if (!frameLoader().shouldContinueForNavigationPolicy( m_request, SubstituteData(), this, CheckContentSecurityPolicy, m_navigationType, NavigationPolicyCurrentTab, replacesCurrentHistoryItem(), isClientRedirect(), nullptr)) { @@ -372,7 +384,7 @@ DCHECK(timing().fetchStart()); appendRedirect(requestURL); didRedirect(redirectResponse.url(), requestURL); - frameLoader()->client()->dispatchDidReceiveServerRedirectForProvisionalLoad(); + frameLoaderClient().dispatchDidReceiveServerRedirectForProvisionalLoad(); return true; } @@ -439,7 +451,7 @@ std::unique_ptr<WebDataConsumerHandle> handle) { DCHECK_EQ(m_mainResource, resource); DCHECK(!handle); - DCHECK(frame()); + DCHECK(m_frame); m_applicationCacheHost->didReceiveResponseForMainResource(response); @@ -460,29 +472,29 @@ } if (RuntimeEnabledFeatures::embedderCSPEnforcementEnabled() && - !frameLoader()->requiredCSP().isEmpty()) { + !frameLoader().requiredCSP().isEmpty()) { SecurityOrigin* parentSecurityOrigin = - frame()->tree().parent()->securityContext()->getSecurityOrigin(); + m_frame->tree().parent()->securityContext()->getSecurityOrigin(); if (ContentSecurityPolicy::shouldEnforceEmbeddersPolicy( response, parentSecurityOrigin)) { m_contentSecurityPolicy->addPolicyFromHeaderValue( - frameLoader()->requiredCSP(), ContentSecurityPolicyHeaderTypeEnforce, + frameLoader().requiredCSP(), ContentSecurityPolicyHeaderTypeEnforce, ContentSecurityPolicyHeaderSourceHTTP); } else { ContentSecurityPolicy* embeddingCSP = ContentSecurityPolicy::create(); embeddingCSP->addPolicyFromHeaderValue( - frameLoader()->requiredCSP(), ContentSecurityPolicyHeaderTypeEnforce, + frameLoader().requiredCSP(), ContentSecurityPolicyHeaderTypeEnforce, ContentSecurityPolicyHeaderSourceHTTP); if (!embeddingCSP->subsumes(*m_contentSecurityPolicy)) { String message = "Refused to display '" + response.url().elidedString() + "' because it has not opted-into the following policy " "required by its embedder: '" + - frameLoader()->requiredCSP() + "'."; + frameLoader().requiredCSP() + "'."; ConsoleMessage* consoleMessage = ConsoleMessage::createForRequest( SecurityMessageSource, ErrorMessageLevel, message, response.url(), mainResourceIdentifier()); - frame()->document()->addConsoleMessage(consoleMessage); + m_frame->document()->addConsoleMessage(consoleMessage); cancelLoadAfterCSPDenied(response); return; } @@ -605,7 +617,7 @@ if (isArchiveMIMEType(response().mimeType())) return; commitIfReady(); - if (!frameLoader()) + if (!m_frame) return; commitData(data, length); @@ -630,8 +642,8 @@ // frame have any loads active, so go ahead and kill all the loads. m_fetcher->stopFetching(); - if (frameLoader() && !sentDidFinishLoad()) - frameLoader()->loadFailed(this, ResourceError::cancelledError(url())); + if (m_frame && !sentDidFinishLoad()) + frameLoader().loadFailed(this, ResourceError::cancelledError(url())); // If that load cancellation triggered another detach, leave. // (fast/frames/detach-frame-nested-no-crash.html is an example of this.) @@ -692,7 +704,7 @@ return false; if (m_request.url().isEmpty() && - !frameLoader()->stateMachine()->creatingInitialEmptyDocument()) + !frameLoader().stateMachine()->creatingInitialEmptyDocument()) m_request.setURL(blankURL()); m_response = ResourceResponse(m_request.url(), "text/html", 0, nullAtom, String());
diff --git a/third_party/WebKit/Source/core/loader/DocumentLoader.h b/third_party/WebKit/Source/core/loader/DocumentLoader.h index 9a4acde..1117913 100644 --- a/third_party/WebKit/Source/core/loader/DocumentLoader.h +++ b/third_party/WebKit/Source/core/loader/DocumentLoader.h
@@ -60,6 +60,7 @@ class DocumentInit; class LocalFrame; class FrameLoader; +class FrameLoaderClient; class ResourceTimingInfo; class WebDocumentSubresourceFilter; struct ViewportDescriptionWrapper; @@ -74,6 +75,8 @@ const ResourceRequest& request, const SubstituteData& data, ClientRedirectPolicy clientRedirectPolicy) { + DCHECK(frame); + return new DocumentLoader(frame, request, data, clientRedirectPolicy); } ~DocumentLoader() override; @@ -200,7 +203,10 @@ const KURL& overridingURL = KURL()); void endWriting(); - FrameLoader* frameLoader() const; + // Use these method only where it's guaranteed that |m_frame| hasn't been + // cleared. + FrameLoader& frameLoader() const; + FrameLoaderClient& frameLoaderClient() const; void commitIfReady(); void commitData(const char* bytes, size_t length); @@ -210,6 +216,8 @@ void finishedLoading(double finishTime); void cancelLoadAfterCSPDenied(const ResourceResponse&); + + // RawResourceClient implementation bool redirectReceived(Resource*, const ResourceRequest&, const ResourceResponse&) final; @@ -217,10 +225,13 @@ const ResourceResponse&, std::unique_ptr<WebDataConsumerHandle>) final; void dataReceived(Resource*, const char* data, size_t length) final; - void processData(const char* data, size_t length); + + // ResourceClient implementation void notifyFinished(Resource*) final; String debugName() const override { return "DocumentLoader"; } + void processData(const char* data, size_t length); + bool maybeLoadEmpty(); bool isRedirectAfterPost(const ResourceRequest&, const ResourceResponse&);
diff --git a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp index cd542a6..885be895 100644 --- a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp +++ b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
@@ -455,16 +455,15 @@ effectiveAllowCredentials(), crossOriginRequest.httpMethod(), crossOriginRequest.httpHeaderFields()); if (canSkipPreflight && !shouldForcePreflight) { - if (getSecurityOrigin()) - crossOriginRequest.setHTTPOrigin(getSecurityOrigin()); - if (m_overrideReferrer) - crossOriginRequest.setHTTPReferrer(m_referrerAfterRedirect); - prepareCrossOriginRequest(crossOriginRequest); loadRequest(crossOriginRequest, crossOriginOptions); } else { - ResourceRequest preflightRequest = createAccessControlPreflightRequest( - crossOriginRequest, getSecurityOrigin()); + ResourceRequest preflightRequest = + createAccessControlPreflightRequest(crossOriginRequest); + // TODO(tyoshino): Call prepareCrossOriginRequest(preflightRequest) to + // also set the referrer header. + if (getSecurityOrigin()) + preflightRequest.setHTTPOrigin(getSecurityOrigin()); // Create a ResourceLoaderOptions for preflight. ResourceLoaderOptions preflightOptions = crossOriginOptions; @@ -473,7 +472,6 @@ m_actualRequest = crossOriginRequest; m_actualOptions = crossOriginOptions; - prepareCrossOriginRequest(crossOriginRequest); loadRequest(preflightRequest, preflightOptions); } }
diff --git a/third_party/WebKit/Source/core/loader/EmptyClients.cpp b/third_party/WebKit/Source/core/loader/EmptyClients.cpp index 6fc8717d..c9d0e5e 100644 --- a/third_party/WebKit/Source/core/loader/EmptyClients.cpp +++ b/third_party/WebKit/Source/core/loader/EmptyClients.cpp
@@ -147,6 +147,8 @@ const ResourceRequest& request, const SubstituteData& substituteData, ClientRedirectPolicy clientRedirectPolicy) { + DCHECK(frame); + return DocumentLoader::create(frame, request, substituteData, clientRedirectPolicy); }
diff --git a/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp b/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp index 3098477a..77a7519 100644 --- a/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp +++ b/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
@@ -184,7 +184,7 @@ const bool is2G = networkStateNotifier().connectionType() == WebConnectionTypeCellular2G; WebEffectiveConnectionType effectiveConnection = - document.frame()->loader().client()->getEffectiveConnectionType(); + document.frame()->client()->getEffectiveConnectionType(); const bool is2GOrLike2G = is2G || isConnectionEffectively2G(effectiveConnection); @@ -228,6 +228,10 @@ return frame; } +FrameLoaderClient* FrameFetchContext::frameLoaderClient() const { + return frame()->client(); +} + void FrameFetchContext::addAdditionalRequestHeaders(ResourceRequest& request, FetchResourceType type) { bool isMainResource = type == FetchMainResource; @@ -391,7 +395,7 @@ void FrameFetchContext::prepareRequest(ResourceRequest& request) { frame()->loader().applyUserAgent(request); - frame()->loader().client()->dispatchWillSendRequest(request); + frameLoaderClient()->dispatchWillSendRequest(request); } void FrameFetchContext::dispatchWillSendRequest( @@ -495,7 +499,7 @@ ResourceRequest request(resource->url()); request.setFrameType(frameType); request.setRequestContext(requestContext); - frame()->loader().client()->dispatchDidLoadResourceFromMemoryCache( + frameLoaderClient()->dispatchDidLoadResourceFromMemoryCache( request, resource->response()); dispatchWillSendRequest(identifier, request, ResourceResponse(), resource->options().initiatorInfo); @@ -591,7 +595,7 @@ } bool FrameFetchContext::allowImage(bool imagesEnabled, const KURL& url) const { - return frame()->loader().client()->allowImage(imagesEnabled, url); + return frameLoaderClient()->allowImage(imagesEnabled, url); } void FrameFetchContext::printAccessDeniedMessage(const KURL& url) const { @@ -732,17 +736,17 @@ if (type == Resource::Script || type == Resource::ImportResource) { DCHECK(frame()); - if (!frame()->loader().client()->allowScriptFromSource( + if (!frameLoaderClient()->allowScriptFromSource( !frame()->settings() || frame()->settings()->getScriptEnabled(), url)) { - frame()->loader().client()->didNotAllowScript(); + frameLoaderClient()->didNotAllowScript(); // TODO(estark): Use a different ResourceRequestBlockedReason here, since // this check has nothing to do with CSP. https://crbug.com/600795 return ResourceRequestBlockedReason::CSP; } } else if (type == Resource::Media || type == Resource::TextTrack) { DCHECK(frame()); - if (!frame()->loader().client()->allowMedia(url)) + if (!frameLoaderClient()->allowMedia(url)) return ResourceRequestBlockedReason::Other; } @@ -819,20 +823,20 @@ // m_documentLoader is null while loading resources from an HTML import. In // such cases whether the request is controlled by ServiceWorker or not is // determined by the document loader of the frame. - return frame()->loader().client()->isControlledByServiceWorker( + return frameLoaderClient()->isControlledByServiceWorker( *frame()->loader().documentLoader()); } int64_t FrameFetchContext::serviceWorkerID() const { DCHECK(m_documentLoader || frame()->loader().documentLoader()); if (m_documentLoader) { - return m_documentLoader->frame()->loader().client()->serviceWorkerID( + return m_documentLoader->frame()->client()->serviceWorkerID( *m_documentLoader); } // m_documentLoader is null while loading resources from an HTML import. // In such cases a service worker ID could be retrieved from the document // loader of the frame. - return frame()->loader().client()->serviceWorkerID( + return frameLoaderClient()->serviceWorkerID( *frame()->loader().documentLoader()); } @@ -1046,7 +1050,7 @@ } frame()->loader().progress().incrementProgress(identifier, response); - frame()->loader().client()->dispatchDidReceiveResponse(response); + frameLoaderClient()->dispatchDidReceiveResponse(response); DocumentLoader* documentLoader = masterDocumentLoader(); InspectorInstrumentation::didReceiveResourceResponse( frame(), identifier, documentLoader, response, resource);
diff --git a/third_party/WebKit/Source/core/loader/FrameFetchContext.h b/third_party/WebKit/Source/core/loader/FrameFetchContext.h index 7d54e3b..92c9b49 100644 --- a/third_party/WebKit/Source/core/loader/FrameFetchContext.h +++ b/third_party/WebKit/Source/core/loader/FrameFetchContext.h
@@ -46,6 +46,7 @@ class ClientHintsPreferences; class Document; class DocumentLoader; +class FrameLoaderClient; class LocalFrame; class ResourceError; class ResourceResponse; @@ -175,6 +176,9 @@ LocalFrame* frameOfImportsController() const; LocalFrame* frame() const; + + FrameLoaderClient* frameLoaderClient() const; + void printAccessDeniedMessage(const KURL&) const; ResourceRequestBlockedReason canRequestInternal( Resource::Type,
diff --git a/third_party/WebKit/Source/core/loader/ProgressTracker.cpp b/third_party/WebKit/Source/core/loader/ProgressTracker.cpp index 4618b7d..e94adab3 100644 --- a/third_party/WebKit/Source/core/loader/ProgressTracker.cpp +++ b/third_party/WebKit/Source/core/loader/ProgressTracker.cpp
@@ -99,9 +99,13 @@ m_finishedParsing = false; } +FrameLoaderClient* ProgressTracker::frameLoaderClient() const { + return m_frame->client(); +} + void ProgressTracker::progressStarted() { if (!m_frame->isLoading()) - m_frame->loader().client()->didStartLoading(NavigationToDifferentDocument); + frameLoaderClient()->didStartLoading(NavigationToDifferentDocument); reset(); m_progressValue = initialProgressValue; m_frame->setIsLoading(true); @@ -113,7 +117,7 @@ m_frame->setIsLoading(false); sendFinalProgress(); reset(); - m_frame->loader().client()->didStopLoading(); + frameLoaderClient()->didStopLoading(); InspectorInstrumentation::frameStoppedLoading(m_frame); } @@ -126,7 +130,7 @@ if (m_progressValue == 1) return; m_progressValue = 1; - m_frame->loader().client()->progressEstimateChanged(m_progressValue); + frameLoaderClient()->progressEstimateChanged(m_progressValue); } void ProgressTracker::willStartLoading(unsigned long identifier, @@ -220,7 +224,7 @@ m_progressValue - m_lastNotifiedProgressValue; if (notificationProgressDelta >= progressNotificationInterval || notifiedProgressTimeDelta >= progressNotificationTimeInterval) { - m_frame->loader().client()->progressEstimateChanged(m_progressValue); + frameLoaderClient()->progressEstimateChanged(m_progressValue); m_lastNotifiedProgressValue = m_progressValue; m_lastNotifiedProgressTime = now; }
diff --git a/third_party/WebKit/Source/core/loader/ProgressTracker.h b/third_party/WebKit/Source/core/loader/ProgressTracker.h index 46dd165..3ecbe24 100644 --- a/third_party/WebKit/Source/core/loader/ProgressTracker.h +++ b/third_party/WebKit/Source/core/loader/ProgressTracker.h
@@ -37,6 +37,7 @@ namespace blink { +class FrameLoaderClient; class LocalFrame; class ResourceResponse; struct ProgressItem; @@ -70,8 +71,8 @@ private: explicit ProgressTracker(LocalFrame*); - void incrementProgressForMainResourceOnly(unsigned long identifier, - int length); + FrameLoaderClient* frameLoaderClient() const; + void maybeSendProgress(); void sendFinalProgress(); void reset();
diff --git a/third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp b/third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp index 5ed2efe..01b2c84 100644 --- a/third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp +++ b/third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp
@@ -680,6 +680,8 @@ const ResourceRequest& request, const SubstituteData& data, ClientRedirectPolicy clientRedirectPolicy) { + DCHECK(frame); + WebDataSourceImpl* ds = WebDataSourceImpl::create(frame, request, data, clientRedirectPolicy); if (m_webFrame->client())
diff --git a/third_party/WebKit/Source/web/WebDataSourceImpl.cpp b/third_party/WebKit/Source/web/WebDataSourceImpl.cpp index be9c293..10f8721 100644 --- a/third_party/WebKit/Source/web/WebDataSourceImpl.cpp +++ b/third_party/WebKit/Source/web/WebDataSourceImpl.cpp
@@ -45,6 +45,8 @@ const ResourceRequest& request, const SubstituteData& data, ClientRedirectPolicy clientRedirectPolicy) { + DCHECK(frame); + return new WebDataSourceImpl(frame, request, data, clientRedirectPolicy); }
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index b8ed3e9..744dac8 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -8590,6 +8590,17 @@ </summary> </histogram> +<histogram name="CustomTabs.MayLaunchUrlType" enum="MayLaunchUrlType"> + <owner>lizeb@chromium.org</owner> + <summary> + Android only. Which flavors of mayLaunchUrl() were used before a tab launch. + Can be low and/or high confidence. Recorded at the same time as + CustomTabs.WarmupStateOnLaunch for the buckets "Session, No Warmup, + Warmup called from another UID", "Session, No Warmup" and + "Session, Warmup". + </summary> +</histogram> + <histogram name="CustomTabs.NonDefaultSessionPrerenderMatched" enum="BooleanMatched"> <owner>lizeb@chromium.org</owner> @@ -39061,6 +39072,9 @@ </histogram> <histogram name="NewTabPage.ContentSuggestions.MenuOpenedScore" units="score"> + <obsolete> + Replaced by NewTabPage.ContentSuggestions.MenuOpenedScoreNormalized. + </obsolete> <owner>treib@chromium.org</owner> <summary> Android: The relevance score of a suggestion card on the NTP whose @@ -39069,6 +39083,20 @@ </summary> </histogram> +<histogram name="NewTabPage.ContentSuggestions.MenuOpenedScoreNormalized" + enum="NormalizedScore"> + <owner>tschumann@chromium.org</owner> + <summary> + Android: The relevance score of a suggestion card on the NTP whose + long-press menu was opened, analogous to + NewTabPage.ContentSuggestions.OpenedScoreNormalized. Scores (which are + typically floats within (0,1]) get reported as discrete integers within + [1,10]. For instance, the discrete value 1 represents score values from + (0.0, 0.1]. The discrete value 11 is the overflow bucket for unexpectedly + high scores. + </summary> +</histogram> + <histogram name="NewTabPage.ContentSuggestions.MoreButtonClicked" units="index"> <owner>treib@chromium.org</owner> <summary> @@ -39154,6 +39182,9 @@ </histogram> <histogram name="NewTabPage.ContentSuggestions.OpenedScore" units="score"> + <obsolete> + Replaced by NewTabPage.ContentSuggestions.OpenedScoreNormalized. + </obsolete> <owner>treib@chromium.org</owner> <summary> Android: The score of a suggestion card on the NTP that is clicked through @@ -39162,6 +39193,20 @@ </summary> </histogram> +<histogram name="NewTabPage.ContentSuggestions.OpenedScoreNormalized" + enum="NormalizedScore"> + <owner>tschumann@chromium.org</owner> + <summary> + Android: The score of a suggestion card on the NTP that is clicked through + to the host website of the content. The recorded score is from the moment + the suggestion was fetched, it could have changed since. Scores (which are + typically floats within (0,1]) get reported as discrete integers within + [1,10]. For instance, the discrete value 1 represents score values from + (0.0, 0.1]. The discrete value 11 is the overflow bucket for unexpectedly + high scores. + </summary> +</histogram> + <histogram name="NewTabPage.ContentSuggestions.Shown" units="index"> <owner>treib@chromium.org</owner> <summary> @@ -39184,6 +39229,9 @@ </histogram> <histogram name="NewTabPage.ContentSuggestions.ShownScore" units="score"> + <obsolete> + Replaced by NewTabPage.ContentSuggestions.ShownScoreNormalized. + </obsolete> <owner>treib@chromium.org</owner> <summary> Android: The score of a suggestion card that was shown on the NTP. A card is @@ -39192,6 +39240,20 @@ </summary> </histogram> +<histogram name="NewTabPage.ContentSuggestions.ShownScoreNormalized" + enum="NormalizedScore"> + <owner>tschumann@chromium.org</owner> + <summary> + Android: The score of a suggestion card that was shown on the NTP. A card is + considered shown when at least 1/3 of its height is visible on the screen. + For each card, at most one impression is recorded per NTP instance. Scores + (which are typically floats within (0,1]) get reported as discrete integers + within [1,10]. For instance, the discrete value 1 represents score values + from (0.0, 0.1]. The discrete value 11 is the overflow bucket for + unexpectedly high scores. + </summary> +</histogram> + <histogram name="NewTabPage.ContentSuggestions.TimeSinceLastBackgroundFetch" units="ms"> <owner>markusheintz@chromium.org</owner> @@ -96542,6 +96604,13 @@ <int value="2" label="Non-Secure"/> </enum> +<enum name="MayLaunchUrlType" type="int"> + <int value="0" label="No MayLaunchUrl() call."/> + <int value="1" label="Low confidence."/> + <int value="2" label="High confidence."/> + <int value="3" label="Low and High confidence."/> +</enum> + <enum name="MediaCommand" type="int"> <int value="0" label="Resume"/> <int value="1" label="Pause"/> @@ -98288,6 +98357,21 @@ <int value="1" label="Timed out"/> </enum> +<enum name="NormalizedScore" type="int"> + <int value="0" label="Underflow bucket"/> + <int value="1" label="Score in (0, 0.1]"/> + <int value="2" label="Score in (0.1, 0.2]"/> + <int value="3" label="Score in (0.2, 0.3]"/> + <int value="4" label="Score in (0.3, 0.4]"/> + <int value="5" label="Score in (0.4, 0.5]"/> + <int value="6" label="Score in (0.5, 0.6]"/> + <int value="7" label="Score in (0.6, 0.7]"/> + <int value="8" label="Score in (0.7, 0.8]"/> + <int value="9" label="Score in (0.8, 0.9]"/> + <int value="10" label="Score in (0.9, 1.0]"/> + <int value="11" label="Overflow bucket"/> +</enum> + <enum name="NoStatePrefetchResponseType" type="int"> <int value="0" label="Sub-resource, cacheable"/> <int value="1" label="Sub-resource, no-store"/> @@ -110478,15 +110562,21 @@ <affected-histogram name="NewTabPage.ContentSuggestions.MenuOpened"/> <affected-histogram name="NewTabPage.ContentSuggestions.MenuOpenedAge"/> <affected-histogram name="NewTabPage.ContentSuggestions.MenuOpenedScore"/> + <affected-histogram + name="NewTabPage.ContentSuggestions.MenuOpenedScoreNormalized"/> <affected-histogram name="NewTabPage.ContentSuggestions.MoreButtonClicked"/> <affected-histogram name="NewTabPage.ContentSuggestions.MoreButtonShown"/> <affected-histogram name="NewTabPage.ContentSuggestions.OpenDisposition"/> <affected-histogram name="NewTabPage.ContentSuggestions.Opened"/> <affected-histogram name="NewTabPage.ContentSuggestions.OpenedAge"/> <affected-histogram name="NewTabPage.ContentSuggestions.OpenedScore"/> + <affected-histogram + name="NewTabPage.ContentSuggestions.OpenedScoreNormalized"/> <affected-histogram name="NewTabPage.ContentSuggestions.Shown"/> <affected-histogram name="NewTabPage.ContentSuggestions.ShownAge"/> <affected-histogram name="NewTabPage.ContentSuggestions.ShownScore"/> + <affected-histogram + name="NewTabPage.ContentSuggestions.ShownScoreNormalized"/> <affected-histogram name="NewTabPage.ContentSuggestions.VisitDuration"/> </histogram_suffixes>