diff --git a/DEPS b/DEPS index 2474de15..34ed576 100644 --- a/DEPS +++ b/DEPS
@@ -235,7 +235,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' + '@' + '488d0d83a207c723d646fdd6c4ed2e9e10ea98b8', # commit position 18498 + Var('chromium_git') + '/external/webrtc/trunk/webrtc.git' + '@' + '5c4d199d7c0358a9b8f35707e0f6de983d04232e', # commit position 18506 'src/third_party/openmax_dl': Var('chromium_git') + '/external/webrtc/deps/third_party/openmax.git' + '@' + Var('openmax_dl_revision'),
diff --git a/base/files/file_path.cc b/base/files/file_path.cc index 734d522d..c0d6c12 100644 --- a/base/files/file_path.cc +++ b/base/files/file_path.cc
@@ -492,7 +492,7 @@ DCHECK(!IsPathAbsolute(appended)); - if (path_.compare(kCurrentDirectory) == 0) { + if (path_.compare(kCurrentDirectory) == 0 && !appended.empty()) { // Append normally doesn't do any normalization, but as a special case, // when appending to kCurrentDirectory, just return a new path for the // component argument. Appending component to kCurrentDirectory would
diff --git a/base/files/file_path_unittest.cc b/base/files/file_path_unittest.cc index 9f33952..dbc47d15f 100644 --- a/base/files/file_path_unittest.cc +++ b/base/files/file_path_unittest.cc
@@ -239,6 +239,7 @@ const struct BinaryTestData cases[] = { { { FPL(""), FPL("cc") }, FPL("cc") }, { { FPL("."), FPL("ff") }, FPL("ff") }, + { { FPL("."), FPL("") }, FPL(".") }, { { FPL("/"), FPL("cc") }, FPL("/cc") }, { { FPL("/aa"), FPL("") }, FPL("/aa") }, { { FPL("/aa/"), FPL("") }, FPL("/aa") },
diff --git a/base/i18n/file_util_icu.cc b/base/i18n/file_util_icu.cc index 6189577..08ff101 100644 --- a/base/i18n/file_util_icu.cc +++ b/base/i18n/file_util_icu.cc
@@ -174,9 +174,9 @@ void NormalizeFileNameEncoding(FilePath* file_name) { #if defined(OS_CHROMEOS) std::string normalized_str; - if (ConvertToUtf8AndNormalize(file_name->BaseName().value(), - kCodepageUTF8, - &normalized_str)) { + if (ConvertToUtf8AndNormalize(file_name->BaseName().value(), kCodepageUTF8, + &normalized_str) && + !normalized_str.empty()) { *file_name = file_name->DirName().Append(FilePath(normalized_str)); } #endif
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java b/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java index e276039..ae8b5e32 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java
@@ -4,7 +4,6 @@ package org.chromium.chrome.browser.photo_picker; -import android.app.Activity; import android.content.Context; import android.content.DialogInterface; import android.content.res.Configuration; @@ -18,9 +17,11 @@ import android.widget.Button; import android.widget.RelativeLayout; +import org.chromium.base.DiscardableReferencePool.DiscardableReference; import org.chromium.base.VisibleForTesting; import org.chromium.base.metrics.RecordHistogram; import org.chromium.chrome.R; +import org.chromium.chrome.browser.ChromeActivity; import org.chromium.chrome.browser.util.ConversionUtils; import org.chromium.chrome.browser.widget.selection.SelectableListLayout; import org.chromium.chrome.browser.widget.selection.SelectionDelegate; @@ -50,8 +51,8 @@ // The view containing the RecyclerView and the toolbar, etc. private SelectableListLayout<PickerBitmap> mSelectableListLayout; - // Our context. - private Context mContext; + // Our activity. + private ChromeActivity mActivity; // The list of images on disk, sorted by last-modified first. private List<PickerBitmap> mPickerBitmaps; @@ -80,13 +81,19 @@ // The {@link SelectionDelegate} keeping track of which images are selected. private SelectionDelegate<PickerBitmap> mSelectionDelegate; - // A low-resolution cache for images. Helpful for cache misses from the high-resolution cache - // to avoid showing gray squares (we show pixelated versions instead until image can be loaded - // off disk, which is much less jarring). - private LruCache<String, Bitmap> mLowResBitmaps; + // A low-resolution cache for images, lazily created. Helpful for cache misses from the + // high-resolution cache to avoid showing gray squares (we show pixelated versions instead until + // image can be loaded off disk, which is much less jarring). + private DiscardableReference<LruCache<String, Bitmap>> mLowResBitmaps; - // A high-resolution cache for images. - private LruCache<String, Bitmap> mHighResBitmaps; + // A high-resolution cache for images, lazily created. + private DiscardableReference<LruCache<String, Bitmap>> mHighResBitmaps; + + // The size of the low-res cache. + private int mCacheSizeLarge; + + // The size of the high-res cache. + private int mCacheSizeSmall; /** * The number of columns to show. Note: mColumns and mPadding (see below) should both be even @@ -113,21 +120,13 @@ // A list of files to use for testing (instead of reading files on disk). private static List<PickerBitmap> sTestFiles; + @SuppressWarnings("unchecked") // mSelectableListLayout public PickerCategoryView(Context context) { super(context); - postConstruction(context); - } - - /** - * A helper function for initializing the PickerCategoryView. - * @param context The context to use. - */ - @SuppressWarnings("unchecked") // mSelectableListLayout - private void postConstruction(Context context) { - mContext = context; + mActivity = (ChromeActivity) context; mDecoderServiceHost = new DecoderServiceHost(this); - mDecoderServiceHost.bind(mContext); + mDecoderServiceHost.bind(mActivity); mSelectionDelegate = new SelectionDelegate<PickerBitmap>(); @@ -147,17 +146,15 @@ calculateGridMetrics(); - mLayoutManager = new GridLayoutManager(mContext, mColumns); + mLayoutManager = new GridLayoutManager(mActivity, mColumns); mRecyclerView.setHasFixedSize(true); mRecyclerView.setLayoutManager(mLayoutManager); mSpacingDecoration = new GridSpacingItemDecoration(mColumns, mPadding); mRecyclerView.addItemDecoration(mSpacingDecoration); final long maxMemory = ConversionUtils.bytesToKilobytes(Runtime.getRuntime().maxMemory()); - final long cacheSizeLarge = maxMemory / 2; // 1/2 of the available memory. - final long cacheSizeSmall = maxMemory / 8; // 1/8th of the available memory. - mLowResBitmaps = new LruCache<String, Bitmap>((int) (cacheSizeSmall)); - mHighResBitmaps = new LruCache<String, Bitmap>((int) (cacheSizeLarge)); + mCacheSizeLarge = (int) (maxMemory / 2); // 1/2 of the available memory. + mCacheSizeSmall = (int) (maxMemory / 8); // 1/8th of the available memory. } @Override @@ -181,7 +178,7 @@ } if (mDecoderServiceHost != null) { - mDecoderServiceHost.unbind(mContext); + mDecoderServiceHost.unbind(mActivity); mDecoderServiceHost = null; } } @@ -284,11 +281,19 @@ } public LruCache<String, Bitmap> getLowResBitmaps() { - return mLowResBitmaps; + if (mLowResBitmaps == null || mLowResBitmaps.get() == null) { + mLowResBitmaps = + mActivity.getReferencePool().put(new LruCache<String, Bitmap>(mCacheSizeSmall)); + } + return mLowResBitmaps.get(); } public LruCache<String, Bitmap> getHighResBitmaps() { - return mHighResBitmaps; + if (mHighResBitmaps == null || mHighResBitmaps.get() == null) { + mHighResBitmaps = + mActivity.getReferencePool().put(new LruCache<String, Bitmap>(mCacheSizeLarge)); + } + return mHighResBitmaps.get(); } public boolean isMultiSelectAllowed() { @@ -316,12 +321,12 @@ */ private void calculateGridMetrics() { Rect appRect = new Rect(); - ((Activity) mContext).getWindow().getDecorView().getWindowVisibleDisplayFrame(appRect); + mActivity.getWindow().getDecorView().getWindowVisibleDisplayFrame(appRect); int width = appRect.width(); int minSize = - mContext.getResources().getDimensionPixelSize(R.dimen.photo_picker_tile_min_size); - mPadding = mContext.getResources().getDimensionPixelSize(R.dimen.photo_picker_tile_gap); + mActivity.getResources().getDimensionPixelSize(R.dimen.photo_picker_tile_min_size); + mPadding = mActivity.getResources().getDimensionPixelSize(R.dimen.photo_picker_tile_gap); mColumns = Math.max(1, (width - mPadding) / (minSize + mPadding)); mImageSize = (width - mPadding * (mColumns + 1)) / (mColumns);
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn index 3dc58f03..19eac32 100644 --- a/chrome/browser/BUILD.gn +++ b/chrome/browser/BUILD.gn
@@ -840,6 +840,8 @@ "page_load_metrics/observers/https_engagement_metrics/https_engagement_service.h", "page_load_metrics/observers/https_engagement_metrics/https_engagement_service_factory.cc", "page_load_metrics/observers/https_engagement_metrics/https_engagement_service_factory.h", + "page_load_metrics/observers/loading_predictor_page_load_metrics_observer.cc", + "page_load_metrics/observers/loading_predictor_page_load_metrics_observer.h", "page_load_metrics/observers/lofi_page_load_metrics_observer.cc", "page_load_metrics/observers/lofi_page_load_metrics_observer.h", "page_load_metrics/observers/media_page_load_metrics_observer.cc", @@ -854,8 +856,6 @@ "page_load_metrics/observers/previews_page_load_metrics_observer.h", "page_load_metrics/observers/protocol_page_load_metrics_observer.cc", "page_load_metrics/observers/protocol_page_load_metrics_observer.h", - "page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.cc", - "page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.h", "page_load_metrics/observers/resource_tracking_page_load_metrics_observer.cc", "page_load_metrics/observers/resource_tracking_page_load_metrics_observer.h", "page_load_metrics/observers/service_worker_page_load_metrics_observer.cc", @@ -975,6 +975,8 @@ "predictors/glowplug_key_value_data.h", "predictors/glowplug_key_value_table.cc", "predictors/glowplug_key_value_table.h", + "predictors/loading_data_collector.cc", + "predictors/loading_data_collector.h", "predictors/loading_predictor.cc", "predictors/loading_predictor.h", "predictors/loading_predictor_android.cc",
diff --git a/chrome/browser/net/loading_predictor_observer.cc b/chrome/browser/net/loading_predictor_observer.cc index 8d48d01..af97a61 100644 --- a/chrome/browser/net/loading_predictor_observer.cc +++ b/chrome/browser/net/loading_predictor_observer.cc
@@ -21,7 +21,7 @@ using content::BrowserThread; using predictors::LoadingPredictor; -using predictors::ResourcePrefetchPredictor; +using predictors::LoadingDataCollector; using URLRequestSummary = predictors::ResourcePrefetchPredictor::URLRequestSummary; @@ -98,7 +98,7 @@ if (resource_type == content::RESOURCE_TYPE_MAIN_FRAME) ReportMainFrameRequestStats(MAIN_FRAME_REQUEST_STATS_TOTAL_REQUESTS); - if (!ResourcePrefetchPredictor::ShouldRecordRequest(request, resource_type)) + if (!LoadingDataCollector::ShouldRecordRequest(request, resource_type)) return; auto summary = base::MakeUnique<URLRequestSummary>(); @@ -130,12 +130,11 @@ ReportMainFrameRequestStats(MAIN_FRAME_REQUEST_STATS_TOTAL_REDIRECTS); } - if (!ResourcePrefetchPredictor::ShouldRecordRedirect(request)) + if (!LoadingDataCollector::ShouldRecordRedirect(request)) return; auto summary = base::MakeUnique<URLRequestSummary>(); - if (!ResourcePrefetchPredictor::URLRequestSummary::SummarizeResponse( - *request, summary.get())) { + if (!URLRequestSummary::SummarizeResponse(*request, summary.get())) { return; } summary->redirect_url = redirect_url; @@ -168,11 +167,10 @@ ReportMainFrameRequestStats(MAIN_FRAME_REQUEST_STATS_TOTAL_RESPONSES); } - if (!ResourcePrefetchPredictor::ShouldRecordResponse(request)) + if (!LoadingDataCollector::ShouldRecordResponse(request)) return; auto summary = base::MakeUnique<URLRequestSummary>(); - if (!ResourcePrefetchPredictor::URLRequestSummary::SummarizeResponse( - *request, summary.get())) { + if (!URLRequestSummary::SummarizeResponse(*request, summary.get())) { return; } @@ -202,7 +200,7 @@ } if (summary->resource_type == content::RESOURCE_TYPE_MAIN_FRAME) predictor_->OnMainFrameRequest(*summary); - predictor_->resource_prefetch_predictor()->RecordURLRequest(*summary); + predictor_->loading_data_collector()->RecordURLRequest(*summary); } void LoadingPredictorObserver::OnRequestRedirectedOnUIThread( @@ -217,7 +215,7 @@ } if (summary->resource_type == content::RESOURCE_TYPE_MAIN_FRAME) predictor_->OnMainFrameRedirect(*summary); - predictor_->resource_prefetch_predictor()->RecordURLRedirect(*summary); + predictor_->loading_data_collector()->RecordURLRedirect(*summary); } void LoadingPredictorObserver::OnResponseStartedOnUIThread( @@ -232,7 +230,7 @@ } if (summary->resource_type == content::RESOURCE_TYPE_MAIN_FRAME) predictor_->OnMainFrameResponse(*summary); - predictor_->resource_prefetch_predictor()->RecordURLResponse(*summary); + predictor_->loading_data_collector()->RecordURLResponse(*summary); } } // namespace chrome_browser_net
diff --git a/chrome/browser/ntp_snippets/download_suggestions_provider.cc b/chrome/browser/ntp_snippets/download_suggestions_provider.cc index 56dc64ac..c3bdf67 100644 --- a/chrome/browser/ntp_snippets/download_suggestions_provider.cc +++ b/chrome/browser/ntp_snippets/download_suggestions_provider.cc
@@ -45,7 +45,7 @@ namespace { const int kDefaultMaxSuggestionsCount = 5; -const int kDefaultMaxDownloadAgeHours = 6 * 7 * 24; // 6 weeks +const int kDefaultMaxDownloadAgeHours = 24; const char kAssetDownloadsPrefix = 'D'; const char kOfflinePageDownloadsPrefix = 'O';
diff --git a/chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc b/chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc index 34d8e02..aa5f26e 100644 --- a/chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc +++ b/chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc
@@ -88,7 +88,7 @@ namespace { -const int kDefaultMaxDownloadAgeHours = 6 * 7 * 24; +const int kDefaultMaxDownloadAgeHours = 24; base::Time CalculateDummyNowTime() { base::Time now; @@ -190,7 +190,7 @@ base::Time current_time = base::Time::Now(); for (int id : ids) { result.push_back(CreateDummyAssetDownload(id, current_time)); - current_time -= base::TimeDelta::FromDays(1); + current_time -= base::TimeDelta::FromMinutes(1); } return result; }
diff --git a/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer.cc similarity index 72% rename from chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.cc rename to chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer.cc index faa43027..aaaa98b 100644 --- a/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer.cc
@@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.h" +#include "chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer.h" #include <memory> @@ -26,32 +26,36 @@ } // namespace internal // static -std::unique_ptr<ResourcePrefetchPredictorPageLoadMetricsObserver> -ResourcePrefetchPredictorPageLoadMetricsObserver::CreateIfNeeded( +std::unique_ptr<LoadingPredictorPageLoadMetricsObserver> +LoadingPredictorPageLoadMetricsObserver::CreateIfNeeded( content::WebContents* web_contents) { auto* loading_predictor = predictors::LoadingPredictorFactory::GetForProfile( Profile::FromBrowserContext(web_contents->GetBrowserContext())); if (!loading_predictor) return nullptr; - return base::MakeUnique<ResourcePrefetchPredictorPageLoadMetricsObserver>( - loading_predictor->resource_prefetch_predictor(), web_contents); + return base::MakeUnique<LoadingPredictorPageLoadMetricsObserver>( + loading_predictor->resource_prefetch_predictor(), + loading_predictor->loading_data_collector(), web_contents); } -ResourcePrefetchPredictorPageLoadMetricsObserver:: - ResourcePrefetchPredictorPageLoadMetricsObserver( +LoadingPredictorPageLoadMetricsObserver:: + LoadingPredictorPageLoadMetricsObserver( predictors::ResourcePrefetchPredictor* predictor, + predictors::LoadingDataCollector* collector, content::WebContents* web_contents) : predictor_(predictor), + collector_(collector), web_contents_(web_contents), record_histograms_(false) { DCHECK(predictor_); + DCHECK(collector_); } -ResourcePrefetchPredictorPageLoadMetricsObserver:: - ~ResourcePrefetchPredictorPageLoadMetricsObserver() {} +LoadingPredictorPageLoadMetricsObserver:: + ~LoadingPredictorPageLoadMetricsObserver() {} page_load_metrics::PageLoadMetricsObserver::ObservePolicy -ResourcePrefetchPredictorPageLoadMetricsObserver::OnStart( +LoadingPredictorPageLoadMetricsObserver::OnStart( content::NavigationHandle* navigation_handle, const GURL& currently_commited_url, bool started_in_foreground) { @@ -63,20 +67,19 @@ } page_load_metrics::PageLoadMetricsObserver::ObservePolicy -ResourcePrefetchPredictorPageLoadMetricsObserver::OnHidden( +LoadingPredictorPageLoadMetricsObserver::OnHidden( const page_load_metrics::mojom::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) { record_histograms_ = false; return CONTINUE_OBSERVING; } -void ResourcePrefetchPredictorPageLoadMetricsObserver:: - OnFirstContentfulPaintInPage( - const page_load_metrics::mojom::PageLoadTiming& timing, - const page_load_metrics::PageLoadExtraInfo& extra_info) { +void LoadingPredictorPageLoadMetricsObserver::OnFirstContentfulPaintInPage( + const page_load_metrics::mojom::PageLoadTiming& timing, + const page_load_metrics::PageLoadExtraInfo& extra_info) { predictors::NavigationID navigation_id(web_contents_); - predictor_->RecordFirstContentfulPaint( + collector_->RecordFirstContentfulPaint( navigation_id, extra_info.navigation_start + timing.paint_timing->first_contentful_paint.value()); if (record_histograms_) { @@ -86,7 +89,7 @@ } } -void ResourcePrefetchPredictorPageLoadMetricsObserver:: +void LoadingPredictorPageLoadMetricsObserver:: OnFirstMeaningfulPaintInMainFrameDocument( const page_load_metrics::mojom::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) {
diff --git a/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer.h similarity index 69% rename from chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.h rename to chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer.h index 3278fbb..6965217 100644 --- a/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer.h
@@ -2,9 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_RESOURCE_PREFETCH_PREDICTOR_PAGE_LOAD_METRICS_OBSERVER_H_ -#define CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_RESOURCE_PREFETCH_PREDICTOR_PAGE_LOAD_METRICS_OBSERVER_H_ +#ifndef CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_LOADING_PREDICTOR_PAGE_LOAD_METRICS_OBSERVER_H_ +#define CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_LOADING_PREDICTOR_PAGE_LOAD_METRICS_OBSERVER_H_ +#include <memory> #include "chrome/browser/page_load_metrics/page_load_metrics_observer.h" namespace content { @@ -13,6 +14,7 @@ namespace predictors { class ResourcePrefetchPredictor; +class LoadingDataCollector; } namespace internal { @@ -24,21 +26,22 @@ // Observer responsible for recording page load metrics relevant to // ResourcePrefetchPredictor. -class ResourcePrefetchPredictorPageLoadMetricsObserver +class LoadingPredictorPageLoadMetricsObserver : public page_load_metrics::PageLoadMetricsObserver { public: - // Returns a ResourcePrefetchPredictorPageLoadMetricsObserver, or nullptr if - // it is not needed. - static std::unique_ptr<ResourcePrefetchPredictorPageLoadMetricsObserver> + // Returns a LoadingPredictorPageLoadMetricsObserver, or nullptr if it is not + // needed. + static std::unique_ptr<LoadingPredictorPageLoadMetricsObserver> CreateIfNeeded(content::WebContents* web_contents); // Public for testing. Normally one should use CreateIfNeeded. Predictor must // outlive this observer. - explicit ResourcePrefetchPredictorPageLoadMetricsObserver( + explicit LoadingPredictorPageLoadMetricsObserver( predictors::ResourcePrefetchPredictor* predictor, + predictors::LoadingDataCollector* collector, content::WebContents* web_contents); - ~ResourcePrefetchPredictorPageLoadMetricsObserver() override; + ~LoadingPredictorPageLoadMetricsObserver() override; // page_load_metrics::PageLoadMetricsObserver: ObservePolicy OnStart(content::NavigationHandle* navigation_handle, @@ -56,10 +59,11 @@ private: predictors::ResourcePrefetchPredictor* predictor_; + predictors::LoadingDataCollector* collector_; content::WebContents* web_contents_; bool record_histograms_; - DISALLOW_COPY_AND_ASSIGN(ResourcePrefetchPredictorPageLoadMetricsObserver); + DISALLOW_COPY_AND_ASSIGN(LoadingPredictorPageLoadMetricsObserver); }; -#endif // CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_RESOURCE_PREFETCH_PREDICTOR_PAGE_LOAD_METRICS_OBSERVER_H_ +#endif // CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_LOADING_PREDICTOR_PAGE_LOAD_METRICS_OBSERVER_H_
diff --git a/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer_unittest.cc b/chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer_unittest.cc similarity index 82% rename from chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer_unittest.cc rename to chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer_unittest.cc index 64cbc254..cfbd42e 100644 --- a/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer_unittest.cc +++ b/chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer_unittest.cc
@@ -2,18 +2,20 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.h" +#include "chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer.h" #include <memory> #include "base/memory/ptr_util.h" #include "chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h" +#include "chrome/browser/predictors/loading_data_collector.h" #include "chrome/browser/predictors/resource_prefetch_common.h" #include "chrome/browser/predictors/resource_prefetch_predictor.h" #include "chrome/test/base/testing_profile.h" #include "content/public/test/test_browser_thread_bundle.h" #include "testing/gmock/include/gmock/gmock.h" +using predictors::LoadingDataCollector; using predictors::ResourcePrefetchPredictor; class MockResourcePrefetchPredictor : public ResourcePrefetchPredictor { @@ -28,7 +30,7 @@ ~MockResourcePrefetchPredictor() override {} }; -class ResourcePrefetchPredictorPageLoadMetricsObserverTest +class LoadingPredictorPageLoadMetricsObserverTest : public page_load_metrics::PageLoadMetricsObserverTestHarness { protected: void SetUp() override { @@ -39,6 +41,7 @@ base::MakeUnique<testing::StrictMock<MockResourcePrefetchPredictor>>( config, profile()); page_load_metrics::InitPageLoadTimingForTest(&timing_); + collector_ = base::MakeUnique<LoadingDataCollector>(predictor_.get()); timing_.navigation_start = base::Time::FromDoubleT(1); timing_.paint_timing->first_paint = base::TimeDelta::FromSeconds(2); timing_.paint_timing->first_contentful_paint = @@ -50,17 +53,17 @@ void RegisterObservers(page_load_metrics::PageLoadTracker* tracker) override { tracker->AddObserver( - base::MakeUnique<ResourcePrefetchPredictorPageLoadMetricsObserver>( - predictor_.get(), web_contents())); + base::MakeUnique<LoadingPredictorPageLoadMetricsObserver>( + predictor_.get(), collector_.get(), web_contents())); } std::unique_ptr<testing::StrictMock<MockResourcePrefetchPredictor>> predictor_; page_load_metrics::mojom::PageLoadTiming timing_; + std::unique_ptr<LoadingDataCollector> collector_; }; -TEST_F(ResourcePrefetchPredictorPageLoadMetricsObserverTest, - PrefetchableIsRecorded) { +TEST_F(LoadingPredictorPageLoadMetricsObserverTest, PrefetchableIsRecorded) { const GURL main_frame_url("https://www.google.com"); EXPECT_CALL(*predictor_.get(), IsUrlPrefetchable(main_frame_url)) .WillOnce(testing::Return(true)); @@ -74,7 +77,7 @@ internal::kHistogramResourcePrefetchPredictorFirstMeaningfulPaint, 1); } -TEST_F(ResourcePrefetchPredictorPageLoadMetricsObserverTest, +TEST_F(LoadingPredictorPageLoadMetricsObserverTest, NotPrefetchableIsNotRecorded) { const GURL main_frame_url("https://www.google.com"); EXPECT_CALL(*predictor_.get(), IsUrlPrefetchable(main_frame_url))
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc b/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc index c0c3791c..4e400fb 100644 --- a/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc +++ b/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc
@@ -4,6 +4,9 @@ #include "chrome/browser/page_load_metrics/page_load_metrics_initialize.h" +#include <memory> +#include <utility> + #include "base/macros.h" #include "base/memory/ptr_util.h" #include "base/timer/timer.h" @@ -22,6 +25,7 @@ #include "chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h" #include "chrome/browser/page_load_metrics/observers/google_captcha_observer.h" #include "chrome/browser/page_load_metrics/observers/https_engagement_metrics/https_engagement_page_load_metrics_observer.h" +#include "chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer.h" #include "chrome/browser/page_load_metrics/observers/lofi_page_load_metrics_observer.h" #include "chrome/browser/page_load_metrics/observers/media_page_load_metrics_observer.h" #include "chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.h" @@ -29,7 +33,6 @@ #include "chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.h" #include "chrome/browser/page_load_metrics/observers/previews_page_load_metrics_observer.h" #include "chrome/browser/page_load_metrics/observers/protocol_page_load_metrics_observer.h" -#include "chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.h" #include "chrome/browser/page_load_metrics/observers/service_worker_page_load_metrics_observer.h" #include "chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.h" #include "chrome/browser/page_load_metrics/observers/tab_restore_page_load_metrics_observer.h" @@ -121,11 +124,11 @@ base::MakeUnique<AndroidPageLoadMetricsObserver>(web_contents_)); #endif // OS_ANDROID std::unique_ptr<page_load_metrics::PageLoadMetricsObserver> - resource_prefetch_predictor_observer = - ResourcePrefetchPredictorPageLoadMetricsObserver::CreateIfNeeded( + loading_predictor_observer = + LoadingPredictorPageLoadMetricsObserver::CreateIfNeeded( web_contents_); - if (resource_prefetch_predictor_observer) - tracker->AddObserver(std::move(resource_prefetch_predictor_observer)); + if (loading_predictor_observer) + tracker->AddObserver(std::move(loading_predictor_observer)); } else { std::unique_ptr<page_load_metrics::PageLoadMetricsObserver> prerender_observer =
diff --git a/chrome/browser/predictors/loading_data_collector.cc b/chrome/browser/predictors/loading_data_collector.cc new file mode 100644 index 0000000..92705ae7 --- /dev/null +++ b/chrome/browser/predictors/loading_data_collector.cc
@@ -0,0 +1,144 @@ +// 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 <string> + +#include "chrome/browser/predictors/loading_data_collector.h" +#include "chrome/browser/profiles/profile.h" +#include "components/mime_util/mime_util.h" +#include "content/public/browser/resource_request_info.h" +#include "content/public/common/resource_type.h" +#include "net/url_request/url_request.h" + +namespace predictors { + +namespace { + +bool g_allow_port_in_urls = false; + +} // namespace + +// static +bool LoadingDataCollector::ShouldRecordRequest( + net::URLRequest* request, + content::ResourceType resource_type) { + const content::ResourceRequestInfo* request_info = + content::ResourceRequestInfo::ForRequest(request); + if (!request_info) + return false; + + if (!request_info->IsMainFrame()) + return false; + + return resource_type == content::RESOURCE_TYPE_MAIN_FRAME && + IsHandledMainPage(request); +} + +// static +bool LoadingDataCollector::ShouldRecordResponse(net::URLRequest* response) { + const content::ResourceRequestInfo* request_info = + content::ResourceRequestInfo::ForRequest(response); + if (!request_info) + return false; + + if (!request_info->IsMainFrame()) + return false; + + content::ResourceType resource_type = request_info->GetResourceType(); + return resource_type == content::RESOURCE_TYPE_MAIN_FRAME + ? IsHandledMainPage(response) + : IsHandledSubresource(response, resource_type); +} + +// static +bool LoadingDataCollector::ShouldRecordRedirect(net::URLRequest* response) { + return ShouldRecordResponse(response); +} + +// static +bool LoadingDataCollector::IsHandledMainPage(net::URLRequest* request) { + const GURL& url = request->url(); + bool bad_port = !g_allow_port_in_urls && url.has_port(); + return url.SchemeIsHTTPOrHTTPS() && !bad_port; +} + +// static +bool LoadingDataCollector::IsHandledSubresource( + net::URLRequest* response, + content::ResourceType resource_type) { + const GURL& url = response->url(); + bool bad_port = !g_allow_port_in_urls && url.has_port(); + if (!response->first_party_for_cookies().SchemeIsHTTPOrHTTPS() || + !url.SchemeIsHTTPOrHTTPS() || bad_port) { + return false; + } + + std::string mime_type; + response->GetMimeType(&mime_type); + if (!IsHandledResourceType(resource_type, mime_type)) + return false; + + if (response->method() != "GET") + return false; + + if (response->original_url().spec().length() > + ResourcePrefetchPredictorTables::kMaxStringLength) { + return false; + } + + if (!response->response_info().headers.get()) + return false; + + return true; +} + +// static +bool LoadingDataCollector::IsHandledResourceType( + content::ResourceType resource_type, + const std::string& mime_type) { + content::ResourceType actual_resource_type = + ResourcePrefetchPredictor::GetResourceType(resource_type, mime_type); + return actual_resource_type == content::RESOURCE_TYPE_STYLESHEET || + actual_resource_type == content::RESOURCE_TYPE_SCRIPT || + actual_resource_type == content::RESOURCE_TYPE_IMAGE || + actual_resource_type == content::RESOURCE_TYPE_FONT_RESOURCE; +} + +// static +void LoadingDataCollector::SetAllowPortInUrlsForTesting(bool state) { + g_allow_port_in_urls = state; +} + +LoadingDataCollector::LoadingDataCollector(ResourcePrefetchPredictor* predictor) + : predictor_(predictor) {} + +LoadingDataCollector::~LoadingDataCollector() {} + +void LoadingDataCollector::RecordURLRequest( + const ResourcePrefetchPredictor::URLRequestSummary& request) { + predictor_->RecordURLRequest(request); +} + +void LoadingDataCollector::RecordURLResponse( + const ResourcePrefetchPredictor::URLRequestSummary& response) { + predictor_->RecordURLResponse(response); +} + +void LoadingDataCollector::RecordURLRedirect( + const ResourcePrefetchPredictor::URLRequestSummary& response) { + predictor_->RecordURLRedirect(response); +} + +void LoadingDataCollector::RecordMainFrameLoadComplete( + const NavigationID& navigation_id) { + predictor_->RecordMainFrameLoadComplete(navigation_id); +} + +void LoadingDataCollector::RecordFirstContentfulPaint( + const NavigationID& navigation_id, + const base::TimeTicks& first_contentful_paint) { + predictor_->RecordFirstContentfulPaint(navigation_id, first_contentful_paint); +} + +} // namespace predictors
diff --git a/chrome/browser/predictors/loading_data_collector.h b/chrome/browser/predictors/loading_data_collector.h new file mode 100644 index 0000000..65274eef --- /dev/null +++ b/chrome/browser/predictors/loading_data_collector.h
@@ -0,0 +1,77 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_PREDICTORS_LOADING_DATA_COLLECTOR_H_ +#define CHROME_BROWSER_PREDICTORS_LOADING_DATA_COLLECTOR_H_ + +#include <string> + +#include "chrome/browser/predictors/resource_prefetch_predictor.h" +#include "content/public/common/resource_type.h" + +namespace net { +class URLRequest; +} + +namespace predictors { + +// Records navigation events as reported by various observers to the database +// and stats collection classes. All the non-static methods of this class need +// to be called on the UI thread. +class LoadingDataCollector + : public base::SupportsWeakPtr<LoadingDataCollector> { + public: + explicit LoadingDataCollector( + predictors::ResourcePrefetchPredictor* predictor); + ~LoadingDataCollector(); + + // Thread safe. + static bool ShouldRecordRequest(net::URLRequest* request, + content::ResourceType resource_type); + static bool ShouldRecordResponse(net::URLRequest* response); + static bool ShouldRecordRedirect(net::URLRequest* response); + + // 'LoadingPredictorObserver' and 'ResourcePrefetchPredictorTabHelper' call + // the below functions to inform the predictor of main frame and resource + // requests. Should only be called if the corresponding Should* functions + // return true. + void RecordURLRequest( + const predictors::ResourcePrefetchPredictor::URLRequestSummary& request); + void RecordURLResponse( + const predictors::ResourcePrefetchPredictor::URLRequestSummary& response); + void RecordURLRedirect( + const predictors::ResourcePrefetchPredictor::URLRequestSummary& response); + + // Called when the main frame of a page completes loading. + void RecordMainFrameLoadComplete(const NavigationID& navigation_id); + + // Called after the main frame's first contentful paint. + void RecordFirstContentfulPaint( + const NavigationID& navigation_id, + const base::TimeTicks& first_contentful_paint); + + private: + friend class ResourcePrefetchPredictorBrowserTest; + + FRIEND_TEST_ALL_PREFIXES(LoadingDataCollectorTest, HandledResourceTypes); + + // Returns true if the main page request is supported for prediction. + static bool IsHandledMainPage(net::URLRequest* request); + + // Returns true if the subresource request is supported for prediction. + static bool IsHandledSubresource(net::URLRequest* request, + content::ResourceType resource_type); + + // Returns true if the subresource has a supported type. + static bool IsHandledResourceType(content::ResourceType resource_type, + const std::string& mime_type); + + static void SetAllowPortInUrlsForTesting(bool state); + + predictors::ResourcePrefetchPredictor* const predictor_; +}; + +} // namespace predictors + +#endif // CHROME_BROWSER_PREDICTORS_LOADING_DATA_COLLECTOR_H_
diff --git a/chrome/browser/predictors/loading_data_collector_unittest.cc b/chrome/browser/predictors/loading_data_collector_unittest.cc new file mode 100644 index 0000000..7c75e1b0 --- /dev/null +++ b/chrome/browser/predictors/loading_data_collector_unittest.cc
@@ -0,0 +1,230 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/predictors/loading_data_collector.h" + +#include <iostream> +#include <memory> +#include <utility> + +#include "base/run_loop.h" +#include "base/test/histogram_tester.h" +#include "chrome/browser/history/history_service_factory.h" +#include "chrome/browser/predictors/loading_test_util.h" +#include "chrome/test/base/testing_profile.h" +#include "content/public/test/test_browser_thread_bundle.h" +#include "net/http/http_response_headers.h" +#include "net/url_request/url_request_context.h" +#include "net/url_request/url_request_job.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +using testing::StrictMock; + +namespace predictors { + +class LoadingDataCollectorTest : public testing::Test { + public: + void SetUp() override { + url_request_job_factory_.Reset(); + url_request_context_.set_job_factory(&url_request_job_factory_); + } + + protected: + content::TestBrowserThreadBundle thread_bundle_; + net::TestURLRequestContext url_request_context_; + MockURLRequestJobFactory url_request_job_factory_; +}; + +TEST_F(LoadingDataCollectorTest, HandledResourceTypes) { + EXPECT_TRUE(LoadingDataCollector::IsHandledResourceType( + content::RESOURCE_TYPE_STYLESHEET, "bogus/mime-type")); + EXPECT_TRUE(LoadingDataCollector::IsHandledResourceType( + content::RESOURCE_TYPE_STYLESHEET, "")); + EXPECT_FALSE(LoadingDataCollector::IsHandledResourceType( + content::RESOURCE_TYPE_WORKER, "text/css")); + EXPECT_FALSE(LoadingDataCollector::IsHandledResourceType( + content::RESOURCE_TYPE_WORKER, "")); + EXPECT_TRUE(LoadingDataCollector::IsHandledResourceType( + content::RESOURCE_TYPE_PREFETCH, "text/css")); + EXPECT_FALSE(LoadingDataCollector::IsHandledResourceType( + content::RESOURCE_TYPE_PREFETCH, "bogus/mime-type")); + EXPECT_FALSE(LoadingDataCollector::IsHandledResourceType( + content::RESOURCE_TYPE_PREFETCH, "")); + EXPECT_TRUE(LoadingDataCollector::IsHandledResourceType( + content::RESOURCE_TYPE_PREFETCH, "application/font-woff")); + EXPECT_TRUE(LoadingDataCollector::IsHandledResourceType( + content::RESOURCE_TYPE_PREFETCH, "font/woff2")); + EXPECT_FALSE(LoadingDataCollector::IsHandledResourceType( + content::RESOURCE_TYPE_XHR, "")); + EXPECT_FALSE(LoadingDataCollector::IsHandledResourceType( + content::RESOURCE_TYPE_XHR, "bogus/mime-type")); + EXPECT_TRUE(LoadingDataCollector::IsHandledResourceType( + content::RESOURCE_TYPE_XHR, "application/javascript")); +} + +TEST_F(LoadingDataCollectorTest, ShouldRecordRequestMainFrame) { + std::unique_ptr<net::URLRequest> http_request = + CreateURLRequest(url_request_context_, GURL("http://www.google.com"), + net::MEDIUM, content::RESOURCE_TYPE_IMAGE, true); + EXPECT_TRUE(LoadingDataCollector::ShouldRecordRequest( + http_request.get(), content::RESOURCE_TYPE_MAIN_FRAME)); + + std::unique_ptr<net::URLRequest> https_request = + CreateURLRequest(url_request_context_, GURL("https://www.google.com"), + net::MEDIUM, content::RESOURCE_TYPE_IMAGE, true); + EXPECT_TRUE(LoadingDataCollector::ShouldRecordRequest( + https_request.get(), content::RESOURCE_TYPE_MAIN_FRAME)); + + std::unique_ptr<net::URLRequest> file_request = + CreateURLRequest(url_request_context_, GURL("file://www.google.com"), + net::MEDIUM, content::RESOURCE_TYPE_IMAGE, true); + EXPECT_FALSE(LoadingDataCollector::ShouldRecordRequest( + file_request.get(), content::RESOURCE_TYPE_MAIN_FRAME)); + + std::unique_ptr<net::URLRequest> https_request_with_port = + CreateURLRequest(url_request_context_, GURL("https://www.google.com:666"), + net::MEDIUM, content::RESOURCE_TYPE_IMAGE, true); + EXPECT_FALSE(LoadingDataCollector::ShouldRecordRequest( + https_request_with_port.get(), content::RESOURCE_TYPE_MAIN_FRAME)); +} + +TEST_F(LoadingDataCollectorTest, ShouldRecordRequestSubResource) { + std::unique_ptr<net::URLRequest> http_request = CreateURLRequest( + url_request_context_, GURL("http://www.google.com/cat.png"), net::MEDIUM, + content::RESOURCE_TYPE_IMAGE, false); + EXPECT_FALSE(LoadingDataCollector::ShouldRecordRequest( + http_request.get(), content::RESOURCE_TYPE_IMAGE)); + + std::unique_ptr<net::URLRequest> https_request = CreateURLRequest( + url_request_context_, GURL("https://www.google.com/cat.png"), net::MEDIUM, + content::RESOURCE_TYPE_IMAGE, false); + EXPECT_FALSE(LoadingDataCollector::ShouldRecordRequest( + https_request.get(), content::RESOURCE_TYPE_IMAGE)); + + std::unique_ptr<net::URLRequest> file_request = CreateURLRequest( + url_request_context_, GURL("file://www.google.com/cat.png"), net::MEDIUM, + content::RESOURCE_TYPE_IMAGE, false); + EXPECT_FALSE(LoadingDataCollector::ShouldRecordRequest( + file_request.get(), content::RESOURCE_TYPE_IMAGE)); + + std::unique_ptr<net::URLRequest> https_request_with_port = CreateURLRequest( + url_request_context_, GURL("https://www.google.com:666/cat.png"), + net::MEDIUM, content::RESOURCE_TYPE_IMAGE, false); + EXPECT_FALSE(LoadingDataCollector::ShouldRecordRequest( + https_request_with_port.get(), content::RESOURCE_TYPE_IMAGE)); +} + +TEST_F(LoadingDataCollectorTest, ShouldRecordResponseMainFrame) { + net::HttpResponseInfo response_info; + response_info.headers = MakeResponseHeaders(""); + url_request_job_factory_.set_response_info(response_info); + + std::unique_ptr<net::URLRequest> http_request = + CreateURLRequest(url_request_context_, GURL("http://www.google.com"), + net::MEDIUM, content::RESOURCE_TYPE_MAIN_FRAME, true); + EXPECT_TRUE(LoadingDataCollector::ShouldRecordResponse(http_request.get())); + + std::unique_ptr<net::URLRequest> https_request = + CreateURLRequest(url_request_context_, GURL("https://www.google.com"), + net::MEDIUM, content::RESOURCE_TYPE_MAIN_FRAME, true); + EXPECT_TRUE(LoadingDataCollector::ShouldRecordResponse(https_request.get())); + + std::unique_ptr<net::URLRequest> file_request = + CreateURLRequest(url_request_context_, GURL("file://www.google.com"), + net::MEDIUM, content::RESOURCE_TYPE_MAIN_FRAME, true); + EXPECT_FALSE(LoadingDataCollector::ShouldRecordResponse(file_request.get())); + + std::unique_ptr<net::URLRequest> https_request_with_port = + CreateURLRequest(url_request_context_, GURL("https://www.google.com:666"), + net::MEDIUM, content::RESOURCE_TYPE_MAIN_FRAME, true); + EXPECT_FALSE(LoadingDataCollector::ShouldRecordResponse( + https_request_with_port.get())); +} + +TEST_F(LoadingDataCollectorTest, ShouldRecordResponseSubresource) { + net::HttpResponseInfo response_info; + response_info.headers = + MakeResponseHeaders("HTTP/1.1 200 OK\n\nSome: Headers\n"); + response_info.was_cached = true; + url_request_job_factory_.set_response_info(response_info); + + // Protocol. + std::unique_ptr<net::URLRequest> http_image_request = CreateURLRequest( + url_request_context_, GURL("http://www.google.com/cat.png"), net::MEDIUM, + content::RESOURCE_TYPE_IMAGE, true); + EXPECT_TRUE( + LoadingDataCollector::ShouldRecordResponse(http_image_request.get())); + + std::unique_ptr<net::URLRequest> https_image_request = CreateURLRequest( + url_request_context_, GURL("https://www.google.com/cat.png"), net::MEDIUM, + content::RESOURCE_TYPE_IMAGE, true); + EXPECT_TRUE( + LoadingDataCollector::ShouldRecordResponse(https_image_request.get())); + + std::unique_ptr<net::URLRequest> https_image_request_with_port = + CreateURLRequest(url_request_context_, + GURL("https://www.google.com:666/cat.png"), net::MEDIUM, + content::RESOURCE_TYPE_IMAGE, true); + EXPECT_FALSE(LoadingDataCollector::ShouldRecordResponse( + https_image_request_with_port.get())); + + std::unique_ptr<net::URLRequest> file_image_request = CreateURLRequest( + url_request_context_, GURL("file://www.google.com/cat.png"), net::MEDIUM, + content::RESOURCE_TYPE_IMAGE, true); + EXPECT_FALSE( + LoadingDataCollector::ShouldRecordResponse(file_image_request.get())); + + // ResourceType. + std::unique_ptr<net::URLRequest> sub_frame_request = CreateURLRequest( + url_request_context_, GURL("http://www.google.com/frame.html"), + net::MEDIUM, content::RESOURCE_TYPE_SUB_FRAME, true); + EXPECT_FALSE( + LoadingDataCollector::ShouldRecordResponse(sub_frame_request.get())); + + std::unique_ptr<net::URLRequest> font_request = CreateURLRequest( + url_request_context_, GURL("http://www.google.com/comic-sans-ms.woff"), + net::MEDIUM, content::RESOURCE_TYPE_FONT_RESOURCE, true); + EXPECT_TRUE(LoadingDataCollector::ShouldRecordResponse(font_request.get())); + + // From MIME Type. + url_request_job_factory_.set_mime_type("image/png"); + std::unique_ptr<net::URLRequest> prefetch_image_request = CreateURLRequest( + url_request_context_, GURL("http://www.google.com/cat.png"), net::MEDIUM, + content::RESOURCE_TYPE_PREFETCH, true); + EXPECT_TRUE( + LoadingDataCollector::ShouldRecordResponse(prefetch_image_request.get())); + + url_request_job_factory_.set_mime_type("image/my-wonderful-format"); + std::unique_ptr<net::URLRequest> prefetch_unknown_image_request = + CreateURLRequest(url_request_context_, + GURL("http://www.google.com/cat.png"), net::MEDIUM, + content::RESOURCE_TYPE_PREFETCH, true); + EXPECT_FALSE(LoadingDataCollector::ShouldRecordResponse( + prefetch_unknown_image_request.get())); + + url_request_job_factory_.set_mime_type("font/woff"); + std::unique_ptr<net::URLRequest> prefetch_font_request = CreateURLRequest( + url_request_context_, GURL("http://www.google.com/comic-sans-ms.woff"), + net::MEDIUM, content::RESOURCE_TYPE_PREFETCH, true); + EXPECT_TRUE( + LoadingDataCollector::ShouldRecordResponse(prefetch_font_request.get())); + + url_request_job_factory_.set_mime_type("font/woff-woff"); + std::unique_ptr<net::URLRequest> prefetch_unknown_font_request = + CreateURLRequest(url_request_context_, + GURL("http://www.google.com/comic-sans-ms.woff"), + net::MEDIUM, content::RESOURCE_TYPE_PREFETCH, true); + EXPECT_FALSE(LoadingDataCollector::ShouldRecordResponse( + prefetch_unknown_font_request.get())); + + // Not main frame. + std::unique_ptr<net::URLRequest> font_request_sub_frame = CreateURLRequest( + url_request_context_, GURL("http://www.google.com/comic-sans-ms.woff"), + net::MEDIUM, content::RESOURCE_TYPE_FONT_RESOURCE, false); + EXPECT_FALSE( + LoadingDataCollector::ShouldRecordResponse(font_request_sub_frame.get())); +} + +} // namespace predictors
diff --git a/chrome/browser/predictors/loading_predictor.cc b/chrome/browser/predictors/loading_predictor.cc index 0bc1956..2af956ff 100644 --- a/chrome/browser/predictors/loading_predictor.cc +++ b/chrome/browser/predictors/loading_predictor.cc
@@ -6,6 +6,7 @@ #include "base/memory/ptr_util.h" #include "base/metrics/histogram_macros.h" +#include "chrome/browser/predictors/loading_data_collector.h" #include "chrome/browser/predictors/loading_stats_collector.h" #include "chrome/browser/predictors/resource_prefetch_common.h" #include "chrome/browser/predictors/resource_prefetch_predictor.h" @@ -22,7 +23,9 @@ base::MakeUnique<ResourcePrefetchPredictor>(config, profile)), stats_collector_(base::MakeUnique<LoadingStatsCollector>( resource_prefetch_predictor_.get(), - config)) { + config)), + loading_data_collector_(base::MakeUnique<LoadingDataCollector>( + resource_prefetch_predictor())) { resource_prefetch_predictor_->SetStatsCollector(stats_collector_.get()); } @@ -50,8 +53,11 @@ resource_prefetch_predictor_->StartInitialization(); } -ResourcePrefetchPredictor* LoadingPredictor::resource_prefetch_predictor() - const { +LoadingDataCollector* LoadingPredictor::loading_data_collector() { + return loading_data_collector_.get(); +} + +ResourcePrefetchPredictor* LoadingPredictor::resource_prefetch_predictor() { return resource_prefetch_predictor_.get(); }
diff --git a/chrome/browser/predictors/loading_predictor.h b/chrome/browser/predictors/loading_predictor.h index eb1d29a..e5c90b81b3 100644 --- a/chrome/browser/predictors/loading_predictor.h +++ b/chrome/browser/predictors/loading_predictor.h
@@ -12,6 +12,7 @@ #include "base/gtest_prod_util.h" #include "base/memory/weak_ptr.h" #include "base/time/time.h" +#include "chrome/browser/predictors/loading_data_collector.h" #include "chrome/browser/predictors/resource_prefetch_common.h" #include "chrome/browser/predictors/resource_prefetch_predictor.h" #include "components/keyed_service/core/keyed_service.h" @@ -52,7 +53,8 @@ void StartInitialization(); // Don't use, internal only. - ResourcePrefetchPredictor* resource_prefetch_predictor() const; + ResourcePrefetchPredictor* resource_prefetch_predictor(); + LoadingDataCollector* loading_data_collector(); // KeyedService: void Shutdown() override; @@ -82,6 +84,7 @@ Profile* profile_; std::unique_ptr<ResourcePrefetchPredictor> resource_prefetch_predictor_; std::unique_ptr<LoadingStatsCollector> stats_collector_; + std::unique_ptr<LoadingDataCollector> loading_data_collector_; std::map<GURL, base::TimeTicks> active_hints_; // Initial URL. std::map<NavigationID, GURL> active_navigations_;
diff --git a/chrome/browser/predictors/loading_predictor_unittest.cc b/chrome/browser/predictors/loading_predictor_unittest.cc index ee090403..67e3497 100644 --- a/chrome/browser/predictors/loading_predictor_unittest.cc +++ b/chrome/browser/predictors/loading_predictor_unittest.cc
@@ -12,7 +12,7 @@ #include "base/run_loop.h" #include "base/test/histogram_tester.h" -#include "chrome/browser/predictors/resource_prefetch_predictor_test_util.h" +#include "chrome/browser/predictors/loading_test_util.h" #include "chrome/test/base/testing_profile.h" #include "content/public/test/test_browser_thread_bundle.h" #include "testing/gmock/include/gmock/gmock.h"
diff --git a/chrome/browser/predictors/loading_stats_collector_unittest.cc b/chrome/browser/predictors/loading_stats_collector_unittest.cc index ad7b716..f79b50b 100644 --- a/chrome/browser/predictors/loading_stats_collector_unittest.cc +++ b/chrome/browser/predictors/loading_stats_collector_unittest.cc
@@ -7,7 +7,7 @@ #include "base/memory/ptr_util.h" #include "base/message_loop/message_loop.h" #include "base/test/histogram_tester.h" -#include "chrome/browser/predictors/resource_prefetch_predictor_test_util.h" +#include "chrome/browser/predictors/loading_test_util.h" #include "chrome/test/base/testing_profile.h" #include "content/public/test/test_browser_thread_bundle.h" #include "testing/gtest/include/gtest/gtest.h"
diff --git a/chrome/browser/predictors/resource_prefetch_predictor_test_util.cc b/chrome/browser/predictors/loading_test_util.cc similarity index 83% rename from chrome/browser/predictors/resource_prefetch_predictor_test_util.cc rename to chrome/browser/predictors/loading_test_util.cc index 4f6e93cd..b740a7e 100644 --- a/chrome/browser/predictors/resource_prefetch_predictor_test_util.cc +++ b/chrome/browser/predictors/loading_test_util.cc
@@ -2,12 +2,24 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/predictors/resource_prefetch_predictor_test_util.h" +#include "chrome/browser/predictors/loading_test_util.h" #include <cmath> +#include <memory> + +#include "content/public/browser/resource_request_info.h" +#include "net/http/http_response_headers.h" +#include "net/url_request/url_request_test_util.h" namespace { +class EmptyURLRequestDelegate : public net::URLRequest::Delegate { + void OnResponseStarted(net::URLRequest* request, int net_error) override {} + void OnReadCompleted(net::URLRequest* request, int bytes_read) override {} +}; + +EmptyURLRequestDelegate g_empty_url_request_delegate; + bool AlmostEqual(const double x, const double y) { return std::fabs(x - y) <= 1e-6; // Arbitrary but close enough. } @@ -211,6 +223,86 @@ config->mode = LoadingPredictorConfig::LEARNING; } +scoped_refptr<net::HttpResponseHeaders> MakeResponseHeaders( + const char* headers) { + return make_scoped_refptr(new net::HttpResponseHeaders( + net::HttpUtil::AssembleRawHeaders(headers, strlen(headers)))); +} + +MockURLRequestJob::MockURLRequestJob(net::URLRequest* request, + const net::HttpResponseInfo& response_info, + const std::string& mime_type) + : net::URLRequestJob(request, nullptr), + response_info_(response_info), + mime_type_(mime_type) {} + +bool MockURLRequestJob::GetMimeType(std::string* mime_type) const { + *mime_type = mime_type_; + return true; +} + +void MockURLRequestJob::Start() { + NotifyHeadersComplete(); +} + +void MockURLRequestJob::GetResponseInfo(net::HttpResponseInfo* info) { + *info = response_info_; +} + +MockURLRequestJobFactory::MockURLRequestJobFactory() {} +MockURLRequestJobFactory::~MockURLRequestJobFactory() {} + +void MockURLRequestJobFactory::Reset() { + response_info_ = net::HttpResponseInfo(); + mime_type_ = std::string(); +} + +net::URLRequestJob* MockURLRequestJobFactory::MaybeCreateJobWithProtocolHandler( + const std::string& scheme, + net::URLRequest* request, + net::NetworkDelegate* network_delegate) const { + return new MockURLRequestJob(request, response_info_, mime_type_); +} + +net::URLRequestJob* MockURLRequestJobFactory::MaybeInterceptRedirect( + net::URLRequest* request, + net::NetworkDelegate* network_delegate, + const GURL& location) const { + return nullptr; +} + +net::URLRequestJob* MockURLRequestJobFactory::MaybeInterceptResponse( + net::URLRequest* request, + net::NetworkDelegate* network_delegate) const { + return nullptr; +} + +bool MockURLRequestJobFactory::IsHandledProtocol( + const std::string& scheme) const { + return true; +} + +bool MockURLRequestJobFactory::IsSafeRedirectTarget( + const GURL& location) const { + return true; +} + +std::unique_ptr<net::URLRequest> CreateURLRequest( + const net::TestURLRequestContext& url_request_context, + const GURL& url, + net::RequestPriority priority, + content::ResourceType resource_type, + bool is_main_frame) { + std::unique_ptr<net::URLRequest> request = url_request_context.CreateRequest( + url, priority, &g_empty_url_request_delegate); + request->set_first_party_for_cookies(url); + content::ResourceRequestInfo::AllocateForTesting( + request.get(), resource_type, nullptr, -1, -1, -1, is_main_frame, false, + false, true, content::PREVIEWS_OFF); + request->Start(); + return request; +} + std::ostream& operator<<(std::ostream& os, const PrefetchData& data) { os << "[" << data.primary_key() << "," << data.last_visit_time() << "]" << std::endl;
diff --git a/chrome/browser/predictors/resource_prefetch_predictor_test_util.h b/chrome/browser/predictors/loading_test_util.h similarity index 73% rename from chrome/browser/predictors/resource_prefetch_predictor_test_util.h rename to chrome/browser/predictors/loading_test_util.h index e34dbcd..2d6e171 100644 --- a/chrome/browser/predictors/resource_prefetch_predictor_test_util.h +++ b/chrome/browser/predictors/loading_test_util.h
@@ -1,15 +1,21 @@ // Copyright 2016 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_TEST_UTIL_H_ -#define CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_TEST_UTIL_H_ +#ifndef CHROME_BROWSER_PREDICTORS_LOADING_TEST_UTIL_H_ +#define CHROME_BROWSER_PREDICTORS_LOADING_TEST_UTIL_H_ +#include <memory> +#include <set> #include <string> #include <vector> #include "chrome/browser/predictors/resource_prefetch_predictor.h" #include "chrome/browser/predictors/resource_prefetch_predictor_tables.h" #include "components/sessions/core/session_id.h" +#include "net/url_request/url_request_context.h" +#include "net/url_request/url_request_job.h" +#include "net/url_request/url_request_job_factory.h" +#include "net/url_request/url_request_test_util.h" #include "testing/gmock/include/gmock/gmock.h" namespace predictors { @@ -100,6 +106,69 @@ void PopulateTestConfig(LoadingPredictorConfig* config, bool small_db = true); +scoped_refptr<net::HttpResponseHeaders> MakeResponseHeaders( + const char* headers); + +class MockURLRequestJob : public net::URLRequestJob { + public: + MockURLRequestJob(net::URLRequest* request, + const net::HttpResponseInfo& response_info, + const std::string& mime_type); + + bool GetMimeType(std::string* mime_type) const override; + + protected: + void Start() override; + void GetResponseInfo(net::HttpResponseInfo* info) override; + + private: + net::HttpResponseInfo response_info_; + std::string mime_type_; +}; + +class MockURLRequestJobFactory : public net::URLRequestJobFactory { + public: + MockURLRequestJobFactory(); + ~MockURLRequestJobFactory() override; + + void Reset(); + + net::URLRequestJob* MaybeCreateJobWithProtocolHandler( + const std::string& scheme, + net::URLRequest* request, + net::NetworkDelegate* network_delegate) const override; + + net::URLRequestJob* MaybeInterceptRedirect( + net::URLRequest* request, + net::NetworkDelegate* network_delegate, + const GURL& location) const override; + + net::URLRequestJob* MaybeInterceptResponse( + net::URLRequest* request, + net::NetworkDelegate* network_delegate) const override; + + bool IsHandledProtocol(const std::string& scheme) const override; + + bool IsSafeRedirectTarget(const GURL& location) const override; + + void set_response_info(const net::HttpResponseInfo& response_info) { + response_info_ = response_info; + } + + void set_mime_type(const std::string& mime_type) { mime_type_ = mime_type; } + + private: + net::HttpResponseInfo response_info_; + std::string mime_type_; +}; + +std::unique_ptr<net::URLRequest> CreateURLRequest( + const net::TestURLRequestContext& url_request_context, + const GURL& url, + net::RequestPriority priority, + content::ResourceType resource_type, + bool is_main_frame); + // For printing failures nicely. std::ostream& operator<<(std::ostream& stream, const PrefetchData& data); std::ostream& operator<<(std::ostream& stream, const ResourceData& resource); @@ -141,4 +210,4 @@ } // namespace precache -#endif // CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_TEST_UTIL_H_ +#endif // CHROME_BROWSER_PREDICTORS_LOADING_TEST_UTIL_H_
diff --git a/chrome/browser/predictors/resource_prefetch_predictor.cc b/chrome/browser/predictors/resource_prefetch_predictor.cc index 08dfc83..1812a87 100644 --- a/chrome/browser/predictors/resource_prefetch_predictor.cc +++ b/chrome/browser/predictors/resource_prefetch_predictor.cc
@@ -55,8 +55,6 @@ const size_t kNumSampleHosts = 50; const size_t kReportReadinessThreshold = 50; -bool g_allow_port_in_urls = false; - // For reporting events of interest that are not tied to any navigation. enum ReportingEvent { REPORTING_EVENT_ALL_HISTORY_CLEARED = 0, @@ -172,94 +170,6 @@ // ResourcePrefetchPredictor static functions. // static -bool ResourcePrefetchPredictor::ShouldRecordRequest( - net::URLRequest* request, - content::ResourceType resource_type) { - const content::ResourceRequestInfo* request_info = - content::ResourceRequestInfo::ForRequest(request); - if (!request_info) - return false; - - if (!request_info->IsMainFrame()) - return false; - - return resource_type == content::RESOURCE_TYPE_MAIN_FRAME && - IsHandledMainPage(request); -} - -// static -bool ResourcePrefetchPredictor::ShouldRecordResponse( - net::URLRequest* response) { - const content::ResourceRequestInfo* request_info = - content::ResourceRequestInfo::ForRequest(response); - if (!request_info) - return false; - - if (!request_info->IsMainFrame()) - return false; - - content::ResourceType resource_type = request_info->GetResourceType(); - return resource_type == content::RESOURCE_TYPE_MAIN_FRAME - ? IsHandledMainPage(response) - : IsHandledSubresource(response, resource_type); -} - -// static -bool ResourcePrefetchPredictor::ShouldRecordRedirect( - net::URLRequest* response) { - return ShouldRecordResponse(response); -} - -// static -bool ResourcePrefetchPredictor::IsHandledMainPage(net::URLRequest* request) { - const GURL& url = request->url(); - bool bad_port = !g_allow_port_in_urls && url.has_port(); - return url.SchemeIsHTTPOrHTTPS() && !bad_port; -} - -// static -bool ResourcePrefetchPredictor::IsHandledSubresource( - net::URLRequest* response, - content::ResourceType resource_type) { - const GURL& url = response->url(); - bool bad_port = !g_allow_port_in_urls && url.has_port(); - if (!response->first_party_for_cookies().SchemeIsHTTPOrHTTPS() || - !url.SchemeIsHTTPOrHTTPS() || bad_port) { - return false; - } - - std::string mime_type; - response->GetMimeType(&mime_type); - if (!IsHandledResourceType(resource_type, mime_type)) - return false; - - if (response->method() != "GET") - return false; - - if (response->original_url().spec().length() > - ResourcePrefetchPredictorTables::kMaxStringLength) { - return false; - } - - if (!response->response_info().headers.get()) - return false; - - return true; -} - -// static -bool ResourcePrefetchPredictor::IsHandledResourceType( - content::ResourceType resource_type, - const std::string& mime_type) { - content::ResourceType actual_resource_type = - GetResourceType(resource_type, mime_type); - return actual_resource_type == content::RESOURCE_TYPE_STYLESHEET || - actual_resource_type == content::RESOURCE_TYPE_SCRIPT || - actual_resource_type == content::RESOURCE_TYPE_IMAGE || - actual_resource_type == content::RESOURCE_TYPE_FONT_RESOURCE; -} - -// static content::ResourceType ResourcePrefetchPredictor::GetResourceType( content::ResourceType resource_type, const std::string& mime_type) { @@ -353,11 +263,6 @@ return true; } -// static -void ResourcePrefetchPredictor::SetAllowPortInUrlsForTesting(bool state) { - g_allow_port_in_urls = state; -} - //////////////////////////////////////////////////////////////////////////////// // ResourcePrefetchPredictor nested types.
diff --git a/chrome/browser/predictors/resource_prefetch_predictor.h b/chrome/browser/predictors/resource_prefetch_predictor.h index 2c5b984..a9e9c0325 100644 --- a/chrome/browser/predictors/resource_prefetch_predictor.h +++ b/chrome/browser/predictors/resource_prefetch_predictor.h
@@ -76,16 +76,16 @@ // // The overall flow of the resource prefetching algorithm is as follows: // -// * ResourcePrefetchPredictorObserver - Listens for URL requests, responses and +// * LoadingPredictorObserver - Listens for URL requests, responses and // redirects (client-side redirects are not supported) on the IO thread (via // ResourceDispatcherHostDelegate) and posts tasks to the -// ResourcePrefetchPredictor on the UI thread. This is owned by the -// ProfileIOData for the profile. +// LoadingDataCollector on the UI thread. This is owned by the ProfileIOData +// for the profile. // * ResourcePrefetchPredictorTables - Persists ResourcePrefetchPredictor data // to a sql database. Runs entirely on the DB thread. Owned by the // PredictorDatabase. // * ResourcePrefetchPredictor - Learns about resource requirements per URL in -// the UI thread through the ResourcePrefetchPredictorObserver and persists +// the UI thread through the LoadingPredictorObserver and persists // it to disk in the DB thread through the ResourcePrefetchPredictorTables. It // initiates resource prefetching using the ResourcePrefetcherManager. Owned // by profile. @@ -200,12 +200,6 @@ virtual void StartInitialization(); virtual void Shutdown(); - // Thread safe. - static bool ShouldRecordRequest(net::URLRequest* request, - content::ResourceType resource_type); - static bool ShouldRecordResponse(net::URLRequest* response); - static bool ShouldRecordRedirect(net::URLRequest* response); - // Determines the resource type from the declared one, falling back to MIME // type detection when it is not explicit. static content::ResourceType GetResourceType( @@ -218,21 +212,6 @@ const std::string& mime_type, content::ResourceType fallback); - // 'ResourcePrefetchPredictorObserver' calls the below functions to inform the - // predictor of main frame and resource requests. Should only be called if the - // corresponding Should* functions return true. - void RecordURLRequest(const URLRequestSummary& request); - void RecordURLResponse(const URLRequestSummary& response); - void RecordURLRedirect(const URLRequestSummary& response); - - // Called when the main frame of a page completes loading. - void RecordMainFrameLoadComplete(const NavigationID& navigation_id); - - // Called after the main frame's first contentful paint. - void RecordFirstContentfulPaint( - const NavigationID& navigation_id, - const base::TimeTicks& first_contentful_paint); - // Called when ResourcePrefetcher is finished, i.e. there is nothing pending // in flight. void OnPrefetchingFinished( @@ -265,6 +244,21 @@ Prediction* prediction) const; private: + // 'LoadingPredictorObserver' calls the below functions to inform the + // predictor of main frame and resource requests. Should only be called if the + // corresponding Should* functions return true. + void RecordURLRequest(const URLRequestSummary& request); + void RecordURLResponse(const URLRequestSummary& response); + void RecordURLRedirect(const URLRequestSummary& response); + + // Called when the main frame of a page completes loading. + void RecordMainFrameLoadComplete(const NavigationID& navigation_id); + + // Called after the main frame's first contentful paint. + void RecordFirstContentfulPaint( + const NavigationID& navigation_id, + const base::TimeTicks& first_contentful_paint); + // Starts prefetching for |main_frame_url| from a |prediction|. void StartPrefetching(const GURL& main_frame_url, const Prediction& prediction); @@ -275,6 +269,7 @@ friend class LoadingPredictor; friend class ::PredictorsHandler; + friend class LoadingDataCollector; friend class ResourcePrefetchPredictorTest; friend class ResourcePrefetchPredictorBrowserTest; @@ -302,7 +297,6 @@ FRIEND_TEST_ALL_PREFIXES(ResourcePrefetchPredictorTest, OnSubresourceResponse); FRIEND_TEST_ALL_PREFIXES(ResourcePrefetchPredictorTest, GetCorrectPLT); - FRIEND_TEST_ALL_PREFIXES(ResourcePrefetchPredictorTest, HandledResourceTypes); FRIEND_TEST_ALL_PREFIXES(ResourcePrefetchPredictorTest, PopulatePrefetcherRequest); FRIEND_TEST_ALL_PREFIXES(ResourcePrefetchPredictorTest, PopulateFromManifest); @@ -332,22 +326,9 @@ typedef std::map<NavigationID, std::unique_ptr<PageRequestSummary>> NavigationMap; - // Returns true if the main page request is supported for prediction. - static bool IsHandledMainPage(net::URLRequest* request); - - // Returns true if the subresource request is supported for prediction. - static bool IsHandledSubresource(net::URLRequest* request, - content::ResourceType resource_type); - - // Returns true if the subresource has a supported type. - static bool IsHandledResourceType(content::ResourceType resource_type, - const std::string& mime_type); - // Returns true if the request (should have a response in it) is "no-store". static bool IsNoStore(const net::URLRequest& request); - static void SetAllowPortInUrlsForTesting(bool state); - // Functions called on different network events pertaining to the loading of // main frame resource or sub resources. void OnMainFrameRequest(const URLRequestSummary& request);
diff --git a/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc b/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc index 3a426a1..f56fae5 100644 --- a/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc +++ b/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
@@ -11,10 +11,11 @@ #include "base/strings/string_util.h" #include "base/test/histogram_tester.h" #include "chrome/browser/browsing_data/browsing_data_helper.h" +#include "chrome/browser/predictors/loading_data_collector.h" #include "chrome/browser/predictors/loading_predictor.h" #include "chrome/browser/predictors/loading_predictor_factory.h" #include "chrome/browser/predictors/loading_stats_collector.h" -#include "chrome/browser/predictors/resource_prefetch_predictor_test_util.h" +#include "chrome/browser/predictors/loading_test_util.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" @@ -367,13 +368,13 @@ ASSERT_TRUE(predictor_); resource_prefetch_predictor_ = predictor_->resource_prefetch_predictor(); // URLs from the test server contain a port number. - ResourcePrefetchPredictor::SetAllowPortInUrlsForTesting(true); + LoadingDataCollector::SetAllowPortInUrlsForTesting(true); EnsurePredictorInitialized(); histogram_tester_.reset(new base::HistogramTester()); } void TearDownOnMainThread() override { - ResourcePrefetchPredictor::SetAllowPortInUrlsForTesting(false); + LoadingDataCollector::SetAllowPortInUrlsForTesting(false); } void TestLearningAndPrefetching(
diff --git a/chrome/browser/predictors/resource_prefetch_predictor_tab_helper.cc b/chrome/browser/predictors/resource_prefetch_predictor_tab_helper.cc index 3b9a1fd..015a06d 100644 --- a/chrome/browser/predictors/resource_prefetch_predictor_tab_helper.cc +++ b/chrome/browser/predictors/resource_prefetch_predictor_tab_helper.cc
@@ -34,10 +34,9 @@ if (!loading_predictor) return; - auto* resource_prefetch_predictor = - loading_predictor->resource_prefetch_predictor(); + auto* collector = loading_predictor->loading_data_collector(); NavigationID navigation_id(web_contents()); - resource_prefetch_predictor->RecordMainFrameLoadComplete(navigation_id); + collector->RecordMainFrameLoadComplete(navigation_id); } void ResourcePrefetchPredictorTabHelper::DidLoadResourceFromMemoryCache( @@ -59,9 +58,8 @@ ResourcePrefetchPredictor::GetResourceTypeFromMimeType( mime_type, resource_type); summary.was_cached = true; - auto* resource_prefetch_predictor = - loading_predictor->resource_prefetch_predictor(); - resource_prefetch_predictor->RecordURLResponse(summary); + auto* collector = loading_predictor->loading_data_collector(); + collector->RecordURLResponse(summary); } } // namespace predictors
diff --git a/chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc b/chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc index 265c57d4..34fd28f 100644 --- a/chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc +++ b/chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc
@@ -10,9 +10,9 @@ #include "base/message_loop/message_loop.h" #include "base/run_loop.h" #include "base/strings/utf_string_conversions.h" +#include "chrome/browser/predictors/loading_test_util.h" #include "chrome/browser/predictors/predictor_database.h" #include "chrome/browser/predictors/resource_prefetch_predictor_tables.h" -#include "chrome/browser/predictors/resource_prefetch_predictor_test_util.h" #include "chrome/test/base/testing_profile.h" #include "content/public/test/test_browser_thread_bundle.h" #include "net/base/request_priority.h"
diff --git a/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc b/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc index a3761dd..0ec87e20 100644 --- a/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc +++ b/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
@@ -15,20 +15,18 @@ #include "base/time/time.h" #include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/predictors/loading_predictor.h" +#include "chrome/browser/predictors/loading_test_util.h" #include "chrome/browser/predictors/resource_prefetch_predictor_tables.h" -#include "chrome/browser/predictors/resource_prefetch_predictor_test_util.h" #include "chrome/browser/predictors/resource_prefetcher_manager.h" #include "chrome/test/base/testing_profile.h" #include "components/history/core/browser/history_service.h" #include "components/history/core/browser/history_types.h" #include "components/sessions/core/session_id.h" -#include "content/public/browser/resource_request_info.h" #include "content/public/test/test_browser_thread_bundle.h" #include "net/http/http_response_headers.h" #include "net/traffic_annotation/network_traffic_annotation_test_helper.h" #include "net/url_request/url_request_context.h" #include "net/url_request/url_request_job.h" -#include "net/url_request/url_request_test_util.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -44,86 +42,6 @@ using OriginDataMap = std::map<std::string, OriginData>; using ManifestDataMap = std::map<std::string, precache::PrecacheManifest>; -scoped_refptr<net::HttpResponseHeaders> MakeResponseHeaders( - const char* headers) { - return make_scoped_refptr(new net::HttpResponseHeaders( - net::HttpUtil::AssembleRawHeaders(headers, strlen(headers)))); -} - -class EmptyURLRequestDelegate : public net::URLRequest::Delegate { - void OnResponseStarted(net::URLRequest* request, int net_error) override {} - void OnReadCompleted(net::URLRequest* request, int bytes_read) override {} -}; - -class MockURLRequestJob : public net::URLRequestJob { - public: - MockURLRequestJob(net::URLRequest* request, - const net::HttpResponseInfo& response_info, - const std::string& mime_type) - : net::URLRequestJob(request, nullptr), - response_info_(response_info), - mime_type_(mime_type) {} - - bool GetMimeType(std::string* mime_type) const override { - *mime_type = mime_type_; - return true; - } - - protected: - void Start() override { NotifyHeadersComplete(); } - void GetResponseInfo(net::HttpResponseInfo* info) override { - *info = response_info_; - } - - private: - net::HttpResponseInfo response_info_; - std::string mime_type_; -}; - -class MockURLRequestJobFactory : public net::URLRequestJobFactory { - public: - MockURLRequestJobFactory() {} - ~MockURLRequestJobFactory() override {} - - net::URLRequestJob* MaybeCreateJobWithProtocolHandler( - const std::string& scheme, - net::URLRequest* request, - net::NetworkDelegate* network_delegate) const override { - return new MockURLRequestJob(request, response_info_, mime_type_); - } - - net::URLRequestJob* MaybeInterceptRedirect( - net::URLRequest* request, - net::NetworkDelegate* network_delegate, - const GURL& location) const override { - return nullptr; - } - - net::URLRequestJob* MaybeInterceptResponse( - net::URLRequest* request, - net::NetworkDelegate* network_delegate) const override { - return nullptr; - } - - bool IsHandledProtocol(const std::string& scheme) const override { - return true; - } - - bool IsSafeRedirectTarget(const GURL& location) const override { - return true; - } - - void set_response_info(const net::HttpResponseInfo& response_info) { - response_info_ = response_info; - } - - void set_mime_type(const std::string& mime_type) { mime_type_ = mime_type; } - - private: - net::HttpResponseInfo response_info_; - std::string mime_type_; -}; - template <typename T> class FakeGlowplugKeyValueTable : public GlowplugKeyValueTable<T> { public: @@ -240,23 +158,6 @@ return summary; } - std::unique_ptr<net::URLRequest> CreateURLRequest( - const GURL& url, - net::RequestPriority priority, - content::ResourceType resource_type, - bool is_main_frame) { - std::unique_ptr<net::URLRequest> request = - url_request_context_.CreateRequest(url, priority, - &url_request_delegate_, - TRAFFIC_ANNOTATION_FOR_TESTS); - request->set_first_party_for_cookies(url); - content::ResourceRequestInfo::AllocateForTesting( - request.get(), resource_type, nullptr, -1, -1, -1, is_main_frame, false, - false, true, content::PREVIEWS_OFF); - request->Start(); - return request; - } - void InitializePredictor() { loading_predictor_->StartInitialization(); base::RunLoop loop; @@ -291,7 +192,6 @@ OriginDataMap test_origin_data_; MockURLRequestJobFactory url_request_job_factory_; - EmptyURLRequestDelegate url_request_delegate_; std::unique_ptr<base::HistogramTester> histogram_tester_; }; @@ -321,6 +221,7 @@ EXPECT_EQ(predictor_->initialization_state_, ResourcePrefetchPredictor::INITIALIZED); + url_request_job_factory_.Reset(); url_request_context_.set_job_factory(&url_request_job_factory_); histogram_tester_.reset(new base::HistogramTester()); @@ -1413,197 +1314,6 @@ ->subresource_requests[2]); } -TEST_F(ResourcePrefetchPredictorTest, HandledResourceTypes) { - EXPECT_TRUE(ResourcePrefetchPredictor::IsHandledResourceType( - content::RESOURCE_TYPE_STYLESHEET, "bogus/mime-type")); - EXPECT_TRUE(ResourcePrefetchPredictor::IsHandledResourceType( - content::RESOURCE_TYPE_STYLESHEET, "")); - EXPECT_FALSE(ResourcePrefetchPredictor::IsHandledResourceType( - content::RESOURCE_TYPE_WORKER, "text/css")); - EXPECT_FALSE(ResourcePrefetchPredictor::IsHandledResourceType( - content::RESOURCE_TYPE_WORKER, "")); - EXPECT_TRUE(ResourcePrefetchPredictor::IsHandledResourceType( - content::RESOURCE_TYPE_PREFETCH, "text/css")); - EXPECT_FALSE(ResourcePrefetchPredictor::IsHandledResourceType( - content::RESOURCE_TYPE_PREFETCH, "bogus/mime-type")); - EXPECT_FALSE(ResourcePrefetchPredictor::IsHandledResourceType( - content::RESOURCE_TYPE_PREFETCH, "")); - EXPECT_TRUE(ResourcePrefetchPredictor::IsHandledResourceType( - content::RESOURCE_TYPE_PREFETCH, "application/font-woff")); - EXPECT_TRUE(ResourcePrefetchPredictor::IsHandledResourceType( - content::RESOURCE_TYPE_PREFETCH, "font/woff2")); - EXPECT_FALSE(ResourcePrefetchPredictor::IsHandledResourceType( - content::RESOURCE_TYPE_XHR, "")); - EXPECT_FALSE(ResourcePrefetchPredictor::IsHandledResourceType( - content::RESOURCE_TYPE_XHR, "bogus/mime-type")); - EXPECT_TRUE(ResourcePrefetchPredictor::IsHandledResourceType( - content::RESOURCE_TYPE_XHR, "application/javascript")); -} - -TEST_F(ResourcePrefetchPredictorTest, ShouldRecordRequestMainFrame) { - std::unique_ptr<net::URLRequest> http_request = - CreateURLRequest(GURL("http://www.google.com"), net::MEDIUM, - content::RESOURCE_TYPE_IMAGE, true); - EXPECT_TRUE(ResourcePrefetchPredictor::ShouldRecordRequest( - http_request.get(), content::RESOURCE_TYPE_MAIN_FRAME)); - - std::unique_ptr<net::URLRequest> https_request = - CreateURLRequest(GURL("https://www.google.com"), net::MEDIUM, - content::RESOURCE_TYPE_IMAGE, true); - EXPECT_TRUE(ResourcePrefetchPredictor::ShouldRecordRequest( - https_request.get(), content::RESOURCE_TYPE_MAIN_FRAME)); - - std::unique_ptr<net::URLRequest> file_request = - CreateURLRequest(GURL("file://www.google.com"), net::MEDIUM, - content::RESOURCE_TYPE_IMAGE, true); - EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordRequest( - file_request.get(), content::RESOURCE_TYPE_MAIN_FRAME)); - - std::unique_ptr<net::URLRequest> https_request_with_port = - CreateURLRequest(GURL("https://www.google.com:666"), net::MEDIUM, - content::RESOURCE_TYPE_IMAGE, true); - EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordRequest( - https_request_with_port.get(), content::RESOURCE_TYPE_MAIN_FRAME)); -} - -TEST_F(ResourcePrefetchPredictorTest, ShouldRecordRequestSubResource) { - std::unique_ptr<net::URLRequest> http_request = - CreateURLRequest(GURL("http://www.google.com/cat.png"), net::MEDIUM, - content::RESOURCE_TYPE_IMAGE, false); - EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordRequest( - http_request.get(), content::RESOURCE_TYPE_IMAGE)); - - std::unique_ptr<net::URLRequest> https_request = - CreateURLRequest(GURL("https://www.google.com/cat.png"), net::MEDIUM, - content::RESOURCE_TYPE_IMAGE, false); - EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordRequest( - https_request.get(), content::RESOURCE_TYPE_IMAGE)); - - std::unique_ptr<net::URLRequest> file_request = - CreateURLRequest(GURL("file://www.google.com/cat.png"), net::MEDIUM, - content::RESOURCE_TYPE_IMAGE, false); - EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordRequest( - file_request.get(), content::RESOURCE_TYPE_IMAGE)); - - std::unique_ptr<net::URLRequest> https_request_with_port = - CreateURLRequest(GURL("https://www.google.com:666/cat.png"), net::MEDIUM, - content::RESOURCE_TYPE_IMAGE, false); - EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordRequest( - https_request_with_port.get(), content::RESOURCE_TYPE_IMAGE)); -} - -TEST_F(ResourcePrefetchPredictorTest, ShouldRecordResponseMainFrame) { - net::HttpResponseInfo response_info; - response_info.headers = MakeResponseHeaders(""); - url_request_job_factory_.set_response_info(response_info); - - std::unique_ptr<net::URLRequest> http_request = - CreateURLRequest(GURL("http://www.google.com"), net::MEDIUM, - content::RESOURCE_TYPE_MAIN_FRAME, true); - EXPECT_TRUE( - ResourcePrefetchPredictor::ShouldRecordResponse(http_request.get())); - - std::unique_ptr<net::URLRequest> https_request = - CreateURLRequest(GURL("https://www.google.com"), net::MEDIUM, - content::RESOURCE_TYPE_MAIN_FRAME, true); - EXPECT_TRUE( - ResourcePrefetchPredictor::ShouldRecordResponse(https_request.get())); - - std::unique_ptr<net::URLRequest> file_request = - CreateURLRequest(GURL("file://www.google.com"), net::MEDIUM, - content::RESOURCE_TYPE_MAIN_FRAME, true); - EXPECT_FALSE( - ResourcePrefetchPredictor::ShouldRecordResponse(file_request.get())); - - std::unique_ptr<net::URLRequest> https_request_with_port = - CreateURLRequest(GURL("https://www.google.com:666"), net::MEDIUM, - content::RESOURCE_TYPE_MAIN_FRAME, true); - EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordResponse( - https_request_with_port.get())); -} - -TEST_F(ResourcePrefetchPredictorTest, ShouldRecordResponseSubresource) { - net::HttpResponseInfo response_info; - response_info.headers = - MakeResponseHeaders("HTTP/1.1 200 OK\n\nSome: Headers\n"); - response_info.was_cached = true; - url_request_job_factory_.set_response_info(response_info); - - // Protocol. - std::unique_ptr<net::URLRequest> http_image_request = - CreateURLRequest(GURL("http://www.google.com/cat.png"), net::MEDIUM, - content::RESOURCE_TYPE_IMAGE, true); - EXPECT_TRUE(ResourcePrefetchPredictor::ShouldRecordResponse( - http_image_request.get())); - - std::unique_ptr<net::URLRequest> https_image_request = - CreateURLRequest(GURL("https://www.google.com/cat.png"), net::MEDIUM, - content::RESOURCE_TYPE_IMAGE, true); - EXPECT_TRUE(ResourcePrefetchPredictor::ShouldRecordResponse( - https_image_request.get())); - - std::unique_ptr<net::URLRequest> https_image_request_with_port = - CreateURLRequest(GURL("https://www.google.com:666/cat.png"), net::MEDIUM, - content::RESOURCE_TYPE_IMAGE, true); - EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordResponse( - https_image_request_with_port.get())); - - std::unique_ptr<net::URLRequest> file_image_request = - CreateURLRequest(GURL("file://www.google.com/cat.png"), net::MEDIUM, - content::RESOURCE_TYPE_IMAGE, true); - EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordResponse( - file_image_request.get())); - - // ResourceType. - std::unique_ptr<net::URLRequest> sub_frame_request = - CreateURLRequest(GURL("http://www.google.com/frame.html"), net::MEDIUM, - content::RESOURCE_TYPE_SUB_FRAME, true); - EXPECT_FALSE( - ResourcePrefetchPredictor::ShouldRecordResponse(sub_frame_request.get())); - - std::unique_ptr<net::URLRequest> font_request = - CreateURLRequest(GURL("http://www.google.com/comic-sans-ms.woff"), - net::MEDIUM, content::RESOURCE_TYPE_FONT_RESOURCE, true); - EXPECT_TRUE( - ResourcePrefetchPredictor::ShouldRecordResponse(font_request.get())); - - // From MIME Type. - url_request_job_factory_.set_mime_type("image/png"); - std::unique_ptr<net::URLRequest> prefetch_image_request = - CreateURLRequest(GURL("http://www.google.com/cat.png"), net::MEDIUM, - content::RESOURCE_TYPE_PREFETCH, true); - EXPECT_TRUE(ResourcePrefetchPredictor::ShouldRecordResponse( - prefetch_image_request.get())); - - url_request_job_factory_.set_mime_type("image/my-wonderful-format"); - std::unique_ptr<net::URLRequest> prefetch_unknown_image_request = - CreateURLRequest(GURL("http://www.google.com/cat.png"), net::MEDIUM, - content::RESOURCE_TYPE_PREFETCH, true); - EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordResponse( - prefetch_unknown_image_request.get())); - - url_request_job_factory_.set_mime_type("font/woff"); - std::unique_ptr<net::URLRequest> prefetch_font_request = - CreateURLRequest(GURL("http://www.google.com/comic-sans-ms.woff"), - net::MEDIUM, content::RESOURCE_TYPE_PREFETCH, true); - EXPECT_TRUE(ResourcePrefetchPredictor::ShouldRecordResponse( - prefetch_font_request.get())); - - url_request_job_factory_.set_mime_type("font/woff-woff"); - std::unique_ptr<net::URLRequest> prefetch_unknown_font_request = - CreateURLRequest(GURL("http://www.google.com/comic-sans-ms.woff"), - net::MEDIUM, content::RESOURCE_TYPE_PREFETCH, true); - EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordResponse( - prefetch_unknown_font_request.get())); - - // Not main frame. - std::unique_ptr<net::URLRequest> font_request_sub_frame = CreateURLRequest( - GURL("http://www.google.com/comic-sans-ms.woff"), net::MEDIUM, - content::RESOURCE_TYPE_FONT_RESOURCE, false); - EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordResponse( - font_request_sub_frame.get())); -} - TEST_F(ResourcePrefetchPredictorTest, SummarizeResponse) { net::HttpResponseInfo response_info; response_info.headers = @@ -1613,7 +1323,8 @@ GURL url("http://www.google.com/cat.png"); std::unique_ptr<net::URLRequest> request = - CreateURLRequest(url, net::MEDIUM, content::RESOURCE_TYPE_IMAGE, true); + CreateURLRequest(url_request_context_, url, net::MEDIUM, + content::RESOURCE_TYPE_IMAGE, true); URLRequestSummary summary; EXPECT_TRUE(URLRequestSummary::SummarizeResponse(*request, &summary)); EXPECT_EQ(url, summary.resource_url); @@ -1636,9 +1347,9 @@ url_request_job_factory_.set_response_info(response_info); url_request_job_factory_.set_mime_type("image/png"); - std::unique_ptr<net::URLRequest> request = - CreateURLRequest(GURL("http://www.google.com/cat.png"), net::MEDIUM, - content::RESOURCE_TYPE_PREFETCH, true); + std::unique_ptr<net::URLRequest> request = CreateURLRequest( + url_request_context_, GURL("http://www.google.com/cat.png"), net::MEDIUM, + content::RESOURCE_TYPE_PREFETCH, true); URLRequestSummary summary; EXPECT_TRUE(URLRequestSummary::SummarizeResponse(*request, &summary)); EXPECT_EQ(content::RESOURCE_TYPE_IMAGE, summary.resource_type); @@ -1651,9 +1362,9 @@ "Some: Headers\n"); url_request_job_factory_.set_response_info(response_info); - std::unique_ptr<net::URLRequest> request_no_validators = - CreateURLRequest(GURL("http://www.google.com/cat.png"), net::MEDIUM, - content::RESOURCE_TYPE_PREFETCH, true); + std::unique_ptr<net::URLRequest> request_no_validators = CreateURLRequest( + url_request_context_, GURL("http://www.google.com/cat.png"), net::MEDIUM, + content::RESOURCE_TYPE_PREFETCH, true); URLRequestSummary summary; EXPECT_TRUE( @@ -1665,9 +1376,9 @@ "ETag: \"Cr66\"\n" "Cache-Control: no-cache\n"); url_request_job_factory_.set_response_info(response_info); - std::unique_ptr<net::URLRequest> request_etag = - CreateURLRequest(GURL("http://www.google.com/cat.png"), net::MEDIUM, - content::RESOURCE_TYPE_PREFETCH, true); + std::unique_ptr<net::URLRequest> request_etag = CreateURLRequest( + url_request_context_, GURL("http://www.google.com/cat.png"), net::MEDIUM, + content::RESOURCE_TYPE_PREFETCH, true); EXPECT_TRUE(URLRequestSummary::SummarizeResponse(*request_etag, &summary)); EXPECT_TRUE(summary.has_validators); EXPECT_TRUE(summary.always_revalidate);
diff --git a/chrome/browser/predictors/resource_prefetcher_unittest.cc b/chrome/browser/predictors/resource_prefetcher_unittest.cc index 8623e6b..1347fe6 100644 --- a/chrome/browser/predictors/resource_prefetcher_unittest.cc +++ b/chrome/browser/predictors/resource_prefetcher_unittest.cc
@@ -14,7 +14,7 @@ #include "base/memory/ref_counted.h" #include "base/message_loop/message_loop.h" #include "base/test/histogram_tester.h" -#include "chrome/browser/predictors/resource_prefetch_predictor_test_util.h" +#include "chrome/browser/predictors/loading_test_util.h" #include "chrome/browser/predictors/resource_prefetcher_manager.h" #include "chrome/test/base/testing_profile.h" #include "content/public/test/test_browser_thread.h"
diff --git a/chrome/renderer/autofill/password_generation_agent_browsertest.cc b/chrome/renderer/autofill/password_generation_agent_browsertest.cc index ecfa0783..6f0ff9e6 100644 --- a/chrome/renderer/autofill/password_generation_agent_browsertest.cc +++ b/chrome/renderer/autofill/password_generation_agent_browsertest.cc
@@ -91,13 +91,7 @@ bool available) { FocusField(element_id); base::RunLoop().RunUntilIdle(); - fake_pw_client_.Flush(); - bool called = fake_pw_client_.called_show_pw_generation_popup(); - if (available) - ASSERT_TRUE(called); - else - ASSERT_FALSE(called); - + ASSERT_EQ(available, GetCalledShowPasswordGenerationPopup()); fake_pw_client_.reset_called_show_pw_generation_popup(); } @@ -249,6 +243,18 @@ " <INPUT type = 'submit' value = 'LOGIN' />" "</FORM>"; +const char kCurrentAndNewPasswordAutocompleteAttributeFormHTML[] = + "<FORM name = 'blah' action = 'http://www.random.com/'> " + " <INPUT type = 'password' id = 'old_password' " + " autocomplete='current-password'/>" + " <INPUT type = 'password' id = 'new_password' " + " autocomplete='new-password'/>" + " <INPUT type = 'password' id = 'confirm_password' " + " autocomplete='new-password'/>" + " <INPUT type = 'button' id = 'dummy'/> " + " <INPUT type = 'submit' value = 'LOGIN' />" + "</FORM>"; + const char kPasswordChangeFormHTML[] = "<FORM name = 'ChangeWithUsernameForm' action = 'http://www.bidule.com'> " " <INPUT type = 'text' id = 'username'/> " @@ -605,21 +611,27 @@ LoadHTMLWithUserGesture(kBothAutocompleteAttributesFormHTML); SetNotBlacklistedMessage(password_generation_, kBothAutocompleteAttributesFormHTML); - ExpectGenerationAvailable("first_password", true); - // Only setting one of the two attributes doesn't trigger generation. + // Only username autocomplete attribute enabled doesn't trigger generation. LoadHTMLWithUserGesture(kUsernameAutocompleteAttributeFormHTML); SetNotBlacklistedMessage(password_generation_, kUsernameAutocompleteAttributeFormHTML); - ExpectGenerationAvailable("first_password", false); + // Only new-password autocomplete attribute enabled does trigger generation. LoadHTMLWithUserGesture(kNewPasswordAutocompleteAttributeFormHTML); SetNotBlacklistedMessage(password_generation_, kNewPasswordAutocompleteAttributeFormHTML); + ExpectGenerationAvailable("first_password", true); - ExpectGenerationAvailable("first_password", false); + // Generation is triggered if the form has only password fields. + LoadHTMLWithUserGesture(kCurrentAndNewPasswordAutocompleteAttributeFormHTML); + SetNotBlacklistedMessage(password_generation_, + kCurrentAndNewPasswordAutocompleteAttributeFormHTML); + ExpectGenerationAvailable("old_password", false); + ExpectGenerationAvailable("new_password", true); + ExpectGenerationAvailable("confirm_password", false); } TEST_F(PasswordGenerationAgentTest, ChangePasswordFormDetectionTest) {
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 38063ac..f5cd21b 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn
@@ -1362,9 +1362,9 @@ "../browser/policy/policy_network_browsertest.cc", "../browser/policy/policy_prefs_browsertest.cc", "../browser/policy/policy_startup_browsertest.cc", + "../browser/predictors/loading_test_util.cc", + "../browser/predictors/loading_test_util.h", "../browser/predictors/resource_prefetch_predictor_browsertest.cc", - "../browser/predictors/resource_prefetch_predictor_test_util.cc", - "../browser/predictors/resource_prefetch_predictor_test_util.h", "../browser/prefetch/prefetch_browsertest.cc", "../browser/prefs/pref_functional_browsertest.cc", "../browser/prefs/pref_service_browsertest.cc", @@ -3138,13 +3138,13 @@ "../browser/page_load_metrics/observers/document_write_page_load_metrics_observer_unittest.cc", "../browser/page_load_metrics/observers/from_gws_page_load_metrics_observer_unittest.cc", "../browser/page_load_metrics/observers/google_captcha_observer_unittest.cc", + "../browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer_unittest.cc", "../browser/page_load_metrics/observers/lofi_page_load_metrics_observer_unittest.cc", "../browser/page_load_metrics/observers/media_page_load_metrics_observer_unittest.cc", "../browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.cc", "../browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h", "../browser/page_load_metrics/observers/previews_page_load_metrics_observer_unittest.cc", "../browser/page_load_metrics/observers/protocol_page_load_metrics_observer_unittest.cc", - "../browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer_unittest.cc", "../browser/page_load_metrics/observers/resource_tracking_page_load_metrics_observer_unittest.cc", "../browser/page_load_metrics/observers/service_worker_page_load_metrics_observer_unittest.cc", "../browser/page_load_metrics/observers/subresource_filter_metrics_observer_unittest.cc", @@ -3176,12 +3176,13 @@ "../browser/policy/profile_policy_connector_unittest.cc", "../browser/predictors/autocomplete_action_predictor_table_unittest.cc", "../browser/predictors/autocomplete_action_predictor_unittest.cc", + "../browser/predictors/loading_data_collector_unittest.cc", "../browser/predictors/loading_predictor_unittest.cc", "../browser/predictors/loading_stats_collector_unittest.cc", + "../browser/predictors/loading_test_util.cc", + "../browser/predictors/loading_test_util.h", "../browser/predictors/resource_prefetch_common_unittest.cc", "../browser/predictors/resource_prefetch_predictor_tables_unittest.cc", - "../browser/predictors/resource_prefetch_predictor_test_util.cc", - "../browser/predictors/resource_prefetch_predictor_test_util.h", "../browser/predictors/resource_prefetch_predictor_unittest.cc", "../browser/predictors/resource_prefetcher_unittest.cc", "../browser/prefs/chrome_command_line_pref_store_proxy_unittest.cc",
diff --git a/components/autofill/content/renderer/password_form_conversion_utils.cc b/components/autofill/content/renderer/password_form_conversion_utils.cc index 825f446..c47feeb 100644 --- a/components/autofill/content/renderer/password_form_conversion_utils.cc +++ b/components/autofill/content/renderer/password_form_conversion_utils.cc
@@ -740,31 +740,37 @@ } bool HasAutocompleteAttributeValue(const blink::WebInputElement& element, - const char* value_in_lowercase) { - std::string autocomplete_value_lowercase = base::ToLowerASCII( - base::UTF16ToUTF8(element.GetAttribute("autocomplete").Utf16())); + base::StringPiece value_in_lowercase) { + CR_DEFINE_STATIC_LOCAL(WebString, kAutocomplete, ("autocomplete")); + const std::string autocomplete_value = + element.GetAttribute(kAutocomplete) + .Utf8(WebString::UTF8ConversionMode::kStrictReplacingErrorsWithFFFD); - std::vector<base::StringPiece> tokens = base::SplitStringPiece( - autocomplete_value_lowercase, base::kWhitespaceASCII, - base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY); - - return base::ContainsValue(tokens, value_in_lowercase); + std::vector<base::StringPiece> tokens = + base::SplitStringPiece(autocomplete_value, base::kWhitespaceASCII, + base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY); + return std::find_if(tokens.begin(), tokens.end(), + [value_in_lowercase](base::StringPiece token) { + return base::LowerCaseEqualsASCII(token, + value_in_lowercase); + }) != tokens.end(); } bool HasCreditCardAutocompleteAttributes( const blink::WebInputElement& element) { - std::string autocomplete_value_lowercase = base::ToLowerASCII( - base::UTF16ToUTF8(element.GetAttribute("autocomplete").Utf16())); + CR_DEFINE_STATIC_LOCAL(WebString, kAutocomplete, ("autocomplete")); + const std::string autocomplete_value = + element.GetAttribute(kAutocomplete) + .Utf8(WebString::UTF8ConversionMode::kStrictReplacingErrorsWithFFFD); - for (const auto& token : base::SplitStringPiece( - autocomplete_value_lowercase, base::kWhitespaceASCII, - base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY)) { - if (base::StartsWith(token, kAutocompleteCreditCardPrefix, - base::CompareCase::SENSITIVE)) { - return true; - } - } - return false; + std::vector<base::StringPiece> tokens = + base::SplitStringPiece(autocomplete_value, base::kWhitespaceASCII, + base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY); + return std::find_if( + tokens.begin(), tokens.end(), [](base::StringPiece token) { + return base::StartsWith(token, kAutocompleteCreditCardPrefix, + base::CompareCase::INSENSITIVE_ASCII); + }) != tokens.end(); } bool IsCreditCardVerificationPasswordField(
diff --git a/components/autofill/content/renderer/password_form_conversion_utils.h b/components/autofill/content/renderer/password_form_conversion_utils.h index 0cebba2..3e4f402 100644 --- a/components/autofill/content/renderer/password_form_conversion_utils.h +++ b/components/autofill/content/renderer/password_form_conversion_utils.h
@@ -9,6 +9,7 @@ #include <memory> #include <vector> +#include "base/strings/string_piece.h" #include "components/autofill/core/common/password_form.h" #include "components/autofill/core/common/password_form_field_prediction_map.h" #include "third_party/WebKit/public/platform/WebString.h" @@ -62,7 +63,7 @@ // Checks in a case-insensitive way if the autocomplete attribute for the given // |element| is present and has the specified |value_in_lowercase|. bool HasAutocompleteAttributeValue(const blink::WebInputElement& element, - const char* value_in_lowercase); + base::StringPiece value_in_lowercase); // Checks in a case-insensitive way if credit card autocomplete attributes for // the given |element| are present.
diff --git a/components/autofill/content/renderer/password_generation_agent.cc b/components/autofill/content/renderer/password_generation_agent.cc index bf50f22..f9fe4c9 100644 --- a/components/autofill/content/renderer/password_generation_agent.cc +++ b/components/autofill/content/renderer/password_generation_agent.cc
@@ -4,7 +4,9 @@ #include "components/autofill/content/renderer/password_generation_agent.h" +#include <algorithm> #include <memory> +#include <utility> #include "base/command_line.h" #include "base/logging.h" @@ -39,17 +41,17 @@ using Logger = autofill::SavePasswordProgressLogger; -// Returns true if we think that this form is for account creation. |passwords| -// is filled with the password field(s) in the form. +// Returns true if we think that this form is for account creation. Password +// field(s) of the form are pushed back to |passwords|. bool GetAccountCreationPasswordFields( const std::vector<blink::WebFormControlElement>& control_elements, std::vector<blink::WebInputElement>* passwords) { - for (size_t i = 0; i < control_elements.size(); i++) { + for (const auto& control_element : control_elements) { const blink::WebInputElement* input_element = - ToWebInputElement(&control_elements[i]); - if (input_element && input_element->IsTextField()) { - if (input_element->IsPasswordField()) - passwords->push_back(*input_element); + ToWebInputElement(&control_element); + if (input_element && input_element->IsTextField() && + input_element->IsPasswordField()) { + passwords->push_back(*input_element); } } return !passwords->empty(); @@ -71,10 +73,33 @@ return nullptr; } -// This function returns a vector of password fields into which Chrome should -// fill the generated password. It assumes that |field_signature| describes the -// field where Chrome shows the password generation prompt. It returns no more -// than 2 elements. +// Returns a vector of up to 2 password fields with autocomplete attribute set +// to "new-password". These will be filled with the generated password. +std::vector<blink::WebInputElement> FindNewPasswordElementsMarkedBySite( + const std::vector<blink::WebInputElement>& all_password_elements) { + std::vector<blink::WebInputElement> passwords; + + auto is_new_password_field = [](const blink::WebInputElement& element) { + return HasAutocompleteAttributeValue(element, "new-password"); + }; + + auto field_iter = + std::find_if(all_password_elements.begin(), all_password_elements.end(), + is_new_password_field); + if (field_iter != all_password_elements.end()) { + passwords.push_back(*field_iter++); + field_iter = std::find_if(field_iter, all_password_elements.end(), + is_new_password_field); + if (field_iter != all_password_elements.end()) + passwords.push_back(*field_iter); + } + + return passwords; +} + +// Returns a vector of up to 2 password fields into which Chrome should fill the +// generated password. It assumes that |field_signature| describes the field +// where Chrome shows the password generation prompt. std::vector<blink::WebInputElement> FindPasswordElementsForGeneration( const std::vector<blink::WebInputElement>& all_password_elements, const PasswordFormGenerationData& generation_data) { @@ -85,11 +110,12 @@ const blink::WebInputElement& input = *iter; FieldSignature signature = CalculateFieldSignatureByNameAndType( input.NameForAutofill().Utf16(), input.FormControlType().Utf8()); - if (signature == generation_data.field_signature) + if (signature == generation_data.field_signature) { generation_field_iter = iter; - else if (generation_data.confirmation_field_signature && - signature == *generation_data.confirmation_field_signature) + } else if (generation_data.confirmation_field_signature && + signature == *generation_data.confirmation_field_signature) { confirmation_field_iter = iter; + } } std::vector<blink::WebInputElement> passwords; @@ -114,17 +140,12 @@ } } -bool AutocompleteAttributesSetForGeneration(const PasswordForm& form) { - return form.username_marked_by_site && form.new_password_marked_by_site; -} - } // namespace PasswordGenerationAgent::AccountCreationFormData::AccountCreationFormData( linked_ptr<PasswordForm> password_form, std::vector<blink::WebInputElement> passwords) - : form(password_form), - password_elements(passwords) {} + : form(password_form), password_elements(std::move(passwords)) {} PasswordGenerationAgent::AccountCreationFormData::AccountCreationFormData( const AccountCreationFormData& other) = default; @@ -277,9 +298,8 @@ &passwords)) { if (form_classifier_enabled_) RunFormClassifierAndSaveVote(forms[i], *password_form); - AccountCreationFormData ac_form_data( - make_linked_ptr(password_form.release()), passwords); - possible_account_creation_forms_.push_back(ac_form_data); + possible_account_creation_forms_.emplace_back( + make_linked_ptr(password_form.release()), std::move(passwords)); } } @@ -389,8 +409,11 @@ for (auto& possible_form_data : possible_account_creation_forms_) { PasswordForm* possible_password_form = possible_form_data.form.get(); const PasswordFormGenerationData* generation_data = nullptr; + + std::vector<blink::WebInputElement> password_elements; if (base::CommandLine::ForCurrentProcess()->HasSwitch( switches::kLocalHeuristicsOnlyForPasswordGeneration)) { + password_elements = possible_form_data.password_elements; VLOG(2) << "Bypassing additional checks."; } else if (!ContainsURL(not_blacklisted_password_form_origins_, possible_password_form->origin)) { @@ -399,28 +422,31 @@ } else { generation_data = FindFormGenerationData(generation_enabled_forms_, *possible_password_form); - if (!generation_data) { - if (AutocompleteAttributesSetForGeneration(*possible_password_form)) { - LogMessage(Logger::STRING_GENERATION_RENDERER_AUTOCOMPLETE_ATTRIBUTE); - password_generation::LogPasswordGenerationEvent( - password_generation::AUTOCOMPLETE_ATTRIBUTES_ENABLED_GENERATION); - } else { + if (generation_data) { + password_elements = FindPasswordElementsForGeneration( + possible_form_data.password_elements, *generation_data); + } else { + if (!possible_password_form->new_password_marked_by_site) { LogMessage(Logger::STRING_GENERATION_RENDERER_NO_SERVER_SIGNAL); continue; } + + LogMessage(Logger::STRING_GENERATION_RENDERER_AUTOCOMPLETE_ATTRIBUTE); + password_generation::LogPasswordGenerationEvent( + password_generation::AUTOCOMPLETE_ATTRIBUTES_ENABLED_GENERATION); + + password_elements = FindNewPasswordElementsMarkedBySite( + possible_form_data.password_elements); } } + LogMessage(Logger::STRING_GENERATION_RENDERER_ELIGIBLE_FORM_FOUND); - std::vector<blink::WebInputElement> password_elements = - generation_data - ? FindPasswordElementsForGeneration( - possible_form_data.password_elements, *generation_data) - : possible_form_data.password_elements; if (password_elements.empty()) { // It might be if JavaScript changes field names. LogMessage(Logger::STRING_GENERATION_RENDERER_NO_FIELD_FOUND); return; } + generation_form_data_.reset(new AccountCreationFormData( possible_form_data.form, std::move(password_elements))); generation_element_ = generation_form_data_->password_elements[0];
diff --git a/content/browser/download/download_browsertest.cc b/content/browser/download/download_browsertest.cc index b2a20a7..f913d6f 100644 --- a/content/browser/download/download_browsertest.cc +++ b/content/browser/download/download_browsertest.cc
@@ -2320,6 +2320,34 @@ EXPECT_EQ(document_url.spec(), requests.back()->referrer); } +// Test that the referrer header is dropped for HTTP downloads from HTTPS. +IN_PROC_BROWSER_TEST_F(DownloadContentTest, ReferrerForHTTPS) { + net::EmbeddedTestServer https_origin( + net::EmbeddedTestServer::Type::TYPE_HTTPS); + net::EmbeddedTestServer http_origin(net::EmbeddedTestServer::Type::TYPE_HTTP); + https_origin.ServeFilesFromDirectory(GetTestFilePath("download", "")); + http_origin.RegisterRequestHandler(CreateBasicResponseHandler( + "/download", base::StringPairs(), "application/octet-stream", "Hello")); + ASSERT_TRUE(https_origin.InitializeAndListen()); + ASSERT_TRUE(http_origin.InitializeAndListen()); + + GURL download_url = http_origin.GetURL("/download"); + GURL referrer_url = https_origin.GetURL( + std::string("/download-link.html?dl=") + download_url.spec()); + + https_origin.StartAcceptingConnections(); + http_origin.StartAcceptingConnections(); + + DownloadItem* download = StartDownloadAndReturnItem(shell(), referrer_url); + WaitForCompletion(download); + + ASSERT_EQ(5, download->GetReceivedBytes()); + EXPECT_EQ("", download->GetReferrerUrl().spec()); + + ASSERT_TRUE(https_origin.ShutdownAndWaitUntilComplete()); + ASSERT_TRUE(http_origin.ShutdownAndWaitUntilComplete()); +} + // Check that the cookie policy is correctly updated when downloading a file // that redirects cross origin. IN_PROC_BROWSER_TEST_F(DownloadContentTest, CookiePolicy) {
diff --git a/content/browser/download/url_downloader.cc b/content/browser/download/url_downloader.cc index 169e975..be20bb891 100644 --- a/content/browser/download/url_downloader.cc +++ b/content/browser/download/url_downloader.cc
@@ -68,7 +68,9 @@ std::unique_ptr<net::URLRequest> request, const Referrer& referrer, bool is_parallel_request) { - Referrer::SetReferrerForRequest(request.get(), referrer); + Referrer sanitized_referrer = + Referrer::SanitizeForRequest(request->url(), referrer); + Referrer::SetReferrerForRequest(request.get(), sanitized_referrer); if (request->url().SchemeIs(url::kBlobScheme)) return nullptr;
diff --git a/content/renderer/media/media_stream_audio_processor.cc b/content/renderer/media/media_stream_audio_processor.cc index d5314117..45c3b20 100644 --- a/content/renderer/media/media_stream_audio_processor.cc +++ b/content/renderer/media/media_stream_audio_processor.cc
@@ -502,8 +502,13 @@ apm_config.echo_canceller3.enabled = enable; audio_processing_->ApplyConfig(apm_config); - DCHECK(echo_information_); - echo_information_.reset(new EchoInformation()); + if (apm_config.echo_canceller3.enabled) { + // Do not log any echo information when AEC3 is active, as the echo + // information then will not be properly updated. + echo_information_.reset(); + } else { + echo_information_ = base::MakeUnique<EchoInformation>(); + } } void MediaStreamAudioProcessor::OnIpcClosing() { @@ -552,11 +557,9 @@ int audio_delay_milliseconds) { DCHECK(render_thread_checker_.CalledOnValidThread()); #if defined(OS_ANDROID) - DCHECK(audio_processing_->echo_control_mobile()->is_enabled()); DCHECK(!audio_processing_->echo_cancellation()->is_enabled()); #else DCHECK(!audio_processing_->echo_control_mobile()->is_enabled()); - DCHECK(audio_processing_->echo_cancellation()->is_enabled()); #endif TRACE_EVENT0("audio", "MediaStreamAudioProcessor::OnPlayoutData"); @@ -696,12 +699,18 @@ if (echo_cancellation) { EnableEchoCancellation(audio_processing_.get()); - // Prepare for logging echo information. If there are data remaining in - // |echo_information_| we simply discard it. - echo_information_.reset(new EchoInformation()); - apm_config.echo_canceller3.enabled = override_aec3_.value_or( base::FeatureList::IsEnabled(features::kWebRtcUseEchoCanceller3)); + + if (!apm_config.echo_canceller3.enabled) { + // Prepare for logging echo information. If there are data remaining in + // |echo_information_| we simply discard it. + echo_information_ = base::MakeUnique<EchoInformation>(); + } else { + // Do not log any echo information when AEC3 is active, as the echo + // information then will not be properly updated. + echo_information_.reset(); + } } else { apm_config.echo_canceller3.enabled = false; }
diff --git a/headless/BUILD.gn b/headless/BUILD.gn index 15816d5..67096986 100644 --- a/headless/BUILD.gn +++ b/headless/BUILD.gn
@@ -8,6 +8,7 @@ import("//mojo/public/tools/bindings/mojom.gni") import("//printing/features/features.gni") import("//testing/test.gni") +import("//third_party/closure_compiler/compile_js.gni") import("//tools/grit/grit_rule.gni") import("//tools/grit/repack.gni") @@ -157,7 +158,10 @@ ] generated_devtools_api = [] +generated_devtools_api_js = [] foreach(domain, devtools_domains) { + generated_devtools_api_js += + [ "$target_gen_dir/public/devtools_js/" + domain + ".js" ] generated_devtools_api += [ "$target_gen_dir/public/devtools/domains/" + domain + ".cc", "$target_gen_dir/public/devtools/domains/" + domain + ".h", @@ -179,11 +183,12 @@ "$root_gen_dir/blink/core/inspector/protocol.json", ] - outputs = generated_devtools_api + outputs = generated_devtools_api + generated_devtools_api_js sources = [ "lib/browser/devtools_api/domain_cc.template", "lib/browser/devtools_api/domain_h.template", + "lib/browser/devtools_api/domain_js.template", "lib/browser/devtools_api/domain_type_conversions_h.template", "lib/browser/devtools_api/domain_types_cc.template", "lib/browser/devtools_api/domain_types_forward_declarations_h.template", @@ -198,6 +203,13 @@ ] } +js_library("js_devtools_bindings_lib") { + sources = [ "lib/browser/devtools_api/devtools_connection.js" ] + + generated_devtools_api_js + deps = [] + extra_deps = [ ":gen_devtools_client_api" ] +} + if (headless_fontconfig_utils) { static_library("headless_fontconfig_utils") { sources = [ @@ -503,6 +515,61 @@ } } +if (!is_win) { + js_binary("js_devtools_bindings_test") { + sources = [ + "test/bindings_test.js", + ] + deps = [ + ":js_devtools_bindings_lib", + ] + externs_list = [ "lib/tab_socket_externs.js" ] + outputs = [ + "$target_gen_dir/devtools_bindings_test.js", + ] + config_files = [] + closure_flags = [ + "jscomp_error=checkTypes", + "dependency_mode=STRICT", + + # Currently the bindings do not support property renaming so we can't use + # ADVANCED_OPTIMIZATIONS here. We could add support via either moving all + # the types to externs, or via applying the same renaming to the json + # dictionaries sent between C++ and JS. The closure compiler can produce + # files which contain those mappings with the appropriate flag. + "compilation_level=SIMPLE", + "language_out=ES5_STRICT", + "entry_point=chromium.BindingsTest", + ] + } + + grit("headless_browsertest_resources_grit") { + source = "headless_browsertest_resources.grd" + outputs = [ + "grit/headless_browsertest_resources.h", + "$root_gen_dir/headless/headless_browsertest_resources.pak", + ] + grit_flags = [ + "-E", + "gen_root=" + rebase_path(root_gen_dir), + ] + deps = [ + ":js_devtools_bindings_test", + ] + resource_ids = "lib/headless_browsertest_resource_ids" + } + + repack("headless_browser_tests_pak") { + sources = [ + "$root_gen_dir/headless/headless_browsertest_resources.pak", + ] + output = "$root_out_dir/headless_browser_tests.pak" + deps = [ + ":headless_browsertest_resources_grit", + ] + } +} + test("headless_browsertests") { sources = [ "lib/frame_id_browsertest.cc", @@ -557,6 +624,13 @@ "//testing/gtest", ] + # The js_binary rule doesn't currently work on windows. + if (!is_win) { + data += [ "$root_out_dir/headless_browser_tests.pak" ] + sources += [ "test/headless_js_bindings_browsertest.cc" ] + deps += [ ":headless_browser_tests_pak" ] + } + if (enable_basic_printing) { deps += [ "//components/printing/browser",
diff --git a/headless/headless_browsertest_resources.grd b/headless/headless_browsertest_resources.grd new file mode 100644 index 0000000..08da556 --- /dev/null +++ b/headless/headless_browsertest_resources.grd
@@ -0,0 +1,15 @@ +<?xml version="1.0" encoding="UTF-8"?> +<grit latest_public_release="0" current_release="1" output_all_resource_defines="false"> + <outputs> + <output filename="grit/headless_browsertest_resources.h" type="rc_header"> + <emit emit_type='prepend'></emit> + </output> + <output filename="headless_browsertest_resources.pak" type="data_package" /> + </outputs> + <translations /> + <release seq="1"> + <includes> + <include name="DEVTOOLS_BINDINGS_TEST" file="${gen_root}\headless\devtools_bindings_test.js" use_base_dir="false" type="BINDATA" /> + </includes> + </release> +</grit>
diff --git a/headless/lib/browser/devtools_api/client_api_generator.py b/headless/lib/browser/devtools_api/client_api_generator.py index 6b9936d..7a8ea34 100644 --- a/headless/lib/browser/devtools_api/client_api_generator.py +++ b/headless/lib/browser/devtools_api/client_api_generator.py
@@ -4,9 +4,11 @@ import argparse import collections +import functools import os.path import re import sys + try: import json except ImportError: @@ -61,6 +63,22 @@ return name.lower() +def Shorten(js_name, domain_name): + short_name = domain_name + '.' + long_name = 'chromium.DevTools.' + short_name + return js_name.replace(long_name, short_name) + + +def ShortForm(domain, js_name): + if not 'js_dependencies' in domain: + return js_name + + for dependency in domain['js_dependencies']: + js_name = Shorten(js_name, dependency) + js_name = Shorten(js_name, domain['domain']) + return js_name + + def SanitizeLiteral(literal): return { # Rename null enumeration values to avoid a clash with the NULL macro. @@ -120,6 +138,7 @@ def CreateUserTypeDefinition(domain, type): namespace = CamelCaseToHackerStyle(domain['domain']) return { + 'js_type': '!chromium.DevTools.%s.%s' % (domain['domain'], type['id']), 'return_type': 'std::unique_ptr<headless::%s::%s>' % ( namespace, type['id']), 'pass_type': 'std::unique_ptr<headless::%s::%s>' % ( @@ -137,6 +156,7 @@ def CreateEnumTypeDefinition(domain_name, type): namespace = CamelCaseToHackerStyle(domain_name) return { + 'js_type': '!chromium.DevTools.%s.%s' % (domain_name, type['id']), 'return_type': 'headless::%s::%s' % (namespace, type['id']), 'pass_type': 'headless::%s::%s' % (namespace, type['id']), 'to_raw_type': '%s', @@ -151,6 +171,7 @@ def CreateObjectTypeDefinition(): return { + 'js_type': 'Object', 'return_type': 'std::unique_ptr<base::DictionaryValue>', 'pass_type': 'std::unique_ptr<base::DictionaryValue>', 'to_raw_type': '*%s', @@ -166,6 +187,7 @@ def WrapObjectTypeDefinition(type): id = type.get('id', 'base::Value') return { + 'js_type': '!Object', 'return_type': 'std::unique_ptr<%s>' % id, 'pass_type': 'std::unique_ptr<%s>' % id, 'to_raw_type': '*%s', @@ -180,6 +202,7 @@ def CreateAnyTypeDefinition(): return { + 'js_type': '*', 'return_type': 'std::unique_ptr<base::Value>', 'pass_type': 'std::unique_ptr<base::Value>', 'to_raw_type': '*%s', @@ -194,6 +217,7 @@ def CreateStringTypeDefinition(): return { + 'js_type': 'string', 'return_type': 'std::string', 'pass_type': 'const std::string&', 'to_pass_type': '%s', @@ -212,7 +236,13 @@ 'integer': 'int', 'boolean': 'bool', } + js_typedefs = { + 'number': 'number', + 'integer': 'number', + 'boolean': 'boolean', + } return { + 'js_type': js_typedefs[type], 'return_type': typedefs[type], 'pass_type': typedefs[type], 'to_pass_type': '%s', @@ -236,6 +266,7 @@ def WrapArrayDefinition(type): return { + 'js_type': '!Array.<%s>' % type['js_type'], 'return_type': 'std::vector<%s>' % type['type'], 'pass_type': 'std::vector<%s>' % type['type'], 'to_raw_type': '%s', @@ -377,6 +408,7 @@ including itself.""" direct_deps = collections.defaultdict(set) + types_required = collections.defaultdict(set) def GetDomainDepsFromRefs(domain_name, json): if isinstance(json, list): @@ -393,9 +425,11 @@ if '.' in json['$ref']: dep = json['$ref'].split('.')[0] direct_deps[domain_name].add(dep) + types_required[domain_name].add(json['$ref']) for domain in json_api['domains']: direct_deps[domain['domain']] = set(domain.get('dependencies', [])) + types_required[domain['domain']] = set(domain.get('types_required', [])) GetDomainDepsFromRefs(domain['domain'], domain) def TraverseDependencies(domain, deps): @@ -409,6 +443,15 @@ for domain in json_api['domains']: domain_deps = set() TraverseDependencies(domain['domain'], domain_deps) + if 'dependencies' in domain: + domain['js_dependencies'] = domain['dependencies'] + else: + domain['js_dependencies'] = [] + + domain['js_forward_declarations'] = [] + for type in types_required[domain['domain']]: + if not type.split('.')[0] in domain['js_dependencies']: + domain['js_forward_declarations'].append(type) domain['dependencies'] = sorted(domain_deps) @@ -456,6 +499,7 @@ template_context = { 'domain': domain, 'resolve_type': ResolveType, + 'short_form': functools.partial(ShortForm, domain), } domain_name = CamelCaseToHackerStyle(domain['domain']) output_file = '%s/%s.%s' % (output_dirname, @@ -470,6 +514,10 @@ jinja_env, os.path.join(output_dirname, 'devtools', 'domains'), json_api, 'domain', ['cc', 'h'], lambda domain_name: domain_name) + GeneratePerDomain( + jinja_env, os.path.join(output_dirname, 'devtools_js'), json_api, + 'domain', ['js'], + lambda domain_name: domain_name) def GenerateTypes(jinja_env, output_dirname, json_api):
diff --git a/headless/lib/browser/devtools_api/devtools_connection.js b/headless/lib/browser/devtools_api/devtools_connection.js new file mode 100644 index 0000000..82993e0 --- /dev/null +++ b/headless/lib/browser/devtools_api/devtools_connection.js
@@ -0,0 +1,157 @@ +// 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. + +/** + * @fileoverview Contains a class which marshals DevTools protocol messages over + * a provided low level message transport. This transport might be a headless + * TabSocket, or a WebSocket or a mock for testing. + */ + +'use strict'; + +goog.module('chromium.DevTools.Connection'); + +/** + * Handles sending and receiving DevTools JSON protocol messages over the + * provided low level message transport. + */ +class Connection { + /** + * @param {!Object} transport The API providing transport for devtools + * commands. + */ + constructor(transport) { + /** @private {!Object} */ + this.transport_ = transport; + + /** @private {number} */ + this.commandId_ = 1; + + /** + * An object containing pending DevTools protocol commands keyed by id. + * + * @private {!Map<number, !Connection.CallbackFunction>} + */ + this.pendingCommands_ = new Map(); + + /** @private {number} */ + this.nextListenerId_ = 1; + + /** + * An object containing DevTools protocol events we are listening for keyed + * by name. + * + * @private {!Map<string, !Map<number, !Connection.CallbackFunction>>} + */ + this.eventListeners_ = new Map(); + + /** + * Used for removing event listeners by id. + * + * @private {!Map<number, string>} + */ + this.eventListenerIdToEventName_ = new Map(); + + this.transport_.onmessage = this.onMessage_.bind(this); + } + + /** + * Listens for DevTools protocol events of the specified name and issues the + * callback upon reception. + * + * @param {string} eventName Name of the DevTools protocol event to listen + * for. + * @param {!Connection.CallbackFunction} listener The callback issued when we + * receive a DevTools protocol event corresponding to the given name. + * @return {number} The id of this event listener. + */ + addEventListener(eventName, listener) { + if (!this.eventListeners_.has(eventName)) { + this.eventListeners_.set(eventName, new Map()); + } + let id = this.nextListenerId_++; + this.eventListeners_.get(eventName).set(id, listener); + this.eventListenerIdToEventName_.set(id, eventName); + return id; + } + + + /** + * Removes an event listener previously added by + * <code>addEventListener</code>. + * + * @param {number} id The id of the event listener to remove. + * @return {boolean} Whether the event listener was actually removed. + */ + removeEventListener(id) { + if (!this.eventListenerIdToEventName_.has(id)) return false; + let eventName = this.eventListenerIdToEventName_.get(id); + this.eventListenerIdToEventName_.delete(id); + // This shouldn't happen, but lets check anyway. + if (!this.eventListeners_.has(eventName)) return false; + return this.eventListeners_.get(eventName).delete(id); + } + + + /** + * Issues a DevTools protocol command and returns a promise for the results. + * + * @param {string} method The name of the DevTools protocol command method. + * @param {!Object=} params An object containing the command parameters if + * any. + * @return {!Promise<!Object>} A promise for the results object. + */ + sendDevToolsMessage(method, params = {}) { + let id = this.commandId_; + // We increment by two because these bindings are intended to be used in + // conjunction with HeadlessDevToolsClient::RawProtocolListener and using + // odd numbers for js generated IDs lets the implementation of = + // OnProtocolMessage easily distinguish between C++ and JS generated + // commands and route the response accordingly. + this.commandId_ += 2; + // Note the names are in quotes to prevent closure compiler name mangling. + this.transport_.send( + JSON.stringify({'method': method, 'id': id, 'params': params})); + return new Promise((resolve, reject) => { + this.pendingCommands_.set(id, resolve); + }); + } + + + /** + * @param {string} jsonMessage An object containing a DevTools protocol + * message. + * @private + */ + onMessage_(jsonMessage) { + const message = JSON.parse(jsonMessage); + if (message.hasOwnProperty('id')) { + if (!this.pendingCommands_.has(message.id)) + throw new Error('Unrecognized id:' + jsonMessage); + if (message.hasOwnProperty('error')) + throw new Error('DevTools protocol error: ' + message.error); + this.pendingCommands_.get(message.id)(message.result); + this.pendingCommands_.delete(message.id); + } else { + if (!message.hasOwnProperty('method') || + !message.hasOwnProperty('params')) { + throw new Error('Bad message:' + jsonMessage); + } + const method = message['method']; + const params = message['params']; + if (this.eventListeners_.has(method)) { + this.eventListeners_.get(method).forEach(function(listener) { + listener(params); + }); + } + } + } +} + +/** + * @typedef {function(Object): undefined|function(string): undefined} + */ +Connection.CallbackFunction; + +exports = Connection;
diff --git a/headless/lib/browser/devtools_api/domain_js.template b/headless/lib/browser/devtools_api/domain_js.template new file mode 100644 index 0000000..8bce5ed9 --- /dev/null +++ b/headless/lib/browser/devtools_api/domain_js.template
@@ -0,0 +1,133 @@ +// 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. + +/** + * @fileoverview Generated DevTools bindings for the {{domain.domain}} Domain. + * Note bindings are not generated for experimental commands and events, if you + * need to use one of those you must use the + * <code>chromium.DevTools.Connection</code> class directly. + * + * TODO(alexclarke): Consider generating bindings for experimental stuff too. + */ +'use strict'; + +goog.module('chromium.DevTools.{{domain.domain}}'); +const Connection = goog.require('chromium.DevTools.Connection'); +{% for domain_name in domain.js_dependencies %} +const {{domain_name}} = goog.require('chromium.DevTools.{{domain_name}}'); +{% endfor %} +{% for forward_declaration in domain.js_forward_declarations %} +goog.forwardDeclare('chromium.DevTools.{{forward_declaration}}'); +{% endfor %} + +/** + * Bindings for the {{domain.domain}} DevTools Domain. + */ +class {{domain.domain}} { + /** + * @param {!Connection} connection The DevTools connection. + */ + constructor(connection) { + /** @private {!Connection} */ + this.connection_ = connection; + }; +{# Generate non-experimental commands. #} +{% for command in domain.commands %} + {% if command.experimental %}{% continue %}{% endif %} + {% set method_name = command.name | sanitize_literal | to_title_case %} + {% if command.parameters|length > 0 %} + {% set param_name = 'params' %} + {% set param_type = '{' + domain.domain + '.' + method_name + 'Params}' %} + {% else %} + {% set param_name = 'opt_params' %} + {% set param_type = '{' + domain.domain + '.' + method_name + 'Params=}' %} + {% endif %} + {% set result_type = '{!Promise<' + domain.domain + '.' + method_name + 'Result>}' %} + + /** + {% if command.description %} + * {{ command.description }} + {% endif %} + * @param {{param_type}} {{param_name}} + * @return {{result_type}} + */ + {{method_name}}({{param_name}}) { + return /** @type {{result_type}} **/( + this.connection_.sendDevToolsMessage('{{domain.domain}}.{{command.name}}', + {{param_name}})); + } +{% endfor %} + +{# Generate non-experimental events. #} +{% for event in domain.events %} + {% if event.experimental %}{% continue %}{% endif %} + {% set param_type = '{!function(!' + domain.domain + '.' + event.name | to_title_case + 'Params)}' %} + + /** + {% if event.description %} + * {{ event.description }} + {% endif %} + * @param {{param_type}} listener + */ + On{{event.name | to_title_case}}(listener) { + this.connection_.addEventListener( + '{{domain.domain}}.{{event.name}}', /** @type {!function(!Object): undefined} */ (listener)); + } +{% endfor %} +} + +{# Generate enums. #} +{% for type in domain.types %} + {% if not "enum" in type %}{% continue %}{% endif %} +/** + {% if type.description %} + * {{type.description}} + * + {% endif %} + * @enum {string} + */ +{{domain.domain}}.{{type.id}} = { + {% for literal in type.enum %} + {{ literal | sanitize_literal | dash_to_camelcase | camelcase_to_hacker_style | upper }}: "{{ literal }}"{{',' if not loop.last}} + {% endfor %} +}; + +{% endfor %} + +{# Generate types. #} +{% for type in domain.types %} + {% if not (type.type == "object") or not ("properties" in type) %}{% continue %}{% endif %} +/** + {% if type.description %} + * {{type.description}} + * + {% endif %} + {% for property in type.properties %} + {% if property.description %} + * {{property.name}}: {{property.description}} + {% endif %} + {% endfor %} + * + {% if type.properties %} + * @typedef {{ '{{' }} + {% for property in type.properties %} + {% if property.optional %} + * {{property.name}}: ({{ short_form(resolve_type(property).js_type) }}|undefined){% if not loop.last %},{% endif %} + + {% else %} + * {{property.name}}: {{ short_form(resolve_type(property).js_type) }}{% if not loop.last %}, {% endif %} + + {% endif %} + {% endfor %} + * {{ '}}' }} + {% else %} + * @typedef {undefined} + {% endif %} + */ +{{domain.domain}}.{{type.id}}; + + +{% endfor %} + +exports = {{domain.domain}};
diff --git a/headless/lib/headless_browsertest_resource_ids b/headless/lib/headless_browsertest_resource_ids index fad4a66..9eaf2c63 100644 --- a/headless/lib/headless_browsertest_resource_ids +++ b/headless/lib/headless_browsertest_resource_ids
@@ -1,7 +1,7 @@ { "SRCDIR": "../..", - "headless/lib/headless_browsertest_resources.grd": { + "headless/headless_browsertest_resources.grd": { "includes": [31000], } }
diff --git a/headless/lib/tab_socket_externs.js b/headless/lib/tab_socket_externs.js new file mode 100644 index 0000000..96428747 --- /dev/null +++ b/headless/lib/tab_socket_externs.js
@@ -0,0 +1,12 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +/** + * @fileoverview Headless TabSocket externs. + * @externs + */ + +window.TabSocket = {}; +window.TabSocket.send = function(a) {}; +window.TabSocket.onmessage = {};
diff --git a/headless/test/bindings_test.js b/headless/test/bindings_test.js new file mode 100644 index 0000000..cc035e1 --- /dev/null +++ b/headless/test/bindings_test.js
@@ -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. + +goog.module('chromium.BindingsTest'); +// Needed to let C++ invoke the test. +goog.module.declareLegacyNamespace(); + +const Connection = goog.require('chromium.DevTools.Connection'); +const DOM = goog.require('chromium.DevTools.DOM'); +const Runtime = goog.require('chromium.DevTools.Runtime'); + +/** + * A trivial test which is invoked from C++ by HeadlessJsBindingsTest. + */ +class BindingsTest { + constructor() {} + + /** + * Evaluates 1+1 and returns the result over the chromium.DevTools.Connection. + */ + evalOneAddOne() { + let connection = new Connection(window.TabSocket); + let runtime = new Runtime(connection); + runtime.Evaluate({'expression': '1+1'}).then(function(message) { + connection.sendDevToolsMessage( + '__Result', + {'result': JSON.stringify(message.result.value)}); + }); + } + + /** + * Evaluates 1+1 and returns the result over the chromium.DevTools.Connection. + */ + listenForChildNodeCountUpdated() { + let connection = new Connection(window.TabSocket); + let dom = new DOM(connection); + dom.OnChildNodeCountUpdated(function(params) { + connection.sendDevToolsMessage('__Result', + {'result': JSON.stringify(params)}); + }); + dom.Enable().then(function() { + return dom.GetDocument({}); + }).then(function() { + // Create a new div which should trigger the event. + let div = document.createElement('div'); + document.body.appendChild(div); + }); + } +} + +exports = BindingsTest;
diff --git a/headless/test/headless_js_bindings_browsertest.cc b/headless/test/headless_js_bindings_browsertest.cc new file mode 100644 index 0000000..7b9fad5 --- /dev/null +++ b/headless/test/headless_js_bindings_browsertest.cc
@@ -0,0 +1,160 @@ +// 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 <memory> +#include <string> +#include <vector> + +#include "base/base64.h" +#include "base/json/json_reader.h" +#include "base/path_service.h" +#include "base/threading/thread_restrictions.h" +#include "content/public/test/browser_test.h" +#include "headless/grit/headless_browsertest_resources.h" +#include "headless/public/devtools/domains/runtime.h" +#include "headless/public/headless_browser.h" +#include "headless/public/headless_devtools_client.h" +#include "headless/public/headless_tab_socket.h" +#include "headless/public/headless_web_contents.h" +#include "headless/test/headless_browser_test.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "ui/base/resource/resource_bundle.h" + +namespace headless { + +class HeadlessJsBindingsTest + : public HeadlessAsyncDevTooledBrowserTest, + public HeadlessTabSocket::Listener, + public HeadlessDevToolsClient::RawProtocolListener { + public: + void SetUp() override { + options()->mojo_service_names.insert("headless::TabSocket"); + HeadlessAsyncDevTooledBrowserTest::SetUp(); + } + + void SetUpOnMainThread() override { + base::ThreadRestrictions::SetIOAllowed(true); + base::FilePath pak_path; + ASSERT_TRUE(PathService::Get(base::DIR_MODULE, &pak_path)); + pak_path = pak_path.AppendASCII("headless_browser_tests.pak"); + ResourceBundle::GetSharedInstance().AddDataPackFromPath( + pak_path, ui::SCALE_FACTOR_NONE); + } + + void RunDevTooledTest() override { + headless_tab_socket_ = web_contents_->GetHeadlessTabSocket(); + DCHECK(headless_tab_socket_); + headless_tab_socket_->SetListener(this); + devtools_client_->SetRawProtocolListener(this); + devtools_client_->GetRuntime()->Evaluate( + ResourceBundle::GetSharedInstance() + .GetRawDataResource(DEVTOOLS_BINDINGS_TEST) + .as_string(), + base::Bind(&HeadlessJsBindingsTest::FailOnJsEvaluateException, + base::Unretained(this))); + RunJsBindingsTest(); + } + + virtual void RunJsBindingsTest() = 0; + virtual std::string GetExpectedResult() = 0; + + void FailOnJsEvaluateException( + std::unique_ptr<runtime::EvaluateResult> result) { + if (!result->HasExceptionDetails()) + return; + + FinishAsynchronousTest(); + + const runtime::ExceptionDetails* exception_details = + result->GetExceptionDetails(); + FAIL() << exception_details->GetText() + << (exception_details->HasException() + ? exception_details->GetException()->GetDescription().c_str() + : ""); + } + + void OnMessageFromTab(const std::string& json_message) override { + std::unique_ptr<base::Value> message = + base::JSONReader::Read(json_message, base::JSON_PARSE_RFC); + const base::DictionaryValue* message_dict; + const base::DictionaryValue* params_dict; + std::string method; + int id; + if (!message || !message->GetAsDictionary(&message_dict) || + !message_dict->GetString("method", &method) || + !message_dict->GetDictionary("params", ¶ms_dict) || + !message_dict->GetInteger("id", &id)) { + FinishAsynchronousTest(); + FAIL() << "Badly formed message " << json_message; + return; + } + + if (method == "__Result") { + std::string result; + params_dict->GetString("result", &result); + EXPECT_EQ(GetExpectedResult(), result); + FinishAsynchronousTest(); + return; + } + + devtools_client_->SendRawDevToolsMessage(json_message); + } + + HeadlessWebContents::Builder::TabSocketType GetTabSocketType() override { + return HeadlessWebContents::Builder::TabSocketType::MAIN_WORLD; + } + + bool OnProtocolMessage(const std::string& devtools_agent_host_id, + const std::string& json_message, + const base::DictionaryValue& parsed_message) override { + int id; + // If |parsed_message| contains an id we know this is a message reply. + if (parsed_message.GetInteger("id", &id)) { + // We are only interested in message replies (ones with an id) where the + // id is odd. The reason is HeadlessDevToolsClientImpl uses even/oddness + // to distinguish between commands send from the C++ bindings and those + // via HeadlessDevToolsClientImpl::SendRawDevToolsMessage. + if ((id % 2) == 0) + return false; + } + + headless_tab_socket_->SendMessageToTab(json_message); + return true; + } + + private: + HeadlessTabSocket* headless_tab_socket_; +}; + +class SimpleCommandJsBindingsTest : public HeadlessJsBindingsTest { + public: + void RunJsBindingsTest() override { + devtools_client_->GetRuntime()->Evaluate( + "new chromium.BindingsTest().evalOneAddOne();", + base::Bind(&HeadlessJsBindingsTest::FailOnJsEvaluateException, + base::Unretained(this))); + } + + std::string GetExpectedResult() override { return "2"; } +}; + +HEADLESS_ASYNC_DEVTOOLED_TEST_F(SimpleCommandJsBindingsTest); + +class SimpleEventJsBindingsTest : public HeadlessJsBindingsTest { + public: + void RunJsBindingsTest() override { + devtools_client_->GetRuntime()->Evaluate( + "new chromium.BindingsTest().listenForChildNodeCountUpdated();", + base::Bind(&HeadlessJsBindingsTest::FailOnJsEvaluateException, + base::Unretained(this))); + } + + std::string GetExpectedResult() override { + return "{\"nodeId\":4,\"childNodeCount\":1}"; + } +}; + +HEADLESS_ASYNC_DEVTOOLED_TEST_F(SimpleEventJsBindingsTest); +} // namespace headless
diff --git a/ios/chrome/browser/tabs/tab.mm b/ios/chrome/browser/tabs/tab.mm index 49ab25d1..07b97175 100644 --- a/ios/chrome/browser/tabs/tab.mm +++ b/ios/chrome/browser/tabs/tab.mm
@@ -571,7 +571,7 @@ } - (BOOL)loadFinished { - return [self.webController loadPhase] == web::PAGE_LOADED; + return self.webState && !self.webState->IsLoading(); } - (void)setIsVoiceSearchResultsTab:(BOOL)isVoiceSearchResultsTab { @@ -765,13 +765,12 @@ // If the page has finished loading, take a snapshot. If the page is still // loading, do nothing, as CRWWebController will automatically take a // snapshot once the load completes. - BOOL loadingFinished = self.webController.loadPhase == web::PAGE_LOADED; - if (loadingFinished) + if ([self loadFinished]) [self updateSnapshotWithOverlay:YES visibleFrameOnly:YES]; [[OmniboxGeolocationController sharedInstance] finishPageLoadForTab:self - loadSuccess:loadingFinished]; + loadSuccess:[self loadFinished]]; [self countMainFrameLoad]; } @@ -1461,7 +1460,7 @@ - (void)webState:(web::WebState*)webState didLoadPageWithSuccess:(BOOL)loadSuccess { - DCHECK(self.webController.loadPhase == web::PAGE_LOADED); + DCHECK([self loadFinished]); // Cancel prerendering if response is "application/octet-stream". It can be a // video file which should not be played from preload tab (crbug.com/436813).
diff --git a/ios/chrome/browser/tabs/tab_unittest.mm b/ios/chrome/browser/tabs/tab_unittest.mm index 8ff8547..bd5e988 100644 --- a/ios/chrome/browser/tabs/tab_unittest.mm +++ b/ios/chrome/browser/tabs/tab_unittest.mm
@@ -118,9 +118,6 @@ namespace { -const web::LoadPhase kPageLoading = web::PAGE_LOADING; -const web::LoadPhase kPageLoaded = web::PAGE_LOADED; - // Observer of a QueryHistory request. class HistoryQueryResultsObserver : public base::RefCountedThreadSafe<HistoryQueryResultsObserver> { @@ -258,8 +255,7 @@ [tab_ navigationManager]->GetLastCommittedItem()->SetTitle(new_title); web_state_impl_->OnTitleChanged(); - [[[(id)mock_web_controller_ expect] - andReturnValue:OCMOCK_VALUE(kPageLoaded)] loadPhase]; + web_state_impl_->SetIsLoading(false); web_state_impl_->OnPageLoaded(redirectUrl, true); } @@ -271,10 +267,8 @@ web::NavigationManager::WebLoadParams params(url); params.transition_type = ui::PAGE_TRANSITION_TYPED; [tab_ navigationManager]->LoadURLWithParams(params); - [[[(id)mock_web_controller_ expect] - andReturnValue:OCMOCK_VALUE(kPageLoading)] loadPhase]; - [[[(id)mock_web_controller_ expect] - andReturnValue:OCMOCK_VALUE(kPageLoaded)] loadPhase]; + web_state_impl_->SetIsLoading(true); + web_state_impl_->SetIsLoading(false); web_state_impl_->OnPageLoaded(url, true); web_state_impl_->OnTitleChanged(); }
diff --git a/ios/chrome/browser/ui/reading_list/BUILD.gn b/ios/chrome/browser/ui/reading_list/BUILD.gn index f58c9d5d..ef4bca6 100644 --- a/ios/chrome/browser/ui/reading_list/BUILD.gn +++ b/ios/chrome/browser/ui/reading_list/BUILD.gn
@@ -3,10 +3,7 @@ # found in the LICENSE file. source_set("reading_list") { - configs += [ "//build/config/compiler:enable_arc" ] sources = [ - "number_badge_view.h", - "number_badge_view.mm", "offline_page_native_content.h", "offline_page_native_content.mm", "reading_list_collection_view_controller.h", @@ -16,10 +13,6 @@ "reading_list_collection_view_item_accessibility_delegate.h", "reading_list_coordinator.h", "reading_list_coordinator.mm", - "reading_list_data_sink.h", - "reading_list_data_source.h", - "reading_list_empty_collection_background.h", - "reading_list_empty_collection_background.mm", "reading_list_mediator.h", "reading_list_mediator.mm", "reading_list_menu_notification_delegate.h", @@ -27,17 +20,16 @@ "reading_list_menu_notifier.mm", "reading_list_side_swipe_provider.h", "reading_list_side_swipe_provider.mm", - "reading_list_toolbar.h", - "reading_list_toolbar.mm", "reading_list_utils.h", "reading_list_utils.mm", "reading_list_view_controller.h", "reading_list_view_controller.mm", ] deps = [ + ":reading_list_ui", ":resources", "//base", - "//components/prefs", + "//components/favicon/core", "//components/reading_list/core", "//components/reading_list/ios", "//components/strings", @@ -47,11 +39,8 @@ "//ios/chrome/browser/browser_state", "//ios/chrome/browser/favicon", "//ios/chrome/browser/reading_list", - "//ios/chrome/browser/tabs", "//ios/chrome/browser/ui", "//ios/chrome/browser/ui/alert_coordinator", - "//ios/chrome/browser/ui/collection_view/cells", - "//ios/chrome/browser/ui/colors", "//ios/chrome/browser/ui/favicon", "//ios/chrome/browser/ui/favicon:favicon_ui", "//ios/chrome/browser/ui/keyboard", @@ -59,12 +48,8 @@ "//ios/chrome/browser/ui/side_swipe", "//ios/chrome/browser/ui/static_content", "//ios/chrome/browser/ui/util", - "//ios/chrome/common", - "//ios/third_party/material_components_ios", - "//ios/third_party/material_roboto_font_loader_ios", "//ios/web", "//ios/web:reload_type", - "//net", "//ui/base", "//ui/strings", "//url", @@ -74,6 +59,42 @@ ] allow_circular_includes_from = [ "//ios/chrome/browser/ui/side_swipe" ] libs = [ "UIKit.framework" ] + configs += [ "//build/config/compiler:enable_arc" ] +} + +source_set("reading_list_ui") { + configs += [ "//build/config/compiler:enable_arc" ] + sources = [ + "number_badge_view.h", + "number_badge_view.mm", + "reading_list_collection_view_cell.h", + "reading_list_collection_view_cell.mm", + "reading_list_data_sink.h", + "reading_list_data_source.h", + "reading_list_empty_collection_background.h", + "reading_list_empty_collection_background.mm", + "reading_list_toolbar.h", + "reading_list_toolbar.mm", + ] + deps = [ + ":resources", + "//base", + "//components/strings", + "//components/url_formatter", + "//ios/chrome/app/strings", + "//ios/chrome/browser", + "//ios/chrome/browser/ui", + "//ios/chrome/browser/ui/alert_coordinator", + "//ios/chrome/browser/ui/collection_view/cells", + "//ios/chrome/browser/ui/colors", + "//ios/chrome/browser/ui/favicon:favicon_ui", + "//ios/chrome/browser/ui/util", + "//ios/chrome/common", + "//ios/third_party/material_components_ios", + "//ios/third_party/material_roboto_font_loader_ios", + "//ui/base", + ] + libs = [ "UIKit.framework" ] } source_set("unit_tests") { @@ -87,6 +108,7 @@ ] deps = [ ":reading_list", + ":reading_list_ui", "//base", "//base/test:test_support", "//components/favicon/core", @@ -116,6 +138,7 @@ ] deps = [ ":reading_list", + ":reading_list_ui", "//base", "//base/test:test_support", "//components/reading_list/core",
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_collection_view_cell.h b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_cell.h new file mode 100644 index 0000000..2fe93d76 --- /dev/null +++ b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_cell.h
@@ -0,0 +1,36 @@ +// 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_CHROME_BROWSER_UI_READING_LIST_READING_LIST_COLLECTION_VIEW_CELL_H_ +#define IOS_CHROME_BROWSER_UI_READING_LIST_READING_LIST_COLLECTION_VIEW_CELL_H_ + +#import "ios/third_party/material_components_ios/src/components/CollectionCells/src/MaterialCollectionCells.h" + +@class FaviconViewNew; + +typedef NS_ENUM(NSInteger, ReadingListUIDistillationStatus) { + ReadingListUIDistillationStatusPending, + ReadingListUIDistillationStatusSuccess, + ReadingListUIDistillationStatusFailure +}; + +// Cell for ReadingListCollectionViewItem. +@interface ReadingListCell : MDCCollectionViewCell + +// Title label. +@property(nonatomic, readonly, strong) UILabel* titleLabel; +// Subtitle label. +@property(nonatomic, readonly, strong) UILabel* subtitleLabel; +// Timestamp of the distillation in microseconds since Jan 1st 1970. +@property(nonatomic, assign) int64_t distillationDate; +// Size of the distilled files. +@property(nonatomic, assign) int64_t distillationSize; +// View for displaying the favicon for the reading list entry. +@property(nonatomic, readonly, strong) FaviconViewNew* faviconView; +// Status of the offline version. Updates the visual indicator when updated. +@property(nonatomic, assign) ReadingListUIDistillationStatus distillationState; + +@end + +#endif // IOS_CHROME_BROWSER_UI_READING_LIST_READING_LIST_COLLECTION_VIEW_CELL_H_
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_collection_view_cell.mm b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_cell.mm new file mode 100644 index 0000000..6fc635d --- /dev/null +++ b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_cell.mm
@@ -0,0 +1,275 @@ +// 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/chrome/browser/ui/reading_list/reading_list_collection_view_cell.h" + +#include "base/strings/sys_string_conversions.h" +#include "base/time/time.h" +#import "ios/chrome/browser/ui/colors/MDCPalette+CrAdditions.h" +#import "ios/chrome/browser/ui/favicon/favicon_view.h" +#import "ios/chrome/browser/ui/uikit_ui_util.h" +#include "ios/chrome/grit/ios_strings.h" +#import "ios/third_party/material_components_ios/src/components/Palettes/src/MaterialPalettes.h" +#import "ios/third_party/material_components_ios/src/components/Typography/src/MaterialTypography.h" +#include "ui/base/l10n/l10n_util.h" +#include "ui/base/l10n/time_format.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +namespace { +NSString* kSuccessImageString = @"distillation_success"; +NSString* kFailureImageString = @"distillation_fail"; + +// Height of the cell. +const CGFloat kCellHeight = 72; + +// Distillation indicator constants. +const CGFloat kDistillationIndicatorSize = 18; + +// Margin for the elements displayed in the cell. +const CGFloat kMargin = 16; + +// Transparency of the distillation size and date. +const CGFloat kInfoTextTransparency = 0.38; +} // namespace + +@implementation ReadingListCell { + UIImageView* _downloadIndicator; + UILayoutGuide* _textGuide; + + UILabel* _distillationSizeLabel; + UILabel* _distillationDateLabel; + + // View containing |_distillationSizeLabel| and |_distillationDateLabel|. + UIView* _infoView; + + // Whether |_infoView| is visible. + BOOL _showInfo; +} +@synthesize faviconView = _faviconView; +@synthesize titleLabel = _titleLabel; +@synthesize subtitleLabel = _subtitleLabel; +@synthesize distillationDate = _distillationDate; +@synthesize distillationSize = _distillationSize; +@synthesize distillationState = _distillationState; + +- (instancetype)initWithFrame:(CGRect)frame { + self = [super initWithFrame:frame]; + if (self) { + id<MDCTypographyFontLoading> fontLoader = [MDCTypography fontLoader]; + CGFloat faviconSize = kFaviconPreferredSize; + _titleLabel = [[UILabel alloc] init]; + _titleLabel.font = [fontLoader mediumFontOfSize:16]; + _titleLabel.textColor = [[MDCPalette greyPalette] tint900]; + _titleLabel.translatesAutoresizingMaskIntoConstraints = NO; + _titleLabel.accessibilityIdentifier = @"Reading List Item title"; + + _subtitleLabel = [[UILabel alloc] init]; + _subtitleLabel.font = [fontLoader mediumFontOfSize:14]; + _subtitleLabel.textColor = [[MDCPalette greyPalette] tint500]; + _subtitleLabel.translatesAutoresizingMaskIntoConstraints = NO; + _subtitleLabel.accessibilityIdentifier = @"Reading List Item subtitle"; + + _distillationDateLabel = [[UILabel alloc] init]; + _distillationDateLabel.font = [fontLoader mediumFontOfSize:12]; + [_distillationDateLabel + setContentHuggingPriority:UILayoutPriorityDefaultHigh + forAxis:UILayoutConstraintAxisHorizontal]; + _distillationDateLabel.textColor = + [UIColor colorWithWhite:0 alpha:kInfoTextTransparency]; + _distillationDateLabel.translatesAutoresizingMaskIntoConstraints = NO; + _distillationDateLabel.accessibilityIdentifier = + @"Reading List Item distillation date"; + + _distillationSizeLabel = [[UILabel alloc] init]; + _distillationSizeLabel.font = [fontLoader mediumFontOfSize:12]; + [_distillationSizeLabel + setContentHuggingPriority:UILayoutPriorityDefaultHigh + forAxis:UILayoutConstraintAxisHorizontal]; + _distillationSizeLabel.textColor = + [UIColor colorWithWhite:0 alpha:kInfoTextTransparency]; + _distillationSizeLabel.translatesAutoresizingMaskIntoConstraints = NO; + _distillationSizeLabel.accessibilityIdentifier = + @"Reading List Item distillation size"; + + _faviconView = [[FaviconViewNew alloc] init]; + CGFloat fontSize = floorf(faviconSize / 2); + [_faviconView setFont:[fontLoader regularFontOfSize:fontSize]]; + _faviconView.translatesAutoresizingMaskIntoConstraints = NO; + + _downloadIndicator = [[UIImageView alloc] init]; + [_downloadIndicator setTranslatesAutoresizingMaskIntoConstraints:NO]; + _downloadIndicator.accessibilityIdentifier = + @"Reading List Item download indicator"; + [_faviconView addSubview:_downloadIndicator]; + + [self.contentView addSubview:_faviconView]; + [self.contentView addSubview:_titleLabel]; + [self.contentView addSubview:_subtitleLabel]; + + _infoView = [[UIView alloc] initWithFrame:CGRectZero]; + [_infoView addSubview:_distillationDateLabel]; + [_infoView addSubview:_distillationSizeLabel]; + _infoView.translatesAutoresizingMaskIntoConstraints = NO; + + _textGuide = [[UILayoutGuide alloc] init]; + [self.contentView addLayoutGuide:_textGuide]; + + ApplyVisualConstraintsWithMetrics( + @[ + @"H:|[date]-(>=margin)-[size]|", + @"V:[title][subtitle]", + @"H:|-(margin)-[favicon]-(margin)-[title]-(>=margin)-|", + @"H:[favicon]-(margin)-[subtitle]-(>=margin)-|", + @"V:|[date]|", + @"V:|[size]|", + ], + @{ + @"favicon" : _faviconView, + @"title" : _titleLabel, + @"subtitle" : _subtitleLabel, + @"date" : _distillationDateLabel, + @"size" : _distillationSizeLabel, + }, + @{ + @"margin" : @(kMargin), + }); + + // Sets the bottom of the text. Lower the priority so we can add the details + // later. + NSLayoutConstraint* bottomTextConstraint = [_textGuide.bottomAnchor + constraintEqualToAnchor:_subtitleLabel.bottomAnchor]; + bottomTextConstraint.priority = UILayoutPriorityDefaultHigh; + NSLayoutConstraint* topTextConstraint = + [_textGuide.topAnchor constraintEqualToAnchor:_titleLabel.topAnchor]; + + [NSLayoutConstraint activateConstraints:@[ + // Height for the cell. + [self.contentView.heightAnchor constraintEqualToConstant:kCellHeight], + + // Text constraints. + topTextConstraint, + bottomTextConstraint, + + // Favicons are always the same size. + [_faviconView.widthAnchor constraintEqualToConstant:faviconSize], + [_faviconView.heightAnchor constraintEqualToConstant:faviconSize], + + // Center the content (favicon and text) vertically. + [_faviconView.centerYAnchor + constraintEqualToAnchor:self.contentView.centerYAnchor], + [_textGuide.centerYAnchor + constraintEqualToAnchor:self.contentView.centerYAnchor], + + // Place the download indicator in the bottom right corner of the favicon. + [[_downloadIndicator centerXAnchor] + constraintEqualToAnchor:_faviconView.trailingAnchor], + [[_downloadIndicator centerYAnchor] + constraintEqualToAnchor:_faviconView.bottomAnchor], + [[_downloadIndicator widthAnchor] + constraintEqualToConstant:kDistillationIndicatorSize], + [[_downloadIndicator heightAnchor] + constraintEqualToConstant:kDistillationIndicatorSize], + ]]; + + self.editingSelectorColor = [[MDCPalette cr_bluePalette] tint500]; + } + return self; +} + +- (void)setDistillationState: + (ReadingListUIDistillationStatus)distillationState { + if (_distillationState == distillationState) + return; + + _distillationState = distillationState; + switch (distillationState) { + case ReadingListUIDistillationStatusFailure: + [_downloadIndicator setImage:[UIImage imageNamed:kFailureImageString]]; + break; + + case ReadingListUIDistillationStatusSuccess: + [_downloadIndicator setImage:[UIImage imageNamed:kSuccessImageString]]; + break; + + case ReadingListUIDistillationStatusPending: + [_downloadIndicator setImage:nil]; + break; + } +} + +- (void)setShowInfo:(BOOL)show { + if (_showInfo == show) { + return; + } + _showInfo = show; + if (!show) { + [_infoView removeFromSuperview]; + return; + } + [self.contentView addSubview:_infoView]; + ApplyVisualConstraintsWithMetrics( + @[ + @"H:|-(margin)-[favicon]-(margin)-[detail]-(margin)-|", + ], + @{ + @"favicon" : _faviconView, + @"detail" : _infoView, + }, + @{ + @"margin" : @(kMargin), + }); + [NSLayoutConstraint activateConstraints:@[ + [_infoView.topAnchor constraintEqualToAnchor:_subtitleLabel.bottomAnchor], + [_infoView.bottomAnchor constraintEqualToAnchor:_textGuide.bottomAnchor], + ]]; +} + +- (void)setDistillationSize:(int64_t)distillationSize { + [_distillationSizeLabel + setText:[NSByteCountFormatter + stringFromByteCount:distillationSize + countStyle:NSByteCountFormatterCountStyleFile]]; + _distillationSize = distillationSize; + BOOL showInfo = _distillationSize != 0 && _distillationDate != 0; + [self setShowInfo:showInfo]; +} + +- (void)setDistillationDate:(int64_t)distillationDate { + int64_t now = (base::Time::Now() - base::Time::UnixEpoch()).InMicroseconds(); + int64_t elapsed = now - distillationDate; + NSString* text; + if (elapsed < base::Time::kMicrosecondsPerMinute) { + // This will also catch items added in the future. In that case, show the + // "just now" string. + text = l10n_util::GetNSString(IDS_IOS_READING_LIST_JUST_NOW); + } else { + text = base::SysUTF16ToNSString(ui::TimeFormat::SimpleWithMonthAndYear( + ui::TimeFormat::FORMAT_ELAPSED, ui::TimeFormat::LENGTH_LONG, + base::TimeDelta::FromMicroseconds(elapsed), true)); + } + + [_distillationDateLabel setText:text]; + _distillationDate = distillationDate; + BOOL showInfo = _distillationSize != 0 && _distillationDate != 0; + [self setShowInfo:showInfo]; +} + +#pragma mark - UICollectionViewCell + +- (void)prepareForReuse { + self.titleLabel.text = nil; + self.subtitleLabel.text = nil; + self.distillationState = ReadingListUIDistillationStatusPending; + self.distillationDate = 0; + self.distillationSize = 0; + [self setShowInfo:NO]; + [self.faviconView configureWithAttributes:nil]; + self.accessibilityCustomActions = nil; + [super prepareForReuse]; +} + +@end
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.h b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.h index 43d6817c..4814b9a1 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.h +++ b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.h
@@ -9,12 +9,7 @@ #import "ios/chrome/browser/ui/reading_list/reading_list_toolbar.h" -namespace favicon { -class LargeIconService; -} - @class ReadingListCollectionViewController; -@class ReadingListCollectionViewItem; @protocol ReadingListDataSource; // Audience for the ReadingListCollectionViewController @@ -39,16 +34,14 @@ - (void)readingListCollectionViewController: (ReadingListCollectionViewController*) readingListCollectionViewController - displayContextMenuForItem: - (ReadingListCollectionViewItem*)readingListItem + displayContextMenuForItem:(CollectionViewItem*)readingListItem atPoint:(CGPoint)menuLocation; // Opens the entry corresponding to the |readingListItem|. - (void) readingListCollectionViewController: (ReadingListCollectionViewController*)readingListCollectionViewController - openItem: - (ReadingListCollectionViewItem*)readingListItem; + openItem:(CollectionViewItem*)readingListItem; // Opens the entry corresponding to the |item| in a new tab, |incognito| or not. - (void)readingListCollectionViewController: @@ -70,7 +63,6 @@ : CollectionViewController<ReadingListToolbarActions> - (instancetype)initWithDataSource:(id<ReadingListDataSource>)dataSource - largeIconService:(favicon::LargeIconService*)largeIconService toolbar:(ReadingListToolbar*)toolbar NS_DESIGNATED_INITIALIZER; - (instancetype)initWithStyle:(CollectionViewControllerStyle)style @@ -82,9 +74,6 @@ audience; @property(nonatomic, weak) id<ReadingListDataSource> dataSource; -@property(nonatomic, assign, readonly) - favicon::LargeIconService* largeIconService; - // Prepares this view controller to be dismissed. - (void)willBeDismissed;
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm index 4e330d6..710b5349 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm +++ b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm
@@ -27,7 +27,6 @@ #import "ios/chrome/browser/ui/reading_list/reading_list_data_source.h" #import "ios/chrome/browser/ui/reading_list/reading_list_empty_collection_background.h" #import "ios/chrome/browser/ui/reading_list/reading_list_toolbar.h" -#import "ios/chrome/browser/ui/reading_list/reading_list_utils.h" #import "ios/chrome/browser/ui/uikit_ui_util.h" #include "ios/chrome/browser/ui/url_loader.h" #include "ios/chrome/grit/ios_strings.h" @@ -73,9 +72,6 @@ BOOL _dataSourceHasBeenModified; } -// Lazily instantiated. -@property(nonatomic, strong, readonly) - FaviconAttributesProvider* attributesProvider; // Whether the data source modifications should be taken into account. @property(nonatomic, assign) BOOL shouldMonitorDataSource; @@ -86,16 +82,8 @@ // Fills section |sectionIdentifier| with the items from |array|. - (void)loadItemsFromArray:(NSArray<ReadingListCollectionViewItem*>*)array toSection:(SectionIdentifier)sectionIdentifier; -// Whether the data source has changed. -- (BOOL)hasDataSourceChanged; -// Returns whether there is a difference between the elements contained in the -// |sectionIdentifier| and those in the |array|. -- (BOOL)section:(SectionIdentifier)sectionIdentifier - isDifferentOfArray:(NSArray<ReadingListCollectionViewItem*>*)array; // Reloads the data if a changed occured during editing - (void)applyPendingUpdates; -// Fetches the |faviconURL| of this |item|, then reconfigures the item. -- (void)fetchFaviconForItem:(ReadingListCollectionViewItem*)item; // Returns whether there are elements in the section identified by // |sectionIdentifier|. - (BOOL)hasItemInSection:(SectionIdentifier)sectionIdentifier; @@ -168,21 +156,17 @@ @synthesize delegate = _delegate; @synthesize dataSource = _dataSource; -@synthesize largeIconService = _largeIconService; -@synthesize attributesProvider = _attributesProvider; @synthesize shouldMonitorDataSource = _shouldMonitorDataSource; #pragma mark lifecycle - (instancetype)initWithDataSource:(id<ReadingListDataSource>)dataSource - largeIconService:(favicon::LargeIconService*)largeIconService toolbar:(ReadingListToolbar*)toolbar { self = [super initWithStyle:CollectionViewControllerStyleAppBar]; if (self) { _toolbar = toolbar; _dataSource = dataSource; - _largeIconService = largeIconService; _emptyCollectionBackground = [[ReadingListEmptyCollectionBackground alloc] init]; @@ -196,21 +180,6 @@ #pragma mark - properties -- (FaviconAttributesProvider*)attributesProvider { - if (_attributesProvider) { - return _attributesProvider; - } - - // Accept any favicon even the smallest ones (16x16) as it is better than the - // fallback icon. - // Pass 1 as minimum size to avoid empty favicons. - _attributesProvider = [[FaviconAttributesProvider alloc] - initWithFaviconSize:kFaviconPreferredSize - minFaviconSize:1 - largeIconService:self.largeIconService]; - return _attributesProvider; -} - - (void)setToolbarState:(ReadingListToolbarState)toolbarState { [_toolbar setState:toolbarState]; } @@ -259,12 +228,10 @@ if (self.editor.editing) { [self updateToolbarState]; } else { - ReadingListCollectionViewItem* readingListItem = - base::mac::ObjCCastStrict<ReadingListCollectionViewItem>( - [self.collectionViewModel itemAtIndexPath:indexPath]); - - [self.delegate readingListCollectionViewController:self - openItem:readingListItem]; + [self.delegate + readingListCollectionViewController:self + openItem:[self.collectionViewModel + itemAtIndexPath:indexPath]]; } } @@ -312,34 +279,42 @@ } } -- (void)readingListModelDidApplyChanges { - if (!self.shouldMonitorDataSource) { - return; - } - +- (void)dataSourceChanged { // If we are editing and monitoring the model updates, set a flag to reload // the data at the end of the editing. - if ([self.editor isEditing]) { + if (self.editor.editing) { _dataSourceHasBeenModified = YES; - return; - } - - // Ignore single element updates when the data source is doing batch updates. - if ([self.dataSource isPerformingBatchUpdates]) { - return; - } - - if ([self hasDataSourceChanged]) + } else { [self reloadData]; + } } -- (void)readingListModelCompletedBatchUpdates { - if (!self.shouldMonitorDataSource) { - return; +- (NSArray<CollectionViewItem*>*)readItems { + if (![self.collectionViewModel + hasSectionForSectionIdentifier:SectionIdentifierRead]) { + return nil; } + return [self.collectionViewModel + itemsInSectionWithIdentifier:SectionIdentifierRead]; +} - if ([self hasDataSourceChanged]) - [self reloadData]; +- (NSArray<CollectionViewItem*>*)unreadItems { + if (![self.collectionViewModel + hasSectionForSectionIdentifier:SectionIdentifierUnread]) { + return nil; + } + return [self.collectionViewModel + itemsInSectionWithIdentifier:SectionIdentifierUnread]; +} + +- (void)itemHasChangedAfterDelay:(CollectionViewItem*)item { + if ([self.collectionViewModel hasItem:item]) { + [self reconfigureCellsForItems:@[ item ]]; + } +} + +- (void)itemsHaveChanged:(NSArray<CollectionViewItem*>*)items { + [self reconfigureCellsForItems:items]; } #pragma mark - Public methods @@ -417,6 +392,7 @@ - (void)loadModel { [super loadModel]; + _dataSourceHasBeenModified = NO; if (!self.dataSource.hasElements) { [self collectionIsEmpty]; @@ -439,7 +415,7 @@ forSectionWithIdentifier:sectionIdentifier]; for (ReadingListCollectionViewItem* item in items) { item.type = ItemTypeItem; - [self fetchFaviconForItem:item]; + [self.dataSource fetchFaviconForItem:item]; item.accessibilityDelegate = self; [model addItem:item toSectionWithIdentifier:sectionIdentifier]; } @@ -464,85 +440,6 @@ } } -- (BOOL)hasDataSourceChanged { - NSMutableArray<ReadingListCollectionViewItem*>* readArray = - [NSMutableArray array]; - NSMutableArray<ReadingListCollectionViewItem*>* unreadArray = - [NSMutableArray array]; - [self.dataSource fillReadItems:readArray unreadItems:unreadArray]; - - if ([self section:SectionIdentifierRead isDifferentOfArray:readArray]) - return YES; - if ([self section:SectionIdentifierUnread isDifferentOfArray:unreadArray]) - return YES; - - return NO; -} - -- (BOOL)section:(SectionIdentifier)sectionIdentifier - isDifferentOfArray:(NSArray<ReadingListCollectionViewItem*>*)array { - if (![self.collectionViewModel - hasSectionForSectionIdentifier:sectionIdentifier]) { - return array.count > 0; - } - - NSArray* items = - [self.collectionViewModel itemsInSectionWithIdentifier:sectionIdentifier]; - if (items.count != array.count) - return YES; - - NSMutableArray<ReadingListCollectionViewItem*>* itemsToReconfigure = - [NSMutableArray array]; - - NSInteger index = 0; - for (ReadingListCollectionViewItem* newItem in array) { - ReadingListCollectionViewItem* oldItem = - base::mac::ObjCCastStrict<ReadingListCollectionViewItem>(items[index]); - if (oldItem.url == newItem.url) { - if (![oldItem isEqual:newItem]) { - oldItem.title = newItem.title; - oldItem.subtitle = newItem.subtitle; - oldItem.distillationState = newItem.distillationState; - oldItem.distillationDate = newItem.distillationDate; - oldItem.distillationSize = newItem.distillationSize; - [itemsToReconfigure addObject:oldItem]; - } - if (oldItem.faviconPageURL != newItem.faviconPageURL) { - oldItem.faviconPageURL = newItem.faviconPageURL; - [self fetchFaviconForItem:oldItem]; - } - } - if (![oldItem isEqual:newItem]) { - return YES; - } - index++; - } - [self reconfigureCellsForItems:itemsToReconfigure]; - return NO; -} - -- (void)fetchFaviconForItem:(ReadingListCollectionViewItem*)item { - __weak ReadingListCollectionViewItem* weakItem = item; - __weak ReadingListCollectionViewController* weakSelf = self; - void (^completionBlock)(FaviconAttributes* attributes) = - ^(FaviconAttributes* attributes) { - ReadingListCollectionViewItem* strongItem = weakItem; - ReadingListCollectionViewController* strongSelf = weakSelf; - if (!strongSelf || !strongItem) { - return; - } - - strongItem.attributes = attributes; - - if ([strongSelf.collectionViewModel hasItem:strongItem]) { - [strongSelf reconfigureCellsForItems:@[ strongItem ]]; - } - }; - - [self.attributesProvider fetchFaviconAttributesForURL:item.faviconPageURL - completion:completionBlock]; -} - - (BOOL)hasItemInSection:(SectionIdentifier)sectionIdentifier { if (![self.collectionViewModel hasSectionForSectionIdentifier:sectionIdentifier]) { @@ -586,18 +483,13 @@ CollectionViewItem* touchedItem = [self.collectionViewModel itemAtIndexPath:touchedItemIndexPath]; - if (touchedItem == [self.collectionViewModel - headerForSection:touchedItemIndexPath.section] || - ![touchedItem isKindOfClass:[ReadingListCollectionViewItem class]]) { + if (touchedItem.type != ItemTypeItem) { // Do not trigger context menu on headers. return; } - ReadingListCollectionViewItem* readingListItem = - base::mac::ObjCCastStrict<ReadingListCollectionViewItem>(touchedItem); - [self.delegate readingListCollectionViewController:self - displayContextMenuForItem:readingListItem + displayContextMenuForItem:touchedItem atPoint:touchLocation]; } @@ -870,8 +762,7 @@ - (void)updateItemsInSectionIdentifier:(SectionIdentifier)identifier usingEntryUpdater:(EntryUpdater)updater { - self.shouldMonitorDataSource = NO; - auto token = [self.dataSource beginBatchUpdates]; + [self.dataSource beginBatchUpdates]; NSArray* readItems = [self.collectionViewModel itemsInSectionWithIdentifier:identifier]; // Read the objects in reverse order to keep the order (last modified first). @@ -881,14 +772,12 @@ if (updater) updater(readingListItem.url); } - token.reset(); - self.shouldMonitorDataSource = YES; + [self.dataSource endBatchUpdates]; } - (void)updateIndexPaths:(NSArray<NSIndexPath*>*)indexPaths usingEntryUpdater:(EntryUpdater)updater { - self.shouldMonitorDataSource = NO; - auto token = [self.dataSource beginBatchUpdates]; + [self.dataSource beginBatchUpdates]; // Read the objects in reverse order to keep the order (last modified first). for (NSIndexPath* index in [indexPaths reverseObjectEnumerator]) { CollectionViewItem* cell = [self.collectionViewModel itemAtIndexPath:index]; @@ -897,9 +786,7 @@ if (updater) updater(readingListItem.url); } - // Leave the batch update while it is not monitored. - token.reset(); - self.shouldMonitorDataSource = YES; + [self.dataSource endBatchUpdates]; } - (void)logDeletionHistogramsForEntry:(const GURL&)url {
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller_unittest.mm b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller_unittest.mm index 5f7b5a9..251bd5c 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller_unittest.mm +++ b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller_unittest.mm
@@ -61,14 +61,14 @@ reading_list_model_.reset(new ReadingListModelImpl( nullptr, nullptr, base::MakeUnique<base::DefaultClock>())); - mediator_ = - [[ReadingListMediator alloc] initWithModel:reading_list_model_.get()]; large_icon_service_.reset(new favicon::LargeIconService( &mock_favicon_service_, base::ThreadTaskRunnerHandle::Get(), /*image_fetcher=*/nullptr)); + mediator_ = + [[ReadingListMediator alloc] initWithModel:reading_list_model_.get() + largeIconService:large_icon_service_.get()]; reading_list_view_controller_ = [[ReadingListCollectionViewController alloc] initWithDataSource:mediator_ - largeIconService:large_icon_service_.get() toolbar:nil]; mock_delegate_ = [OCMockObject
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.h b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.h index f9aeb54..17ed4a15 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.h +++ b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.h
@@ -6,18 +6,13 @@ #define IOS_CHROME_BROWSER_UI_READING_LIST_READING_LIST_COLLECTION_VIEW_ITEM_H_ #import "ios/chrome/browser/ui/collection_view/cells/collection_view_text_item.h" -#import "ios/third_party/material_components_ios/src/components/CollectionCells/src/MaterialCollectionCells.h" + +#import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_cell.h" class GURL; @class FaviconAttributes; @protocol ReadingListCollectionViewItemAccessibilityDelegate; -typedef NS_ENUM(NSInteger, ReadingListUIDistillationStatus) { - ReadingListUIDistillationStatusPending, - ReadingListUIDistillationStatusSuccess, - ReadingListUIDistillationStatusFailure -}; - // Collection view item for representing a ReadingListEntry. @interface ReadingListCollectionViewItem : CollectionViewItem @@ -54,23 +49,4 @@ @end -@class FaviconViewNew; -// Cell for ReadingListCollectionViewItem. -@interface ReadingListCell : MDCCollectionViewCell - -// Title label. -@property(nonatomic, readonly, strong) UILabel* titleLabel; -// Subtitle label. -@property(nonatomic, readonly, strong) UILabel* subtitleLabel; -// Timestamp of the distillation in microseconds since Jan 1st 1970. -@property(nonatomic, assign) int64_t distillationDate; -// Size of the distilled files. -@property(nonatomic, assign) int64_t distillationSize; -// View for displaying the favicon for the reading list entry. -@property(nonatomic, readonly, strong) FaviconViewNew* faviconView; -// Status of the offline version. Updates the visual indicator when updated. -@property(nonatomic, assign) ReadingListUIDistillationStatus distillationState; - -@end - #endif // IOS_CHROME_BROWSER_UI_READING_LIST_READING_LIST_COLLECTION_VIEW_ITEM_H_
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.mm b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.mm index 05365dcbe..aae2d7e 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.mm +++ b/ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.mm
@@ -5,40 +5,18 @@ #import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.h" #include "base/strings/sys_string_conversions.h" -#include "base/time/time.h" -#import "ios/chrome/browser/ui/colors/MDCPalette+CrAdditions.h" #import "ios/chrome/browser/ui/favicon/favicon_view.h" +#import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_cell.h" #import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_item_accessibility_delegate.h" -#import "ios/chrome/browser/ui/uikit_ui_util.h" #import "ios/chrome/browser/ui/util/pasteboard_util.h" #include "ios/chrome/grit/ios_strings.h" -#import "ios/third_party/material_components_ios/src/components/Palettes/src/MaterialPalettes.h" -#import "ios/third_party/material_components_ios/src/components/Typography/src/MaterialTypography.h" #include "ui/base/l10n/l10n_util.h" -#include "ui/base/l10n/time_format.h" #import "url/gurl.h" #if !defined(__has_feature) || !__has_feature(objc_arc) #error "This file requires ARC support." #endif -namespace { -NSString* kSuccessImageString = @"distillation_success"; -NSString* kFailureImageString = @"distillation_fail"; - -// Height of the cell. -const CGFloat kCellHeight = 72; - -// Distillation indicator constants. -const CGFloat kDistillationIndicatorSize = 18; - -// Margin for the elements displayed in the cell. -const CGFloat kMargin = 16; - -// Transparency of the distillation size and date. -const CGFloat kInfoTextTransparency = 0.38; -} // namespace - #pragma mark - ReadingListCollectionViewItem @interface ReadingListCollectionViewItem () { @@ -224,243 +202,3 @@ } @end - -#pragma mark - ReadingListCell - -@implementation ReadingListCell { - UIImageView* _downloadIndicator; - UILayoutGuide* _textGuide; - - UILabel* _distillationSizeLabel; - UILabel* _distillationDateLabel; - - // View containing |_distillationSizeLabel| and |_distillationDateLabel|. - UIView* _infoView; - - // Whether |_infoView| is visible. - BOOL _showInfo; -} -@synthesize faviconView = _faviconView; -@synthesize titleLabel = _titleLabel; -@synthesize subtitleLabel = _subtitleLabel; -@synthesize distillationDate = _distillationDate; -@synthesize distillationSize = _distillationSize; -@synthesize distillationState = _distillationState; - -- (instancetype)initWithFrame:(CGRect)frame { - self = [super initWithFrame:frame]; - if (self) { - id<MDCTypographyFontLoading> fontLoader = [MDCTypography fontLoader]; - CGFloat faviconSize = kFaviconPreferredSize; - _titleLabel = [[UILabel alloc] init]; - _titleLabel.font = [fontLoader mediumFontOfSize:16]; - _titleLabel.textColor = [[MDCPalette greyPalette] tint900]; - _titleLabel.translatesAutoresizingMaskIntoConstraints = NO; - _titleLabel.accessibilityIdentifier = @"Reading List Item title"; - - _subtitleLabel = [[UILabel alloc] init]; - _subtitleLabel.font = [fontLoader mediumFontOfSize:14]; - _subtitleLabel.textColor = [[MDCPalette greyPalette] tint500]; - _subtitleLabel.translatesAutoresizingMaskIntoConstraints = NO; - _subtitleLabel.accessibilityIdentifier = @"Reading List Item subtitle"; - - _distillationDateLabel = [[UILabel alloc] init]; - _distillationDateLabel.font = [fontLoader mediumFontOfSize:12]; - [_distillationDateLabel - setContentHuggingPriority:UILayoutPriorityDefaultHigh - forAxis:UILayoutConstraintAxisHorizontal]; - _distillationDateLabel.textColor = - [UIColor colorWithWhite:0 alpha:kInfoTextTransparency]; - _distillationDateLabel.translatesAutoresizingMaskIntoConstraints = NO; - _distillationDateLabel.accessibilityIdentifier = - @"Reading List Item distillation date"; - - _distillationSizeLabel = [[UILabel alloc] init]; - _distillationSizeLabel.font = [fontLoader mediumFontOfSize:12]; - [_distillationSizeLabel - setContentHuggingPriority:UILayoutPriorityDefaultHigh - forAxis:UILayoutConstraintAxisHorizontal]; - _distillationSizeLabel.textColor = - [UIColor colorWithWhite:0 alpha:kInfoTextTransparency]; - _distillationSizeLabel.translatesAutoresizingMaskIntoConstraints = NO; - _distillationSizeLabel.accessibilityIdentifier = - @"Reading List Item distillation size"; - - _faviconView = [[FaviconViewNew alloc] init]; - CGFloat fontSize = floorf(faviconSize / 2); - [_faviconView setFont:[fontLoader regularFontOfSize:fontSize]]; - _faviconView.translatesAutoresizingMaskIntoConstraints = NO; - - _downloadIndicator = [[UIImageView alloc] init]; - [_downloadIndicator setTranslatesAutoresizingMaskIntoConstraints:NO]; - _downloadIndicator.accessibilityIdentifier = - @"Reading List Item download indicator"; - [_faviconView addSubview:_downloadIndicator]; - - [self.contentView addSubview:_faviconView]; - [self.contentView addSubview:_titleLabel]; - [self.contentView addSubview:_subtitleLabel]; - - _infoView = [[UIView alloc] initWithFrame:CGRectZero]; - [_infoView addSubview:_distillationDateLabel]; - [_infoView addSubview:_distillationSizeLabel]; - _infoView.translatesAutoresizingMaskIntoConstraints = NO; - - _textGuide = [[UILayoutGuide alloc] init]; - [self.contentView addLayoutGuide:_textGuide]; - - ApplyVisualConstraintsWithMetrics( - @[ - @"H:|[date]-(>=margin)-[size]|", - @"V:[title][subtitle]", - @"H:|-(margin)-[favicon]-(margin)-[title]-(>=margin)-|", - @"H:[favicon]-(margin)-[subtitle]-(>=margin)-|", - @"V:|[date]|", - @"V:|[size]|", - ], - @{ - @"favicon" : _faviconView, - @"title" : _titleLabel, - @"subtitle" : _subtitleLabel, - @"date" : _distillationDateLabel, - @"size" : _distillationSizeLabel, - }, - @{ - @"margin" : @(kMargin), - }); - - // Sets the bottom of the text. Lower the priority so we can add the details - // later. - NSLayoutConstraint* bottomTextConstraint = [_textGuide.bottomAnchor - constraintEqualToAnchor:_subtitleLabel.bottomAnchor]; - bottomTextConstraint.priority = UILayoutPriorityDefaultHigh; - NSLayoutConstraint* topTextConstraint = - [_textGuide.topAnchor constraintEqualToAnchor:_titleLabel.topAnchor]; - - [NSLayoutConstraint activateConstraints:@[ - // Height for the cell. - [self.contentView.heightAnchor constraintEqualToConstant:kCellHeight], - - // Text constraints. - topTextConstraint, - bottomTextConstraint, - - // Favicons are always the same size. - [_faviconView.widthAnchor constraintEqualToConstant:faviconSize], - [_faviconView.heightAnchor constraintEqualToConstant:faviconSize], - - // Center the content (favicon and text) vertically. - [_faviconView.centerYAnchor - constraintEqualToAnchor:self.contentView.centerYAnchor], - [_textGuide.centerYAnchor - constraintEqualToAnchor:self.contentView.centerYAnchor], - - // Place the download indicator in the bottom right corner of the favicon. - [[_downloadIndicator centerXAnchor] - constraintEqualToAnchor:_faviconView.trailingAnchor], - [[_downloadIndicator centerYAnchor] - constraintEqualToAnchor:_faviconView.bottomAnchor], - [[_downloadIndicator widthAnchor] - constraintEqualToConstant:kDistillationIndicatorSize], - [[_downloadIndicator heightAnchor] - constraintEqualToConstant:kDistillationIndicatorSize], - ]]; - - self.editingSelectorColor = [[MDCPalette cr_bluePalette] tint500]; - } - return self; -} - -- (void)setDistillationState: - (ReadingListUIDistillationStatus)distillationState { - if (_distillationState == distillationState) - return; - - _distillationState = distillationState; - switch (distillationState) { - case ReadingListUIDistillationStatusFailure: - [_downloadIndicator setImage:[UIImage imageNamed:kFailureImageString]]; - break; - - case ReadingListUIDistillationStatusSuccess: - [_downloadIndicator setImage:[UIImage imageNamed:kSuccessImageString]]; - break; - - case ReadingListUIDistillationStatusPending: - [_downloadIndicator setImage:nil]; - break; - } -} - -- (void)setShowInfo:(BOOL)show { - if (_showInfo == show) { - return; - } - _showInfo = show; - if (!show) { - [_infoView removeFromSuperview]; - return; - } - [self.contentView addSubview:_infoView]; - ApplyVisualConstraintsWithMetrics( - @[ - @"H:|-(margin)-[favicon]-(margin)-[detail]-(margin)-|", - ], - @{ - @"favicon" : _faviconView, - @"detail" : _infoView, - }, - @{ - @"margin" : @(kMargin), - }); - [NSLayoutConstraint activateConstraints:@[ - [_infoView.topAnchor constraintEqualToAnchor:_subtitleLabel.bottomAnchor], - [_infoView.bottomAnchor constraintEqualToAnchor:_textGuide.bottomAnchor], - ]]; -} - -- (void)setDistillationSize:(int64_t)distillationSize { - [_distillationSizeLabel - setText:[NSByteCountFormatter - stringFromByteCount:distillationSize - countStyle:NSByteCountFormatterCountStyleFile]]; - _distillationSize = distillationSize; - BOOL showInfo = _distillationSize != 0 && _distillationDate != 0; - [self setShowInfo:showInfo]; -} - -- (void)setDistillationDate:(int64_t)distillationDate { - int64_t now = (base::Time::Now() - base::Time::UnixEpoch()).InMicroseconds(); - int64_t elapsed = now - distillationDate; - NSString* text; - if (elapsed < base::Time::kMicrosecondsPerMinute) { - // This will also catch items added in the future. In that case, show the - // "just now" string. - text = l10n_util::GetNSString(IDS_IOS_READING_LIST_JUST_NOW); - } else { - text = base::SysUTF16ToNSString(ui::TimeFormat::SimpleWithMonthAndYear( - ui::TimeFormat::FORMAT_ELAPSED, ui::TimeFormat::LENGTH_LONG, - base::TimeDelta::FromMicroseconds(elapsed), true)); - } - - [_distillationDateLabel setText:text]; - _distillationDate = distillationDate; - BOOL showInfo = _distillationSize != 0 && _distillationDate != 0; - [self setShowInfo:showInfo]; -} - -#pragma mark - UICollectionViewCell - -- (void)prepareForReuse { - self.titleLabel.text = nil; - self.subtitleLabel.text = nil; - self.distillationState = ReadingListUIDistillationStatusPending; - self.distillationDate = 0; - self.distillationSize = 0; - [self setShowInfo:NO]; - [self.faviconView configureWithAttributes:nil]; - self.accessibilityCustomActions = nil; - [super prepareForReuse]; -} - -@end
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_coordinator.h b/ios/chrome/browser/ui/reading_list/reading_list_coordinator.h index 1d6480b..6ec1d9b 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_coordinator.h +++ b/ios/chrome/browser/ui/reading_list/reading_list_coordinator.h
@@ -12,12 +12,16 @@ class ChromeBrowserState; } +@class ReadingListMediator; @protocol UrlLoader; // Coordinator for Reading List, displaying the Reading List when starting. @interface ReadingListCoordinator : ChromeCoordinator<ReadingListCollectionViewControllerDelegate> +// Mediator used by this coordinator. Reset when |-start| is called. +@property(nonatomic, strong, nullable) ReadingListMediator* mediator; + - (nullable instancetype) initWithBaseViewController:(nullable UIViewController*)viewController browserState:(nonnull ios::ChromeBrowserState*)browserState
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_coordinator.mm b/ios/chrome/browser/ui/reading_list/reading_list_coordinator.mm index 01b657d..22dc97b 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_coordinator.mm +++ b/ios/chrome/browser/ui/reading_list/reading_list_coordinator.mm
@@ -57,7 +57,6 @@ // Used to load the Reading List pages. @property(nonatomic, weak) id<UrlLoader> URLLoader; @property(nonatomic, strong) ReadingListViewController* containerViewController; -@property(nonatomic, strong) ReadingListMediator* mediator; @property(nonatomic, strong) AlertCoordinator* alertCoordinator; @end @@ -92,12 +91,14 @@ favicon::LargeIconService* largeIconService = IOSChromeLargeIconServiceFactory::GetForBrowserState(self.browserState); - self.mediator = [[ReadingListMediator alloc] initWithModel:model]; + self.mediator = + [[ReadingListMediator alloc] initWithModel:model + largeIconService:largeIconService]; ReadingListToolbar* toolbar = [[ReadingListToolbar alloc] init]; ReadingListCollectionViewController* collectionViewController = [[ReadingListCollectionViewController alloc] initWithDataSource:self.mediator - largeIconService:largeIconService + toolbar:toolbar]; collectionViewController.delegate = self; @@ -131,16 +132,16 @@ - (void)readingListCollectionViewController: (ReadingListCollectionViewController*) readingListCollectionViewController - displayContextMenuForItem: - (ReadingListCollectionViewItem*)readingListItem + displayContextMenuForItem:(CollectionViewItem*)item atPoint:(CGPoint)menuLocation { if (!self.containerViewController) { return; } - const ReadingListEntry* entry = - [readingListCollectionViewController.dataSource - entryWithURL:readingListItem.url]; + ReadingListCollectionViewItem* readingListItem = + base::mac::ObjCCastStrict<ReadingListCollectionViewItem>(item); + + const ReadingListEntry* entry = [self.mediator entryFromItem:item]; if (!entry) { [readingListCollectionViewController reloadData]; @@ -230,11 +231,8 @@ - (void) readingListCollectionViewController: (ReadingListCollectionViewController*)readingListCollectionViewController - openItem: - (ReadingListCollectionViewItem*)readingListItem { - const ReadingListEntry* entry = - [readingListCollectionViewController.dataSource - entryWithURL:readingListItem.url]; + openItem:(CollectionViewItem*)readingListItem { + const ReadingListEntry* entry = [self.mediator entryFromItem:readingListItem]; if (!entry) { [readingListCollectionViewController reloadData];
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_coordinator_unittest.mm b/ios/chrome/browser/ui/reading_list/reading_list_coordinator_unittest.mm index fa3f0ac..722543a 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_coordinator_unittest.mm +++ b/ios/chrome/browser/ui/reading_list/reading_list_coordinator_unittest.mm
@@ -12,6 +12,7 @@ #include "components/reading_list/core/reading_list_entry.h" #include "components/reading_list/core/reading_list_model_impl.h" #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h" +#include "ios/chrome/browser/reading_list/offline_url_utils.h" #import "ios/chrome/browser/ui/collection_view/collection_view_model.h" #import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.h" #import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.h" @@ -43,13 +44,14 @@ @property(nonatomic, readonly) const web::Referrer& referrer; @property(nonatomic, assign) ui::PageTransition transition; @property(nonatomic, assign) BOOL rendererInitiated; - +@property(nonatomic, assign) BOOL inIncognito; @end @implementation UrlLoaderStub @synthesize transition = _transition; @synthesize rendererInitiated = _rendererInitiated; +@synthesize inIncognito = _inIncognito; - (void)loadURL:(const GURL&)url referrer:(const web::Referrer&)referrer @@ -72,6 +74,9 @@ inIncognito:(BOOL)inIncognito inBackground:(BOOL)inBackground appendTo:(OpenPosition)appendTo { + _url = url; + _referrer = referrer; + self.inIncognito = inIncognito; } - (void)loadSessionTab:(const sessions::SessionTab*)sessionTab { @@ -102,15 +107,17 @@ reading_list_model_.reset(new ReadingListModelImpl( nullptr, nullptr, base::MakeUnique<base::DefaultClock>())); - mediator_ = - [[ReadingListMediator alloc] initWithModel:reading_list_model_.get()]; large_icon_service_.reset(new favicon::LargeIconService( &mock_favicon_service_, base::ThreadTaskRunnerHandle::Get(), /*image_fetcher=*/nullptr)); + mediator_ = + [[ReadingListMediator alloc] initWithModel:reading_list_model_.get() + largeIconService:large_icon_service_.get()]; coordinator_ = [[ReadingListCoordinator alloc] initWithBaseViewController:nil browserState:browser_state_.get() loader:loader_mock_]; + coordinator_.mediator = mediator_; EXPECT_CALL(mock_favicon_service_, GetLargestRawFaviconForPageURL(_, _, _, _, _)) @@ -128,7 +135,6 @@ GetAReadingListCollectionViewController() { return [[ReadingListCollectionViewController alloc] initWithDataSource:mediator_ - largeIconService:large_icon_service_.get() toolbar:nil]; } @@ -148,8 +154,6 @@ // Setup. GURL url("https://chromium.org"); std::string title("Chromium"); - std::unique_ptr<ReadingListEntry> entry = - base::MakeUnique<ReadingListEntry>(url, title, base::Time::FromTimeT(10)); ReadingListModel* model = GetReadingListModel(); model->AddEntry(url, title, reading_list::ADDED_VIA_CURRENT_APP); @@ -170,3 +174,58 @@ loader.transition)); EXPECT_EQ(NO, loader.rendererInitiated); } + +TEST_F(ReadingListCoordinatorTest, OpenItemOffline) { + // Setup. + GURL url("https://chromium.org"); + std::string title("Chromium"); + ReadingListModel* model = GetReadingListModel(); + model->AddEntry(url, title, reading_list::ADDED_VIA_CURRENT_APP); + base::FilePath distilled_path("test"); + GURL distilled_url("https://distilled.com"); + model->SetEntryDistilledInfo(url, distilled_path, distilled_url, 123, + base::Time::FromTimeT(10)); + + ReadingListCollectionViewItem* item = [[ReadingListCollectionViewItem alloc] + initWithType:0 + url:url + distillationState:ReadingListUIDistillationStatusSuccess]; + GURL offlineURL = + reading_list::OfflineURLForPath(distilled_path, url, distilled_url); + ASSERT_FALSE(model->GetEntryByURL(url)->IsRead()); + + // Action. + [GetCoordinator() readingListCollectionViewController: + GetAReadingListCollectionViewController() + openItemOfflineInNewTab:item]; + + // Tests. + UrlLoaderStub* loader = GetLoaderStub(); + EXPECT_EQ(offlineURL, loader.url); + EXPECT_FALSE(loader.inIncognito); + EXPECT_TRUE(model->GetEntryByURL(url)->IsRead()); +} + +TEST_F(ReadingListCoordinatorTest, OpenItemInNewTab) { + // Setup. + GURL url("https://chromium.org"); + std::string title("Chromium"); + ReadingListModel* model = GetReadingListModel(); + model->AddEntry(url, title, reading_list::ADDED_VIA_CURRENT_APP); + + ReadingListCollectionViewItem* item = [[ReadingListCollectionViewItem alloc] + initWithType:0 + url:url + distillationState:ReadingListUIDistillationStatusSuccess]; + + // Action. + [GetCoordinator() readingListCollectionViewController: + GetAReadingListCollectionViewController() + openItemInNewTab:item + incognito:YES]; + + // Tests. + UrlLoaderStub* loader = GetLoaderStub(); + EXPECT_EQ(url, loader.url); + EXPECT_TRUE(loader.inIncognito); +}
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_data_sink.h b/ios/chrome/browser/ui/reading_list/reading_list_data_sink.h index 87eae5d..29203ef9 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_data_sink.h +++ b/ios/chrome/browser/ui/reading_list/reading_list_data_sink.h
@@ -13,10 +13,21 @@ // Called by the data source when it is ready. - (void)dataSourceReady:(id<ReadingListDataSource>)dataSource; +// Notifies the DataSink that the DataSource has changed and the items should be +// reloaded. +- (void)dataSourceChanged; -// TODO(crbug.com/721758): Group and rename those two methods. -- (void)readingListModelDidApplyChanges; -- (void)readingListModelCompletedBatchUpdates; +// Returns the read items displayed. +- (NSArray<CollectionViewItem*>*)readItems; +// Returns the unread items displayed. +- (NSArray<CollectionViewItem*>*)unreadItems; + +// Notifies the DataSink that the |item| has changed and it should be reloaded +// if it is still displayed. +- (void)itemHasChangedAfterDelay:(CollectionViewItem*)item; +// Notifies the DataSink that the |items| have changed and must be reloaded. The +// |items| must be presented. +- (void)itemsHaveChanged:(NSArray<CollectionViewItem*>*)items; @end
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_data_source.h b/ios/chrome/browser/ui/reading_list/reading_list_data_source.h index 500a8e3..88931b70 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_data_source.h +++ b/ios/chrome/browser/ui/reading_list/reading_list_data_source.h
@@ -7,9 +7,6 @@ #include <memory> -// TODO(crbug.com/721758): Batch updates should be done in the mediator. -#include "components/reading_list/core/reading_list_model.h" - class GURL; class ReadingListEntry; @class CollectionViewItem; @@ -50,10 +47,16 @@ // TODO(crbug.com/721758): Return ReadingListItem directly. - (const ReadingListEntry* _Nullable)entryWithURL:(const GURL&)URL; -// TODO(crbug.com/721758): Batch updates should be done in the mediator. -- (std::unique_ptr<ReadingListModel::ScopedReadingListBatchUpdate>) - beginBatchUpdates; -- (BOOL)isPerformingBatchUpdates; +// Fetches the |faviconURL| of this |item|, notifies the data sink when +// receiving the favicon. +- (void)fetchFaviconForItem:(nonnull ReadingListCollectionViewItem*)item; + +// Prepares the data source for batch updates. The UI is not notified for the +// updates happenning between |-beginBatchUpdates| and |-endBatchUpdates|. +- (void)beginBatchUpdates; +// Notifies the data source that the batch updates are over. After calling this +// function the UI is notified when the model changes. +- (void)endBatchUpdates; @end
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_mediator.h b/ios/chrome/browser/ui/reading_list/reading_list_mediator.h index ba7962b1..6042d87 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_mediator.h +++ b/ios/chrome/browser/ui/reading_list/reading_list_mediator.h
@@ -9,6 +9,10 @@ #import "ios/chrome/browser/ui/reading_list/reading_list_data_source.h" +namespace favicon { +class LargeIconService; +} + class ReadingListModel; // Mediator between the Model and the UI. @@ -17,8 +21,16 @@ - (nullable instancetype)init NS_UNAVAILABLE; - (nullable instancetype)initWithModel:(nonnull ReadingListModel*)model + largeIconService: + (nonnull favicon::LargeIconService*)largeIconService NS_DESIGNATED_INITIALIZER; +// Returns the entry corresponding to the |item|. The item should be of type +// ReadingListCollectionViewItem. Returns nullptr if there is no corresponding +// entry. +- (nullable const ReadingListEntry*)entryFromItem: + (nonnull CollectionViewItem*)item; + @end #endif // IOS_CHROME_BROWSER_UI_READING_LIST_READING_LIST_MEDIATOR_H_
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm b/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm index 16c8463..b32ef24 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm +++ b/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm
@@ -9,9 +9,12 @@ #import "base/mac/foundation_util.h" #include "base/metrics/histogram_macros.h" #include "base/strings/sys_string_conversions.h" +#include "components/favicon/core/large_icon_service.h" #include "components/reading_list/core/reading_list_model.h" #import "components/reading_list/ios/reading_list_model_bridge_observer.h" #include "components/url_formatter/url_formatter.h" +#import "ios/chrome/browser/ui/favicon/favicon_attributes_provider.h" +#import "ios/chrome/browser/ui/favicon/favicon_view.h" #import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.h" #import "ios/chrome/browser/ui/reading_list/reading_list_data_sink.h" #import "ios/chrome/browser/ui/reading_list/reading_list_utils.h" @@ -28,23 +31,39 @@ @interface ReadingListMediator ()<ReadingListModelBridgeObserver> { std::unique_ptr<ReadingListModelBridge> _modelBridge; + std::unique_ptr<ReadingListModel::ScopedReadingListBatchUpdate> _batchToken; } @property(nonatomic, assign) ReadingListModel* model; +@property(nonatomic, assign) BOOL shouldMonitorModel; + +// Lazily instantiated. +@property(nonatomic, strong, readonly) + FaviconAttributesProvider* attributesProvider; + +@property(nonatomic, assign, readonly) + favicon::LargeIconService* largeIconService; + @end @implementation ReadingListMediator @synthesize model = _model; @synthesize dataSink = _dataSink; +@synthesize shouldMonitorModel = _shouldMonitorModel; +@synthesize attributesProvider = _attributesProvider; +@synthesize largeIconService = _largeIconService; #pragma mark - Public -- (instancetype)initWithModel:(ReadingListModel*)model { +- (instancetype)initWithModel:(ReadingListModel*)model + largeIconService:(favicon::LargeIconService*)largeIconService { self = [super init]; if (self) { _model = model; + _largeIconService = largeIconService; + _shouldMonitorModel = YES; // This triggers the callback method. Should be created last. _modelBridge.reset(new ReadingListModelBridge(self, model)); @@ -52,6 +71,12 @@ return self; } +- (const ReadingListEntry*)entryFromItem:(CollectionViewItem*)item { + ReadingListCollectionViewItem* readingListItem = + base::mac::ObjCCastStrict<ReadingListCollectionViewItem>(item); + return self.model->GetEntryByURL(readingListItem.url); +} + #pragma mark - ReadingListDataSource - (BOOL)isEntryRead:(CollectionViewItem*)item { @@ -115,17 +140,53 @@ DCHECK(self.model->Keys().size() == [readArray count] + [unreadArray count]); } -- (std::unique_ptr<ReadingListModel::ScopedReadingListBatchUpdate>) - beginBatchUpdates { - return self.model->BeginBatchUpdates(); +- (void)fetchFaviconForItem:(ReadingListCollectionViewItem*)item { + __weak ReadingListCollectionViewItem* weakItem = item; + __weak ReadingListMediator* weakSelf = self; + void (^completionBlock)(FaviconAttributes* attributes) = + ^(FaviconAttributes* attributes) { + ReadingListCollectionViewItem* strongItem = weakItem; + ReadingListMediator* strongSelf = weakSelf; + if (!strongSelf || !strongItem) { + return; + } + + strongItem.attributes = attributes; + + [strongSelf.dataSink itemHasChangedAfterDelay:strongItem]; + }; + + [self.attributesProvider fetchFaviconAttributesForURL:item.faviconPageURL + completion:completionBlock]; } -- (BOOL)isPerformingBatchUpdates { - return self.model->IsPerformingBatchUpdates(); +- (void)beginBatchUpdates { + self.shouldMonitorModel = NO; + _batchToken = self.model->BeginBatchUpdates(); +} + +- (void)endBatchUpdates { + _batchToken.reset(); + self.shouldMonitorModel = YES; } #pragma mark - Properties +- (FaviconAttributesProvider*)attributesProvider { + if (_attributesProvider) { + return _attributesProvider; + } + + // Accept any favicon even the smallest ones (16x16) as it is better than the + // fallback icon. + // Pass 1 as minimum size to avoid empty favicons. + _attributesProvider = [[FaviconAttributesProvider alloc] + initWithFaviconSize:kFaviconPreferredSize + minFaviconSize:1 + largeIconService:self.largeIconService]; + return _attributesProvider; +} + - (void)setDataSink:(id<ReadingListDataSink>)dataSink { _dataSink = dataSink; if (self.model->loaded()) { @@ -155,11 +216,26 @@ } - (void)readingListModelDidApplyChanges:(const ReadingListModel*)model { - [self.dataSink readingListModelDidApplyChanges]; + if (!self.shouldMonitorModel) { + return; + } + + // Ignore single element updates when the data source is doing batch updates. + if (self.model->IsPerformingBatchUpdates()) { + return; + } + + if ([self hasDataSourceChanged]) + [self.dataSink dataSourceChanged]; } - (void)readingListModelCompletedBatchUpdates:(const ReadingListModel*)model { - [self.dataSink readingListModelCompletedBatchUpdates]; + if (!self.shouldMonitorModel) { + return; + } + + if ([self hasDataSourceChanged]) + [self.dataSink dataSourceChanged]; } #pragma mark - Private @@ -196,4 +272,58 @@ return item; } +// Whether the data source has changed. +- (BOOL)hasDataSourceChanged { + NSMutableArray<ReadingListCollectionViewItem*>* readArray = + [NSMutableArray array]; + NSMutableArray<ReadingListCollectionViewItem*>* unreadArray = + [NSMutableArray array]; + [self fillReadItems:readArray unreadItems:unreadArray]; + + return [self currentSection:[self.dataSink readItems] + isDifferentOfArray:readArray] || + [self currentSection:[self.dataSink unreadItems] + isDifferentOfArray:unreadArray]; +} + +// Returns whether there is a difference between the elements contained in the +// |sectionIdentifier| and those in the |array|. The comparison is done with the +// URL of the elements. If an element exist in both, the one in |currentSection| +// will be overwriten with the informations contained in the one from|array|. +- (BOOL)currentSection:(NSArray<CollectionViewItem*>*)currentSection + isDifferentOfArray:(NSArray<ReadingListCollectionViewItem*>*)array { + if (currentSection.count != array.count) + return YES; + + NSMutableArray<ReadingListCollectionViewItem*>* itemsToReconfigure = + [NSMutableArray array]; + + NSInteger index = 0; + for (ReadingListCollectionViewItem* newItem in array) { + ReadingListCollectionViewItem* oldItem = + base::mac::ObjCCastStrict<ReadingListCollectionViewItem>( + currentSection[index]); + if (oldItem.url == newItem.url) { + if (![oldItem isEqual:newItem]) { + oldItem.title = newItem.title; + oldItem.subtitle = newItem.subtitle; + oldItem.distillationState = newItem.distillationState; + oldItem.distillationDate = newItem.distillationDate; + oldItem.distillationSize = newItem.distillationSize; + [itemsToReconfigure addObject:oldItem]; + } + if (oldItem.faviconPageURL != newItem.faviconPageURL) { + oldItem.faviconPageURL = newItem.faviconPageURL; + [self fetchFaviconForItem:oldItem]; + } + } + if (![oldItem isEqual:newItem]) { + return YES; + } + index++; + } + [self.dataSink itemsHaveChanged:itemsToReconfigure]; + return NO; +} + @end
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_mediator_unittest.mm b/ios/chrome/browser/ui/reading_list/reading_list_mediator_unittest.mm index 2394879..7bd20967 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_mediator_unittest.mm +++ b/ios/chrome/browser/ui/reading_list/reading_list_mediator_unittest.mm
@@ -7,9 +7,15 @@ #include "base/memory/ptr_util.h" #include "base/strings/sys_string_conversions.h" #include "base/test/simple_test_clock.h" +#include "base/threading/thread_task_runner_handle.h" +#include "components/favicon/core/large_icon_service.h" +#include "components/favicon/core/test/mock_favicon_service.h" #include "components/reading_list/core/reading_list_model_impl.h" #include "components/url_formatter/url_formatter.h" +#include "ios/chrome/browser/favicon/ios_chrome_large_icon_service_factory.h" #import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.h" +#include "ios/web/public/test/test_web_thread_bundle.h" +#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" @@ -17,6 +23,8 @@ #error "This file requires ARC support." #endif +using testing::_; + class ReadingListMediatorTest : public PlatformTest { public: ReadingListMediatorTest() { @@ -25,6 +33,10 @@ clock_ = clock.get(); model_ = base::MakeUnique<ReadingListModelImpl>(nullptr, nullptr, std::move(clock)); + EXPECT_CALL(mock_favicon_service_, + GetLargestRawFaviconForPageURL(_, _, _, _, _)) + .WillRepeatedly( + favicon::PostReply<5>(favicon_base::FaviconRawBitmapResult())); no_title_entry_url_ = GURL("http://chromium.org/unread3"); // The first 3 have the same update time on purpose. @@ -43,14 +55,26 @@ reading_list::ADDED_VIA_CURRENT_APP); model_->SetReadStatus(GURL("http://chromium.org/read2"), true); - mediator_ = [[ReadingListMediator alloc] initWithModel:model_.get()]; + large_icon_service_.reset(new favicon::LargeIconService( + &mock_favicon_service_, base::ThreadTaskRunnerHandle::Get(), + /*image_fetcher=*/nullptr)); + + mediator_ = + [[ReadingListMediator alloc] initWithModel:model_.get() + largeIconService:large_icon_service_.get()]; } protected: + testing::StrictMock<favicon::MockFaviconService> mock_favicon_service_; std::unique_ptr<ReadingListModelImpl> model_; ReadingListMediator* mediator_; base::SimpleTestClock* clock_; GURL no_title_entry_url_; + std::unique_ptr<favicon::LargeIconService> large_icon_service_; + + private: + web::TestWebThreadBundle thread_bundle_; + DISALLOW_COPY_AND_ASSIGN(ReadingListMediatorTest); }; TEST_F(ReadingListMediatorTest, fillItems) {
diff --git a/ios/chrome/browser/ui/reading_list/reading_list_utils.h b/ios/chrome/browser/ui/reading_list/reading_list_utils.h index 941a91a..08ebdb6 100644 --- a/ios/chrome/browser/ui/reading_list/reading_list_utils.h +++ b/ios/chrome/browser/ui/reading_list/reading_list_utils.h
@@ -6,7 +6,7 @@ #define IOS_CHROME_BROWSER_UI_READING_LIST_READING_LIST_UTILS_H_ #include "components/reading_list/core/reading_list_entry.h" -#import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.h" +#import "ios/chrome/browser/ui/reading_list/reading_list_collection_view_cell.h" namespace reading_list {
diff --git a/ios/chrome/browser/ui/tools_menu/BUILD.gn b/ios/chrome/browser/ui/tools_menu/BUILD.gn index ba00ce7..7865a46 100644 --- a/ios/chrome/browser/ui/tools_menu/BUILD.gn +++ b/ios/chrome/browser/ui/tools_menu/BUILD.gn
@@ -30,6 +30,7 @@ "//ios/chrome/browser/ui/commands", "//ios/chrome/browser/ui/popup_menu", "//ios/chrome/browser/ui/reading_list", + "//ios/chrome/browser/ui/reading_list:reading_list_ui", "//ios/chrome/browser/ui/toolbar:resource_macros", "//ios/chrome/common", "//ios/public/provider/chrome/browser",
diff --git a/net/cookies/cookie_monster_perftest.cc b/net/cookies/cookie_monster_perftest.cc index dc891c0..eb9ce7b9 100644 --- a/net/cookies/cookie_monster_perftest.cc +++ b/net/cookies/cookie_monster_perftest.cc
@@ -154,8 +154,7 @@ timer3.Done(); } -// TODO(xunjieli): Renable after crbug.com/730000 is fixed. -TEST_F(CookieMonsterTest, DISABLED_TestAddCookieOnManyHosts) { +TEST_F(CookieMonsterTest, TestAddCookieOnManyHosts) { std::unique_ptr<CookieMonster> cm(new CookieMonster(nullptr, nullptr)); std::string cookie(kCookieLine); std::vector<GURL> gurls; // just wanna have ffffuunnn
diff --git a/third_party/WebKit/Source/core/dom/ContainerNode.cpp b/third_party/WebKit/Source/core/dom/ContainerNode.cpp index 5cb7eb8a..1549a554 100644 --- a/third_party/WebKit/Source/core/dom/ContainerNode.cpp +++ b/third_party/WebKit/Source/core/dom/ContainerNode.cpp
@@ -224,6 +224,8 @@ exception_state); } +// TODO(tkent): Fold CheckAcceptChildGuaranteedNodeTypes() into +// EnsurePreInsertionValidity(). bool ContainerNode::CheckAcceptChildGuaranteedNodeTypes( const Node& new_child, const Node* next, @@ -253,9 +255,6 @@ // ProcessingInstruction, or Comment node, throw a HierarchyRequestError. // 5. If either node is a Text node and parent is a document, or node is a // doctype and parent is not a document, throw a HierarchyRequestError. - // - // TODO(tkent): IsChildTypeAllowed() is unnecessary for - // RecheckNodeInsertionStructuralPrereq(). if (!IsChildTypeAllowed(new_child)) { exception_state.ThrowDOMException( kHierarchyRequestError, @@ -280,9 +279,18 @@ // node. Firefox and Edge don't throw in this case. return false; } - if (!CheckAcceptChildGuaranteedNodeTypes(*child, next, nullptr, - exception_state)) - return false; + if (IsDocumentNode()) { + // For Document, no need to check ContainsConsideringHostElements() + // because a Document node can't be a child of other nodes. + // However, status of existing doctype or root element might be changed + // and we need to check it again. + if (!ToDocument(this)->CanAcceptChild(*child, next, nullptr, + exception_state)) + return false; + } else { + if (ContainsConsideringHostElements(*child, exception_state)) + return false; + } } return CheckReferenceChildParent(*this, next, nullptr, exception_state); }
diff --git a/third_party/closure_compiler/compile_js.gni b/third_party/closure_compiler/compile_js.gni index 958ac1b..329bb75 100644 --- a/third_party/closure_compiler/compile_js.gni +++ b/third_party/closure_compiler/compile_js.gni
@@ -17,6 +17,10 @@ # deps: # List of js_library targets to depend on # +# extra_deps: +# List of any other rules to depend on. E.g. a rule that generates js source +# files +# # Example: # js_library("apple_tree") { # sources = ["tree_main.js"] @@ -36,6 +40,7 @@ [ "sources", "deps", + "extra_deps", ]) output_file = "$target_gen_dir/$target_name.js_library" outputs = [ @@ -55,6 +60,12 @@ args += [ rebase_path(dep_output_path, root_build_dir) ] } } + if (defined(extra_deps)) { + if (!defined(deps)) { + deps = [] + } + deps += extra_deps + } } }
diff --git a/third_party/ocmock/OCMock/OCMFunctions.m b/third_party/ocmock/OCMock/OCMFunctions.m index e1ec9fa..13ff12c 100644 --- a/third_party/ocmock/OCMock/OCMFunctions.m +++ b/third_party/ocmock/OCMock/OCMFunctions.m
@@ -77,7 +77,7 @@ * For some types some runtime functions throw exceptions, which is why we wrap this in an * exception handler just below. */ -static BOOL OCMEqualTypesAllowingOpaqueStructsInternal(const char *type1, const char *type2) +static BOOL OCMEqualTypesAllowingOpaqueStructsInternal(const char *type1, const char *type2, BOOL pointers) { type1 = OCMTypeWithoutQualifiers(type1); type2 = OCMTypeWithoutQualifiers(type2); @@ -104,8 +104,12 @@ BOOL type2Opaque = (type2Equals == NULL || (type2End < type2Equals) || type2Equals[1] == endChar); const char *type1NameEnd = (type1Equals == NULL || (type1End < type1Equals)) ? type1End : type1Equals; const char *type2NameEnd = (type2Equals == NULL || (type2End < type2Equals)) ? type2End : type2Equals; - intptr_t type1NameLen = type1NameEnd - type1; - intptr_t type2NameLen = type2NameEnd - type2; + intptr_t type1NameLen = type1NameEnd - type1 - 1; + intptr_t type2NameLen = type2NameEnd - type2 - 1; + + /* If one type is unnammed and opaque, assume equality if they are pointers. */ + if (!type1NameLen || !type2NameLen) + return pointers && (type1Opaque || type2Opaque); /* If the names are not equal, return NO */ if (type1NameLen != type2NameLen || strncmp(type1, type2, type1NameLen)) @@ -131,7 +135,7 @@ /* for a pointer, make sure the other is a pointer, then recursively compare the rest */ if (type2[0] != type1[0]) return NO; - return OCMEqualTypesAllowingOpaqueStructs(type1 + 1, type2 + 1); + return OCMEqualTypesAllowingOpaqueStructsInternal(type1 + 1, type2 + 1, YES); case '?': return type2[0] == '?'; @@ -156,7 +160,7 @@ { @try { - return OCMEqualTypesAllowingOpaqueStructsInternal(type1, type2); + return OCMEqualTypesAllowingOpaqueStructsInternal(type1, type2, NO); } @catch (NSException *e) {
diff --git a/third_party/ocmock/README.chromium b/third_party/ocmock/README.chromium index 4b609000..5d237f8 100644 --- a/third_party/ocmock/README.chromium +++ b/third_party/ocmock/README.chromium
@@ -29,3 +29,8 @@ because our version of clang does not support __builtin_types_compatible_p. As we are building with Objective-C++, we need to add 'extern "C"' in OCMFunctions.h to have C linkage. + +Chromium patches in a fix for crbug.com/731129 to assume that unnamed types +are equal to named ones (see bug for example of types). This is probably due +to differences between the type embedded by Chromium version of clang and by +the introspection used by Xcode 9 SDK.
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 2947589b..3064958 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -61324,8 +61324,8 @@ <owner>alexilin@chromium.org</owner> <summary> Records stats about requests, redirects, and responses observed by the - ResourcePrefetchPredictorObserver. These stats are useful as a baseline for - other stats. + LoadingPredictorObserver. These stats are useful as a baseline for other + stats. </summary> </histogram>
diff --git a/ui/base/win/open_file_name_win.cc b/ui/base/win/open_file_name_win.cc index 77dc41f..ccc7203 100644 --- a/ui/base/win/open_file_name_win.cc +++ b/ui/base/win/open_file_name_win.cc
@@ -73,7 +73,7 @@ base::FilePath directory; std::vector<base::FilePath> filenames; GetResult(&directory, &filenames); - if (filenames.size() != 1) + if (filenames.size() != 1 || filenames[0].empty()) return base::FilePath(); return directory.Append(filenames[0]); }