diff --git a/DEPS b/DEPS index 09ff504..fb0219b 100644 --- a/DEPS +++ b/DEPS
@@ -40,11 +40,11 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling Skia # and whatever else without interference from each other. - 'skia_revision': '3f142b6a785ab7af64383fccf98ae2528cdd71e3', + 'skia_revision': '8dadd9e8936a1c8f44fccdfde7f883dfe2d89c96', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other. - 'v8_revision': 'f6aefd1d65e1ce2bef3c0d4928c56cc738051e0f', + 'v8_revision': '2d82e0a3a5d19dfea81cd74771dc031c47e6b15e', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling swarming_client # and whatever else without interference from each other. @@ -64,7 +64,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling PDFium # and whatever else without interference from each other. - 'pdfium_revision': '21ae2b7297e005576afeb9f0230d1f69b3abc857', + 'pdfium_revision': '09065aeee59e51e88de054c8dab3b0e538c0893f', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling openmax_dl # and whatever else without interference from each other.
diff --git a/base/trace_event/memory_dump_manager.cc b/base/trace_event/memory_dump_manager.cc index ebd7380..18b255a 100644 --- a/base/trace_event/memory_dump_manager.cc +++ b/base/trace_event/memory_dump_manager.cc
@@ -53,19 +53,14 @@ // TODO(ssid): Fix all the dump providers to unregister if needed and clear the // blacklist, crbug.com/643438. const char* const kStrictThreadCheckBlacklist[] = { - "android::ResourceManagerImpl", "AndroidGraphics", "BrowserGpuMemoryBufferManager", "ClientDiscardableSharedMemoryManager", "ContextProviderCommandBuffer", - "DOMStorage", "DiscardableSharedMemoryManager", "FontCaches", "GpuMemoryBufferVideoFramePool", "IndexedDBBackingStore", - "LeveldbValueStore", - "MemoryCache", - "Skia", "Sql", "ThreadLocalEventBuffer", "TraceLog", @@ -79,10 +74,8 @@ "gpu::BufferManager", "gpu::MappedMemoryManager", "gpu::RenderbufferManager", - "gpu::TextureManager", "gpu::TransferBufferManager", "sql::Connection", - "SyncDirectory", "BlacklistTestDumpProvider" // for testing };
diff --git a/build/config/BUILD.gn b/build/config/BUILD.gn index 80427f26..09cb18f 100644 --- a/build/config/BUILD.gn +++ b/build/config/BUILD.gn
@@ -221,6 +221,12 @@ defines += [ "DYNAMIC_ANNOTATIONS_ENABLED=0" ] } } + + if (is_ios) { + # Disable NSAssert and GTMDevAssert (from Google Toolbox for Mac). This + # follows XCode's default behavior for Release builds. + defines += [ "NS_BLOCK_ASSERTIONS=1" ] + } } # Default libraries ------------------------------------------------------------
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/fullscreen/FullscreenHtmlApiHandler.java b/chrome/android/java/src/org/chromium/chrome/browser/fullscreen/FullscreenHtmlApiHandler.java index 59542057..29a4532 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/fullscreen/FullscreenHtmlApiHandler.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/fullscreen/FullscreenHtmlApiHandler.java
@@ -298,9 +298,6 @@ contentView.addOnLayoutChangeListener(mFullscreenOnLayoutChangeListener); contentView.setSystemUiVisibility(systemUiVisibility); - // Request a layout so the updated system visibility takes affect. - contentView.getRootView().requestLayout(); - mContentViewCoreInFullscreen = contentViewCore; mTabInFullscreen = tab; }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/init/AsyncInitializationActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/init/AsyncInitializationActivity.java index 7339ca29..b53c8c4 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/init/AsyncInitializationActivity.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/init/AsyncInitializationActivity.java
@@ -80,8 +80,7 @@ } @Override - // TODO(estevenson): Replace with Build.VERSION_CODES.N when available. - @TargetApi(24) + @TargetApi(Build.VERSION_CODES.N) protected void attachBaseContext(Context newBase) { super.attachBaseContext(newBase);
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java index b683aa3..c6cc0ea 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
@@ -63,6 +63,7 @@ import org.chromium.chrome.browser.search_engines.TemplateUrlService.TemplateUrlServiceObserver; import org.chromium.chrome.browser.snackbar.Snackbar; import org.chromium.chrome.browser.snackbar.SnackbarManager.SnackbarController; +import org.chromium.chrome.browser.suggestions.SuggestionsMetricsReporter; import org.chromium.chrome.browser.sync.SyncSessionsMetrics; import org.chromium.chrome.browser.tab.EmptyTabObserver; import org.chromium.chrome.browser.tab.Tab; @@ -327,56 +328,17 @@ return matchByHost ? UrlUtilities.sameHost(url1, url2) : url1.equals(url2); } - @Override - public void trackSnippetsPageImpression(int[] categories, int[] suggestionsPerCategory) { - mSnippetsBridge.onPageShown(categories, suggestionsPerCategory); + public SuggestionsMetricsReporter getSuggestionsMetricsReporter() { + return mSnippetsBridge; + } + + public void trackSnippetOpened(int windowOpenDisposition, SnippetArticle article) { + mSnippetsBridge.onSuggestionOpened(article, windowOpenDisposition); } @Override - public void trackSnippetImpression(SnippetArticle article) { - mSnippetsBridge.onSuggestionShown(article); - } - - @Override - public void trackSnippetMenuOpened(SnippetArticle article) { - mSnippetsBridge.onSuggestionMenuOpened(article); - } - - @Override - public void trackSnippetCategoryActionImpression(int category, int position) { - mSnippetsBridge.onMoreButtonShown(category, position); - } - - @Override - public void trackSnippetCategoryActionClick(int category, int position) { - mSnippetsBridge.onMoreButtonClicked(category, position); - switch (category) { - case KnownCategories.BOOKMARKS: - NewTabPageUma.recordAction(NewTabPageUma.ACTION_OPENED_BOOKMARKS_MANAGER); - break; - // MORE button in both categories leads to the recent tabs manager - case KnownCategories.FOREIGN_TABS: - case KnownCategories.RECENT_TABS: - NewTabPageUma.recordAction(NewTabPageUma.ACTION_OPENED_RECENT_TABS_MANAGER); - break; - case KnownCategories.DOWNLOADS: - NewTabPageUma.recordAction(NewTabPageUma.ACTION_OPENED_DOWNLOADS_MANAGER); - break; - default: - // No action associated - break; - } - } - - public void trackSnippetOpened( - int windowOpenDisposition, SnippetArticle article, int categoryIndex) { - mSnippetsBridge.onSuggestionOpened(article, categoryIndex, windowOpenDisposition); - } - - @Override - public void openSnippet( - int windowOpenDisposition, SnippetArticle article, int categoryIndex) { - trackSnippetOpened(windowOpenDisposition, article, categoryIndex); + public void openSnippet(int windowOpenDisposition, SnippetArticle article) { + trackSnippetOpened(windowOpenDisposition, article); NewTabPageUma.recordAction(NewTabPageUma.ACTION_OPENED_SNIPPET); if (article.mIsAssetDownload) {
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java index 1915b3c..141dbabca 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
@@ -59,6 +59,7 @@ import org.chromium.chrome.browser.offlinepages.OfflinePageBridge; import org.chromium.chrome.browser.profiles.MostVisitedSites.MostVisitedURLsObserver; import org.chromium.chrome.browser.profiles.Profile; +import org.chromium.chrome.browser.suggestions.SuggestionsMetricsReporter; import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.util.MathUtils; import org.chromium.chrome.browser.util.ViewUtils; @@ -164,45 +165,11 @@ void navigateToDownloadManager(); /** - * Tracks per-page-load metrics for content suggestions. - * @param categories The categories of content suggestions. - * @param suggestionsPerCategory The number of content suggestions in each category. - */ - void trackSnippetsPageImpression(int[] categories, int[] suggestionsPerCategory); - - /** - * Tracks impression metrics for a content suggestion. - * @param article The content suggestion that was shown to the user. - */ - void trackSnippetImpression(SnippetArticle article); - - /** - * Tracks impression metrics for the long-press menu for a content suggestion. - * @param article The content suggestion for which the long-press menu was opened. - */ - void trackSnippetMenuOpened(SnippetArticle article); - - /** - * Tracks impression metrics for a category's action button ("More"). - * @param category The category for which the action button was shown. - * @param position The position of the action button within the category. - */ - void trackSnippetCategoryActionImpression(int category, int position); - - /** - * Tracks click metrics for a category's action button ("More"). - * @param category The category for which the action button was clicked. - * @param position The position of the action button within the category. - */ - void trackSnippetCategoryActionClick(int category, int position); - - /** * Opens a content suggestion and records related metrics. * @param windowOpenDisposition How to open (current tab, new tab, new window etc). * @param article The content suggestion to open. - * @param categoryIndex The index of the category |article| belongs to. */ - void openSnippet(int windowOpenDisposition, SnippetArticle article, int categoryIndex); + void openSnippet(int windowOpenDisposition, SnippetArticle article); /** * Animates the search box up into the omnibox and bring up the keyboard. @@ -301,6 +268,13 @@ */ @Nullable ContextMenuManager getContextMenuManager(); + + /** + * @return The suggestion metrics reporter. Will be {@code null} if the + * {@link NewTabPageView} is not done initialising. + */ + @Nullable + SuggestionsMetricsReporter getSuggestionsMetricsReporter(); } /**
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ActionItem.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ActionItem.java index a1a4e11..34e6edc1 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ActionItem.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ActionItem.java
@@ -14,7 +14,9 @@ import org.chromium.chrome.browser.ntp.ContextMenuManager.Delegate; import org.chromium.chrome.browser.ntp.NewTabPageView.NewTabPageManager; import org.chromium.chrome.browser.ntp.UiConfig; +import org.chromium.chrome.browser.ntp.snippets.CategoryInt; import org.chromium.chrome.browser.ntp.snippets.SnippetsConfig; +import org.chromium.chrome.browser.suggestions.SuggestionsRanker; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -24,7 +26,7 @@ * Note: Use {@link #refreshVisibility()} to update the visibility of the button instead of calling * {@link #setVisible(boolean)} directly. */ -class ActionItem extends OptionalLeaf { +public class ActionItem extends OptionalLeaf { @IntDef({ACTION_NONE, ACTION_VIEW_ALL, ACTION_FETCH_MORE, ACTION_RELOAD}) @Retention(RetentionPolicy.SOURCE) public @interface Action {} @@ -35,14 +37,17 @@ private final SuggestionsCategoryInfo mCategoryInfo; private final SuggestionsSection mParentSection; + private final SuggestionsRanker mSuggestionsRanker; @Action private int mCurrentAction = ACTION_NONE; private boolean mImpressionTracked; + private int mPerSectionRank = -1; - public ActionItem(SuggestionsSection section) { + public ActionItem(SuggestionsSection section, SuggestionsRanker ranker) { mCategoryInfo = section.getCategoryInfo(); mParentSection = section; + mSuggestionsRanker = ranker; } /** Call this instead of {@link #setVisible(boolean)} to update the visibility. */ @@ -59,17 +64,26 @@ @Override protected void onBindViewHolder(NewTabPageViewHolder holder) { assert holder instanceof ViewHolder; + mSuggestionsRanker.rankActionItem(this, mParentSection); ((ViewHolder) holder).onBindViewHolder(this); } - private int getPosition() { - // TODO(dgn): looks dodgy. Confirm that's what we want. - return mParentSection.getSuggestionsCount(); + @CategoryInt + public int getCategory() { + return mCategoryInfo.getCategory(); + } + + public void setPerSectionRank(int perSectionRank) { + mPerSectionRank = perSectionRank; + } + + public int getPerSectionRank() { + return mPerSectionRank; } @VisibleForTesting void performAction(NewTabPageManager manager) { - manager.trackSnippetCategoryActionClick(mCategoryInfo.getCategory(), getPosition()); + manager.getSuggestionsMetricsReporter().onMoreButtonClicked(this); switch (mCurrentAction) { case ACTION_VIEW_ALL: @@ -97,6 +111,7 @@ return ACTION_NONE; } + /** ViewHolder associated to {@link ItemViewType#ACTION}. */ public static class ViewHolder extends CardViewHolder implements ContextMenuManager.Delegate { private ActionItem mActionListItem; @@ -117,9 +132,7 @@ public void onImpression() { if (mActionListItem != null && !mActionListItem.mImpressionTracked) { mActionListItem.mImpressionTracked = true; - manager.trackSnippetCategoryActionImpression( - mActionListItem.mCategoryInfo.getCategory(), - mActionListItem.getPosition()); + manager.getSuggestionsMetricsReporter().onMoreButtonShown(mActionListItem); } } });
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java index f75a9f3f8..b7edb85 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java
@@ -15,6 +15,7 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetsConfig; import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource; import org.chromium.chrome.browser.offlinepages.OfflinePageBridge; +import org.chromium.chrome.browser.suggestions.SuggestionsRanker; import java.util.LinkedHashMap; import java.util.List; @@ -32,10 +33,13 @@ private final Map<Integer, SuggestionsSection> mSections = new LinkedHashMap<>(); private final NewTabPageManager mNewTabPageManager; private final OfflinePageBridge mOfflinePageBridge; + private final SuggestionsRanker mSuggestionsRanker; public SectionList(NewTabPageManager newTabPageManager, OfflinePageBridge offlinePageBridge) { + mSuggestionsRanker = new SuggestionsRanker(); mNewTabPageManager = newTabPageManager; mNewTabPageManager.getSuggestionsSource().setObserver(this); + mNewTabPageManager.getSuggestionsMetricsReporter().setRanker(mSuggestionsRanker); mOfflinePageBridge = offlinePageBridge; resetSections(/* alwaysAllowEmptySections = */ false); } @@ -61,12 +65,11 @@ suggestionsPerCategory[categoryIndex] = resetSection(category, categoryStatus, alwaysAllowEmptySections); - SuggestionsSection section = mSections.get(category); - if (section != null) section.setCategoryIndex(categoryIndex); ++categoryIndex; } - mNewTabPageManager.trackSnippetsPageImpression(categories, suggestionsPerCategory); + mNewTabPageManager.getSuggestionsMetricsReporter().onPageShown( + categories, suggestionsPerCategory); } /** @@ -95,8 +98,10 @@ // Create the section if needed. if (section == null) { - section = new SuggestionsSection(this, mNewTabPageManager, mOfflinePageBridge, info); + section = new SuggestionsSection( + this, mNewTabPageManager, mSuggestionsRanker, mOfflinePageBridge, info); mSections.put(category, section); + mSuggestionsRanker.registerCategory(category); addChild(section); } @@ -186,17 +191,6 @@ */ private void setSuggestions(@CategoryInt int category, List<SnippetArticle> suggestions, @CategoryStatusEnum int status, boolean replaceExisting) { - // Count the number of suggestions before this category. - int globalPositionOffset = 0; - for (Map.Entry<Integer, SuggestionsSection> entry : mSections.entrySet()) { - if (entry.getKey() == category) break; - globalPositionOffset += entry.getValue().getSuggestionsCount(); - } - // Assign global indices to the new suggestions. - for (SnippetArticle suggestion : suggestions) { - suggestion.mGlobalPosition = globalPositionOffset + suggestion.mPosition; - } - mSections.get(category).setSuggestions(suggestions, status, replaceExisting); }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java index acb529e..2d7b839 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
@@ -19,6 +19,7 @@ import org.chromium.chrome.browser.offlinepages.ClientId; import org.chromium.chrome.browser.offlinepages.OfflinePageBridge; import org.chromium.chrome.browser.offlinepages.OfflinePageItem; +import org.chromium.chrome.browser.suggestions.SuggestionsRanker; import java.util.ArrayList; import java.util.Iterator; @@ -34,6 +35,7 @@ private final Delegate mDelegate; private final SuggestionsCategoryInfo mCategoryInfo; private final OfflinePageBridge mOfflinePageBridge; + private final SuggestionsRanker mSuggestionsRanker; // Children private final SectionHeader mHeader; @@ -60,15 +62,17 @@ } public SuggestionsSection(Delegate delegate, NewTabPageManager manager, - OfflinePageBridge offlinePageBridge, SuggestionsCategoryInfo info) { + SuggestionsRanker ranker, OfflinePageBridge offlinePageBridge, + SuggestionsCategoryInfo info) { mDelegate = delegate; mCategoryInfo = info; mOfflinePageBridge = offlinePageBridge; + mSuggestionsRanker = ranker; mHeader = new SectionHeader(info.getTitle()); mSuggestionsList = new SuggestionsList(manager, info); mStatus = StatusItem.createNoSuggestionsItem(info); - mMoreButton = new ActionItem(this); + mMoreButton = new ActionItem(this, ranker); mProgressIndicator = new ProgressItem(); addChildren(mHeader, mSuggestionsList, mStatus, mMoreButton, mProgressIndicator); @@ -80,7 +84,6 @@ private final List<SnippetArticle> mSuggestions = new ArrayList<>(); private final NewTabPageManager mNewTabPageManager; private final SuggestionsCategoryInfo mCategoryInfo; - private int mCategoryIndex; public SuggestionsList(NewTabPageManager newTabPageManager, SuggestionsCategoryInfo categoryInfo) { @@ -88,14 +91,6 @@ mCategoryInfo = categoryInfo; } - public void setCategoryIndex(int categoryIndex) { - mCategoryIndex = categoryIndex; - } - - public int getCategoryIndex() { - return mCategoryIndex; - } - @Override public int getItemCount() { return mSuggestions.size(); @@ -114,8 +109,7 @@ checkIndex(position); assert holder instanceof SnippetArticleViewHolder; ((SnippetArticleViewHolder) holder) - .onBindViewHolder( - getSuggestionAt(position), mCategoryInfo, payloads, mCategoryIndex); + .onBindViewHolder(getSuggestionAt(position), mCategoryInfo, payloads); } @Override @@ -359,6 +353,7 @@ mSuggestionsList.addAll(suggestions); for (SnippetArticle article : suggestions) { + mSuggestionsRanker.rankSuggestion(article); if (!article.requiresExactOfflinePage()) { updateSnippetOfflineAvailability(article); } @@ -436,14 +431,6 @@ return mCategoryInfo; } - public int getCategoryIndex() { - return mSuggestionsList.getCategoryIndex(); - } - - public void setCategoryIndex(int categoryIndex) { - mSuggestionsList.setCategoryIndex(categoryIndex); - } - public String getHeaderText() { return mHeader.getHeaderText(); }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java index bc37e383a..95f20e50 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java
@@ -36,11 +36,11 @@ /** The score expressing relative quality of the article for the user. */ public final float mScore; - /** The position of this article within its section. */ - public final int mPosition; + /** The rank of this article within its section. */ + private int mPerSectionRank = -1; - /** The position of this article in the complete list. Populated by NewTabPageAdapter. */ - public int mGlobalPosition = -1; + /** The global rank of this article in the complete list. */ + private int mGlobalRank = -1; /** Bitmap of the thumbnail, fetched lazily, when the RecyclerView wants to show the snippet. */ private Bitmap mThumbnailBitmap; @@ -67,7 +67,7 @@ * Creates a SnippetArticleListItem object that will hold the data. */ public SnippetArticle(int category, String idWithinCategory, String title, String publisher, - String previewText, String url, long timestamp, float score, int position) { + String previewText, String url, long timestamp, float score) { mCategory = category; mIdWithinCategory = idWithinCategory; mTitle = title; @@ -76,7 +76,6 @@ mUrl = url; mPublishTimestampMilliseconds = timestamp; mScore = score; - mPosition = position; } @Override @@ -208,4 +207,17 @@ // For debugging purposes. Displays the first 42 characters of the title. return String.format("{%s, %1.42s}", getClass().getSimpleName(), mTitle); } + + public void setRank(int perSectionRank, int globalRank) { + mPerSectionRank = perSectionRank; + mGlobalRank = globalRank; + } + + public int getGlobalRank() { + return mGlobalRank; + } + + public int getPerSectionRank() { + return mPerSectionRank; + } }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java index 7bb3616..92cdf2b 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
@@ -70,7 +70,6 @@ private FetchImageCallback mImageCallback; private SnippetArticle mArticle; private SuggestionsCategoryInfo mCategoryInfo; - private int mCategoryIndex; private int mPublisherFaviconSizePx; private final boolean mUseFaviconService; @@ -111,19 +110,19 @@ @Override public void onImpression() { if (mArticle != null && mArticle.trackImpression()) { - mNewTabPageManager.trackSnippetImpression(mArticle); + mNewTabPageManager.getSuggestionsMetricsReporter().onSuggestionShown(mArticle); mRecyclerView.onSnippetImpression(); } } @Override public void onCardTapped() { - mNewTabPageManager.openSnippet(WindowOpenDisposition.CURRENT_TAB, mArticle, mCategoryIndex); + mNewTabPageManager.openSnippet(WindowOpenDisposition.CURRENT_TAB, mArticle); } @Override public void openItem(int windowDisposition) { - mNewTabPageManager.openSnippet(windowDisposition, mArticle, mCategoryIndex); + mNewTabPageManager.openSnippet(windowDisposition, mArticle); } @Override @@ -152,7 +151,7 @@ @Override public void onContextMenuCreated() { - mNewTabPageManager.trackSnippetMenuOpened(mArticle); + mNewTabPageManager.getSuggestionsMetricsReporter().onSuggestionMenuOpened(mArticle); } @Override @@ -224,8 +223,8 @@ BidiFormatter.getInstance().unicodeWrap(article.mPublisher), relativeTimeSpan); } - public void onBindViewHolder(SnippetArticle article, SuggestionsCategoryInfo categoryInfo, - List<Object> payloads, int categoryIndex) { + public void onBindViewHolder( + SnippetArticle article, SuggestionsCategoryInfo categoryInfo, List<Object> payloads) { if (!payloads.isEmpty() && article.equals(mArticle)) { performPartialBind(payloads); return; @@ -235,7 +234,6 @@ mArticle = article; mCategoryInfo = categoryInfo; - mCategoryIndex = categoryIndex; updateLayout(); mHeadlineTextView.setText(mArticle.mTitle);
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java index a9928ce1..87e5e00 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java
@@ -8,9 +8,13 @@ import org.chromium.base.Callback; import org.chromium.base.annotations.CalledByNative; +import org.chromium.chrome.browser.ntp.NewTabPageUma; +import org.chromium.chrome.browser.ntp.cards.ActionItem; import org.chromium.chrome.browser.ntp.cards.SuggestionsCategoryInfo; import org.chromium.chrome.browser.ntp.snippets.CategoryStatus.CategoryStatusEnum; import org.chromium.chrome.browser.profiles.Profile; +import org.chromium.chrome.browser.suggestions.SuggestionsMetricsReporter; +import org.chromium.chrome.browser.suggestions.SuggestionsRanker; import java.util.ArrayList; import java.util.List; @@ -18,11 +22,12 @@ /** * Provides access to the snippets to display on the NTP using the C++ ContentSuggestionsService. */ -public class SnippetsBridge implements SuggestionsSource { +public class SnippetsBridge implements SuggestionsSource, SuggestionsMetricsReporter { private static final String TAG = "SnippetsBridge"; private long mNativeSnippetsBridge; private SuggestionsSource.Observer mObserver; + private SuggestionsRanker mSuggestionsRanker; public static boolean isCategoryStatusAvailable(@CategoryStatusEnum int status) { // Note: This code is duplicated in content_suggestions_category_status.cc. @@ -119,8 +124,8 @@ @Override public void dismissSuggestion(SnippetArticle suggestion) { assert mNativeSnippetsBridge != 0; - nativeDismissSuggestion(mNativeSnippetsBridge, suggestion.mUrl, suggestion.mGlobalPosition, - suggestion.mCategory, suggestion.mPosition, suggestion.mIdWithinCategory); + nativeDismissSuggestion(mNativeSnippetsBridge, suggestion.mUrl, suggestion.getGlobalRank(), + suggestion.mCategory, suggestion.getPerSectionRank(), suggestion.mIdWithinCategory); } @Override @@ -135,41 +140,66 @@ nativeRestoreDismissedCategories(mNativeSnippetsBridge); } + @Override public void onPageShown(int[] categories, int[] suggestionsPerCategory) { assert mNativeSnippetsBridge != 0; nativeOnPageShown(mNativeSnippetsBridge, categories, suggestionsPerCategory); } + @Override public void onSuggestionShown(SnippetArticle suggestion) { assert mNativeSnippetsBridge != 0; - nativeOnSuggestionShown(mNativeSnippetsBridge, suggestion.mGlobalPosition, - suggestion.mCategory, suggestion.mPosition, + nativeOnSuggestionShown(mNativeSnippetsBridge, suggestion.getGlobalRank(), + suggestion.mCategory, suggestion.getPerSectionRank(), suggestion.mPublishTimestampMilliseconds, suggestion.mScore); } - public void onSuggestionOpened( - SnippetArticle suggestion, int categoryIndex, int windowOpenDisposition) { + @Override + public void onSuggestionOpened(SnippetArticle suggestion, int windowOpenDisposition) { assert mNativeSnippetsBridge != 0; - nativeOnSuggestionOpened(mNativeSnippetsBridge, suggestion.mGlobalPosition, - suggestion.mCategory, categoryIndex, suggestion.mPosition, + int categoryIndex = mSuggestionsRanker.getCategoryRank(suggestion.mCategory); + nativeOnSuggestionOpened(mNativeSnippetsBridge, suggestion.getGlobalRank(), + suggestion.mCategory, categoryIndex, suggestion.getPerSectionRank(), suggestion.mPublishTimestampMilliseconds, suggestion.mScore, windowOpenDisposition); } + @Override public void onSuggestionMenuOpened(SnippetArticle suggestion) { assert mNativeSnippetsBridge != 0; - nativeOnSuggestionMenuOpened(mNativeSnippetsBridge, suggestion.mGlobalPosition, - suggestion.mCategory, suggestion.mPosition, + nativeOnSuggestionMenuOpened(mNativeSnippetsBridge, suggestion.getGlobalRank(), + suggestion.mCategory, suggestion.getPerSectionRank(), suggestion.mPublishTimestampMilliseconds, suggestion.mScore); } - public void onMoreButtonShown(int category, int position) { + @Override + public void onMoreButtonShown(ActionItem actionItem) { assert mNativeSnippetsBridge != 0; - nativeOnMoreButtonShown(mNativeSnippetsBridge, category, position); + nativeOnMoreButtonShown( + mNativeSnippetsBridge, actionItem.getCategory(), actionItem.getPerSectionRank()); } - public void onMoreButtonClicked(int category, int position) { + @Override + public void onMoreButtonClicked(ActionItem actionItem) { assert mNativeSnippetsBridge != 0; - nativeOnMoreButtonClicked(mNativeSnippetsBridge, category, position); + @CategoryInt + int category = actionItem.getCategory(); + nativeOnMoreButtonClicked(mNativeSnippetsBridge, category, actionItem.getPerSectionRank()); + switch (category) { + case KnownCategories.BOOKMARKS: + NewTabPageUma.recordAction(NewTabPageUma.ACTION_OPENED_BOOKMARKS_MANAGER); + break; + // MORE button in both categories leads to the recent tabs manager + case KnownCategories.FOREIGN_TABS: + case KnownCategories.RECENT_TABS: + NewTabPageUma.recordAction(NewTabPageUma.ACTION_OPENED_RECENT_TABS_MANAGER); + break; + case KnownCategories.DOWNLOADS: + NewTabPageUma.recordAction(NewTabPageUma.ACTION_OPENED_DOWNLOADS_MANAGER); + break; + default: + // No action associated + break; + } } /** @@ -194,21 +224,19 @@ nativeOnSuggestionTargetVisited(category, visitTimeMs); } - /** - * Sets the recipient for the fetched snippets. - * - * An observer needs to be set before the native code attempts to transmit snippets them to - * java. Upon registration, the observer will be notified of already fetched snippets. - * - * @param observer object to notify when snippets are received. - */ @Override - public void setObserver(SuggestionsSource.Observer observer) { + public void setObserver(Observer observer) { assert observer != null; mObserver = observer; } @Override + public void setRanker(SuggestionsRanker suggestionsRanker) { + assert suggestionsRanker != null; + mSuggestionsRanker = suggestionsRanker; + } + + @Override public void fetchSuggestions(@CategoryInt int category, String[] displayedSuggestionIds) { nativeFetch(mNativeSnippetsBridge, category, displayedSuggestionIds); } @@ -234,7 +262,7 @@ long timestamp, float score) { int position = suggestions.size(); suggestions.add(new SnippetArticle( - category, id, title, publisher, previewText, url, timestamp, score, position)); + category, id, title, publisher, previewText, url, timestamp, score)); return suggestions.get(position); } @@ -306,18 +334,18 @@ private native void nativeFetch( long nativeNTPSnippetsBridge, int category, String[] knownSuggestions); private native void nativeDismissSuggestion(long nativeNTPSnippetsBridge, String url, - int globalPosition, int category, int categoryPosition, String idWithinCategory); + int globalPosition, int category, int positionInCategory, String idWithinCategory); private native void nativeDismissCategory(long nativeNTPSnippetsBridge, int category); private native void nativeRestoreDismissedCategories(long nativeNTPSnippetsBridge); private native void nativeOnPageShown( long nativeNTPSnippetsBridge, int[] categories, int[] suggestionsPerCategory); private native void nativeOnSuggestionShown(long nativeNTPSnippetsBridge, int globalPosition, - int category, int categoryPosition, long publishTimestampMs, float score); + int category, int positionInCategory, long publishTimestampMs, float score); private native void nativeOnSuggestionOpened(long nativeNTPSnippetsBridge, int globalPosition, - int category, int categoryIndex, int categoryPosition, long publishTimestampMs, + int category, int categoryIndex, int positionInCategory, long publishTimestampMs, float score, int windowOpenDisposition); private native void nativeOnSuggestionMenuOpened(long nativeNTPSnippetsBridge, - int globalPosition, int category, int categoryPosition, long publishTimestampMs, + int globalPosition, int category, int positionInCategory, long publishTimestampMs, float score); private native void nativeOnMoreButtonShown( long nativeNTPSnippetsBridge, int category, int position);
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/suggestions/ContentSuggestionsActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/suggestions/ContentSuggestionsActivity.java index 8ca0ed4..0d66ebc7 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/suggestions/ContentSuggestionsActivity.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/suggestions/ContentSuggestionsActivity.java
@@ -94,23 +94,7 @@ public void navigateToDownloadManager() {} @Override - public void trackSnippetsPageImpression(int[] categories, int[] suggestionsPerCategory) {} - - @Override - public void trackSnippetImpression(SnippetArticle article) {} - - @Override - public void trackSnippetMenuOpened(SnippetArticle article) {} - - @Override - public void trackSnippetCategoryActionImpression(int category, int position) {} - - @Override - public void trackSnippetCategoryActionClick(int category, int position) {} - - @Override - public void openSnippet( - int windowOpenDisposition, SnippetArticle article, int categoryIndex) {} + public void openSnippet(int windowOpenDisposition, SnippetArticle article) {} @Override public void focusSearchBox(boolean beginVoiceSearch, String pastedText) {} @@ -163,6 +147,11 @@ public ContextMenuManager getContextMenuManager() { return mContextMenuManager; } + + @Override + public SuggestionsMetricsReporter getSuggestionsMetricsReporter() { + return mSnippetsBridge; + } } @Override @@ -198,6 +187,10 @@ for (DestructionObserver observer : mDestructionObservers) { observer.onDestroy(); } + + mSnippetsBridge.destroy(); + mSnippetsBridge = null; + super.onDestroy(); } }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsMetricsReporter.java b/chrome/android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsMetricsReporter.java new file mode 100644 index 0000000..b2aa548b --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsMetricsReporter.java
@@ -0,0 +1,56 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.chrome.browser.suggestions; + +import org.chromium.chrome.browser.ntp.cards.ActionItem; +import org.chromium.chrome.browser.ntp.snippets.CategoryInt; +import org.chromium.chrome.browser.ntp.snippets.SnippetArticle; + +/** + * Exposes UMA related methods. + */ +public interface SuggestionsMetricsReporter { + /** + * Tracks per-page-load metrics for content suggestions. + * @param categories The categories of content suggestions. + * @param suggestionsPerCategory The number of content suggestions in each category. + */ + void onPageShown(int[] categories, int[] suggestionsPerCategory); + + /** + * Tracks impression metrics for a content suggestion. + * @param suggestion The content suggestion that was shown to the user. + */ + void onSuggestionShown(SnippetArticle suggestion); + + /** + * Tracks interaction metrics for a content suggestion. + * @param suggestion The content suggestion that the user opened. + * @param windowOpenDisposition How the suggestion was opened (current tab, new tab, + * new window etc). + */ + void onSuggestionOpened(SnippetArticle suggestion, int windowOpenDisposition); + + /** + * Tracks impression metrics for the long-press menu for a content suggestion. + * @param suggestion The content suggestion for which the long-press menu was opened. + */ + void onSuggestionMenuOpened(SnippetArticle suggestion); + + /** + * Tracks impression metrics for a category's action button ("More"). + * @param category The action button that was shown. + */ + void onMoreButtonShown(@CategoryInt ActionItem category); + + /** + * Tracks click metrics for a category's action button ("More"). + * @param category The action button that was clicked. + */ + void onMoreButtonClicked(@CategoryInt ActionItem category); + + /** Sets the ranker to use to compute some of the reported metrics. */ + void setRanker(SuggestionsRanker suggestionsRanker); +}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsRanker.java b/chrome/android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsRanker.java new file mode 100644 index 0000000..49b546e --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsRanker.java
@@ -0,0 +1,61 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.chrome.browser.suggestions; + +import org.chromium.chrome.browser.ntp.cards.ActionItem; +import org.chromium.chrome.browser.ntp.cards.SuggestionsSection; +import org.chromium.chrome.browser.ntp.snippets.CategoryInt; +import org.chromium.chrome.browser.ntp.snippets.SnippetArticle; + +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * Attributes ranks to suggestions and related elements. + * + * Ranks here are 0-based scores attributed based on the position or loading order of the + * elements. + */ +public class SuggestionsRanker { + private final Map<Integer, Integer> mSuggestionsAddedPerSection = new LinkedHashMap<>(); + private int mTotalAddedSuggestions; + + /** + * Attributes a per section rank to the provided action item. + * @see ActionItem#getPerSectionRank() + */ + public void rankActionItem(ActionItem actionItem, SuggestionsSection section) { + if (actionItem.getPerSectionRank() != -1) return; // Item was already ranked. + actionItem.setPerSectionRank(section.getSuggestionsCount()); + } + + /** + * Attributes global and per section rank to the provided suggestion. + * @see SnippetArticle#getPerSectionRank() + * @see SnippetArticle#getGlobalRank() + */ + public void rankSuggestion(SnippetArticle suggestion) { + int globalRank = mTotalAddedSuggestions++; + int perSectionRank = mSuggestionsAddedPerSection.get(suggestion.mCategory); + mSuggestionsAddedPerSection.put(suggestion.mCategory, perSectionRank + 1); + + suggestion.setRank(perSectionRank, globalRank); + } + + public void registerCategory(@CategoryInt int category) { + // Check we are not simply resetting an already registered category. + if (mSuggestionsAddedPerSection.containsKey(category)) return; + mSuggestionsAddedPerSection.put(category, 0); + } + + public int getCategoryRank(@CategoryInt int category) { + int rank = 0; + for (Integer key : mSuggestionsAddedPerSection.keySet()) { + if (key == category) return rank; + ++rank; + } + return -1; + } +}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java index 149a3e2c..a32ff99e 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java
@@ -20,6 +20,8 @@ import org.chromium.chrome.browser.ChromeApplication; import org.chromium.chrome.browser.ShortcutHelper; import org.chromium.chrome.browser.banners.InstallerDelegate; +import org.chromium.chrome.browser.externalauth.ExternalAuthUtils; +import org.chromium.chrome.browser.externalauth.UserRecoverableErrorHandler; import org.chromium.chrome.browser.util.IntentUtils; import java.io.File; @@ -62,8 +64,11 @@ } @CalledByNative - private boolean hasGooglePlayWebApkInstallDelegate() { - return mGooglePlayWebApkInstallDelegate != null; + private boolean canUseGooglePlayInstallService() { + return mGooglePlayWebApkInstallDelegate != null + && ExternalAuthUtils.getInstance().canUseGooglePlayServices( + ContextUtils.getApplicationContext(), + new UserRecoverableErrorHandler.Silent()); } @CalledByNative
diff --git a/chrome/android/java_sources.gni b/chrome/android/java_sources.gni index 4ea2d7e..f0f2631 100644 --- a/chrome/android/java_sources.gni +++ b/chrome/android/java_sources.gni
@@ -918,6 +918,8 @@ "java/src/org/chromium/chrome/browser/snackbar/undo/UndoBarController.java", "java/src/org/chromium/chrome/browser/ssl/SecurityStateModel.java", "java/src/org/chromium/chrome/browser/suggestions/ContentSuggestionsActivity.java", + "java/src/org/chromium/chrome/browser/suggestions/SuggestionsMetricsReporter.java", + "java/src/org/chromium/chrome/browser/suggestions/SuggestionsRanker.java", "java/src/org/chromium/chrome/browser/superviseduser/SupervisedUserContentProvider.java", "java/src/org/chromium/chrome/browser/sync/GmsCoreSyncListener.java", "java/src/org/chromium/chrome/browser/sync/GoogleServiceAuthError.java", @@ -1521,6 +1523,7 @@ "junit/src/org/chromium/chrome/browser/ntp/cards/InnerNodeTest.java", "junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java", "junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java", + "junit/src/org/chromium/chrome/browser/ntp/cards/SectionListTest.java", "junit/src/org/chromium/chrome/browser/offlinepages/BackgroundSchedulerTest.java", "junit/src/org/chromium/chrome/browser/offlinepages/BackgroundOfflinerTaskTest.java", "junit/src/org/chromium/chrome/browser/offlinepages/ClientIdTest.java",
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerViewTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerViewTest.java index e4c6804..5cd10e94 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerViewTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerViewTest.java
@@ -301,7 +301,7 @@ for (int i = 0; i < suggestionsCount; i++) { String url = mTestServer.getURL(TEST_PAGE) + "#" + i; suggestions.add(new SnippetArticle(KnownCategories.ARTICLES, "id" + i, "title" + i, - "publisher" + i, "previewText" + i, url, 1466614774 + i, 10f, 0)); + "publisher" + i, "previewText" + i, url, 1466614774 + i, 10f)); } return suggestions; }
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/snippets/ArticleSnippetsTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/snippets/ArticleSnippetsTest.java index ab722ec..091e335 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/snippets/ArticleSnippetsTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/snippets/ArticleSnippetsTest.java
@@ -26,12 +26,15 @@ import org.chromium.chrome.browser.ntp.NewTabPage.DestructionObserver; import org.chromium.chrome.browser.ntp.NewTabPageView.NewTabPageManager; import org.chromium.chrome.browser.ntp.UiConfig; +import org.chromium.chrome.browser.ntp.cards.ActionItem; import org.chromium.chrome.browser.ntp.cards.NewTabPageAdapter; import org.chromium.chrome.browser.ntp.cards.NewTabPageRecyclerView; import org.chromium.chrome.browser.ntp.cards.SuggestionsCategoryInfo; import org.chromium.chrome.browser.offlinepages.OfflinePageBridge; import org.chromium.chrome.browser.profiles.MostVisitedSites.MostVisitedURLsObserver; import org.chromium.chrome.browser.profiles.Profile; +import org.chromium.chrome.browser.suggestions.SuggestionsMetricsReporter; +import org.chromium.chrome.browser.suggestions.SuggestionsRanker; import org.chromium.chrome.test.ChromeActivityTestCaseBase; import org.chromium.chrome.test.util.RenderUtils.ViewRenderer; @@ -128,51 +131,30 @@ int fullCategory = 0; @CategoryInt int minimalCategory = 1; - SnippetArticle shortSnippet = new SnippetArticle( - fullCategory, - "id1", - "Snippet", - "Publisher", - "Preview Text", - "www.google.com", - 1466614774, // Timestamp - 10f, // Score - 0); // Position + SnippetArticle shortSnippet = new SnippetArticle(fullCategory, "id1", "Snippet", + "Publisher", "Preview Text", "www.google.com", + 1466614774, // Timestamp + 10f); // Score shortSnippet.setThumbnailBitmap(BitmapFactory.decodeResource(getActivity().getResources(), R.drawable.signin_promo_illustration)); - SnippetArticle longSnippet = new SnippetArticle( - fullCategory, - "id2", + SnippetArticle longSnippet = new SnippetArticle(fullCategory, "id2", new String(new char[20]).replace("\0", "Snippet "), new String(new char[20]).replace("\0", "Publisher "), - new String(new char[80]).replace("\0", "Preview Text "), - "www.google.com", - 1466614074, // Timestamp - 20f, // Score - 1); // Position + new String(new char[80]).replace("\0", "Preview Text "), "www.google.com", + 1466614074, // Timestamp + 20f); // Score - SnippetArticle minimalSnippet = new SnippetArticle( - minimalCategory, - "id3", - new String(new char[20]).replace("\0", "Bookmark "), - "Publisher", - "This should not be displayed", - "www.google.com", - 1466614774, // Timestamp - 10f, // Score - 0); // Position + SnippetArticle minimalSnippet = new SnippetArticle(minimalCategory, "id3", + new String(new char[20]).replace("\0", "Bookmark "), "Publisher", + "This should not be displayed", "www.google.com", + 1466614774, // Timestamp + 10f); // Score - SnippetArticle minimalSnippet2 = new SnippetArticle( - minimalCategory, - "id4", - "Bookmark", - "Publisher", - "This should not be displayed", - "www.google.com", - 1466614774, // Timestamp - 10f, // Score - 0); // Position + SnippetArticle minimalSnippet2 = new SnippetArticle(minimalCategory, "id4", "Bookmark", + "Publisher", "This should not be displayed", "www.google.com", + 1466614774, // Timestamp + 10f); // Score mSnippetsSource.setInfoForCategory( fullCategory, new SuggestionsCategoryInfo(fullCategory, "Section Title", @@ -212,6 +194,8 @@ // TODO(dgn): provide a RecyclerView if we need to test the context menu. private ContextMenuManager mContextMenuManager = new ContextMenuManager(getActivity(), this, null); + private SuggestionsMetricsReporter mSuggestionsMetricsReporter = + new DummySuggestionsMetricsReporter(); @Override public void getLocalFaviconImageForURL( @@ -265,23 +249,7 @@ } @Override - public void trackSnippetsPageImpression(int[] categories, int[] suggestionsPerCategory) {} - - @Override - public void trackSnippetImpression(SnippetArticle article) {} - - @Override - public void trackSnippetMenuOpened(SnippetArticle article) {} - - @Override - public void trackSnippetCategoryActionImpression(int category, int position) {} - - @Override - public void trackSnippetCategoryActionClick(int category, int position) {} - - @Override - public void openSnippet( - int windowOpenDisposition, SnippetArticle article, int categoryIndex) { + public void openSnippet(int windowOpenDisposition, SnippetArticle article) { throw new UnsupportedOperationException(); } @@ -363,5 +331,32 @@ public ContextMenuManager getContextMenuManager() { return mContextMenuManager; } + + public SuggestionsMetricsReporter getSuggestionsMetricsReporter() { + return mSuggestionsMetricsReporter; + } + } + + private static class DummySuggestionsMetricsReporter implements SuggestionsMetricsReporter { + @Override + public void onPageShown(int[] categories, int[] suggestionsPerCategory) {} + + @Override + public void onSuggestionShown(SnippetArticle suggestion) {} + + @Override + public void onSuggestionOpened(SnippetArticle suggestion, int windowOpenDisposition) {} + + @Override + public void onSuggestionMenuOpened(SnippetArticle suggestion) {} + + @Override + public void onMoreButtonShown(@CategoryInt ActionItem category) {} + + @Override + public void onMoreButtonClicked(@CategoryInt ActionItem category) {} + + @Override + public void setRanker(SuggestionsRanker suggestionsRanker) {} } }
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/ContentSuggestionsTestUtils.java b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/ContentSuggestionsTestUtils.java index 0979f149..df22287 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/ContentSuggestionsTestUtils.java +++ b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/ContentSuggestionsTestUtils.java
@@ -19,27 +19,54 @@ public final class ContentSuggestionsTestUtils { private ContentSuggestionsTestUtils() {} - public static List<SnippetArticle> createDummySuggestions(int count) { + public static List<SnippetArticle> createDummySuggestions( + int count, @CategoryInt int category) { List<SnippetArticle> suggestions = new ArrayList<>(); for (int index = 0; index < count; index++) { - suggestions.add(new SnippetArticle(KnownCategories.BOOKMARKS, - "https://site.com/url" + index, "title" + index, "pub" + index, "txt" + index, - "https://site.com/url" + index, 0, 0, 0)); + suggestions.add( + new SnippetArticle(category, "https://site.com/url" + index, "title" + index, + "pub" + index, "txt" + index, "https://site.com/url" + index, 0, 0)); } return suggestions; } - /** Registers a category that has a reload action and is shown if empty. */ - public static void registerCategory(FakeSuggestionsSource suggestionsSource, + /** + * @deprecated The hardcoded category is a common source of bugs. Prefer + * {@link #createDummySuggestions(int, int)} + */ + @Deprecated + public static List<SnippetArticle> createDummySuggestions(int count) { + return createDummySuggestions(count, KnownCategories.BOOKMARKS); + } + + /** + * Registers a category according to the provided category info. + * @return the suggestions added to the newly registered category. + */ + public static List<SnippetArticle> registerCategory(FakeSuggestionsSource suggestionsSource, @CategoryInt int category, int suggestionCount) { + // Important: showIfEmpty flag to true. + SuggestionsCategoryInfo categoryInfo = + new CategoryInfoBuilder(category).withReloadAction().showIfEmpty().build(); + return registerCategory(suggestionsSource, categoryInfo, suggestionCount); + } + + /** + * Registers a category that has a reload action and is shown if empty. + * @return the suggestions added to the newly registered category. + */ + public static List<SnippetArticle> registerCategory(FakeSuggestionsSource suggestionsSource, + SuggestionsCategoryInfo categoryInfo, int suggestionCount) { // FakeSuggestionSource does not provide suggestions if the category's status is not // AVAILABLE. - suggestionsSource.setStatusForCategory(category, CategoryStatus.AVAILABLE); - // Important: showIfEmpty flag to true. - suggestionsSource.setInfoForCategory(category, - new CategoryInfoBuilder(category).withReloadAction().showIfEmpty().build()); - suggestionsSource.setSuggestionsForCategory( - category, createDummySuggestions(suggestionCount)); + suggestionsSource.setStatusForCategory( + categoryInfo.getCategory(), CategoryStatus.AVAILABLE); + suggestionsSource.setInfoForCategory(categoryInfo.getCategory(), categoryInfo); + + List<SnippetArticle> suggestions = + createDummySuggestions(suggestionCount, categoryInfo.getCategory()); + suggestionsSource.setSuggestionsForCategory(categoryInfo.getCategory(), suggestions); + return suggestions; } public static String viewTypeToString(@ItemViewType int viewType) {
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java index 2c50fc0..6b3fa15 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java +++ b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
@@ -67,6 +67,7 @@ import org.chromium.chrome.browser.signin.SigninManager; import org.chromium.chrome.browser.signin.SigninManager.SignInAllowedObserver; import org.chromium.chrome.browser.signin.SigninManager.SignInStateObserver; +import org.chromium.chrome.browser.suggestions.SuggestionsMetricsReporter; import org.chromium.testing.local.LocalRobolectricTestRunner; import java.util.ArrayList; @@ -210,6 +211,8 @@ new CategoryInfoBuilder(category).showIfEmpty().build()); when(mNewTabPageManager.getSuggestionsSource()).thenReturn(mSource); + when(mNewTabPageManager.getSuggestionsMetricsReporter()) + .thenReturn(mock(SuggestionsMetricsReporter.class)); when(mNewTabPageManager.isCurrentPage()).thenReturn(true); reloadNtp(); @@ -232,7 +235,8 @@ assertItemsFor(sectionWithStatusCard().withProgress()); final int numSuggestions = 3; - List<SnippetArticle> suggestions = createDummySuggestions(numSuggestions); + List<SnippetArticle> suggestions = + createDummySuggestions(numSuggestions, KnownCategories.ARTICLES); mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.AVAILABLE); mSource.setSuggestionsForCategory(KnownCategories.ARTICLES, suggestions); @@ -245,16 +249,18 @@ @Test @Feature({"Ntp"}) public void testSuggestionLoadingInitiallyEmpty() { + final int category = KnownCategories.ARTICLES; + // If we don't get anything, we should be in the same situation as the initial one. - mSource.setSuggestionsForCategory( - KnownCategories.ARTICLES, new ArrayList<SnippetArticle>()); + mSource.setSuggestionsForCategory(category, new ArrayList<SnippetArticle>()); assertItemsFor(sectionWithStatusCard().withProgress()); // We should load new suggestions when we get notified about them. final int numSuggestions = 5; - List<SnippetArticle> suggestions = createDummySuggestions(numSuggestions); - mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.AVAILABLE); - mSource.setSuggestionsForCategory(KnownCategories.ARTICLES, suggestions); + + List<SnippetArticle> suggestions = createDummySuggestions(numSuggestions, category); + mSource.setStatusForCategory(category, CategoryStatus.AVAILABLE); + mSource.setSuggestionsForCategory(category, suggestions); assertItemsFor(section(numSuggestions)); } @@ -265,7 +271,7 @@ @Test @Feature({"Ntp"}) public void testSuggestionClearing() { - List<SnippetArticle> suggestions = createDummySuggestions(4); + List<SnippetArticle> suggestions = createDummySuggestions(4, KnownCategories.ARTICLES); mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.AVAILABLE); mSource.setSuggestionsForCategory(KnownCategories.ARTICLES, suggestions); assertItemsFor(section(4)); @@ -281,7 +287,7 @@ assertItemsFor(sectionWithStatusCard()); // The adapter should now be waiting for new suggestions. - suggestions = createDummySuggestions(6); + suggestions = createDummySuggestions(6, KnownCategories.ARTICLES); mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.AVAILABLE); mSource.setSuggestionsForCategory(KnownCategories.ARTICLES, suggestions); assertItemsFor(section(6)); @@ -293,7 +299,7 @@ @Test @Feature({"Ntp"}) public void testSuggestionLoadingBlock() { - List<SnippetArticle> suggestions = createDummySuggestions(3); + List<SnippetArticle> suggestions = createDummySuggestions(3, KnownCategories.ARTICLES); // By default, status is INITIALIZING, so we can load suggestions. mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.AVAILABLE); @@ -301,8 +307,8 @@ assertItemsFor(section(3)); // Add another snippet. - suggestions.add(new SnippetArticle(0, "https://site.com/url1", "title1", "pub1", "txt1", - "https://site.com/url1", 0, 0, 0)); + suggestions.add(new SnippetArticle(KnownCategories.ARTICLES, "https://site.com/url1", + "title1", "pub1", "txt1", "https://site.com/url1", 0, 0)); // When snippets are disabled, we should not be able to load them. mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.SIGNED_OUT); @@ -350,7 +356,7 @@ @Test @Feature({"Ntp"}) public void testSectionClearingWhenUnavailable() { - List<SnippetArticle> snippets = createDummySuggestions(5); + List<SnippetArticle> snippets = createDummySuggestions(5, KnownCategories.ARTICLES); mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.AVAILABLE); mSource.setSuggestionsForCategory(KnownCategories.ARTICLES, snippets); assertItemsFor(section(5)); @@ -382,7 +388,7 @@ @Test @Feature({"Ntp"}) public void testUIUntouchedWhenNotProvided() { - List<SnippetArticle> snippets = createDummySuggestions(4); + List<SnippetArticle> snippets = createDummySuggestions(4, KnownCategories.ARTICLES); mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.AVAILABLE); mSource.setSuggestionsForCategory(KnownCategories.ARTICLES, snippets); assertItemsFor(section(4)); @@ -402,11 +408,11 @@ @Test @Feature({"Ntp"}) public void testUIUpdatesOnNewSuggestionsWhenOtherSectionSeen() { - List<SnippetArticle> snippets = createDummySuggestions(4); + List<SnippetArticle> snippets = createDummySuggestions(4, KnownCategories.ARTICLES); mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.AVAILABLE); mSource.setSuggestionsForCategory(KnownCategories.ARTICLES, snippets); - List<SnippetArticle> bookmarks = createDummySuggestions(2); + List<SnippetArticle> bookmarks = createDummySuggestions(2, KnownCategories.BOOKMARKS); mSource.setStatusForCategory(KnownCategories.BOOKMARKS, CategoryStatus.AVAILABLE); mSource.setInfoForCategory(KnownCategories.BOOKMARKS, new CategoryInfoBuilder(KnownCategories.BOOKMARKS).showIfEmpty().build()); @@ -419,7 +425,7 @@ .getSectionForTesting(KnownCategories.BOOKMARKS) .childSeen(2); - List<SnippetArticle> newSnippets = createDummySuggestions(3); + List<SnippetArticle> newSnippets = createDummySuggestions(3, KnownCategories.ARTICLES); mSource.setSuggestionsForCategory(KnownCategories.ARTICLES, newSnippets); assertItemsFor(section(3), section(2)); @@ -434,12 +440,11 @@ @Test @Feature({"Ntp"}) public void testUIUpdatesOnNewSuggestionsWhenFirstOfOtherSectionIsSeen() { - List<SnippetArticle> snippets = createDummySuggestions(4); - SnippetArticle earlier = snippets.get(0); + List<SnippetArticle> snippets = createDummySuggestions(4, KnownCategories.ARTICLES); mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.AVAILABLE); mSource.setSuggestionsForCategory(KnownCategories.ARTICLES, snippets); - List<SnippetArticle> bookmarks = createDummySuggestions(1); + List<SnippetArticle> bookmarks = createDummySuggestions(1, KnownCategories.BOOKMARKS); mSource.setStatusForCategory(KnownCategories.BOOKMARKS, CategoryStatus.AVAILABLE); mSource.setInfoForCategory(KnownCategories.BOOKMARKS, new CategoryInfoBuilder(KnownCategories.BOOKMARKS).showIfEmpty().build()); @@ -452,7 +457,7 @@ .getSectionForTesting(KnownCategories.BOOKMARKS) .childSeen(1); - List<SnippetArticle> newSnippets = createDummySuggestions(3); + List<SnippetArticle> newSnippets = createDummySuggestions(3, KnownCategories.ARTICLES); SnippetArticle newer = newSnippets.get(0); mSource.setSuggestionsForCategory(KnownCategories.ARTICLES, newSnippets); @@ -481,7 +486,8 @@ assertItemsFor(sectionWithStatusCard().withProgress()); // 1.2 - With suggestions - List<SnippetArticle> articles = Collections.unmodifiableList(createDummySuggestions(3)); + List<SnippetArticle> articles = + Collections.unmodifiableList(createDummySuggestions(3, category)); suggestionsSource.setStatusForCategory(category, CategoryStatus.AVAILABLE); suggestionsSource.setSuggestionsForCategory(category, articles); assertItemsFor(section(3)); @@ -536,7 +542,8 @@ assertItemsFor(sectionWithStatusCard().withActionButton().withProgress()); // 1.2 - With suggestions. - List<SnippetArticle> articles = Collections.unmodifiableList(createDummySuggestions(3)); + List<SnippetArticle> articles = + Collections.unmodifiableList(createDummySuggestions(3, category)); suggestionsSource.setStatusForCategory(category, CategoryStatus.AVAILABLE); suggestionsSource.setSuggestionsForCategory(category, articles); assertItemsFor(section(3).withActionButton()); @@ -581,7 +588,7 @@ @Test @Feature({"Ntp"}) public void testSuggestionInvalidated() { - List<SnippetArticle> articles = createDummySuggestions(3); + List<SnippetArticle> articles = createDummySuggestions(3, KnownCategories.ARTICLES); mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.AVAILABLE); mSource.setSuggestionsForCategory(KnownCategories.ARTICLES, articles); assertItemsFor(section(3)); @@ -598,13 +605,13 @@ @Test @Feature({"Ntp"}) public void testDynamicCategories() { - List<SnippetArticle> articles = createDummySuggestions(3); + List<SnippetArticle> articles = createDummySuggestions(3, KnownCategories.ARTICLES); mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.AVAILABLE); mSource.setSuggestionsForCategory(KnownCategories.ARTICLES, articles); assertItemsFor(section(3)); int dynamicCategory1 = 1010; - List<SnippetArticle> dynamics1 = createDummySuggestions(5); + List<SnippetArticle> dynamics1 = createDummySuggestions(5, KnownCategories.ARTICLES); mSource.setInfoForCategory(dynamicCategory1, new CategoryInfoBuilder(dynamicCategory1) .withViewAllAction() .build()); @@ -615,7 +622,7 @@ assertItemsFor(section(3), section(5).withActionButton()); int dynamicCategory2 = 1011; - List<SnippetArticle> dynamics2 = createDummySuggestions(11); + List<SnippetArticle> dynamics2 = createDummySuggestions(11, KnownCategories.ARTICLES); mSource.setInfoForCategory(dynamicCategory2, new CategoryInfoBuilder(dynamicCategory1).build()); mSource.setStatusForCategory(dynamicCategory2, CategoryStatus.AVAILABLE); @@ -752,8 +759,8 @@ final int newSuggestionCount = 7; reset(dataObserver); - suggestionsSource.setSuggestionsForCategory( - KnownCategories.ARTICLES, createDummySuggestions(newSuggestionCount)); + suggestionsSource.setSuggestionsForCategory(KnownCategories.ARTICLES, + createDummySuggestions(newSuggestionCount, KnownCategories.ARTICLES)); verify(dataObserver).onItemRangeInserted(2, newSuggestionCount); verify(dataObserver).onItemRangeChanged(5 + newSuggestionCount, 1, null); // Spacer refresh verify(dataObserver, times(2)).onItemRangeRemoved(2 + newSuggestionCount, 1); @@ -772,7 +779,7 @@ verifyNoMoreInteractions(dataObserver); reset(dataObserver); suggestionsSource.setSuggestionsForCategory( - KnownCategories.ARTICLES, createDummySuggestions(0)); + KnownCategories.ARTICLES, createDummySuggestions(0, KnownCategories.ARTICLES)); mAdapter.getSectionListForTesting().onCategoryStatusChanged( KnownCategories.ARTICLES, CategoryStatus.SIGNED_OUT); verify(dataObserver).onItemRangeRemoved(2, newSuggestionCount); @@ -946,7 +953,8 @@ // Prepare some suggestions. They should not load because the category is dismissed on // the current NTP. mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.AVAILABLE); - mSource.setSuggestionsForCategory(KnownCategories.ARTICLES, createDummySuggestions(1)); + mSource.setSuggestionsForCategory( + KnownCategories.ARTICLES, createDummySuggestions(1, KnownCategories.ARTICLES)); mSource.setInfoForCategory(KnownCategories.ARTICLES, new CategoryInfoBuilder(KnownCategories.ARTICLES).build()); assertEquals(4, mAdapter.getItemCount()); // TODO(dgn): rewrite with section descriptors.
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SectionListTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SectionListTest.java new file mode 100644 index 0000000..452238f --- /dev/null +++ b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SectionListTest.java
@@ -0,0 +1,190 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.chrome.browser.ntp.cards; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import static org.chromium.chrome.browser.ntp.cards.ContentSuggestionsTestUtils.createDummySuggestions; +import static org.chromium.chrome.browser.ntp.cards.ContentSuggestionsTestUtils.registerCategory; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.robolectric.annotation.Config; + +import org.chromium.base.Callback; +import org.chromium.base.test.util.Feature; +import org.chromium.chrome.browser.ntp.NewTabPageView.NewTabPageManager; +import org.chromium.chrome.browser.ntp.cards.ContentSuggestionsTestUtils.CategoryInfoBuilder; +import org.chromium.chrome.browser.ntp.snippets.CategoryInt; +import org.chromium.chrome.browser.ntp.snippets.FakeSuggestionsSource; +import org.chromium.chrome.browser.ntp.snippets.KnownCategories; +import org.chromium.chrome.browser.ntp.snippets.SectionHeaderViewHolder; +import org.chromium.chrome.browser.ntp.snippets.SnippetArticle; +import org.chromium.chrome.browser.ntp.snippets.SnippetArticleViewHolder; +import org.chromium.chrome.browser.offlinepages.OfflinePageBridge; +import org.chromium.chrome.browser.suggestions.SuggestionsMetricsReporter; +import org.chromium.testing.local.LocalRobolectricTestRunner; + +import java.util.Collections; +import java.util.List; + +/** + * Unit tests for {@link SuggestionsSection}. + */ +@RunWith(LocalRobolectricTestRunner.class) +@Config(manifest = Config.NONE) +public class SectionListTest { + @Mock + private NewTabPageManager mNtpManager; + @Mock + private OfflinePageBridge mOfflinePageBridge; + @Mock + private SuggestionsMetricsReporter mMetricsReporter; + private FakeSuggestionsSource mSuggestionSource; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + mSuggestionSource = new FakeSuggestionsSource(); + + when(mNtpManager.getSuggestionsSource()).thenReturn(mSuggestionSource); + when(mNtpManager.getSuggestionsMetricsReporter()).thenReturn(mMetricsReporter); + } + + @Test + @Feature({"Ntp"}) + public void testGetSuggestionRank() { + // Setup the section list the following way: + // + // Item type | local rank | global rank + // -----------+------------+------------- + // HEADER | | + // ARTICLE | 0 | 0 + // ARTICLE | 1 | 1 + // ARTICLE | 2 | 2 + // HEADER | | + // STATUS | | + // ACTION | 0 | + // BOOKMARK | 0 | 3 + // BOOKMARK | 1 | 4 + // BOOKMARK | 2 | 5 + // BOOKMARK | 3 | 6 + List<SnippetArticle> articles = + registerCategory(mSuggestionSource, KnownCategories.ARTICLES, 3); + registerCategory(mSuggestionSource, KnownCategories.DOWNLOADS, 0); + List<SnippetArticle> bookmarks = + registerCategory(mSuggestionSource, KnownCategories.BOOKMARKS, 4); + + SectionList sectionList = new SectionList(mNtpManager, mOfflinePageBridge); + + assertThat(articles.get(0).getGlobalRank(), equalTo(0)); + assertThat(articles.get(0).getPerSectionRank(), equalTo(0)); + assertThat(articles.get(2).getGlobalRank(), equalTo(2)); + assertThat(articles.get(2).getPerSectionRank(), equalTo(2)); + assertThat(bookmarks.get(1).getGlobalRank(), equalTo(4)); + assertThat(bookmarks.get(1).getPerSectionRank(), equalTo(1)); + + // Test ranks after changes: remove then add some items. + @SuppressWarnings("unchecked") + Callback<String> cb = mock(Callback.class); + sectionList.dismissItem(2, cb); + + // Using sublists to skip the first items and avoid using existing ones + List<SnippetArticle> newArticles = + createDummySuggestions(5, KnownCategories.ARTICLES).subList(3, 5); + List<SnippetArticle> newBookmarks = + createDummySuggestions(6, KnownCategories.BOOKMARKS).subList(4, 6); + + sectionList.onMoreSuggestions(KnownCategories.ARTICLES, newArticles); + sectionList.onMoreSuggestions(KnownCategories.BOOKMARKS, newBookmarks); + + // After the changes we should have: + // Item type | local rank | global rank + // -----------+------------+------------- + // HEADER | | + // ARTICLE | 0 | 0 + // ARTICLE | 1 | 1 + // | - | - (deleted) + // ARTICLE | 3 | 7 (new) + // ARTICLE | 4 | 8 (new) + // HEADER | | + // STATUS | | + // ACTION | 0 | + // BOOKMARK | 0 | 3 + // BOOKMARK | 1 | 4 + // BOOKMARK | 2 | 5 + // BOOKMARK | 3 | 6 + // BOOKMARK | 4 | 9 (new) + // BOOKMARK | 5 | 10 (new) + + // The old ranks should not change. + assertThat(articles.get(0).getGlobalRank(), equalTo(0)); + assertThat(articles.get(0).getPerSectionRank(), equalTo(0)); + assertThat(articles.get(2).getGlobalRank(), equalTo(2)); + assertThat(articles.get(2).getPerSectionRank(), equalTo(2)); + assertThat(bookmarks.get(1).getGlobalRank(), equalTo(4)); + assertThat(bookmarks.get(1).getPerSectionRank(), equalTo(1)); + + // New ranks take into account the previously existing items. + assertThat(newArticles.get(1).getGlobalRank(), equalTo(8)); + assertThat(newArticles.get(1).getPerSectionRank(), equalTo(4)); + assertThat(newBookmarks.get(1).getGlobalRank(), equalTo(10)); + assertThat(newBookmarks.get(1).getPerSectionRank(), equalTo(5)); + } + + @Test + @Feature({"Ntp"}) + public void testGetActionItemRank() { + registerCategory(mSuggestionSource, KnownCategories.ARTICLES, 0); + registerCategory(mSuggestionSource, + new CategoryInfoBuilder(KnownCategories.DOWNLOADS).withViewAllAction().build(), 3); + + SectionList sectionList = new SectionList(mNtpManager, mOfflinePageBridge); + bindViewHolders(sectionList); + + assertThat(sectionList.getSectionForTesting(KnownCategories.ARTICLES) + .getActionItem() + .getPerSectionRank(), + equalTo(0)); + assertThat(sectionList.getSectionForTesting(KnownCategories.DOWNLOADS) + .getActionItem() + .getPerSectionRank(), + equalTo(3)); + } + + private static void bindViewHolders(InnerNode node) { + bindViewHolders(node, 0, node.getItemCount()); + } + + private static void bindViewHolders(InnerNode node, int startIndex, int endIndex) { + for (int i = startIndex; i < endIndex; ++i) { + node.onBindViewHolder( + makeViewHolder(node.getItemViewType(i)), i, Collections.emptyList()); + } + } + + private static NewTabPageViewHolder makeViewHolder(@CategoryInt int viewType) { + switch (viewType) { + case ItemViewType.SNIPPET: + return mock(SnippetArticleViewHolder.class); + case ItemViewType.HEADER: + return mock(SectionHeaderViewHolder.class); + case ItemViewType.STATUS: + return mock(StatusCardViewHolder.class); + case ItemViewType.ACTION: + return mock(ActionItem.ViewHolder.class); + case ItemViewType.PROGRESS: + return mock(ProgressViewHolder.class); + default: + return mock(NewTabPageViewHolder.class); + } + } +}
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java index 1336e85..5d965e1 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java +++ b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java
@@ -39,6 +39,8 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetArticle; import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource; import org.chromium.chrome.browser.offlinepages.OfflinePageItem; +import org.chromium.chrome.browser.suggestions.SuggestionsMetricsReporter; +import org.chromium.chrome.browser.suggestions.SuggestionsRanker; import org.chromium.testing.local.LocalRobolectricTestRunner; import java.util.Arrays; @@ -462,7 +464,8 @@ } private SuggestionsSection createSection(SuggestionsCategoryInfo info) { - SuggestionsSection section = new SuggestionsSection(mDelegate, mManager, mBridge, info); + SuggestionsSection section = new SuggestionsSection( + mDelegate, mManager, mock(SuggestionsRanker.class), mBridge, info); section.setParent(mParent); return section; } @@ -475,6 +478,8 @@ SuggestionsSource suggestionsSource = mock(SuggestionsSource.class); NewTabPageManager manager = mock(NewTabPageManager.class); when(manager.getSuggestionsSource()).thenReturn(suggestionsSource); + when(manager.getSuggestionsMetricsReporter()) + .thenReturn(mock(SuggestionsMetricsReporter.class)); if (action != ActionItem.ACTION_NONE) { section.getActionItem().performAction(manager);
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index 8f4d0b4..dc4b5f27 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd
@@ -15604,6 +15604,16 @@ <message name="IDS_NATIVE_ANDROID_HISTORY_MANAGER_DESCRIPTION" desc="Description of the flag that enables the native Android history UI." translateable="false"> Show the native Android UI for browsing history. </message> + + <!-- Play Services LSD permission prompt chrome://flags strings --> + <if expr="is_android"> + <message name="IDS_FLAGS_LSD_PERMISSION_PROMPT_NAME" desc="Name for the flag to enable LSD permission prompts on Android" translateable="false"> + Location Settings Dialog Permission Prompt + </message> + <message name="IDS_FLAGS_LSD_PERMISSION_PROMPT_DESCRIPTION" desc="Description for the flag to enable LSD permission prompts on Android." translateable="false"> + Whether to use the Google Play Services Location Settings Dialog permission dialog. + </message> + </if> </if> </messages> </release>
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc index 6df2def..deff54ca 100644 --- a/chrome/browser/about_flags.cc +++ b/chrome/browser/about_flags.cc
@@ -2185,7 +2185,13 @@ #endif // OS_ANDROID {"enable-faster-location-reload", IDS_FLAGS_FASTER_LOCATION_RELOAD_NAME, IDS_FLAGS_FASTER_LOCATION_RELOAD_DESCRIPTION, kOsAll, - FEATURE_VALUE_TYPE(features::kFasterLocationReload)} + FEATURE_VALUE_TYPE(features::kFasterLocationReload)}, +#if defined(OS_ANDROID) + {"lsd-permission-prompt", IDS_FLAGS_LSD_PERMISSION_PROMPT_NAME, + IDS_FLAGS_LSD_PERMISSION_PROMPT_DESCRIPTION, kOsAndroid, + FEATURE_VALUE_TYPE(features::kLsdPermissionPrompt)}, +#endif + // NOTE: Adding new command-line switches requires adding corresponding // entries to enum "LoginCustomFlags" in histograms.xml. See note in
diff --git a/chrome/browser/android/ntp/ntp_snippets_bridge.cc b/chrome/browser/android/ntp/ntp_snippets_bridge.cc index d920bfd..3bc97fa3 100644 --- a/chrome/browser/android/ntp/ntp_snippets_bridge.cc +++ b/chrome/browser/android/ntp/ntp_snippets_bridge.cc
@@ -262,7 +262,7 @@ const JavaParamRef<jstring>& jurl, jint global_position, jint j_category_id, - jint category_position, + jint position_in_category, const JavaParamRef<jstring>& id_within_category) { Category category = Category::FromIDValue(j_category_id); @@ -272,14 +272,14 @@ history_service_->QueryURL( GURL(ConvertJavaStringToUTF8(env, jurl)), /*want_visits=*/false, base::Bind( - [](int global_position, Category category, int category_position, + [](int global_position, Category category, int position_in_category, bool success, const history::URLRow& row, const history::VisitVector& visit_vector) { bool visited = success && row.visit_count() != 0; ntp_snippets::metrics::OnSuggestionDismissed( - global_position, category, category_position, visited); + global_position, category, position_in_category, visited); }, - global_position, category, category_position), + global_position, category, position_in_category), &tracker_); } @@ -325,7 +325,7 @@ const JavaParamRef<jobject>& obj, jint global_position, jint j_category_id, - jint category_position, + jint position_in_category, jlong publish_timestamp_ms, jfloat score) { PrefService* pref_service = ProfileManager::GetLastUsedProfile()->GetPrefs(); @@ -334,8 +334,8 @@ ntp_snippets::prefs::kLastSuccessfulBackgroundFetchTime)); ntp_snippets::metrics::OnSuggestionShown( - global_position, Category::FromIDValue(j_category_id), category_position, - base::Time::FromJavaTime(publish_timestamp_ms), + global_position, Category::FromIDValue(j_category_id), + position_in_category, base::Time::FromJavaTime(publish_timestamp_ms), last_background_fetch_time, score); if (global_position == 0) { content_suggestions_service_->user_classifier()->OnEvent( @@ -348,19 +348,19 @@ jint global_position, jint j_category_id, jint category_index, - jint category_position, + jint position_in_category, jlong publish_timestamp_ms, jfloat score, int windowOpenDisposition) { + const Category category = Category::FromIDValue(j_category_id); ntp_snippets::metrics::OnSuggestionOpened( - global_position, Category::FromIDValue(j_category_id), category_position, + global_position, category, category_index, position_in_category, base::Time::FromJavaTime(publish_timestamp_ms), score, static_cast<WindowOpenDisposition>(windowOpenDisposition)); // TODO(vitaliii): Add ContentSuggestionsService::OnSuggestionOpened and // notify the ranker and the classifier there instead. Do not expose both of // them at all. See crbug.com/674080. - content_suggestions_service_->category_ranker()->OnSuggestionOpened( - Category::FromIDValue(j_category_id)); + content_suggestions_service_->category_ranker()->OnSuggestionOpened(category); content_suggestions_service_->user_classifier()->OnEvent( ntp_snippets::UserClassifier::Metric::SUGGESTIONS_USED); } @@ -369,12 +369,13 @@ const JavaParamRef<jobject>& obj, jint global_position, jint j_category_id, - jint category_position, + jint position_in_category, jlong publish_timestamp_ms, jfloat score) { ntp_snippets::metrics::OnSuggestionMenuOpened( - global_position, Category::FromIDValue(j_category_id), category_position, - base::Time::FromJavaTime(publish_timestamp_ms), score); + global_position, Category::FromIDValue(j_category_id), + position_in_category, base::Time::FromJavaTime(publish_timestamp_ms), + score); } void NTPSnippetsBridge::OnMoreButtonShown(JNIEnv* env,
diff --git a/chrome/browser/android/ntp/ntp_snippets_bridge.h b/chrome/browser/android/ntp/ntp_snippets_bridge.h index 2e3f051..f7af988 100644 --- a/chrome/browser/android/ntp/ntp_snippets_bridge.h +++ b/chrome/browser/android/ntp/ntp_snippets_bridge.h
@@ -76,7 +76,7 @@ const base::android::JavaParamRef<jstring>& jurl, jint global_position, jint j_category_id, - jint category_position, + jint position_in_category, const base::android::JavaParamRef<jstring>& id_within_category); void DismissCategory(JNIEnv* env, @@ -97,7 +97,7 @@ const base::android::JavaParamRef<jobject>& obj, jint global_position, jint j_category_id, - jint category_position, + jint position_in_category, jlong publish_timestamp_ms, jfloat score); @@ -106,7 +106,7 @@ jint global_position, jint j_category_id, jint category_index, - jint category_position, + jint position_in_category, jlong publish_timestamp_ms, jfloat score, int windowOpenDisposition); @@ -115,7 +115,7 @@ const base::android::JavaParamRef<jobject>& obj, jint global_position, jint j_category_id, - jint category_position, + jint position_in_category, jlong publish_timestamp_ms, jfloat score);
diff --git a/chrome/browser/android/webapk/webapk_installer.cc b/chrome/browser/android/webapk/webapk_installer.cc index 1384c59..cf8c3e5 100644 --- a/chrome/browser/android/webapk/webapk_installer.cc +++ b/chrome/browser/android/webapk/webapk_installer.cc
@@ -310,9 +310,9 @@ env, java_ref_, java_file_path); } -bool WebApkInstaller::HasGooglePlayWebApkInstallDelegate() { +bool WebApkInstaller::CanUseGooglePlayInstallService() { JNIEnv* env = base::android::AttachCurrentThread(); - return Java_WebApkInstaller_hasGooglePlayWebApkInstallDelegate( + return Java_WebApkInstaller_canUseGooglePlayInstallService( env, java_ref_); } @@ -423,7 +423,7 @@ return; } - if (HasGooglePlayWebApkInstallDelegate()) { + if (CanUseGooglePlayInstallService()) { int version = 1; base::StringToInt(response->version(), &version); if (!InstallOrUpdateWebApkFromGooglePlay(
diff --git a/chrome/browser/android/webapk/webapk_installer.h b/chrome/browser/android/webapk/webapk_installer.h index a987e04..94b807c 100644 --- a/chrome/browser/android/webapk/webapk_installer.h +++ b/chrome/browser/android/webapk/webapk_installer.h
@@ -124,10 +124,11 @@ JNIEnv* env, const base::android::ScopedJavaLocalRef<jstring>& java_file_path); - // Returns whether the Google Play install delegate is available. + // Returns whether Google Play Services can be used and the install delegate + // is available. // Note: it is possible that this delegate is null even when installing // WebAPKs using Google Play is enabled. - virtual bool HasGooglePlayWebApkInstallDelegate(); + virtual bool CanUseGooglePlayInstallService(); // Called when the package name of the WebAPK is available and the install // or update request is handled by Google Play.
diff --git a/chrome/browser/android/webapk/webapk_installer_unittest.cc b/chrome/browser/android/webapk/webapk_installer_unittest.cc index 84265e7..1fddb62 100644 --- a/chrome/browser/android/webapk/webapk_installer_unittest.cc +++ b/chrome/browser/android/webapk/webapk_installer_unittest.cc
@@ -54,17 +54,17 @@ // WebApkInstaller subclass where // WebApkInstaller::StartInstallingDownloadedWebApk() and // WebApkInstaller::StartUpdateUsingDownloadedWebApk() and -// WebApkInstaller::HasGooglePlayWebApkInstallDelegate() and +// WebApkInstaller::CanUseGooglePlayInstallService() and // WebApkInstaller::InstallOrUpdateWebApkFromGooglePlay() are stubbed out. class TestWebApkInstaller : public WebApkInstaller { public: TestWebApkInstaller(content::BrowserContext* browser_context, const ShortcutInfo& shortcut_info, const SkBitmap& shortcut_icon, - bool has_google_play_webapk_install_delegate) + bool can_use_google_play_install_service) : WebApkInstaller(browser_context, shortcut_info, shortcut_icon), - has_google_play_webapk_install_delegate_( - has_google_play_webapk_install_delegate) {} + can_use_google_play_install_service_( + can_use_google_play_install_service) {} bool StartInstallingDownloadedWebApk( JNIEnv* env, @@ -80,8 +80,8 @@ return true; } - bool HasGooglePlayWebApkInstallDelegate() override { - return has_google_play_webapk_install_delegate_; + bool CanUseGooglePlayInstallService() override { + return can_use_google_play_install_service_; } bool InstallOrUpdateWebApkFromGooglePlay(const std::string& package_name, @@ -98,8 +98,9 @@ } private: - // Whether the Google Play install delegate is available. - bool has_google_play_webapk_install_delegate_; + // Whether the Google Play Services can be used and the install delegate is + // available. + bool can_use_google_play_install_service_; DISALLOW_COPY_AND_ASSIGN(TestWebApkInstaller); }; @@ -111,12 +112,13 @@ const GURL& best_icon_url) : browser_context_(browser_context), best_icon_url_(best_icon_url), - has_google_play_webapk_install_delegate_(false) {} + can_use_google_play_install_service_(false) {} ~WebApkInstallerRunner() {} - void SetHasGooglePlayWebApkInstallDelegate(bool has_delegate) { - has_google_play_webapk_install_delegate_ = has_delegate; + void SetCanUseGooglePlayInstallService( + bool can_use_google_play_install_service) { + can_use_google_play_install_service_ = can_use_google_play_install_service; } void RunInstallWebApk() { @@ -151,7 +153,7 @@ // WebApkInstaller owns itself. WebApkInstaller* installer = new TestWebApkInstaller(browser_context_, info, SkBitmap(), - has_google_play_webapk_install_delegate_); + can_use_google_play_install_service_); installer->SetTimeoutMs(100); return installer; } @@ -181,8 +183,9 @@ // Whether the installation process succeeded. bool success_; - // Whether the Google Play install delegate is available. - bool has_google_play_webapk_install_delegate_; + // Whether Google Play Service can be used and the install delegate is + // available. + bool can_use_google_play_install_service_; DISALLOW_COPY_AND_ASSIGN(WebApkInstallerRunner); }; @@ -427,7 +430,7 @@ // Test installation succeeds using Google Play. TEST_F(WebApkInstallerTest, InstallFromGooglePlaySuccess) { std::unique_ptr<WebApkInstallerRunner> runner = CreateWebApkInstallerRunner(); - runner->SetHasGooglePlayWebApkInstallDelegate(true); + runner->SetCanUseGooglePlayInstallService(true); runner->RunInstallWebApk(); EXPECT_TRUE(runner->success()); }
diff --git a/chrome/browser/chrome_content_utility_manifest_overlay.json b/chrome/browser/chrome_content_utility_manifest_overlay.json index 8dd89f3..eb6541048 100644 --- a/chrome/browser/chrome_content_utility_manifest_overlay.json +++ b/chrome/browser/chrome_content_utility_manifest_overlay.json
@@ -8,6 +8,7 @@ "chrome::mojom::ResourceUsageReporter", "chrome::mojom::ShellHandler", "extensions::mojom::MediaParser", + "extensions::mojom::WiFiCredentialsGetter", "net::interfaces::ProxyResolverFactory", "safe_json::mojom::SafeJsonParser" ]
diff --git a/chrome/browser/extensions/api/networking_private/networking_private_credentials_getter_browsertest.cc b/chrome/browser/extensions/api/networking_private/networking_private_credentials_getter_browsertest.cc new file mode 100644 index 0000000..5e1e31f --- /dev/null +++ b/chrome/browser/extensions/api/networking_private/networking_private_credentials_getter_browsertest.cc
@@ -0,0 +1,79 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "base/bind.h" +#include "base/callback.h" +#include "base/macros.h" +#include "base/run_loop.h" +#include "chrome/browser/extensions/api/networking_private/networking_private_credentials_getter.h" +#include "chrome/common/extensions/wifi_credentials_getter.mojom.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "content/public/browser/browser_thread.h" + +#if !defined(OS_WIN) +#error This test is OS_WIN only. +#endif + +class NetworkingPrivateCredentialsGetterTest : public InProcessBrowserTest { + public: + NetworkingPrivateCredentialsGetterTest() = default; + + void RunTest(bool use_test_network) { + base::RunLoop run_loop; + quit_closure_ = run_loop.QuitClosure(); + + if (use_test_network) + network_ = extensions::mojom::WiFiCredentialsGetter::kWiFiTestNetwork; + + done_called_ = false; + content::BrowserThread::PostBlockingPoolTask( + FROM_HERE, + base::Bind(&NetworkingPrivateCredentialsGetterTest::GetCredentials, + base::Unretained(this))); + run_loop.Run(); + + EXPECT_TRUE(done_called_); + } + + private: + void GetCredentials() { + std::unique_ptr<extensions::NetworkingPrivateCredentialsGetter> getter( + extensions::NetworkingPrivateCredentialsGetter::Create()); + getter->Start( + network_, "public_key", + base::Bind(&NetworkingPrivateCredentialsGetterTest::CredentialsDone, + base::Unretained(this))); + } + + void CredentialsDone(const std::string& key_data, const std::string& error) { + done_called_ = true; + + if (!network_.empty()) { + EXPECT_EQ(network_, key_data); + EXPECT_EQ("", error); + } else { + EXPECT_EQ("", key_data); + EXPECT_FALSE(error.empty()); + } + + content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE, + quit_closure_); + } + + base::Closure quit_closure_; + std::string network_; + bool done_called_; + + DISALLOW_COPY_AND_ASSIGN(NetworkingPrivateCredentialsGetterTest); +}; + +IN_PROC_BROWSER_TEST_F(NetworkingPrivateCredentialsGetterTest, + GetCredentialsSuccess) { + RunTest(true); +} + +IN_PROC_BROWSER_TEST_F(NetworkingPrivateCredentialsGetterTest, + GetCredentialsFailure) { + RunTest(false); +}
diff --git a/chrome/browser/extensions/api/networking_private/networking_private_credentials_getter_win.cc b/chrome/browser/extensions/api/networking_private/networking_private_credentials_getter_win.cc index b5a047d7..824a4a2 100644 --- a/chrome/browser/extensions/api/networking_private/networking_private_credentials_getter_win.cc +++ b/chrome/browser/extensions/api/networking_private/networking_private_credentials_getter_win.cc
@@ -10,113 +10,123 @@ #include "base/bind.h" #include "base/macros.h" #include "base/strings/string_piece.h" -#include "base/strings/stringprintf.h" -#include "base/threading/sequenced_worker_pool.h" -#include "base/threading/thread_task_runner_handle.h" #include "chrome/browser/extensions/api/networking_private/networking_private_crypto.h" -#include "chrome/common/extensions/chrome_utility_extensions_messages.h" +#include "chrome/common/extensions/wifi_credentials_getter.mojom.h" #include "chrome/grit/generated_resources.h" #include "content/public/browser/browser_thread.h" -#include "content/public/browser/utility_process_host.h" -#include "content/public/browser/utility_process_host_client.h" +#include "content/public/browser/utility_process_mojo_client.h" #include "ui/base/l10n/l10n_util.h" -using content::BrowserThread; -using content::UtilityProcessHost; -using content::UtilityProcessHostClient; -using extensions::NetworkingPrivateCredentialsGetter; - namespace { -class CredentialsGetterHostClient : public UtilityProcessHostClient { +using extensions::NetworkingPrivateCredentialsGetter; + +class CredentialsGetterHostClient { public: - explicit CredentialsGetterHostClient(const std::string& public_key); + static CredentialsGetterHostClient* Create(const std::string& public_key) { + return new CredentialsGetterHostClient(public_key); + } - // UtilityProcessHostClient - bool OnMessageReceived(const IPC::Message& message) override; - void OnProcessCrashed(int exit_code) override; - void OnProcessLaunchFailed(int error_code) override; - - // IPC message handlers. - void OnGotCredentials(const std::string& key_data, bool success); - - // Starts the utility process that gets wifi passphrase from system. - void StartProcessOnIOThread( + void GetWiFiCredentialsOnIOThread( const std::string& network_guid, const NetworkingPrivateCredentialsGetter::CredentialsCallback& callback); private: - ~CredentialsGetterHostClient() override; + explicit CredentialsGetterHostClient(const std::string& public_key) + : public_key_(public_key.begin(), public_key.end()) {} - // Public key used to encrypt results + ~CredentialsGetterHostClient() = default; + + // Credentials result handler. + void GetWiFiCredentialsDone(bool success, const std::string& key_data); + + // Report the result to |callback_|. + void ReportResult(bool success, const std::string& key_data); + + // Public key used to encrypt the result. std::vector<uint8_t> public_key_; - // Callback for reporting the result. + // Callback for reporting the encrypted result. NetworkingPrivateCredentialsGetter::CredentialsCallback callback_; + // Utility process used to get the credentials. + std::unique_ptr<content::UtilityProcessMojoClient< + extensions::mojom::WiFiCredentialsGetter>> + utility_process_mojo_client_; + + // WiFi network to get the credentials from. + std::string wifi_network_; + DISALLOW_COPY_AND_ASSIGN(CredentialsGetterHostClient); }; -CredentialsGetterHostClient::CredentialsGetterHostClient( - const std::string& public_key) - : public_key_(public_key.begin(), public_key.end()) { -} - -bool CredentialsGetterHostClient::OnMessageReceived( - const IPC::Message& message) { - bool handled = true; - IPC_BEGIN_MESSAGE_MAP(CredentialsGetterHostClient, message) - IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_GotWiFiCredentials, OnGotCredentials) - IPC_MESSAGE_UNHANDLED(handled = false) - IPC_END_MESSAGE_MAP() - return handled; -} - -void CredentialsGetterHostClient::OnProcessCrashed(int exit_code) { - callback_.Run( - "", base::StringPrintf("Process Crashed with code %08x.", exit_code)); -} - -void CredentialsGetterHostClient::OnProcessLaunchFailed(int error_code) { - callback_.Run("", base::StringPrintf("Process Launch Failed with code %08x.", - error_code)); -} - -void CredentialsGetterHostClient::OnGotCredentials(const std::string& key_data, - bool success) { - if (success) { - std::vector<uint8_t> ciphertext; - if (!networking_private_crypto::EncryptByteString( - public_key_, key_data, &ciphertext)) { - callback_.Run("", "Encrypt Credentials Failed"); - return; - } - - std::string base64_encoded_key_data; - base::Base64Encode( - base::StringPiece(reinterpret_cast<const char*>(ciphertext.data()), - ciphertext.size()), - &base64_encoded_key_data); - callback_.Run(base64_encoded_key_data, ""); - } else { - callback_.Run("", "Get Credentials Failed"); - } -} - -void CredentialsGetterHostClient::StartProcessOnIOThread( +void CredentialsGetterHostClient::GetWiFiCredentialsOnIOThread( const std::string& network_guid, const NetworkingPrivateCredentialsGetter::CredentialsCallback& callback) { - DCHECK_CURRENTLY_ON(BrowserThread::IO); + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + DCHECK(!utility_process_mojo_client_); + DCHECK(!callback.is_null()); + + wifi_network_ = network_guid; callback_ = callback; - UtilityProcessHost* host = - UtilityProcessHost::Create(this, base::ThreadTaskRunnerHandle::Get()); - host->SetName(l10n_util::GetStringUTF16( - IDS_UTILITY_PROCESS_WIFI_CREDENTIALS_GETTER_NAME)); - host->ElevatePrivileges(); - host->Send(new ChromeUtilityHostMsg_GetWiFiCredentials(network_guid)); + + const base::string16 utility_process_name = l10n_util::GetStringUTF16( + IDS_UTILITY_PROCESS_WIFI_CREDENTIALS_GETTER_NAME); + + utility_process_mojo_client_.reset( + new content::UtilityProcessMojoClient< + extensions::mojom::WiFiCredentialsGetter>(utility_process_name)); + utility_process_mojo_client_->set_error_callback( + base::Bind(&CredentialsGetterHostClient::GetWiFiCredentialsDone, + base::Unretained(this), false, std::string())); + + utility_process_mojo_client_->set_run_elevated(); + + utility_process_mojo_client_->Start(); // Start the utility process. + + utility_process_mojo_client_->service()->GetWiFiCredentials( + wifi_network_, + base::Bind(&CredentialsGetterHostClient::GetWiFiCredentialsDone, + base::Unretained(this))); } -CredentialsGetterHostClient::~CredentialsGetterHostClient() { +void CredentialsGetterHostClient::GetWiFiCredentialsDone( + bool success, + const std::string& key_data) { + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + + utility_process_mojo_client_.reset(); // Terminate the utility process. + ReportResult(success, key_data); + delete this; +} + +void CredentialsGetterHostClient::ReportResult(bool success, + const std::string& key_data) { + if (!success) { + callback_.Run(std::string(), "Get Credentials Failed"); + return; + } + + if (wifi_network_ == + extensions::mojom::WiFiCredentialsGetter::kWiFiTestNetwork) { + DCHECK_EQ(wifi_network_, key_data); + callback_.Run(key_data, std::string()); + return; + } + + std::vector<uint8_t> ciphertext; + if (!networking_private_crypto::EncryptByteString(public_key_, key_data, + &ciphertext)) { + callback_.Run(std::string(), "Encrypt Credentials Failed"); + return; + } + + std::string base64_encoded_key_data; + base::Base64Encode( + base::StringPiece(reinterpret_cast<const char*>(ciphertext.data()), + ciphertext.size()), + &base64_encoded_key_data); + callback_.Run(base64_encoded_key_data, std::string()); } } // namespace @@ -126,37 +136,35 @@ class NetworkingPrivateCredentialsGetterWin : public NetworkingPrivateCredentialsGetter { public: - NetworkingPrivateCredentialsGetterWin(); + NetworkingPrivateCredentialsGetterWin() = default; void Start(const std::string& network_guid, const std::string& public_key, - const CredentialsCallback& callback) override; + const CredentialsCallback& callback) override { + content::BrowserThread::PostTask( + content::BrowserThread::IO, FROM_HERE, + base::Bind( + &NetworkingPrivateCredentialsGetterWin::GetCredentialsOnIOThread, + network_guid, public_key, callback)); + } private: - ~NetworkingPrivateCredentialsGetterWin() override; + ~NetworkingPrivateCredentialsGetterWin() override = default; + + static void GetCredentialsOnIOThread(const std::string& network_guid, + const std::string& public_key, + const CredentialsCallback& callback) { + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + + // CredentialsGetterHostClient is self deleting. + CredentialsGetterHostClient* client = + CredentialsGetterHostClient::Create(public_key); + client->GetWiFiCredentialsOnIOThread(network_guid, callback); + } DISALLOW_COPY_AND_ASSIGN(NetworkingPrivateCredentialsGetterWin); }; -NetworkingPrivateCredentialsGetterWin::NetworkingPrivateCredentialsGetterWin() { -} - -void NetworkingPrivateCredentialsGetterWin::Start( - const std::string& network_guid, - const std::string& public_key, - const CredentialsCallback& callback) { - BrowserThread::PostTask( - BrowserThread::IO, - FROM_HERE, - base::Bind(&CredentialsGetterHostClient::StartProcessOnIOThread, - new CredentialsGetterHostClient(public_key), - network_guid, - callback)); -} - -NetworkingPrivateCredentialsGetterWin:: - ~NetworkingPrivateCredentialsGetterWin() {} - NetworkingPrivateCredentialsGetter* NetworkingPrivateCredentialsGetter::Create() { return new NetworkingPrivateCredentialsGetterWin();
diff --git a/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc b/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc index 00dd9e6d..5c94f10 100644 --- a/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc +++ b/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc
@@ -34,9 +34,9 @@ #include "components/ntp_snippets/content_suggestions_service.h" #include "components/ntp_snippets/features.h" #include "components/ntp_snippets/ntp_snippets_constants.h" -#include "components/ntp_snippets/remote/ntp_snippets_fetcher.h" #include "components/ntp_snippets/remote/persistent_scheduler.h" #include "components/ntp_snippets/remote/remote_suggestions_database.h" +#include "components/ntp_snippets/remote/remote_suggestions_fetcher.h" #include "components/ntp_snippets/remote/remote_suggestions_provider_impl.h" #include "components/ntp_snippets/remote/remote_suggestions_status_service.h" #include "components/ntp_snippets/remote/scheduling_remote_suggestions_provider.h" @@ -84,9 +84,9 @@ using ntp_snippets::CategoryRanker; using ntp_snippets::ContentSuggestionsService; using ntp_snippets::ForeignSessionsSuggestionsProvider; -using ntp_snippets::NTPSnippetsFetcher; using ntp_snippets::PersistentScheduler; using ntp_snippets::RemoteSuggestionsDatabase; +using ntp_snippets::RemoteSuggestionsFetcher; using ntp_snippets::RemoteSuggestionsProviderImpl; using ntp_snippets::RemoteSuggestionsStatusService; using ntp_snippets::SchedulingRemoteSuggestionsProvider; @@ -183,7 +183,7 @@ auto provider = base::MakeUnique<RemoteSuggestionsProviderImpl>( service, pref_service, g_browser_process->GetApplicationLocale(), service->category_ranker(), - base::MakeUnique<NTPSnippetsFetcher>( + base::MakeUnique<RemoteSuggestionsFetcher>( signin_manager, token_service, request_context, pref_service, language_model, base::Bind(&safe_json::SafeJsonParser::Parse), is_stable_channel ? google_apis::GetAPIKey()
diff --git a/chrome/browser/resources/settings/privacy_page/privacy_page.html b/chrome/browser/resources/settings/privacy_page/privacy_page.html index f9fe8ce7..c9a7031 100644 --- a/chrome/browser/resources/settings/privacy_page/privacy_page.html +++ b/chrome/browser/resources/settings/privacy_page/privacy_page.html
@@ -67,18 +67,17 @@ label="$i18n{linkDoctorPref}"> </settings-toggle-button> </div> - <div class="settings-box"> + <div class="settings-box" hidden="[[!pageVisibility.searchPrediction]]"> <settings-toggle-button class="start" pref="{{prefs.search.suggest_enabled}}" - label="$i18n{searchSuggestPref}" - hidden="[[!pageVisibility.searchPrediction]]"> + label="$i18n{searchSuggestPref}"> </settings-toggle-button> </div> - <div class="settings-box"> + <div class="settings-box" + hidden="[[!pageVisibility.networkPrediction]]"> <settings-toggle-button class="start" pref="{{prefs.net.network_prediction_options}}" - label="$i18n{networkPredictionEnabled}" - hidden="[[!pageVisibility.networkPrediction]]"> + label="$i18n{networkPredictionEnabled}"> </settings-toggle-button> </div> <div class="settings-box">
diff --git a/chrome/browser/ui/webui/OWNERS b/chrome/browser/ui/webui/OWNERS index 855764c9..3be2defe 100644 --- a/chrome/browser/ui/webui/OWNERS +++ b/chrome/browser/ui/webui/OWNERS
@@ -26,5 +26,7 @@ per-file signin_internals_ui*=achuith@chromium.org +per-file snippets_internals*=file://components/ntp_snippets/OWNERS + # Maintaining ownership from this file's original Chrome OS location. per-file device_log_ui*=stevenjb@chromium.org
diff --git a/chrome/browser/ui/webui/snippets_internals_message_handler.cc b/chrome/browser/ui/webui/snippets_internals_message_handler.cc index d2bca2c..4207a04 100644 --- a/chrome/browser/ui/webui/snippets_internals_message_handler.cc +++ b/chrome/browser/ui/webui/snippets_internals_message_handler.cc
@@ -29,7 +29,7 @@ #include "components/ntp_snippets/category_info.h" #include "components/ntp_snippets/features.h" #include "components/ntp_snippets/pref_names.h" -#include "components/ntp_snippets/remote/ntp_snippets_fetcher.h" +#include "components/ntp_snippets/remote/remote_suggestions_fetcher.h" #include "components/ntp_snippets/remote/remote_suggestions_provider.h" #include "components/ntp_snippets/switches.h" #include "components/offline_pages/core/offline_page_feature.h" @@ -306,9 +306,8 @@ SendLastRemoteSuggestionsBackgroundFetchTime(); if (remote_suggestions_provider_) { - const ntp_snippets::NTPSnippetsFetcher* fetcher = - remote_suggestions_provider_ - ->snippets_fetcher_for_testing_and_debugging(); + const ntp_snippets::RemoteSuggestionsFetcher* fetcher = + remote_suggestions_provider_->suggestions_fetcher_for_debugging(); // TODO(fhorschig): Read this string from variations directly. SendString("switch-personalized", fetcher->PersonalizationModeString()); @@ -386,8 +385,7 @@ if (remote_suggestions_provider_) { const std::string& status = - remote_suggestions_provider_ - ->snippets_fetcher_for_testing_and_debugging() + remote_suggestions_provider_->suggestions_fetcher_for_debugging() ->last_status(); if (!status.empty()) SendString("remote-status", "Finished: " + status);
diff --git a/chrome/common/chrome_features.cc b/chrome/common/chrome_features.cc index 6df346e7..0c2394d 100644 --- a/chrome/common/chrome_features.cc +++ b/chrome/common/chrome_features.cc
@@ -107,6 +107,11 @@ "LinuxObsoleteSystemIsEndOfTheLine", base::FEATURE_DISABLED_BY_DEFAULT}; #endif +// Enables or disables the Location Settings Dialog (LSD). The LSD is an Android +// system-level geolocation permission prompt. +const base::Feature kLsdPermissionPrompt{"LsdPermissionPrompt", + base::FEATURE_DISABLED_BY_DEFAULT}; + // Enables or disables the Material Design version of chrome://bookmarks. const base::Feature kMaterialDesignBookmarks{"MaterialDesignBookmarks", base::FEATURE_DISABLED_BY_DEFAULT};
diff --git a/chrome/common/chrome_features.h b/chrome/common/chrome_features.h index 51d6c0b7..298f6b0 100644 --- a/chrome/common/chrome_features.h +++ b/chrome/common/chrome_features.h
@@ -68,6 +68,8 @@ extern const base::Feature kLinuxObsoleteSystemIsEndOfTheLine; #endif +extern const base::Feature kLsdPermissionPrompt; + extern const base::Feature kMaterialDesignBookmarks; #if BUILDFLAG(ENABLE_EXTENSIONS)
diff --git a/chrome/common/extensions/BUILD.gn b/chrome/common/extensions/BUILD.gn index 763ceb7..2be3aaa 100644 --- a/chrome/common/extensions/BUILD.gn +++ b/chrome/common/extensions/BUILD.gn
@@ -28,6 +28,10 @@ "media_parser.mojom", ] + if (is_win) { + sources += [ "wifi_credentials_getter.mojom" ] + } + public_deps = [ "//mojo/common:common_custom_types", ]
diff --git a/chrome/common/extensions/chrome_utility_extensions_messages.h b/chrome/common/extensions/chrome_utility_extensions_messages.h index 51ceb4bd..2a9f3ab 100644 --- a/chrome/common/extensions/chrome_utility_extensions_messages.h +++ b/chrome/common/extensions/chrome_utility_extensions_messages.h
@@ -168,16 +168,3 @@ // Periodic status update about the progress of an operation. IPC_MESSAGE_CONTROL1(ChromeUtilityHostMsg_ImageWriter_Progress, int64_t /* number of bytes processed */) - -#if defined(OS_WIN) -// Get plain-text WiFi credentials from the system (requires UAC privilege -// elevation). -IPC_MESSAGE_CONTROL1(ChromeUtilityHostMsg_GetWiFiCredentials, - std::string /* ssid */) - -// Reply after getting WiFi credentials from the system. |success| is false if -// error occurred. -IPC_MESSAGE_CONTROL2(ChromeUtilityHostMsg_GotWiFiCredentials, - std::string /* key_data */, - bool /* success */) -#endif // defined(OS_WIN)
diff --git a/chrome/common/extensions/wifi_credentials_getter.mojom b/chrome/common/extensions/wifi_credentials_getter.mojom new file mode 100644 index 0000000..e5328b6 --- /dev/null +++ b/chrome/common/extensions/wifi_credentials_getter.mojom
@@ -0,0 +1,17 @@ +// 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. + +// WiFi credentials interface provided by the utility process and exposed +// by mojo policy control to the chrome browser process on OS_WIN. + +module extensions.mojom; + +interface WiFiCredentialsGetter { + const string kWiFiTestNetwork = "chrome://test-get-wifi-credentials"; + + // OS_WIN: Get plain-text WiFi credentials from the system (requires UAC + // privilege elevation). Returns |success| false if an error occurred or + // |success| true and the result in |key_data|. + GetWiFiCredentials(string ssid) => (bool success, string key_data); +};
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 092dacbce..60093e5 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn
@@ -2527,6 +2527,9 @@ ] } } + if (is_win) { + sources += [ "../browser/extensions/api/networking_private/networking_private_credentials_getter_browsertest.cc" ] + } if (is_mac || is_win) { sources += [ "../browser/extensions/api/networking_private/networking_private_apitest.cc",
diff --git a/chrome/utility/chrome_content_utility_client.cc b/chrome/utility/chrome_content_utility_client.cc index 436ee9e..1788cba 100644 --- a/chrome/utility/chrome_content_utility_client.cc +++ b/chrome/utility/chrome_content_utility_client.cc
@@ -200,8 +200,8 @@ extensions::ExtensionsHandler::ExposeInterfacesToBrowser( registry, utility_client, running_elevated); #endif - // If our process runs with elevated privileges, only add elevated - // Mojo services to the interface registry. + // If our process runs with elevated privileges, only add elevated Mojo + // services to the interface registry. if (running_elevated) return;
diff --git a/chrome/utility/chrome_content_utility_ipc_whitelist.cc b/chrome/utility/chrome_content_utility_ipc_whitelist.cc index 8afb0f6..c9fc8c6 100644 --- a/chrome/utility/chrome_content_utility_ipc_whitelist.cc +++ b/chrome/utility/chrome_content_utility_ipc_whitelist.cc
@@ -14,9 +14,6 @@ #if BUILDFLAG(ENABLE_EXTENSIONS) const uint32_t kMessageWhitelist[] = { -#if defined(OS_WIN) - ChromeUtilityHostMsg_GetWiFiCredentials::ID, -#endif // defined(OS_WIN) ChromeUtilityMsg_ImageWriter_Cancel::ID, ChromeUtilityMsg_ImageWriter_Write::ID, ChromeUtilityMsg_ImageWriter_Verify::ID};
diff --git a/chrome/utility/extensions/extensions_handler.cc b/chrome/utility/extensions/extensions_handler.cc index 9992d42..753279f 100644 --- a/chrome/utility/extensions/extensions_handler.cc +++ b/chrome/utility/extensions/extensions_handler.cc
@@ -32,6 +32,7 @@ #endif #if defined(OS_WIN) +#include "chrome/common/extensions/wifi_credentials_getter.mojom.h" #include "chrome/utility/media_galleries/itunes_pref_parser_win.h" #include "components/wifi/wifi_service.h" #endif // defined(OS_WIN) @@ -95,6 +96,46 @@ DISALLOW_COPY_AND_ASSIGN(MediaParserImpl); }; +#if defined(OS_WIN) +class WiFiCredentialsGetterImpl + : public extensions::mojom::WiFiCredentialsGetter { + public: + WiFiCredentialsGetterImpl() = default; + ~WiFiCredentialsGetterImpl() override = default; + + static void Create(extensions::mojom::WiFiCredentialsGetterRequest request) { + mojo::MakeStrongBinding(base::MakeUnique<WiFiCredentialsGetterImpl>(), + std::move(request)); + } + + private: + // extensions::mojom::WiFiCredentialsGetter: + void GetWiFiCredentials(const std::string& ssid, + const GetWiFiCredentialsCallback& callback) override { + if (ssid == kWiFiTestNetwork) { + callback.Run(true, ssid); // test-mode: return the ssid in key_data. + return; + } + + std::unique_ptr<wifi::WiFiService> wifi_service( + wifi::WiFiService::Create()); + wifi_service->Initialize(nullptr); + + std::string key_data; + std::string error; + wifi_service->GetKeyFromSystem(ssid, &key_data, &error); + + const bool success = error.empty(); + if (!success) + key_data.clear(); + + callback.Run(success, key_data); + } + + DISALLOW_COPY_AND_ASSIGN(WiFiCredentialsGetterImpl); +}; +#endif // defined(OS_WIN) + } // namespace namespace extensions { @@ -117,10 +158,14 @@ service_manager::InterfaceRegistry* registry, ChromeContentUtilityClient* utility_client, bool running_elevated) { - // If our process runs with elevated privileges, only add elevated - // Mojo services to the interface registry. - if (running_elevated) + // If our process runs with elevated privileges, only add elevated Mojo + // services to the interface registry. + if (running_elevated) { +#if defined(OS_WIN) + registry->AddInterface(base::Bind(&WiFiCredentialsGetterImpl::Create)); +#endif return; + } registry->AddInterface(base::Bind(&MediaParserImpl::Create, utility_client)); } @@ -144,11 +189,6 @@ OnIndexPicasaAlbumsContents) #endif // defined(OS_WIN) || defined(OS_MACOSX) -#if defined(OS_WIN) - IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_GetWiFiCredentials, - OnGetWiFiCredentials) -#endif // defined(OS_WIN) - IPC_MESSAGE_UNHANDLED(handled = false) IPC_END_MESSAGE_MAP() @@ -227,17 +267,4 @@ } #endif // defined(OS_WIN) || defined(OS_MACOSX) -#if defined(OS_WIN) -void ExtensionsHandler::OnGetWiFiCredentials(const std::string& network_guid) { - std::unique_ptr<wifi::WiFiService> wifi_service(wifi::WiFiService::Create()); - wifi_service->Initialize(NULL); - - std::string key_data; - std::string error; - wifi_service->GetKeyFromSystem(network_guid, &key_data, &error); - - Send(new ChromeUtilityHostMsg_GotWiFiCredentials(key_data, error.empty())); -} -#endif // defined(OS_WIN) - } // namespace extensions
diff --git a/chrome/utility/extensions/extensions_handler.h b/chrome/utility/extensions/extensions_handler.h index 9b9efa1..4c023464 100644 --- a/chrome/utility/extensions/extensions_handler.h +++ b/chrome/utility/extensions/extensions_handler.h
@@ -67,10 +67,6 @@ const std::vector<picasa::FolderINIContents>& folders_inis); #endif // defined(OS_WIN) || defined(OS_MACOSX) -#if defined(OS_WIN) - void OnGetWiFiCredentials(const std::string& network_guid); -#endif // defined(OS_WIN) - UtilityHandler utility_handler_; DISALLOW_COPY_AND_ASSIGN(ExtensionsHandler);
diff --git a/components/cronet/ios/cronet_environment.mm b/components/cronet/ios/cronet_environment.mm index 03f36dd..eb413786 100644 --- a/components/cronet/ios/cronet_environment.mm +++ b/components/cronet/ios/cronet_environment.mm
@@ -22,6 +22,8 @@ #include "base/path_service.h" #include "base/single_thread_task_runner.h" #include "base/synchronization/waitable_event.h" +#include "base/sys_info.h" +#include "base/task_scheduler/task_scheduler.h" #include "base/threading/worker_pool.h" #include "components/cronet/histogram_manager.h" #include "components/cronet/ios/version.h" @@ -117,6 +119,9 @@ if (!g_at_exit_) g_at_exit_ = new base::AtExitManager; + base::TaskScheduler::CreateAndSetSimpleTaskScheduler( + base::SysInfo::NumberOfProcessors()); + url::Initialize(); base::CommandLine::Init(0, nullptr); @@ -246,6 +251,13 @@ CronetEnvironment::~CronetEnvironment() { // net::HTTPProtocolHandlerDelegate::SetInstance(nullptr); + + // TODO(lilyhoughton) right now this is relying on there being + // only one CronetEnvironment (per process). if (when?) that + // changes, so will this have to. + base::TaskScheduler* ts = base::TaskScheduler::GetInstance(); + if (ts) + ts->Shutdown(); } void CronetEnvironment::InitializeOnNetworkThread() {
diff --git a/components/ntp_snippets/BUILD.gn b/components/ntp_snippets/BUILD.gn index 652704b..c29c9133 100644 --- a/components/ntp_snippets/BUILD.gn +++ b/components/ntp_snippets/BUILD.gn
@@ -49,8 +49,6 @@ "pref_util.h", "remote/ntp_snippet.cc", "remote/ntp_snippet.h", - "remote/ntp_snippets_fetcher.cc", - "remote/ntp_snippets_fetcher.h", "remote/ntp_snippets_json_request.cc", "remote/ntp_snippets_json_request.h", "remote/ntp_snippets_request_params.cc", @@ -58,6 +56,8 @@ "remote/persistent_scheduler.h", "remote/remote_suggestions_database.cc", "remote/remote_suggestions_database.h", + "remote/remote_suggestions_fetcher.cc", + "remote/remote_suggestions_fetcher.h", "remote/remote_suggestions_provider.cc", "remote/remote_suggestions_provider.h", "remote/remote_suggestions_provider_impl.cc", @@ -138,9 +138,9 @@ "offline_pages/recent_tab_suggestions_provider_unittest.cc", "physical_web_pages/physical_web_page_suggestions_provider_unittest.cc", "remote/ntp_snippet_unittest.cc", - "remote/ntp_snippets_fetcher_unittest.cc", "remote/ntp_snippets_json_request_unittest.cc", "remote/remote_suggestions_database_unittest.cc", + "remote/remote_suggestions_fetcher_unittest.cc", "remote/remote_suggestions_provider_impl_unittest.cc", "remote/remote_suggestions_status_service_unittest.cc", "remote/request_throttler_unittest.cc",
diff --git a/components/ntp_snippets/content_suggestions_metrics.cc b/components/ntp_snippets/content_suggestions_metrics.cc index 493f8c0..b6c26ed2 100644 --- a/components/ntp_snippets/content_suggestions_metrics.cc +++ b/components/ntp_snippets/content_suggestions_metrics.cc
@@ -22,6 +22,7 @@ const int kMaxSuggestionsPerCategory = 10; const int kMaxSuggestionsTotal = 50; +const int kMaxCategories = 10; const char kHistogramCountOnNtpOpened[] = "NewTabPage.ContentSuggestions.CountOnNtpOpened"; @@ -31,6 +32,8 @@ "NewTabPage.ContentSuggestions.ShownScoreNormalized"; const char kHistogramOpened[] = "NewTabPage.ContentSuggestions.Opened"; const char kHistogramOpenedAge[] = "NewTabPage.ContentSuggestions.OpenedAge"; +const char kHistogramOpenedCategoryIndex[] = + "NewTabPage.ContentSuggestions.OpenedCategoryIndex"; const char kHistogramOpenedScore[] = "NewTabPage.ContentSuggestions.OpenedScoreNormalized"; const char kHistogramOpenDisposition[] = @@ -216,13 +219,13 @@ void OnSuggestionShown(int global_position, Category category, - int category_position, + int position_in_category, base::Time publish_date, base::Time last_background_fetch_time, float score) { UMA_HISTOGRAM_EXACT_LINEAR(kHistogramShown, global_position, kMaxSuggestionsTotal); - LogCategoryHistogramPosition(kHistogramShown, category, category_position, + LogCategoryHistogramPosition(kHistogramShown, category, position_in_category, kMaxSuggestionsPerCategory); base::TimeDelta age = base::Time::Now() - publish_date; @@ -235,7 +238,7 @@ // When the first of the articles suggestions is shown, then we count this as // a single usage of content suggestions. if (category.IsKnownCategory(KnownCategories::ARTICLES) && - category_position == 0) { + position_in_category == 0) { RecordContentSuggestionsUsage(); // Records the time since the last background fetch of the remote content @@ -250,13 +253,19 @@ void OnSuggestionOpened(int global_position, Category category, - int category_position, + int category_index, + int position_in_category, base::Time publish_date, float score, WindowOpenDisposition disposition) { + UMA_HISTOGRAM_EXACT_LINEAR(kHistogramOpenedCategoryIndex, category_index, + kMaxCategories); + LogCategoryHistogramPosition(kHistogramOpenedCategoryIndex, category, + category_index, kMaxCategories); + UMA_HISTOGRAM_EXACT_LINEAR(kHistogramOpened, global_position, kMaxSuggestionsTotal); - LogCategoryHistogramPosition(kHistogramOpened, category, category_position, + LogCategoryHistogramPosition(kHistogramOpened, category, position_in_category, kMaxSuggestionsPerCategory); base::TimeDelta age = base::Time::Now() - publish_date; @@ -281,13 +290,14 @@ void OnSuggestionMenuOpened(int global_position, Category category, - int category_position, + int position_in_category, base::Time publish_date, float score) { UMA_HISTOGRAM_EXACT_LINEAR(kHistogramMenuOpened, global_position, kMaxSuggestionsTotal); LogCategoryHistogramPosition(kHistogramMenuOpened, category, - category_position, kMaxSuggestionsPerCategory); + position_in_category, + kMaxSuggestionsPerCategory); base::TimeDelta age = base::Time::Now() - publish_date; LogCategoryHistogramAge(kHistogramMenuOpenedAge, category, age); @@ -297,18 +307,20 @@ void OnSuggestionDismissed(int global_position, Category category, - int category_position, + int position_in_category, bool visited) { if (visited) { UMA_HISTOGRAM_EXACT_LINEAR(kHistogramDismissedVisited, global_position, kMaxSuggestionsTotal); LogCategoryHistogramPosition(kHistogramDismissedVisited, category, - category_position, kMaxSuggestionsPerCategory); + position_in_category, + kMaxSuggestionsPerCategory); } else { UMA_HISTOGRAM_EXACT_LINEAR(kHistogramDismissedUnvisited, global_position, kMaxSuggestionsTotal); LogCategoryHistogramPosition(kHistogramDismissedUnvisited, category, - category_position, kMaxSuggestionsPerCategory); + position_in_category, + kMaxSuggestionsPerCategory); } }
diff --git a/components/ntp_snippets/content_suggestions_metrics.h b/components/ntp_snippets/content_suggestions_metrics.h index d03fb42..12e2b46 100644 --- a/components/ntp_snippets/content_suggestions_metrics.h +++ b/components/ntp_snippets/content_suggestions_metrics.h
@@ -21,27 +21,30 @@ // Should only be called once per NTP for each suggestion. void OnSuggestionShown(int global_position, Category category, - int category_position, + int position_in_category, base::Time publish_date, base::Time last_background_fetch_time, float score); +// TODO(crbug.com/682160): Take struct, so that one could not mix up the +// order of arguments. void OnSuggestionOpened(int global_position, Category category, - int category_position, + int category_index, + int position_in_category, base::Time publish_date, float score, WindowOpenDisposition disposition); void OnSuggestionMenuOpened(int global_position, Category category, - int category_position, + int position_in_category, base::Time publish_date, float score); void OnSuggestionDismissed(int global_position, Category category, - int category_position, + int position_in_category, bool visited); void OnSuggestionTargetVisited(Category category, base::TimeDelta visit_time);
diff --git a/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.cc b/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.cc index 147cfdb..e7d1ebf 100644 --- a/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.cc +++ b/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.cc
@@ -37,6 +37,14 @@ const size_t kMaxSuggestionsCount = 10; +std::string GetGroupId(const DictionaryValue& page_dictionary) { + std::string group_id; + if (!page_dictionary.GetString(physical_web::kGroupIdKey, &group_id)) { + LOG(DFATAL) << physical_web::kGroupIdKey << " field is missing."; + } + return group_id; +} + std::string GetPageId(const DictionaryValue& page_dictionary) { std::string raw_resolved_url; if (!page_dictionary.GetString(physical_web::kResolvedUrlKey, @@ -46,6 +54,62 @@ return raw_resolved_url; } +bool CompareByDistance(const DictionaryValue* left, + const DictionaryValue* right) { + double left_distance, right_distance; + bool success = + left->GetDouble(physical_web::kDistanceEstimateKey, &left_distance); + success = + right->GetDouble(physical_web::kDistanceEstimateKey, &right_distance) && + success; + if (!success) { + LOG(DFATAL) << "Distance field is missing."; + } + + // When there is no estimate, the value is <= 0, so we implicitly treat it as + // infinity. + bool is_left_estimated = left_distance > 0; + bool is_right_estimated = right_distance > 0; + + if (is_left_estimated != is_right_estimated) + return is_left_estimated; + return left_distance < right_distance; +} + +void FilterOutByGroupId( + std::vector<const DictionaryValue*>* page_dictionaries) { + // |std::unique| only removes duplicates that immediately follow each other. + // Thus, first, we have to sort by group_id and distance and only then remove + // duplicates. + std::sort(page_dictionaries->begin(), page_dictionaries->end(), + [](const DictionaryValue* left, const DictionaryValue* right) { + std::string left_group_id = GetGroupId(*left); + std::string right_group_id = GetGroupId(*right); + + if (left_group_id != right_group_id) { + return left_group_id < right_group_id; + } + + // We want closest pages first, so in case of same group_id we + // sort by distance. + return CompareByDistance(left, right); + }); + + // Each empty group_id must be treated as unique, so we do not apply + // std::unique to them at all. + auto nonempty_group_id_begin = std::find_if( + page_dictionaries->begin(), page_dictionaries->end(), + [](const DictionaryValue* page) { return !GetGroupId(*page).empty(); }); + + auto new_end = std::unique( + nonempty_group_id_begin, page_dictionaries->end(), + [](const DictionaryValue* left, const DictionaryValue* right) { + return GetGroupId(*left) == GetGroupId(*right); + }); + + page_dictionaries->erase(new_end, page_dictionaries->end()); +} + } // namespace PhysicalWebPageSuggestionsProvider::PhysicalWebPageSuggestionsProvider( @@ -222,19 +286,10 @@ StoreDismissedIDsToPrefs(new_dismissed_ids); } + FilterOutByGroupId(&page_dictionaries); + std::sort(page_dictionaries.begin(), page_dictionaries.end(), - [](const DictionaryValue* left, const DictionaryValue* right) { - double left_distance, right_distance; - bool success = left->GetDouble(physical_web::kDistanceEstimateKey, - &left_distance); - success = right->GetDouble(physical_web::kDistanceEstimateKey, - &right_distance) && - success; - if (!success) { - LOG(DFATAL) << "Distance field is missing."; - } - return left_distance < right_distance; - }); + CompareByDistance); std::vector<ContentSuggestion> suggestions; for (const DictionaryValue* page_dictionary : page_dictionaries) {
diff --git a/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider_unittest.cc b/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider_unittest.cc index 763abe0..633fd97 100644 --- a/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider_unittest.cc +++ b/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider_unittest.cc
@@ -179,6 +179,70 @@ } TEST_F(PhysicalWebPageSuggestionsProviderTest, + ShouldConsiderPagesWithoutDistanceEstimateFurthest) { + IgnoreOnCategoryStatusChangedToAvailable(); + IgnoreOnSuggestionInvalidated(); + // |CreateDummyPhysicalWebPages| builds pages with distances 1, 2 and 3 + // respectively. + std::unique_ptr<base::ListValue> pages = + CreateDummyPhysicalWebPages({3, 2, 1}); + DictionaryValue* second_page; + // Set the second page distance estimate to unknown. + ASSERT_TRUE(pages->GetDictionary(1, &second_page)); + second_page->SetDouble(physical_web::kDistanceEstimateKey, -1); + physical_web_data_source()->SetMetadata(std::move(pages)); + EXPECT_CALL( + *observer(), + OnNewSuggestions(_, provided_category(), + ElementsAre(HasUrl("https://resolved_url.com/3"), + HasUrl("https://resolved_url.com/1"), + HasUrl("https://resolved_url.com/2")))); + CreateProvider(); +} + +TEST_F(PhysicalWebPageSuggestionsProviderTest, + ShouldNotShowSuggestionsWithSameGroupId) { + IgnoreOnCategoryStatusChangedToAvailable(); + IgnoreOnSuggestionInvalidated(); + // |CreateDummyPhysicalWebPages| builds pages with distances 1, 2 + // respectively. + std::unique_ptr<base::ListValue> pages = CreateDummyPhysicalWebPages({2, 1}); + for (int i = 0; i < 2; ++i) { + DictionaryValue* page; + ASSERT_TRUE(pages->GetDictionary(i, &page)); + page->SetString(physical_web::kGroupIdKey, "some_group_id"); + } + + physical_web_data_source()->SetMetadata(std::move(pages)); + // The closest page should be reported. + EXPECT_CALL( + *observer(), + OnNewSuggestions(_, provided_category(), + ElementsAre(HasUrl("https://resolved_url.com/2")))); + CreateProvider(); +} + +TEST_F(PhysicalWebPageSuggestionsProviderTest, + ShouldShowSuggestionsWithEmptyGroupId) { + IgnoreOnCategoryStatusChangedToAvailable(); + IgnoreOnSuggestionInvalidated(); + std::unique_ptr<base::ListValue> pages = CreateDummyPhysicalWebPages({1, 2}); + for (int i = 0; i < 2; ++i) { + DictionaryValue* page; + pages->GetDictionary(i, &page); + page->SetString(physical_web::kGroupIdKey, ""); + } + + physical_web_data_source()->SetMetadata(std::move(pages)); + EXPECT_CALL(*observer(), + OnNewSuggestions( + _, provided_category(), + UnorderedElementsAre(HasUrl("https://resolved_url.com/1"), + HasUrl("https://resolved_url.com/2")))); + CreateProvider(); +} + +TEST_F(PhysicalWebPageSuggestionsProviderTest, FetchMoreShouldFilterAndReturnSortedSuggestions) { IgnoreOnCategoryStatusChangedToAvailable(); IgnoreOnSuggestionInvalidated();
diff --git a/components/ntp_snippets/pref_names.h b/components/ntp_snippets/pref_names.h index 60c05e8..7ff783d 100644 --- a/components/ntp_snippets/pref_names.h +++ b/components/ntp_snippets/pref_names.h
@@ -30,11 +30,12 @@ // there is no WiFi connectivity. extern const char kSnippetPersistentFetchingIntervalFallback[]; -// The pref name for today's count of NTPSnippetsFetcher requests, so far. +// The pref name for today's count of RemoteSuggestionsFetcher requests, so far. extern const char kSnippetFetcherRequestCount[]; -// The pref name for today's count of NTPSnippetsFetcher interactive requests. +// The pref name for today's count of RemoteSuggestionsFetcher interactive +// requests. extern const char kSnippetFetcherInteractiveRequestCount[]; -// The pref name for the current day for the counter of NTPSnippetsFetcher +// The pref name for the current day for the counter of RemoteSuggestionsFetcher // requests. extern const char kSnippetFetcherRequestsDay[];
diff --git a/components/ntp_snippets/remote/ntp_snippets_fetcher.cc b/components/ntp_snippets/remote/remote_suggestions_fetcher.cc similarity index 90% rename from components/ntp_snippets/remote/ntp_snippets_fetcher.cc rename to components/ntp_snippets/remote/remote_suggestions_fetcher.cc index 4969440..44392e7 100644 --- a/components/ntp_snippets/remote/ntp_snippets_fetcher.cc +++ b/components/ntp_snippets/remote/remote_suggestions_fetcher.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 "components/ntp_snippets/remote/ntp_snippets_fetcher.h" +#include "components/ntp_snippets/remote/remote_suggestions_fetcher.h" #include <cstdlib> #include <utility> @@ -181,30 +181,29 @@ // The response from the backend might include suggestions from multiple // categories. If only a single category was requested, this function filters // all other categories out. -void FilterCategories(NTPSnippetsFetcher::FetchedCategoriesVector* categories, - base::Optional<Category> exclusive_category) { +void FilterCategories( + RemoteSuggestionsFetcher::FetchedCategoriesVector* categories, + base::Optional<Category> exclusive_category) { if (!exclusive_category.has_value()) { return; } Category exclusive = exclusive_category.value(); auto category_it = std::find_if( categories->begin(), categories->end(), - [&exclusive](const NTPSnippetsFetcher::FetchedCategory& c) -> bool { + [&exclusive](const RemoteSuggestionsFetcher::FetchedCategory& c) -> bool { return c.category == exclusive; }); if (category_it == categories->end()) { categories->clear(); return; } - NTPSnippetsFetcher::FetchedCategory category = std::move(*category_it); + RemoteSuggestionsFetcher::FetchedCategory category = std::move(*category_it); categories->clear(); categories->push_back(std::move(category)); } } // namespace - - CategoryInfo BuildArticleCategoryInfo( const base::Optional<base::string16>& title) { return CategoryInfo( @@ -238,19 +237,20 @@ l10n_util::GetStringUTF16(IDS_NTP_ARTICLE_SUGGESTIONS_SECTION_EMPTY)); } -NTPSnippetsFetcher::FetchedCategory::FetchedCategory(Category c, - CategoryInfo&& info) +RemoteSuggestionsFetcher::FetchedCategory::FetchedCategory(Category c, + CategoryInfo&& info) : category(c), info(info) {} -NTPSnippetsFetcher::FetchedCategory::FetchedCategory(FetchedCategory&&) = +RemoteSuggestionsFetcher::FetchedCategory::FetchedCategory(FetchedCategory&&) = default; -NTPSnippetsFetcher::FetchedCategory::~FetchedCategory() = default; +RemoteSuggestionsFetcher::FetchedCategory::~FetchedCategory() = default; -NTPSnippetsFetcher::FetchedCategory& NTPSnippetsFetcher::FetchedCategory:: -operator=(FetchedCategory&&) = default; +RemoteSuggestionsFetcher::FetchedCategory& +RemoteSuggestionsFetcher::FetchedCategory::operator=(FetchedCategory&&) = + default; -NTPSnippetsFetcher::NTPSnippetsFetcher( +RemoteSuggestionsFetcher::RemoteSuggestionsFetcher( SigninManagerBase* signin_manager, OAuth2TokenService* token_service, scoped_refptr<URLRequestContextGetter> url_request_context_getter, @@ -300,14 +300,15 @@ } } -NTPSnippetsFetcher::~NTPSnippetsFetcher() { +RemoteSuggestionsFetcher::~RemoteSuggestionsFetcher() { if (waiting_for_refresh_token_) { token_service_->RemoveObserver(this); } } -void NTPSnippetsFetcher::FetchSnippets(const NTPSnippetsRequestParams& params, - SnippetsAvailableCallback callback) { +void RemoteSuggestionsFetcher::FetchSnippets( + const NTPSnippetsRequestParams& params, + SnippetsAvailableCallback callback) { if (!DemandQuotaForRequest(params.interactive_request)) { FetchFinished(OptionalFetchedCategories(), std::move(callback), params.interactive_request @@ -357,7 +358,7 @@ } } -void NTPSnippetsFetcher::FetchSnippetsNonAuthenticated( +void RemoteSuggestionsFetcher::FetchSnippetsNonAuthenticated( NTPSnippetsJsonRequest::Builder builder, SnippetsAvailableCallback callback) { // When not providing OAuth token, we need to pass the Google API key. @@ -367,7 +368,7 @@ StartRequest(std::move(builder), std::move(callback)); } -void NTPSnippetsFetcher::FetchSnippetsAuthenticated( +void RemoteSuggestionsFetcher::FetchSnippetsAuthenticated( NTPSnippetsJsonRequest::Builder builder, SnippetsAvailableCallback callback, const std::string& account_id, @@ -380,16 +381,17 @@ StartRequest(std::move(builder), std::move(callback)); } -void NTPSnippetsFetcher::StartRequest(NTPSnippetsJsonRequest::Builder builder, - SnippetsAvailableCallback callback) { +void RemoteSuggestionsFetcher::StartRequest( + NTPSnippetsJsonRequest::Builder builder, + SnippetsAvailableCallback callback) { std::unique_ptr<NTPSnippetsJsonRequest> request = builder.Build(); NTPSnippetsJsonRequest* raw_request = request.get(); - raw_request->Start(base::BindOnce(&NTPSnippetsFetcher::JsonRequestDone, + raw_request->Start(base::BindOnce(&RemoteSuggestionsFetcher::JsonRequestDone, base::Unretained(this), std::move(request), std::move(callback))); } -void NTPSnippetsFetcher::StartTokenRequest() { +void RemoteSuggestionsFetcher::StartTokenRequest() { OAuth2TokenService::ScopeSet scopes; scopes.insert(fetch_api_ == FetchAPI::CHROME_CONTENT_SUGGESTIONS_API ? kContentSuggestionsApiScope @@ -400,7 +402,7 @@ //////////////////////////////////////////////////////////////////////////////// // OAuth2TokenService::Consumer overrides -void NTPSnippetsFetcher::OnGetTokenSuccess( +void RemoteSuggestionsFetcher::OnGetTokenSuccess( const OAuth2TokenService::Request* request, const std::string& access_token, const base::Time& expiration_time) { @@ -420,7 +422,7 @@ } } -void NTPSnippetsFetcher::OnGetTokenFailure( +void RemoteSuggestionsFetcher::OnGetTokenFailure( const OAuth2TokenService::Request* request, const GoogleServiceAuthError& error) { oauth_request_.reset(); @@ -450,7 +452,7 @@ //////////////////////////////////////////////////////////////////////////////// // OAuth2TokenService::Observer overrides -void NTPSnippetsFetcher::OnRefreshTokenAvailable( +void RemoteSuggestionsFetcher::OnRefreshTokenAvailable( const std::string& account_id) { // Only react on tokens for the account the user has signed in with. if (account_id != signin_manager_->GetAuthenticatedAccountId()) { @@ -463,7 +465,7 @@ StartTokenRequest(); } -void NTPSnippetsFetcher::JsonRequestDone( +void RemoteSuggestionsFetcher::JsonRequestDone( std::unique_ptr<NTPSnippetsJsonRequest> request, SnippetsAvailableCallback callback, std::unique_ptr<base::Value> result, @@ -496,10 +498,11 @@ FetchResult::SUCCESS, std::string()); } -void NTPSnippetsFetcher::FetchFinished(OptionalFetchedCategories categories, - SnippetsAvailableCallback callback, - FetchResult fetch_result, - const std::string& error_details) { +void RemoteSuggestionsFetcher::FetchFinished( + OptionalFetchedCategories categories, + SnippetsAvailableCallback callback, + FetchResult fetch_result, + const std::string& error_details) { DCHECK(fetch_result == FetchResult::SUCCESS || !categories.has_value()); last_status_ = FetchResultToString(fetch_result) + error_details; @@ -514,8 +517,9 @@ std::move(categories)); } -bool NTPSnippetsFetcher::JsonToSnippets(const base::Value& parsed, - FetchedCategoriesVector* categories) { +bool RemoteSuggestionsFetcher::JsonToSnippets( + const base::Value& parsed, + FetchedCategoriesVector* categories) { const base::DictionaryValue* top_dict = nullptr; if (!parsed.GetAsDictionary(&top_dict)) { return false; @@ -587,7 +591,7 @@ return false; } -bool NTPSnippetsFetcher::DemandQuotaForRequest(bool interactive_request) { +bool RemoteSuggestionsFetcher::DemandQuotaForRequest(bool interactive_request) { switch (user_classifier_->GetUserClass()) { case UserClassifier::UserClass::RARE_NTP_USER: return request_throttler_rare_ntp_user_.DemandQuotaForRequest( @@ -603,12 +607,12 @@ return false; } -bool NTPSnippetsFetcher::NeedsAuthentication() const { +bool RemoteSuggestionsFetcher::NeedsAuthentication() const { return (personalization_ == Personalization::kPersonal || personalization_ == Personalization::kBoth); } -std::string NTPSnippetsFetcher::PersonalizationModeString() const { +std::string RemoteSuggestionsFetcher::PersonalizationModeString() const { switch (personalization_) { case Personalization::kPersonal: return "Only personalized";
diff --git a/components/ntp_snippets/remote/ntp_snippets_fetcher.h b/components/ntp_snippets/remote/remote_suggestions_fetcher.h similarity index 90% rename from components/ntp_snippets/remote/ntp_snippets_fetcher.h rename to components/ntp_snippets/remote/remote_suggestions_fetcher.h index 19dce66..44fcd6e 100644 --- a/components/ntp_snippets/remote/ntp_snippets_fetcher.h +++ b/components/ntp_snippets/remote/remote_suggestions_fetcher.h
@@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef COMPONENTS_NTP_SNIPPETS_REMOTE_NTP_SNIPPETS_FETCHER_H_ -#define COMPONENTS_NTP_SNIPPETS_REMOTE_NTP_SNIPPETS_FETCHER_H_ +#ifndef COMPONENTS_NTP_SNIPPETS_REMOTE_REMOTE_SUGGESTIONS_FETCHER_H_ +#define COMPONENTS_NTP_SNIPPETS_REMOTE_REMOTE_SUGGESTIONS_FETCHER_H_ #include <memory> #include <queue> @@ -37,7 +37,7 @@ class UserClassifier; // TODO(tschumann): BuildArticleCategoryInfo() and BuildRemoteCategoryInfo() -// don't really belong into this library. However, as the snippets fetcher is +// don't really belong into this library. However, as the fetcher is // providing this data for server-defined remote sections it's a good starting // point. Candiates to add to such a library would be persisting categories // (have all category managment in one place) or turning parsed JSON into @@ -52,11 +52,12 @@ CategoryInfo BuildRemoteCategoryInfo(const base::string16& title, bool allow_fetching_more_results); -// Fetches snippet data for the NTP from the server. +// Fetches suggestion data for the NTP from the server. // TODO(fhorschig): Untangle cyclic dependencies by introducing a -// NTPSnippetsFetcherInterface. (Would be good for mock implementations, too) -class NTPSnippetsFetcher : public OAuth2TokenService::Consumer, - public OAuth2TokenService::Observer { +// RemoteSuggestionsFetcherInterface. (Would be good for mock implementations, +// too!) +class RemoteSuggestionsFetcher : public OAuth2TokenService::Consumer, + public OAuth2TokenService::Observer { public: struct FetchedCategory { Category category; @@ -78,7 +79,7 @@ base::OnceCallback<void(Status status, OptionalFetchedCategories fetched_categories)>; - NTPSnippetsFetcher( + RemoteSuggestionsFetcher( SigninManagerBase* signin_manager, OAuth2TokenService* token_service, scoped_refptr<net::URLRequestContextGetter> url_request_context_getter, @@ -87,7 +88,7 @@ const ParseJSONCallback& parse_json_callback, const std::string& api_key, const UserClassifier* user_classifier); - ~NTPSnippetsFetcher() override; + ~RemoteSuggestionsFetcher() override; // Initiates a fetch from the server. When done (successfully or not), calls // the callback. @@ -103,9 +104,7 @@ const std::string& last_status() const { return last_status_; } // Returns the last JSON fetched from the server. - const std::string& last_json() const { - return last_fetch_json_; - } + const std::string& last_json() const { return last_fetch_json_; } // Returns the personalization setting of the fetcher as used in tests. // TODO(fhorschig): Reconsider these tests and remove this getter. @@ -171,7 +170,7 @@ const std::string& error_details); bool JsonToSnippets(const base::Value& parsed, - NTPSnippetsFetcher::FetchedCategoriesVector* categories); + FetchedCategoriesVector* categories); bool DemandQuotaForRequest(bool interactive_request); @@ -226,10 +225,11 @@ std::string last_status_; std::string last_fetch_json_; - base::WeakPtrFactory<NTPSnippetsFetcher> weak_ptr_factory_; + base::WeakPtrFactory<RemoteSuggestionsFetcher> weak_ptr_factory_; - DISALLOW_COPY_AND_ASSIGN(NTPSnippetsFetcher); + DISALLOW_COPY_AND_ASSIGN(RemoteSuggestionsFetcher); }; + } // namespace ntp_snippets -#endif // COMPONENTS_NTP_SNIPPETS_REMOTE_NTP_SNIPPETS_FETCHER_H_ +#endif // COMPONENTS_NTP_SNIPPETS_REMOTE_REMOTE_SUGGESTIONS_FETCHER_H_
diff --git a/components/ntp_snippets/remote/ntp_snippets_fetcher_unittest.cc b/components/ntp_snippets/remote/remote_suggestions_fetcher_unittest.cc similarity index 95% rename from components/ntp_snippets/remote/ntp_snippets_fetcher_unittest.cc rename to components/ntp_snippets/remote/remote_suggestions_fetcher_unittest.cc index c989ea81..f5f4aa6 100644 --- a/components/ntp_snippets/remote/ntp_snippets_fetcher_unittest.cc +++ b/components/ntp_snippets/remote/remote_suggestions_fetcher_unittest.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 "components/ntp_snippets/remote/ntp_snippets_fetcher.h" +#include "components/ntp_snippets/remote/remote_suggestions_fetcher.h" #include <deque> #include <map> @@ -83,13 +83,15 @@ } MATCHER(IsEmptyArticleList, "is an empty list of articles") { - NTPSnippetsFetcher::OptionalFetchedCategories& fetched_categories = *arg; + RemoteSuggestionsFetcher::OptionalFetchedCategories& fetched_categories = + *arg; return fetched_categories && fetched_categories->size() == 1 && fetched_categories->begin()->snippets.empty(); } MATCHER_P(IsSingleArticle, url, "is a list with the single article %(url)s") { - NTPSnippetsFetcher::OptionalFetchedCategories& fetched_categories = *arg; + RemoteSuggestionsFetcher::OptionalFetchedCategories& fetched_categories = + *arg; if (!fetched_categories) { *result_listener << "got empty categories."; return false; @@ -136,8 +138,8 @@ if (!arg->has_value() || arg->value().size() == 0) { *result_listener << "No category found."; } - return testing::ExplainMatchResult( - info_matcher, arg->value().front().info, result_listener); + return testing::ExplainMatchResult(info_matcher, arg->value().front().info, + result_listener); } class MockSnippetsAvailableCallback { @@ -145,14 +147,14 @@ // Workaround for gMock's lack of support for movable arguments. void WrappedRun( Status status, - NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories) { + RemoteSuggestionsFetcher::OptionalFetchedCategories fetched_categories) { Run(status, &fetched_categories); } - MOCK_METHOD2( - Run, - void(Status status, - NTPSnippetsFetcher::OptionalFetchedCategories* fetched_categories)); + MOCK_METHOD2(Run, + void(Status status, + RemoteSuggestionsFetcher::OptionalFetchedCategories* + fetched_categories)); }; // TODO(fhorschig): Transfer this class' functionality to call delegates @@ -236,7 +238,9 @@ class FailingFakeURLFetcherFactory : public net::URLFetcherFactory { public: std::unique_ptr<net::URLFetcher> CreateURLFetcher( - int id, const GURL& url, net::URLFetcher::RequestType request_type, + int id, + const GURL& url, + net::URLFetcher::RequestType request_type, net::URLFetcherDelegate* d) override { return base::MakeUnique<net::FakeURLFetcher>( url, d, /*response_data=*/std::string(), net::HTTP_NOT_FOUND, @@ -267,9 +271,9 @@ } // namespace -class NTPSnippetsFetcherTestBase : public testing::Test { +class RemoteSuggestionsFetcherTestBase : public testing::Test { public: - explicit NTPSnippetsFetcherTestBase(const GURL& gurl) + explicit RemoteSuggestionsFetcherTestBase(const GURL& gurl) : default_variation_params_( {{"send_top_languages", "true"}, {"send_user_class", "true"}}), params_manager_(ntp_snippets::kStudyName, @@ -294,7 +298,7 @@ } void ResetSnippetsFetcher() { - snippets_fetcher_ = base::MakeUnique<NTPSnippetsFetcher>( + snippets_fetcher_ = base::MakeUnique<RemoteSuggestionsFetcher>( fake_signin_manager_.get(), fake_token_service_.get(), scoped_refptr<net::TestURLRequestContextGetter>( new net::TestURLRequestContextGetter(mock_task_runner_.get())), @@ -305,13 +309,13 @@ mock_task_runner_->GetMockTickClock()); } - NTPSnippetsFetcher::SnippetsAvailableCallback ToSnippetsAvailableCallback( - MockSnippetsAvailableCallback* callback) { + RemoteSuggestionsFetcher::SnippetsAvailableCallback + ToSnippetsAvailableCallback(MockSnippetsAvailableCallback* callback) { return base::BindOnce(&MockSnippetsAvailableCallback::WrappedRun, base::Unretained(callback)); } - NTPSnippetsFetcher& snippets_fetcher() { return *snippets_fetcher_; } + RemoteSuggestionsFetcher& snippets_fetcher() { return *snippets_fetcher_; } MockSnippetsAvailableCallback& mock_callback() { return mock_callback_; } void FastForwardUntilNoTasksRemain() { mock_task_runner_->FastForwardUntilNoTasksRemain(); @@ -369,20 +373,21 @@ std::unique_ptr<AccountTrackerService> account_tracker_; std::unique_ptr<SigninManagerBase> fake_signin_manager_; std::unique_ptr<OAuth2TokenService> fake_token_service_; - std::unique_ptr<NTPSnippetsFetcher> snippets_fetcher_; + std::unique_ptr<RemoteSuggestionsFetcher> snippets_fetcher_; std::unique_ptr<TestingPrefServiceSimple> pref_service_; std::unique_ptr<UserClassifier> user_classifier_; MockSnippetsAvailableCallback mock_callback_; const GURL test_url_; base::HistogramTester histogram_tester_; - DISALLOW_COPY_AND_ASSIGN(NTPSnippetsFetcherTestBase); + DISALLOW_COPY_AND_ASSIGN(RemoteSuggestionsFetcherTestBase); }; -class ChromeReaderSnippetsFetcherTest : public NTPSnippetsFetcherTestBase { +class ChromeReaderSnippetsFetcherTest + : public RemoteSuggestionsFetcherTestBase { public: ChromeReaderSnippetsFetcherTest() - : NTPSnippetsFetcherTestBase(GURL(kTestChromeReaderUrl)) { + : RemoteSuggestionsFetcherTestBase(GURL(kTestChromeReaderUrl)) { default_variation_params_["content_suggestions_backend"] = kChromeReaderServer; SetVariationParam("content_suggestions_backend", kChromeReaderServer); @@ -391,10 +396,11 @@ }; class NTPSnippetsContentSuggestionsFetcherTest - : public NTPSnippetsFetcherTestBase { + : public RemoteSuggestionsFetcherTestBase { public: NTPSnippetsContentSuggestionsFetcherTest() - : NTPSnippetsFetcherTestBase(GURL(kTestChromeContentSuggestionsUrl)) {} + : RemoteSuggestionsFetcherTestBase( + GURL(kTestChromeContentSuggestionsUrl)) {} }; TEST_F(ChromeReaderSnippetsFetcherTest, ShouldNotFetchOnCreation) { @@ -534,7 +540,7 @@ "}]}"; SetFakeResponse(/*response_data=*/kJsonStr, net::HTTP_OK, net::URLRequestStatus::SUCCESS); - NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories; + RemoteSuggestionsFetcher::OptionalFetchedCategories fetched_categories; EXPECT_CALL(mock_callback(), Run(IsSuccess(), _)) .WillOnce(MoveArgument1PointeeTo(&fetched_categories)); snippets_fetcher().FetchSnippets( @@ -596,7 +602,7 @@ "}]}"; SetFakeResponse(/*response_data=*/kJsonStr, net::HTTP_OK, net::URLRequestStatus::SUCCESS); - NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories; + RemoteSuggestionsFetcher::OptionalFetchedCategories fetched_categories; EXPECT_CALL(mock_callback(), Run(IsSuccess(), _)) .WillOnce(MoveArgument1PointeeTo(&fetched_categories)); snippets_fetcher().FetchSnippets( @@ -660,7 +666,7 @@ "}]}"; SetFakeResponse(/*response_data=*/kJsonStr, net::HTTP_OK, net::URLRequestStatus::SUCCESS); - NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories; + RemoteSuggestionsFetcher::OptionalFetchedCategories fetched_categories; EXPECT_CALL(mock_callback(), Run(IsSuccess(), _)) .WillOnce(MoveArgument1PointeeTo(&fetched_categories)); @@ -911,7 +917,8 @@ ::std::ostream& operator<<( ::std::ostream& os, - const NTPSnippetsFetcher::OptionalFetchedCategories& fetched_categories) { + const RemoteSuggestionsFetcher::OptionalFetchedCategories& + fetched_categories) { if (fetched_categories) { // Matchers above aren't any more precise than this, so this is sufficient // for test-failure diagnostics.
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider.h b/components/ntp_snippets/remote/remote_suggestions_provider.h index 8b818b5..8e07099 100644 --- a/components/ntp_snippets/remote/remote_suggestions_provider.h +++ b/components/ntp_snippets/remote/remote_suggestions_provider.h
@@ -13,7 +13,7 @@ namespace ntp_snippets { -class NTPSnippetsFetcher; +class RemoteSuggestionsFetcher; // Retrieves fresh content data (articles) from the server, stores them and // provides them as content suggestions. @@ -50,7 +50,7 @@ virtual void RefetchInTheBackground( std::unique_ptr<FetchStatusCallback> callback) = 0; - virtual const NTPSnippetsFetcher* snippets_fetcher_for_testing_and_debugging() + virtual const RemoteSuggestionsFetcher* suggestions_fetcher_for_debugging() const = 0; protected:
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc b/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc index 4521227ea..dcdbe065 100644 --- a/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc +++ b/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
@@ -265,7 +265,7 @@ PrefService* pref_service, const std::string& application_language_code, CategoryRanker* category_ranker, - std::unique_ptr<NTPSnippetsFetcher> snippets_fetcher, + std::unique_ptr<RemoteSuggestionsFetcher> suggestions_fetcher, std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher, std::unique_ptr<image_fetcher::ImageDecoder> image_decoder, std::unique_ptr<RemoteSuggestionsDatabase> database, @@ -277,7 +277,7 @@ Category::FromKnownCategory(KnownCategories::ARTICLES)), application_language_code_(application_language_code), category_ranker_(category_ranker), - snippets_fetcher_(std::move(snippets_fetcher)), + suggestions_fetcher_(std::move(suggestions_fetcher)), database_(std::move(database)), image_fetcher_(std::move(image_fetcher), std::move(image_decoder), @@ -350,10 +350,9 @@ FetchSnippets(/*interactive_request=*/false, std::move(callback)); } -const NTPSnippetsFetcher* -RemoteSuggestionsProviderImpl::snippets_fetcher_for_testing_and_debugging() - const { - return snippets_fetcher_.get(); +const RemoteSuggestionsFetcher* +RemoteSuggestionsProviderImpl::suggestions_fetcher_for_debugging() const { + return suggestions_fetcher_.get(); } void RemoteSuggestionsProviderImpl::FetchSnippets( @@ -370,7 +369,7 @@ NTPSnippetsRequestParams params = BuildFetchParams(); params.interactive_request = interactive_request; - snippets_fetcher_->FetchSnippets( + suggestions_fetcher_->FetchSnippets( params, base::BindOnce(&RemoteSuggestionsProviderImpl::OnFetchFinished, base::Unretained(this), std::move(callback), interactive_request)); @@ -392,7 +391,7 @@ params.interactive_request = true; params.exclusive_category = category; - snippets_fetcher_->FetchSnippets( + suggestions_fetcher_->FetchSnippets( params, base::BindOnce(&RemoteSuggestionsProviderImpl::OnFetchMoreFinished, base::Unretained(this), callback)); @@ -615,7 +614,7 @@ void RemoteSuggestionsProviderImpl::OnFetchMoreFinished( const FetchDoneCallback& fetching_callback, Status status, - NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories) { + RemoteSuggestionsFetcher::OptionalFetchedCategories fetched_categories) { if (!fetched_categories) { DCHECK(!status.IsSuccess()); CallWithEmptyResults(fetching_callback, status); @@ -676,7 +675,7 @@ std::unique_ptr<FetchStatusCallback> callback, bool interactive_request, Status status, - NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories) { + RemoteSuggestionsFetcher::OptionalFetchedCategories fetched_categories) { if (!ready()) { // TODO(tschumann): What happens if this was a user-triggered, interactive // request? Is the UI waiting indefinitely now? @@ -704,7 +703,7 @@ if (fetched_categories) { // TODO(treib): Reorder |category_contents_| to match the order we received // from the server. crbug.com/653816 - for (NTPSnippetsFetcher::FetchedCategory& fetched_category : + for (RemoteSuggestionsFetcher::FetchedCategory& fetched_category : *fetched_categories) { // TODO(tschumann): Remove this histogram once we only talk to the content // suggestions cloud backend.
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider_impl.h b/components/ntp_snippets/remote/remote_suggestions_provider_impl.h index 9301dd8..e7d28fe 100644 --- a/components/ntp_snippets/remote/remote_suggestions_provider_impl.h +++ b/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
@@ -25,8 +25,8 @@ #include "components/ntp_snippets/content_suggestion.h" #include "components/ntp_snippets/content_suggestions_provider.h" #include "components/ntp_snippets/remote/ntp_snippet.h" -#include "components/ntp_snippets/remote/ntp_snippets_fetcher.h" #include "components/ntp_snippets/remote/ntp_snippets_request_params.h" +#include "components/ntp_snippets/remote/remote_suggestions_fetcher.h" #include "components/ntp_snippets/remote/remote_suggestions_provider.h" #include "components/ntp_snippets/remote/remote_suggestions_status_service.h" #include "components/ntp_snippets/remote/request_throttler.h" @@ -117,7 +117,7 @@ PrefService* pref_service, const std::string& application_language_code, CategoryRanker* category_ranker, - std::unique_ptr<NTPSnippetsFetcher> snippets_fetcher, + std::unique_ptr<RemoteSuggestionsFetcher> suggestions_fetcher, std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher, std::unique_ptr<image_fetcher::ImageDecoder> image_decoder, std::unique_ptr<RemoteSuggestionsDatabase> database, @@ -144,7 +144,7 @@ // TODO(fhorschig): Remove this getter when there is an interface for the // fetcher that allows better mocks. - const NTPSnippetsFetcher* snippets_fetcher_for_testing_and_debugging() + const RemoteSuggestionsFetcher* suggestions_fetcher_for_debugging() const override; // ContentSuggestionsProvider implementation. @@ -303,18 +303,18 @@ void OnDatabaseLoaded(NTPSnippet::PtrVector snippets); void OnDatabaseError(); - // Callback for fetch-more requests with the NTPSnippetsFetcher. + // Callback for fetch-more requests with the RemoteSuggestionsFetcher. void OnFetchMoreFinished( const FetchDoneCallback& fetching_callback, Status status, - NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories); + RemoteSuggestionsFetcher::OptionalFetchedCategories fetched_categories); - // Callback for regular fetch requests with the NTPSnippetsFetcher. + // Callback for regular fetch requests with the RemoteSuggestionsFetcher. void OnFetchFinished( std::unique_ptr<FetchStatusCallback> callback, bool interactive_request, Status status, - NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories); + RemoteSuggestionsFetcher::OptionalFetchedCategories fetched_categories); // Moves all snippets from |to_archive| into the archive of the |content|. // Clears |to_archive|. As the archive is a FIFO buffer of limited size, this @@ -415,10 +415,10 @@ // Ranker that orders the categories. Not owned. CategoryRanker* category_ranker_; - // The snippets fetcher. - std::unique_ptr<NTPSnippetsFetcher> snippets_fetcher_; + // The suggestions fetcher. + std::unique_ptr<RemoteSuggestionsFetcher> suggestions_fetcher_; - // The database for persisting snippets. + // The database for persisting suggestions. std::unique_ptr<RemoteSuggestionsDatabase> database_; base::TimeTicks database_load_start_;
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc b/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc index af4ccc8..9dc9b61 100644 --- a/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc +++ b/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc
@@ -35,9 +35,9 @@ #include "components/ntp_snippets/ntp_snippets_constants.h" #include "components/ntp_snippets/pref_names.h" #include "components/ntp_snippets/remote/ntp_snippet.h" -#include "components/ntp_snippets/remote/ntp_snippets_fetcher.h" #include "components/ntp_snippets/remote/persistent_scheduler.h" #include "components/ntp_snippets/remote/remote_suggestions_database.h" +#include "components/ntp_snippets/remote/remote_suggestions_fetcher.h" #include "components/ntp_snippets/remote/test_utils.h" #include "components/ntp_snippets/user_classifier.h" #include "components/prefs/testing_pref_service.h" @@ -374,6 +374,7 @@ test_url_(kTestContentSuggestionsServerWithAPIKey), category_ranker_(base::MakeUnique<ConstantCategoryRanker>()), user_classifier_(/*pref_service=*/nullptr), + suggestions_fetcher_(nullptr), image_fetcher_(nullptr), image_decoder_(nullptr), database_(nullptr) { @@ -408,11 +409,11 @@ new net::TestURLRequestContextGetter(task_runner.get()); utils_.ResetSigninManager(); - std::unique_ptr<NTPSnippetsFetcher> snippets_fetcher = - base::MakeUnique<NTPSnippetsFetcher>( - utils_.fake_signin_manager(), fake_token_service_.get(), - std::move(request_context_getter), utils_.pref_service(), nullptr, - base::Bind(&ParseJson), kAPIKey, &user_classifier_); + auto suggestions_fetcher = base::MakeUnique<RemoteSuggestionsFetcher>( + utils_.fake_signin_manager(), fake_token_service_.get(), + std::move(request_context_getter), utils_.pref_service(), nullptr, + base::Bind(&ParseJson), kAPIKey, &user_classifier_); + suggestions_fetcher_ = suggestions_fetcher.get(); utils_.fake_signin_manager()->SignIn("foo@bar.com"); @@ -429,7 +430,7 @@ database_ = database.get(); return base::MakeUnique<RemoteSuggestionsProviderImpl>( observer_.get(), utils_.pref_service(), "fr", category_ranker_.get(), - std::move(snippets_fetcher), std::move(image_fetcher), + std::move(suggestions_fetcher), std::move(image_fetcher), std::move(image_decoder), std::move(database), base::MakeUnique<RemoteSuggestionsStatusService>( utils_.fake_signin_manager(), utils_.pref_service())); @@ -488,6 +489,9 @@ protected: const GURL& test_url() { return test_url_; } FakeContentSuggestionsProviderObserver& observer() { return *observer_; } + RemoteSuggestionsFetcher* suggestions_fetcher() { + return suggestions_fetcher_; + } // TODO(tschumann): Make this a strict-mock. We want to avoid unneccesary // network requests. NiceMock<MockImageFetcher>* image_fetcher() { return image_fetcher_; } @@ -538,6 +542,7 @@ std::unique_ptr<CategoryRanker> category_ranker_; UserClassifier user_classifier_; std::unique_ptr<FakeContentSuggestionsProviderObserver> observer_; + RemoteSuggestionsFetcher* suggestions_fetcher_; NiceMock<MockImageFetcher>* image_fetcher_; FakeImageDecoder* image_decoder_; @@ -1080,10 +1085,10 @@ IdEq("http://id-10"))); } -// TODO(tschumann): We don't have test making sure the NTPSnippetsFetcher +// TODO(tschumann): We don't have test making sure the RemoteSuggestionsFetcher // actually gets the proper parameters. Add tests with an injected -// NTPSnippetsFetcher to verify the parameters, including proper handling of -// dismissed and known_ids. +// RemoteSuggestionsFetcher to verify the parameters, including proper handling +// of dismissed and known_ids. namespace { @@ -1115,9 +1120,8 @@ "invalid json string}]}", /*known_ids=*/std::set<std::string>(), base::Bind(&SuggestionsLoaded, &loaded)); - EXPECT_THAT( - service->snippets_fetcher_for_testing_and_debugging()->last_status(), - StartsWith("Received invalid JSON")); + EXPECT_THAT(suggestions_fetcher()->last_status(), + StartsWith("Received invalid JSON")); } TEST_F(RemoteSuggestionsProviderImplTest, @@ -1130,9 +1134,8 @@ GetTestJson({GetIncompleteSnippet()}), /*known_ids=*/std::set<std::string>(), base::Bind(&SuggestionsLoaded, &loaded)); - EXPECT_THAT( - service->snippets_fetcher_for_testing_and_debugging()->last_status(), - StartsWith("Invalid / empty list")); + EXPECT_THAT(suggestions_fetcher()->last_status(), + StartsWith("Invalid / empty list")); } TEST_F(RemoteSuggestionsProviderImplTest, @@ -1164,9 +1167,8 @@ auto service = MakeSnippetsService(); LoadFromJSONString(service.get(), GetTestJson({GetInvalidSnippet()})); - EXPECT_THAT( - service->snippets_fetcher_for_testing_and_debugging()->last_status(), - StartsWith("Received invalid JSON")); + EXPECT_THAT(suggestions_fetcher()->last_status(), + StartsWith("Received invalid JSON")); EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), IsEmpty()); } @@ -1175,14 +1177,11 @@ LoadFromJSONString(service.get(), GetTestJson({GetSnippet()})); ASSERT_THAT(service->GetSnippetsForTesting(articles_category()), SizeIs(1)); - ASSERT_EQ( - "OK", - service->snippets_fetcher_for_testing_and_debugging()->last_status()); + ASSERT_EQ("OK", suggestions_fetcher()->last_status()); LoadFromJSONString(service.get(), GetTestJson({GetInvalidSnippet()})); - EXPECT_THAT( - service->snippets_fetcher_for_testing_and_debugging()->last_status(), - StartsWith("Received invalid JSON")); + EXPECT_THAT(suggestions_fetcher()->last_status(), + StartsWith("Received invalid JSON")); // This should not have changed the existing snippets. EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), SizeIs(1)); } @@ -1191,9 +1190,7 @@ auto service = MakeSnippetsService(); LoadFromJSONString(service.get(), GetTestJson({GetIncompleteSnippet()})); - EXPECT_EQ( - "Invalid / empty list.", - service->snippets_fetcher_for_testing_and_debugging()->last_status()); + EXPECT_EQ("Invalid / empty list.", suggestions_fetcher()->last_status()); EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), IsEmpty()); } @@ -1205,9 +1202,7 @@ ASSERT_THAT(service->GetSnippetsForTesting(articles_category()), SizeIs(1)); LoadFromJSONString(service.get(), GetTestJson({GetIncompleteSnippet()})); - EXPECT_EQ( - "Invalid / empty list.", - service->snippets_fetcher_for_testing_and_debugging()->last_status()); + EXPECT_EQ("Invalid / empty list.", suggestions_fetcher()->last_status()); // This should not have changed the existing snippets. EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), SizeIs(1)); }
diff --git a/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.cc b/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.cc index a746e7b0..0b79eac 100644 --- a/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.cc +++ b/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.cc
@@ -249,9 +249,9 @@ std::move(wrapper_callback))); } -const NTPSnippetsFetcher* SchedulingRemoteSuggestionsProvider:: - snippets_fetcher_for_testing_and_debugging() const { - return provider_->snippets_fetcher_for_testing_and_debugging(); +const RemoteSuggestionsFetcher* +SchedulingRemoteSuggestionsProvider::suggestions_fetcher_for_debugging() const { + return provider_->suggestions_fetcher_for_debugging(); } CategoryStatus SchedulingRemoteSuggestionsProvider::GetCategoryStatus(
diff --git a/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.h b/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.h index 053c9c5..7a42b47 100644 --- a/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.h +++ b/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.h
@@ -79,7 +79,7 @@ std::unique_ptr<ProviderStatusCallback> callback) override; void RefetchInTheBackground( std::unique_ptr<FetchStatusCallback> callback) override; - const NTPSnippetsFetcher* snippets_fetcher_for_testing_and_debugging() + const RemoteSuggestionsFetcher* suggestions_fetcher_for_debugging() const override; // ContentSuggestionsProvider implementation.
diff --git a/components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc b/components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc index a65c61ef..950923f 100644 --- a/components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc +++ b/components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc
@@ -51,7 +51,7 @@ namespace ntp_snippets { -class NTPSnippetsFetcher; +class RemoteSuggestionsFetcher; namespace { @@ -91,8 +91,8 @@ MOCK_METHOD1(RefetchInTheBackground, void(RemoteSuggestionsProvider::FetchStatusCallback)); - MOCK_CONST_METHOD0(snippets_fetcher_for_testing_and_debugging, - const NTPSnippetsFetcher*()); + MOCK_CONST_METHOD0(suggestions_fetcher_for_debugging, + const RemoteSuggestionsFetcher*()); MOCK_METHOD1(GetCategoryStatus, CategoryStatus(Category)); MOCK_METHOD1(GetCategoryInfo, CategoryInfo(Category));
diff --git a/components/password_manager/core/browser/password_manager.cc b/components/password_manager/core/browser/password_manager.cc index 574e49a..a2bbaf12 100644 --- a/components/password_manager/core/browser/password_manager.cc +++ b/components/password_manager/core/browser/password_manager.cc
@@ -382,6 +382,10 @@ all_visible_forms_.clear(); } +bool PasswordManager::IsPasswordFieldDetectedOnPage() { + return !pending_login_managers_.empty(); +} + void PasswordManager::RecordFailure(ProvisionalSaveFailure failure, const GURL& form_origin, BrowserSavePasswordProgressLogger* logger) {
diff --git a/components/password_manager/core/browser/password_manager.h b/components/password_manager/core/browser/password_manager.h index ccc1d89..6cb79e1 100644 --- a/components/password_manager/core/browser/password_manager.h +++ b/components/password_manager/core/browser/password_manager.h
@@ -172,6 +172,9 @@ // visible forms. void DropFormManagers(); + // Returns true if password element is detected on the current page. + bool IsPasswordFieldDetectedOnPage(); + PasswordManagerClient* client() { return client_; } #if defined(UNIT_TEST)
diff --git a/components/password_manager/core/browser/password_manager_metrics_util.cc b/components/password_manager/core/browser/password_manager_metrics_util.cc index 8bc566bc..dc57abb 100644 --- a/components/password_manager/core/browser/password_manager_metrics_util.cc +++ b/components/password_manager/core/browser/password_manager_metrics_util.cc
@@ -131,13 +131,18 @@ void LogPasswordReuse(int password_length, int saved_passwords, - int number_matches) { + int number_matches, + bool password_field_detected) { UMA_HISTOGRAM_COUNTS_100("PasswordManager.PasswordReuse.PasswordLength", password_length); UMA_HISTOGRAM_COUNTS_1000("PasswordManager.PasswordReuse.TotalPasswords", saved_passwords); UMA_HISTOGRAM_COUNTS_1000("PasswordManager.PasswordReuse.NumberOfMatches", number_matches); + UMA_HISTOGRAM_ENUMERATION( + "PasswordManager.PasswordReuse.PasswordFieldDetected", + password_field_detected ? HAS_PASSWORD_FIELD : NO_PASSWORD_FIELD, + PASSWORD_REUSE_PASSWORD_FIELD_DETECTED_COUNT); } } // namespace metrics_util
diff --git a/components/password_manager/core/browser/password_manager_metrics_util.h b/components/password_manager/core/browser/password_manager_metrics_util.h index c2377f1..fbc97bc 100644 --- a/components/password_manager/core/browser/password_manager_metrics_util.h +++ b/components/password_manager/core/browser/password_manager_metrics_util.h
@@ -172,6 +172,12 @@ CREDENTIAL_MANAGER_GET_UNMEDIATED }; +enum PasswordReusePasswordFieldDetected { + NO_PASSWORD_FIELD, + HAS_PASSWORD_FIELD, + PASSWORD_REUSE_PASSWORD_FIELD_DETECTED_COUNT +}; + // A version of the UMA_HISTOGRAM_BOOLEAN macro that allows the |name| // to vary over the program's runtime. void LogUMAHistogramBoolean(const std::string& name, bool sample); @@ -230,7 +236,8 @@ // Log the password reuse. void LogPasswordReuse(int password_length, int saved_passwords, - int number_matches); + int number_matches, + bool password_field_detected); } // namespace metrics_util
diff --git a/components/password_manager/core/browser/password_manager_unittest.cc b/components/password_manager/core/browser/password_manager_unittest.cc index 5ce7d40..da9feef 100644 --- a/components/password_manager/core/browser/password_manager_unittest.cc +++ b/components/password_manager/core/browser/password_manager_unittest.cc
@@ -391,7 +391,9 @@ observed.push_back(form); EXPECT_CALL(*store_, GetLogins(_, _)) .WillRepeatedly(WithArg<1>(InvokeEmptyConsumerWithForms())); + EXPECT_FALSE(manager()->IsPasswordFieldDetectedOnPage()); manager()->OnPasswordFormsParsed(&driver_, observed); + EXPECT_TRUE(manager()->IsPasswordFieldDetectedOnPage()); manager()->OnPasswordFormsRendered(&driver_, observed, true); EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage()) @@ -407,6 +409,7 @@ observed.clear(); manager()->OnPasswordFormsParsed(&driver_, observed); manager()->OnPasswordFormsRendered(&driver_, observed, true); + EXPECT_FALSE(manager()->IsPasswordFieldDetectedOnPage()); // Simulate saving the form, as if the info bar was accepted. EXPECT_CALL(*store_, AddLogin(FormMatches(form)));
diff --git a/components/password_manager/core/browser/password_reuse_detection_manager.cc b/components/password_manager/core/browser/password_reuse_detection_manager.cc index a4bab65..24a7c4f 100644 --- a/components/password_manager/core/browser/password_reuse_detection_manager.cc +++ b/components/password_manager/core/browser/password_reuse_detection_manager.cc
@@ -5,6 +5,7 @@ #include "components/password_manager/core/browser/password_reuse_detection_manager.h" #include "components/password_manager/core/browser/browser_save_password_progress_logger.h" +#include "components/password_manager/core/browser/password_manager.h" #include "components/password_manager/core/browser/password_manager_client.h" #include "components/password_manager/core/browser/password_manager_metrics_util.h" #include "components/password_manager/core/browser/password_manager_util.h" @@ -55,8 +56,10 @@ logger->LogString(BrowserSavePasswordProgressLogger::STRING_REUSE_FOUND, saved_domain); } - metrics_util::LogPasswordReuse(password.size(), saved_passwords, - number_matches); + + metrics_util::LogPasswordReuse( + password.size(), saved_passwords, number_matches, + client_->GetPasswordManager()->IsPasswordFieldDetectedOnPage()); } } // namespace password_manager
diff --git a/components/physical_web/data_source/fake_physical_web_data_source.cc b/components/physical_web/data_source/fake_physical_web_data_source.cc index d5ebbb11..70fea37 100644 --- a/components/physical_web/data_source/fake_physical_web_data_source.cc +++ b/components/physical_web/data_source/fake_physical_web_data_source.cc
@@ -17,6 +17,7 @@ std::unique_ptr<DictionaryValue> CreatePhysicalWebPage( const std::string& resolved_url, double distance_estimate, + const std::string& group_id, int scan_timestamp, const std::string& title, const std::string& description, @@ -24,6 +25,7 @@ auto page = base::MakeUnique<DictionaryValue>(); page->SetString(kScannedUrlKey, scanned_url); page->SetDouble(kDistanceEstimateKey, distance_estimate); + page->SetString(kGroupIdKey, group_id); // TODO(crbug.com/667722): Remove this integer workaround once timestamp is // fixed. page->SetInteger(kScanTimestampKey, scan_timestamp); @@ -38,8 +40,8 @@ int timestamp) { const std::string id_string = base::IntToString(id); return CreatePhysicalWebPage("https://resolved_url.com/" + id_string, - distance, timestamp, "title " + id_string, - "description " + id_string, + distance, /*group_id=*/std::string(), timestamp, + "title " + id_string, "description " + id_string, "https://scanned_url.com/" + id_string); }
diff --git a/components/physical_web/data_source/fake_physical_web_data_source.h b/components/physical_web/data_source/fake_physical_web_data_source.h index 6ccd1de6..c10543e3 100644 --- a/components/physical_web/data_source/fake_physical_web_data_source.h +++ b/components/physical_web/data_source/fake_physical_web_data_source.h
@@ -23,6 +23,7 @@ std::unique_ptr<base::DictionaryValue> CreatePhysicalWebPage( const std::string& resolved_url, double distance_estimate, + const std::string& group_id, int scan_timestamp, const std::string& title, const std::string& description,
diff --git a/content/browser/utility_process_mojo_client_browsertest.cc b/content/browser/utility_process_mojo_client_browsertest.cc index e3f031a6..6ac13b5 100644 --- a/content/browser/utility_process_mojo_client_browsertest.cc +++ b/content/browser/utility_process_mojo_client_browsertest.cc
@@ -20,7 +20,7 @@ // Test fixture used to make different Mojo calls to the utility process. class UtilityProcessMojoClientBrowserTest : public ContentBrowserTest { public: - void StartMojoService(bool disable_sandbox) { + void StartMojoService(bool disable_sandbox, bool run_elevated = false) { mojo_client_.reset(new UtilityProcessMojoClient<mojom::TestService>( base::ASCIIToUTF16("TestMojoProcess"))); @@ -31,6 +31,14 @@ // This test case needs to have the sandbox disabled. if (disable_sandbox) mojo_client_->set_disable_sandbox(); +#if defined(OS_WIN) + // This test case needs utility process privilege elevation. + if (run_elevated) { + CHECK(disable_sandbox); + mojo_client_->set_run_elevated(); + } +#endif // defined(OS_WIN) + mojo_client_->Start(); } @@ -136,6 +144,28 @@ EXPECT_TRUE(sandbox_succeeded_); EXPECT_FALSE(error_happened_); } -#endif +#endif // !defined(OS_ANDROID) + +#if defined(OS_WIN) +// Call a function that succeeds with process elevation. Elevation is only +// available on WIN, and is used to permit UAC prompts for operations that +// need user approval before proceeding. +IN_PROC_BROWSER_TEST_F(UtilityProcessMojoClientBrowserTest, ElevatedSuccess) { + base::RunLoop run_loop; + done_closure_ = run_loop.QuitClosure(); + + bool elevated_utility_process = true; + StartMojoService(true, elevated_utility_process); + + mojo_client_->service()->CreateFolder( + base::Bind(&UtilityProcessMojoClientBrowserTest::OnCreateFolderFinished, + base::Unretained(this))); + + run_loop.Run(); + EXPECT_TRUE(response_received_); + EXPECT_TRUE(sandbox_succeeded_); + EXPECT_FALSE(error_happened_); +} +#endif // defined(OS_WIN) } // namespace content
diff --git a/content/child/runtime_features.cc b/content/child/runtime_features.cc index eca23b3f5..1fcfb01 100644 --- a/content/child/runtime_features.cc +++ b/content/child/runtime_features.cc
@@ -44,6 +44,7 @@ WebRuntimeFeatures::enableAutoplayMutedVideos(true); // Android does not yet support SystemMonitor. WebRuntimeFeatures::enableOnDeviceChange(false); + WebRuntimeFeatures::enableMediaSession(true); #else // defined(OS_ANDROID) WebRuntimeFeatures::enableNavigatorContentUtils(true); if (base::FeatureList::IsEnabled(
diff --git a/content/public/browser/utility_process_mojo_client.h b/content/public/browser/utility_process_mojo_client.h index 29e76fa..cfcb93b 100644 --- a/content/public/browser/utility_process_mojo_client.h +++ b/content/public/browser/utility_process_mojo_client.h
@@ -48,6 +48,14 @@ helper_->set_disable_sandbox(); } +#if defined(OS_WIN) + // Allow the utility process to run with elevated privileges. + void set_run_elevated() { + DCHECK(!start_called_); + helper_->set_run_elevated(); + } +#endif // defined(OS_WIN) + // Starts the utility process and connect to the remote Mojo service. void Start() { DCHECK(thread_checker_.CalledOnValidThread()); @@ -96,15 +104,30 @@ void set_disable_sandbox() { disable_sandbox_ = true; } +#if defined(OS_WIN) + void set_run_elevated() { + disable_sandbox_ = true; + run_elevated_ = true; + } +#endif // defined(OS_WIN) + private: // Starts the utility process and connects to the remote Mojo service. void StartOnIOThread(const std::string& mojo_interface_name, mojo::ScopedMessagePipeHandle interface_pipe) { DCHECK_CURRENTLY_ON(BrowserThread::IO); + utility_host_ = UtilityProcessHost::Create(nullptr, nullptr)->AsWeakPtr(); utility_host_->SetName(process_name_); + if (disable_sandbox_) utility_host_->DisableSandbox(); +#if defined(OS_WIN) + if (run_elevated_) { + DCHECK(disable_sandbox_); + utility_host_->ElevatePrivileges(); + } +#endif // defined(OS_WIN) utility_host_->Start(); @@ -115,6 +138,9 @@ // Properties of the utility process. base::string16 process_name_; bool disable_sandbox_ = false; +#if defined(OS_WIN) + bool run_elevated_ = false; +#endif // defined(OS_WIN) // Must only be accessed on the IO thread. base::WeakPtr<UtilityProcessHost> utility_host_;
diff --git a/content/public/renderer/render_thread.h b/content/public/renderer/render_thread.h index 86e7e9f..3e1759c 100644 --- a/content/public/renderer/render_thread.h +++ b/content/public/renderer/render_thread.h
@@ -106,6 +106,11 @@ // Retrieve the process ID of the browser process. virtual int32_t GetClientId() = 0; + + // Handles for posting tasks to appropriate renderer scheduler task queues. + virtual scoped_refptr<base::SingleThreadTaskRunner> GetTimerTaskRunner() = 0; + virtual scoped_refptr<base::SingleThreadTaskRunner> + GetLoadingTaskRunner() = 0; }; } // namespace content
diff --git a/content/public/test/mock_render_thread.cc b/content/public/test/mock_render_thread.cc index 4c9afbb..2ab1d2a 100644 --- a/content/public/test/mock_render_thread.cc +++ b/content/public/test/mock_render_thread.cc
@@ -249,6 +249,16 @@ return 1; } +scoped_refptr<base::SingleThreadTaskRunner> +MockRenderThread::GetTimerTaskRunner() { + return base::ThreadTaskRunnerHandle::Get(); +} + +scoped_refptr<base::SingleThreadTaskRunner> +MockRenderThread::GetLoadingTaskRunner() { + return base::ThreadTaskRunnerHandle::Get(); +} + #if defined(OS_WIN) void MockRenderThread::PreCacheFont(const LOGFONT& log_font) { }
diff --git a/content/public/test/mock_render_thread.h b/content/public/test/mock_render_thread.h index 63f492c24..67e625d 100644 --- a/content/public/test/mock_render_thread.h +++ b/content/public/test/mock_render_thread.h
@@ -81,6 +81,8 @@ bool ResolveProxy(const GURL& url, std::string* proxy_list) override; base::WaitableEvent* GetShutdownEvent() override; int32_t GetClientId() override; + scoped_refptr<base::SingleThreadTaskRunner> GetTimerTaskRunner() override; + scoped_refptr<base::SingleThreadTaskRunner> GetLoadingTaskRunner() override; #if defined(OS_WIN) void PreCacheFont(const LOGFONT& log_font) override; void ReleaseCachedFonts() override;
diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc index a041df66..9966cc5 100644 --- a/content/renderer/render_thread_impl.cc +++ b/content/renderer/render_thread_impl.cc
@@ -1470,6 +1470,16 @@ return client_id_; } +scoped_refptr<base::SingleThreadTaskRunner> +RenderThreadImpl::GetTimerTaskRunner() { + return renderer_scheduler_->TimerTaskRunner(); +} + +scoped_refptr<base::SingleThreadTaskRunner> +RenderThreadImpl::GetLoadingTaskRunner() { + return renderer_scheduler_->LoadingTaskRunner(); +} + void RenderThreadImpl::OnAssociatedInterfaceRequest( const std::string& name, mojo::ScopedInterfaceEndpointHandle handle) {
diff --git a/content/renderer/render_thread_impl.h b/content/renderer/render_thread_impl.h index 2f00381..9c0cfe3 100644 --- a/content/renderer/render_thread_impl.h +++ b/content/renderer/render_thread_impl.h
@@ -209,6 +209,8 @@ bool ResolveProxy(const GURL& url, std::string* proxy_list) override; base::WaitableEvent* GetShutdownEvent() override; int32_t GetClientId() override; + scoped_refptr<base::SingleThreadTaskRunner> GetTimerTaskRunner() override; + scoped_refptr<base::SingleThreadTaskRunner> GetLoadingTaskRunner() override; // IPC::Listener implementation via ChildThreadImpl: void OnAssociatedInterfaceRequest(
diff --git a/ios/chrome/app/main_controller.mm b/ios/chrome/app/main_controller.mm index 21abdeef5..291afaf 100644 --- a/ios/chrome/app/main_controller.mm +++ b/ios/chrome/app/main_controller.mm
@@ -727,6 +727,8 @@ if (switchFromIncognito) startInIncognito = NO; + [MDCTypography setFontLoader:[MDFRobotoFontLoader sharedInstance]]; + [self createInitialUI:(startInIncognito ? ApplicationMode::INCOGNITO : ApplicationMode::NORMAL)]; @@ -738,8 +740,6 @@ [_browserViewWrangler updateDeviceSharingManager]; - [MDCTypography setFontLoader:[MDFRobotoFontLoader sharedInstance]]; - [self openTabFromLaunchOptions:_launchOptions startupInformation:self appState:self.appState];
diff --git a/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc b/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc index 338e71d..fe6d78da 100644 --- a/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc +++ b/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc
@@ -25,9 +25,9 @@ #include "components/ntp_snippets/content_suggestions_service.h" #include "components/ntp_snippets/features.h" #include "components/ntp_snippets/ntp_snippets_constants.h" -#include "components/ntp_snippets/remote/ntp_snippets_fetcher.h" #include "components/ntp_snippets/remote/persistent_scheduler.h" #include "components/ntp_snippets/remote/remote_suggestions_database.h" +#include "components/ntp_snippets/remote/remote_suggestions_fetcher.h" #include "components/ntp_snippets/remote/remote_suggestions_provider_impl.h" #include "components/ntp_snippets/remote/remote_suggestions_status_service.h" #include "components/ntp_snippets/remote/scheduling_remote_suggestions_provider.h" @@ -52,9 +52,9 @@ using ios::BookmarkModelFactory; using ntp_snippets::BookmarkSuggestionsProvider; using ntp_snippets::ContentSuggestionsService; -using ntp_snippets::NTPSnippetsFetcher; using ntp_snippets::PersistentScheduler; using ntp_snippets::RemoteSuggestionsDatabase; +using ntp_snippets::RemoteSuggestionsFetcher; using ntp_snippets::RemoteSuggestionsProviderImpl; using ntp_snippets::RemoteSuggestionsStatusService; using ntp_snippets::SchedulingRemoteSuggestionsProvider; @@ -163,7 +163,7 @@ auto provider = base::MakeUnique<RemoteSuggestionsProviderImpl>( service.get(), prefs, GetApplicationContext()->GetApplicationLocale(), service->category_ranker(), - base::MakeUnique<NTPSnippetsFetcher>( + base::MakeUnique<RemoteSuggestionsFetcher>( signin_manager, token_service, request_context, prefs, nullptr, base::Bind(&ParseJson), GetChannel() == version_info::Channel::STABLE
diff --git a/ios/chrome/browser/payments/payment_request_coordinator.mm b/ios/chrome/browser/payments/payment_request_coordinator.mm index 5f03a15..11e16da 100644 --- a/ios/chrome/browser/payments/payment_request_coordinator.mm +++ b/ios/chrome/browser/payments/payment_request_coordinator.mm
@@ -165,33 +165,35 @@ if (!_selectedPaymentMethod->billing_address_id().empty()) { autofill::AutofillProfile* address = _personalDataManager->GetProfileByGUID( _selectedPaymentMethod->billing_address_id()); - paymentResponse.details.billing_address.country = - address->GetRawInfo(autofill::ADDRESS_HOME_COUNTRY); - paymentResponse.details.billing_address.address_line.push_back( - address->GetRawInfo(autofill::ADDRESS_HOME_LINE1)); - paymentResponse.details.billing_address.address_line.push_back( - address->GetRawInfo(autofill::ADDRESS_HOME_LINE2)); - paymentResponse.details.billing_address.address_line.push_back( - address->GetRawInfo(autofill::ADDRESS_HOME_LINE3)); - paymentResponse.details.billing_address.region = - address->GetRawInfo(autofill::ADDRESS_HOME_STATE); - paymentResponse.details.billing_address.city = - address->GetRawInfo(autofill::ADDRESS_HOME_CITY); - paymentResponse.details.billing_address.dependent_locality = - address->GetRawInfo(autofill::ADDRESS_HOME_DEPENDENT_LOCALITY); - paymentResponse.details.billing_address.postal_code = - address->GetRawInfo(autofill::ADDRESS_HOME_ZIP); - paymentResponse.details.billing_address.sorting_code = - address->GetRawInfo(autofill::ADDRESS_HOME_SORTING_CODE); - paymentResponse.details.billing_address.language_code = - base::UTF8ToUTF16(address->language_code()); - paymentResponse.details.billing_address.organization = - address->GetRawInfo(autofill::COMPANY_NAME); - paymentResponse.details.billing_address.recipient = - address->GetInfo(autofill::AutofillType(autofill::NAME_FULL), - GetApplicationContext()->GetApplicationLocale()); - paymentResponse.details.billing_address.phone = - address->GetRawInfo(autofill::PHONE_HOME_WHOLE_NUMBER); + if (address) { + paymentResponse.details.billing_address.country = + address->GetRawInfo(autofill::ADDRESS_HOME_COUNTRY); + paymentResponse.details.billing_address.address_line.push_back( + address->GetRawInfo(autofill::ADDRESS_HOME_LINE1)); + paymentResponse.details.billing_address.address_line.push_back( + address->GetRawInfo(autofill::ADDRESS_HOME_LINE2)); + paymentResponse.details.billing_address.address_line.push_back( + address->GetRawInfo(autofill::ADDRESS_HOME_LINE3)); + paymentResponse.details.billing_address.region = + address->GetRawInfo(autofill::ADDRESS_HOME_STATE); + paymentResponse.details.billing_address.city = + address->GetRawInfo(autofill::ADDRESS_HOME_CITY); + paymentResponse.details.billing_address.dependent_locality = + address->GetRawInfo(autofill::ADDRESS_HOME_DEPENDENT_LOCALITY); + paymentResponse.details.billing_address.postal_code = + address->GetRawInfo(autofill::ADDRESS_HOME_ZIP); + paymentResponse.details.billing_address.sorting_code = + address->GetRawInfo(autofill::ADDRESS_HOME_SORTING_CODE); + paymentResponse.details.billing_address.language_code = + base::UTF8ToUTF16(address->language_code()); + paymentResponse.details.billing_address.organization = + address->GetRawInfo(autofill::COMPANY_NAME); + paymentResponse.details.billing_address.recipient = + address->GetInfo(autofill::AutofillType(autofill::NAME_FULL), + GetApplicationContext()->GetApplicationLocale()); + paymentResponse.details.billing_address.phone = + address->GetRawInfo(autofill::PHONE_HOME_WHOLE_NUMBER); + } } [_delegate paymentRequestCoordinatorDidConfirm:paymentResponse];
diff --git a/ios/chrome/browser/ui/ntp/BUILD.gn b/ios/chrome/browser/ui/ntp/BUILD.gn index b5b979e..d5f8a7d 100644 --- a/ios/chrome/browser/ui/ntp/BUILD.gn +++ b/ios/chrome/browser/ui/ntp/BUILD.gn
@@ -69,6 +69,9 @@ "resources/ntp_mv_welcome_favicon@2x.png", "resources/ntp_opentabs@2x.png", "resources/ntp_opentabs@3x.png", + "resources/ntp_opentabs_clock.png", + "resources/ntp_opentabs_clock@2x.png", + "resources/ntp_opentabs_clock@3x.png", "resources/ntp_opentabs_header@2x~ipad.png", "resources/ntp_opentabs_header~ipad.png", "resources/ntp_opentabs_laptop.png",
diff --git a/ios/chrome/browser/ui/ntp/recent_tabs/views/show_full_history_view.mm b/ios/chrome/browser/ui/ntp/recent_tabs/views/show_full_history_view.mm index 74fb81b..c0c25734 100644 --- a/ios/chrome/browser/ui/ntp/recent_tabs/views/show_full_history_view.mm +++ b/ios/chrome/browser/ui/ntp/recent_tabs/views/show_full_history_view.mm
@@ -29,7 +29,7 @@ if (self) { UIImageView* icon = [[UIImageView alloc] initWithImage:nil]; [icon setTranslatesAutoresizingMaskIntoConstraints:NO]; - [icon setImage:[UIImage imageNamed:@"ntp_opentabs"]]; + [icon setImage:[UIImage imageNamed:@"ntp_opentabs_clock"]]; UILabel* label = [[UILabel alloc] initWithFrame:CGRectZero]; [label setTranslatesAutoresizingMaskIntoConstraints:NO];
diff --git a/ios/chrome/browser/ui/ntp/resources/ntp_opentabs_clock.png b/ios/chrome/browser/ui/ntp/resources/ntp_opentabs_clock.png new file mode 100644 index 0000000..fa96864 --- /dev/null +++ b/ios/chrome/browser/ui/ntp/resources/ntp_opentabs_clock.png Binary files differ
diff --git a/ios/chrome/browser/ui/ntp/resources/ntp_opentabs_clock@2x.png b/ios/chrome/browser/ui/ntp/resources/ntp_opentabs_clock@2x.png new file mode 100644 index 0000000..534a04f --- /dev/null +++ b/ios/chrome/browser/ui/ntp/resources/ntp_opentabs_clock@2x.png Binary files differ
diff --git a/ios/chrome/browser/ui/ntp/resources/ntp_opentabs_clock@3x.png b/ios/chrome/browser/ui/ntp/resources/ntp_opentabs_clock@3x.png new file mode 100644 index 0000000..a10996bc --- /dev/null +++ b/ios/chrome/browser/ui/ntp/resources/ntp_opentabs_clock@3x.png Binary files differ
diff --git a/ios/net/crn_http_protocol_handler.mm b/ios/net/crn_http_protocol_handler.mm index c067305..4da8359 100644 --- a/ios/net/crn_http_protocol_handler.mm +++ b/ios/net/crn_http_protocol_handler.mm
@@ -341,11 +341,8 @@ DVLOG(2) << "Redirect, to client:"; LogNSURLRequest(request_); #endif // !defined(NDEBUG) - if (tracker_) { + if (tracker_) tracker_->StopRedirectedRequest(request); - // Add clients from tracker that depend on redirect data. - PushClients(tracker_->ClientsHandlingRedirect(*request, new_url, response)); - } [top_level_client_ wasRedirectedToRequest:request_ nativeRequest:request @@ -501,10 +498,6 @@ long long expectedContentLength = [response expectedContentLength]; if (expectedContentLength > 0) tracker_->CaptureExpectedLength(net_request_, expectedContentLength); - - // Add clients from tracker. - PushClients( - tracker_->ClientsHandlingRequestAndResponse(*net_request_, response)); } // Don't call any function on the response from now on, as the client may be @@ -660,11 +653,6 @@ return; } - if (tracker_) { - // Set up any clients that can operate regardless of the request - PushClients(tracker_->ClientsHandlingAnyRequest()); - } - // Now that all of the network clients are set up, if there was an error with // the URL, it can be raised and all of the clients will have a chance to // handle it. @@ -696,10 +684,6 @@ CopyHttpHeaders(request_, net_request_); - // Add network clients. - if (tracker_) - PushClients(tracker_->ClientsHandlingRequest(*net_request_)); - [top_level_client_ didCreateNativeRequest:net_request_]; SetLoadFlags();
diff --git a/ios/net/request_tracker.h b/ios/net/request_tracker.h index 117b412..098dbc92 100644 --- a/ios/net/request_tracker.h +++ b/ios/net/request_tracker.h
@@ -9,7 +9,6 @@ #include <stdint.h> #include "base/callback_forward.h" -#include "base/mac/scoped_nsobject.h" #include "base/memory/weak_ptr.h" #include "base/threading/thread_checker.h" @@ -19,13 +18,9 @@ class URLRequestContext; } -@class CRNForwardingNetworkClientFactory; -class GURL; - namespace net { -// RequestTracker can be used to observe the network requests and customize the -// behavior of the network stack with CRNForwardingNetworkClients. +// RequestTracker can be used to observe the network requests. // Each network request can be associated with a RequestTracker through the // GetRequestTracker(). // RequestTracker requires a RequestTrackerFactory. @@ -74,32 +69,9 @@ // This function has to be called before using the tracker. virtual void Init(); - // Add a factory that may create network clients for requests going through - // this tracker. - void AddNetworkClientFactory(CRNForwardingNetworkClientFactory* factory); - // Gets the request context associated with the tracker. virtual URLRequestContext* GetRequestContext() = 0; - // Network client generation methods. All of these four ClientsHandling... - // methods return an array of CRNForwardingNetworkClient instances, according - // to the CRNForwardingNetworkClientFactories added to the tracker. The array - // may be empty. The caller is responsible for taking ownership of the clients - // in the array. - - // Returns clients that can handle any request. - NSArray* ClientsHandlingAnyRequest(); - // Returns clients that can handle |request|. - NSArray* ClientsHandlingRequest(const URLRequest& request); - // Returns clients that can handle |request| with |response|. - NSArray* ClientsHandlingRequestAndResponse(const URLRequest& request, - NSURLResponse* response); - // Returns clients that can handle a redirect of |request| to |new_url| based - // on |redirect_response|. - NSArray* ClientsHandlingRedirect(const URLRequest& request, - const GURL& new_url, - NSURLResponse* redirect_response); - // Informs the tracker that a request has started. virtual void StartRequest(URLRequest* request) = 0; @@ -146,10 +118,6 @@ void InvalidateWeakPtrs(); private: - // Array of client factories that may be added by CRNHTTPProtocolHandler. The - // array lives on the IO thread. - base::scoped_nsobject<NSMutableArray> client_factories_; - bool initialized_; CacheMode cache_mode_; base::ThreadChecker thread_checker_;
diff --git a/ios/net/request_tracker.mm b/ios/net/request_tracker.mm index 85dce7f..a28c739 100644 --- a/ios/net/request_tracker.mm +++ b/ios/net/request_tracker.mm
@@ -5,8 +5,6 @@ #include "ios/net/request_tracker.h" #include "base/logging.h" -#import "ios/net/clients/crn_forwarding_network_client.h" -#import "ios/net/clients/crn_forwarding_network_client_factory.h" #if !defined(__has_feature) || !__has_feature(objc_arc) #error "This file requires ARC support." @@ -41,26 +39,8 @@ return g_request_tracker_factory->GetRequestTracker(request, tracker); } -void RequestTracker::AddNetworkClientFactory( - CRNForwardingNetworkClientFactory* factory) { - DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK([[factory clientClass] - isSubclassOfClass:[CRNForwardingNetworkClient class]]); - // Sanity check: We don't already have a factory of the type being added. - DCHECK([client_factories_ indexOfObjectPassingTest:^BOOL(id obj, - NSUInteger idx, - BOOL* stop) { - return [obj clientClass] == [factory clientClass]; - }] == NSNotFound); - [client_factories_ addObject:factory]; - if ([factory requiresOrdering]) { - [client_factories_ sortUsingSelector:@selector(orderRelativeTo:)]; - } -} - RequestTracker::RequestTracker() - : client_factories_([[NSMutableArray alloc] init]), - initialized_(false), + : initialized_(false), cache_mode_(CACHE_NORMAL), weak_ptr_factory_(this) { // RequestTracker can be created from the main thread and used from another @@ -85,64 +65,6 @@ initialized_ = true; } -NSArray* RequestTracker::ClientsHandlingAnyRequest() { - DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(initialized_); - NSMutableArray* applicable_clients = [NSMutableArray array]; - for (CRNForwardingNetworkClientFactory* factory in client_factories_.get()) { - CRNForwardingNetworkClient* client = [factory clientHandlingAnyRequest]; - if (client) - [applicable_clients addObject:client]; - } - return applicable_clients; -} - -NSArray* RequestTracker::ClientsHandlingRequest(const URLRequest& request) { - DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(initialized_); - NSMutableArray* applicable_clients = [NSMutableArray array]; - for (CRNForwardingNetworkClientFactory* factory in client_factories_.get()) { - CRNForwardingNetworkClient* client = - [factory clientHandlingRequest:request]; - if (client) - [applicable_clients addObject:client]; - } - return applicable_clients; -} - -NSArray* RequestTracker::ClientsHandlingRequestAndResponse( - const URLRequest& request, - NSURLResponse* response) { - DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(initialized_); - NSMutableArray* applicable_clients = [NSMutableArray array]; - for (CRNForwardingNetworkClientFactory* factory in client_factories_.get()) { - CRNForwardingNetworkClient* client = - [factory clientHandlingResponse:response request:request]; - if (client) - [applicable_clients addObject:client]; - } - return applicable_clients; -} - -NSArray* RequestTracker::ClientsHandlingRedirect( - const URLRequest& request, - const GURL& new_url, - NSURLResponse* redirect_response) { - DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(initialized_); - NSMutableArray* applicable_clients = [NSMutableArray array]; - for (CRNForwardingNetworkClientFactory* factory in client_factories_.get()) { - CRNForwardingNetworkClient* client = - [factory clientHandlingRedirect:request - url:new_url - response:redirect_response]; - if (client) - [applicable_clients addObject:client]; - } - return applicable_clients; -} - RequestTracker::CacheMode RequestTracker::GetCacheMode() const { DCHECK(thread_checker_.CalledOnValidThread()); return cache_mode_;
diff --git a/pdf/pdf.cc b/pdf/pdf.cc index e6cf5480..917ae472 100644 --- a/pdf/pdf.cc +++ b/pdf/pdf.cc
@@ -123,6 +123,10 @@ void SetPDFUseGDIPrinting(bool enable) { PDFEngineExports::Get()->SetPDFUseGDIPrinting(enable); } + +void SetPDFPostscriptPrintingLevel(int postscript_level) { + PDFEngineExports::Get()->SetPDFPostscriptPrintingLevel(postscript_level); +} #endif // defined(OS_WIN) bool GetPDFDocInfo(const void* pdf_buffer,
diff --git a/pdf/pdf.h b/pdf/pdf.h index c785b551..d18489d 100644 --- a/pdf/pdf.h +++ b/pdf/pdf.h
@@ -82,6 +82,8 @@ PDFEnsureTypefaceCharactersAccessible func); void SetPDFUseGDIPrinting(bool enable); + +void SetPDFPostscriptPrintingLevel(int postscript_level); #endif // defined(OS_WIN) // |page_count| and |max_page_width| are optional and can be NULL.
diff --git a/pdf/pdf_engine.h b/pdf/pdf_engine.h index 54e9f2f..2688f25 100644 --- a/pdf/pdf_engine.h +++ b/pdf/pdf_engine.h
@@ -341,6 +341,8 @@ PDFEnsureTypefaceCharactersAccessible func) = 0; virtual void SetPDFUseGDIPrinting(bool enable) = 0; + + virtual void SetPDFPostscriptPrintingLevel(int postscript_level) = 0; #endif // defined(OS_WIN) // See the definition of RenderPDFPageToBitmap in pdf.cc for details.
diff --git a/pdf/pdfium/fuzzers/BUILD.gn b/pdf/pdfium/fuzzers/BUILD.gn index 1300910..4636a39b 100644 --- a/pdf/pdfium/fuzzers/BUILD.gn +++ b/pdf/pdfium/fuzzers/BUILD.gn
@@ -35,6 +35,13 @@ ] } +fuzzer_test("pdf_codec_a85_fuzzer") { + sources = [] + deps = [ + "//third_party/pdfium/testing/libfuzzer:pdf_codec_a85_fuzzer", + ] +} + fuzzer_test("pdf_codec_fax_fuzzer") { sources = [] deps = [ @@ -42,6 +49,13 @@ ] } +fuzzer_test("pdf_codec_rle_fuzzer") { + sources = [] + deps = [ + "//third_party/pdfium/testing/libfuzzer:pdf_codec_rle_fuzzer", + ] +} + fuzzer_test("pdf_codec_icc_fuzzer") { sources = [] deps = [
diff --git a/pdf/pdfium/pdfium_engine.cc b/pdf/pdfium/pdfium_engine.cc index 111ea87..3e8f5b49 100644 --- a/pdf/pdfium/pdfium_engine.cc +++ b/pdf/pdfium/pdfium_engine.cc
@@ -4103,6 +4103,12 @@ void PDFiumEngineExports::SetPDFUseGDIPrinting(bool enable) { FPDF_SetPrintTextWithGDI(enable); } + +void PDFiumEngineExports::SetPDFPostscriptPrintingLevel( + int postscript_level) { + FPDF_SetPrintPostscriptLevel(postscript_level); +} + #endif // defined(OS_WIN) bool PDFiumEngineExports::RenderPDFPageToBitmap(
diff --git a/pdf/pdfium/pdfium_engine.h b/pdf/pdfium/pdfium_engine.h index e6ff4fcb2..3a525dc 100644 --- a/pdf/pdfium/pdfium_engine.h +++ b/pdf/pdfium/pdfium_engine.h
@@ -775,6 +775,8 @@ PDFEnsureTypefaceCharactersAccessible func) override; void SetPDFUseGDIPrinting(bool enable) override; + + void SetPDFPostscriptPrintingLevel(int postscript_level) override; #endif // defined(OS_WIN) bool RenderPDFPageToBitmap(const void* pdf_buffer, int pdf_buffer_size,
diff --git a/skia/ext/raster_handle_allocator_win.cc b/skia/ext/raster_handle_allocator_win.cc index e78d622e..f5c4bdd 100644 --- a/skia/ext/raster_handle_allocator_win.cc +++ b/skia/ext/raster_handle_allocator_win.cc
@@ -118,7 +118,7 @@ bool is_opaque, HANDLE shared_section, OnFailureType failure_type) { - SkAlphaType alpha = is_opaque ? kPremul_SkAlphaType : kOpaque_SkAlphaType; + SkAlphaType alpha = is_opaque ? kOpaque_SkAlphaType : kPremul_SkAlphaType; SkImageInfo info = SkImageInfo::MakeN32(width, height, alpha); size_t row_bytes = PlatformCanvasStrideForWidth(width);
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index bdec830..132628a 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -2312,4 +2312,5 @@ # Sheriff failures Jan 17 2017 crbug.com/682105 shapedetection/detection-HTMLImageElement.html [ Pass Failure ] +crbug.com/682105 shapedetection/detection-HTMLVideoElement.html [ Pass Failure ] crbug.com/682105 fast/shapedetection/shapedetection-security-test.html [ Pass Failure ]
diff --git a/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip-expected.png b/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip-expected.png index 3a33b31..c563597 100644 --- a/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip-expected.png +++ b/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip-expected.png Binary files differ
diff --git a/third_party/WebKit/LayoutTests/fast/dom/geometry-interface-dom-quad.html b/third_party/WebKit/LayoutTests/fast/dom/geometry-interface-dom-quad.html index 2ffb2b7..071ba7f 100644 --- a/third_party/WebKit/LayoutTests/fast/dom/geometry-interface-dom-quad.html +++ b/third_party/WebKit/LayoutTests/fast/dom/geometry-interface-dom-quad.html
@@ -15,4 +15,16 @@ assert_dom_point_equals(quad.p4, p4); }, "DOMQuad() constructor"); +test(() => { + var p1 = new DOMPoint(0, 1, 3, 4); + var p2 = new DOMPoint(5, 6, 7, 8); + var p3 = new DOMPoint(9, 10, 11, 12); + var p4 = new DOMPoint(13, 14, 15, 16); + var quad = new DOMQuad(p1, p2, p3, p4); + assert_dom_point_equals(quad.toJSON().p1, p1, "p1"); + assert_dom_point_equals(quad.toJSON().p2, p2, "p2"); + assert_dom_point_equals(quad.toJSON().p3, p3, "p3"); + assert_dom_point_equals(quad.toJSON().p4, p4, "p4"); +}, "DOMQuad() toJSON()"); + </script>
diff --git a/third_party/WebKit/LayoutTests/media/deprecated-css-selectors-expected.txt b/third_party/WebKit/LayoutTests/media/deprecated-css-selectors-expected.txt new file mode 100644 index 0000000..6f44507 --- /dev/null +++ b/third_party/WebKit/LayoutTests/media/deprecated-css-selectors-expected.txt
@@ -0,0 +1,11 @@ +CONSOLE WARNING: -internal-media-controls-cast-button selector is deprecated and will be removed in M59, around June 2017. See https://www.chromestatus.com/features/5734009183141888 for more details. +CONSOLE WARNING: -internal-media-controls-overlay-cast-button selector is deprecated and will be removed in M59, around June 2017. See https://www.chromestatus.com/features/5714245488476160 for more details. +CONSOLE WARNING: -internal-media-controls-text-track-list* selectors is deprecated and will be removed in M59, around June 2017. See https://www.chromestatus.com/features/5661431349379072 for more details. +CONSOLE WARNING: -internal-media-controls-text-track-list* selectors is deprecated and will be removed in M59, around June 2017. See https://www.chromestatus.com/features/5661431349379072 for more details. +CONSOLE WARNING: -internal-media-controls-text-track-list* selectors is deprecated and will be removed in M59, around June 2017. See https://www.chromestatus.com/features/5661431349379072 for more details. +CONSOLE WARNING: -internal-media-controls-text-track-list* selectors is deprecated and will be removed in M59, around June 2017. See https://www.chromestatus.com/features/5661431349379072 for more details. +CONSOLE WARNING: -internal-media-controls-text-track-list* selectors is deprecated and will be removed in M59, around June 2017. See https://www.chromestatus.com/features/5661431349379072 for more details. +This is a testharness.js-based test. +PASS Test that -internal-media-controls-* deprecated selectors throw a warning +Harness: the test ran to completion. +
diff --git a/third_party/WebKit/LayoutTests/media/deprecated-css-selectors.html b/third_party/WebKit/LayoutTests/media/deprecated-css-selectors.html new file mode 100644 index 0000000..2624a2b --- /dev/null +++ b/third_party/WebKit/LayoutTests/media/deprecated-css-selectors.html
@@ -0,0 +1,21 @@ +<!DOCTYPE html> +<title>Test that -internal-media-controls-* deprecated selectors throw a warning</title> +<script src="../resources/testharness.js"></script> +<script src="../resources/testharnessreport.js"></script> +<style> +video::-internal-media-controls-cast-button, +video::-internal-media-controls-overlay-cast-button, +video::-internal-media-controls-text-track-list, +video::-internal-media-controls-text-track-list-item, +video::-internal-media-controls-text-track-list-item-input, +video::-internal-media-controls-text-track-list-kind-captions, +video::-internal-media-controls-text-track-list-kind-subtitles { + background-color: blue; +} +</style> +<video controls></video> +<script> +test(() => { + // Doing nothing, we just want to check the warnings. +}); +</script>
diff --git a/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt b/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt index eae9fa9..50dda35d 100644 --- a/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt +++ b/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
@@ -1148,6 +1148,7 @@ getter p3 getter p4 method constructor + method toJSON interface DOMRect : DOMRectReadOnly attribute @@toStringTag getter height
diff --git a/third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp b/third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp index c6fb0b8b..0e8ed5a 100644 --- a/third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp +++ b/third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp
@@ -394,6 +394,9 @@ // NOTE: Some threads (namely utility threads) don't have a scheduler. WebScheduler* scheduler = Platform::current()->currentThread()->scheduler(); + // When timer task runner is used for PerIsolateData, GC tasks are getting + // throttled and memory usage goes up. For now we're using loading task queue + // to prevent this. // TODO(altimin): Consider switching to timerTaskRunner here. v8::Isolate* isolate = V8PerIsolateData::initialize( scheduler ? scheduler->loadingTaskRunner()
diff --git a/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp b/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp index 37b6f8f..221797d 100644 --- a/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp +++ b/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp
@@ -7,6 +7,7 @@ #include "core/css/CSSSelectorList.h" #include "core/css/StyleSheetContents.h" #include "core/css/parser/CSSParserContext.h" +#include "core/frame/Deprecation.h" #include "core/frame/UseCounter.h" #include "platform/RuntimeEnabledFeatures.h" #include "wtf/PtrUtil.h" @@ -14,84 +15,7 @@ namespace blink { -static void recordSelectorStats(const CSSParserContext* context, - const CSSSelectorList& selectorList) { - if (!context->isUseCounterRecordingEnabled()) - return; - - for (const CSSSelector* selector = selectorList.first(); selector; - selector = CSSSelectorList::next(*selector)) { - for (const CSSSelector* current = selector; current; - current = current->tagHistory()) { - UseCounter::Feature feature = UseCounter::NumberOfFeatures; - switch (current->getPseudoType()) { - case CSSSelector::PseudoAny: - feature = UseCounter::CSSSelectorPseudoAny; - break; - case CSSSelector::PseudoUnresolved: - feature = UseCounter::CSSSelectorPseudoUnresolved; - break; - case CSSSelector::PseudoDefined: - feature = UseCounter::CSSSelectorPseudoDefined; - break; - case CSSSelector::PseudoSlotted: - feature = UseCounter::CSSSelectorPseudoSlotted; - break; - case CSSSelector::PseudoContent: - feature = UseCounter::CSSSelectorPseudoContent; - break; - case CSSSelector::PseudoHost: - feature = UseCounter::CSSSelectorPseudoHost; - break; - case CSSSelector::PseudoHostContext: - feature = UseCounter::CSSSelectorPseudoHostContext; - break; - case CSSSelector::PseudoFullScreenAncestor: - feature = UseCounter::CSSSelectorPseudoFullScreenAncestor; - break; - case CSSSelector::PseudoFullScreen: - feature = UseCounter::CSSSelectorPseudoFullScreen; - break; - case CSSSelector::PseudoListBox: - if (context->mode() != UASheetMode) - feature = UseCounter::CSSSelectorInternalPseudoListBox; - break; - case CSSSelector::PseudoWebKitCustomElement: - if (context->mode() != UASheetMode) { - if (current->value() == "-internal-media-controls-cast-button") - feature = UseCounter::CSSSelectorInternalMediaControlsCastButton; - else if (current->value() == - "-internal-media-controls-overlay-cast-button") - feature = - UseCounter::CSSSelectorInternalMediaControlsOverlayCastButton; - } - break; - case CSSSelector::PseudoSpatialNavigationFocus: - if (context->mode() != UASheetMode) - feature = - UseCounter::CSSSelectorInternalPseudoSpatialNavigationFocus; - break; - case CSSSelector::PseudoReadOnly: - if (context->mode() != UASheetMode) - feature = UseCounter::CSSSelectorPseudoReadOnly; - break; - case CSSSelector::PseudoReadWrite: - if (context->mode() != UASheetMode) - feature = UseCounter::CSSSelectorPseudoReadWrite; - break; - default: - break; - } - if (feature != UseCounter::NumberOfFeatures) - context->useCounter()->count(feature); - if (current->relation() == CSSSelector::IndirectAdjacent) - context->useCounter()->count(UseCounter::CSSSelectorIndirectAdjacent); - if (current->selectorList()) - recordSelectorStats(context, *current->selectorList()); - } - } -} - +// static CSSSelectorList CSSSelectorParser::parseSelector( CSSParserTokenRange range, const CSSParserContext* context, @@ -102,7 +26,7 @@ if (!range.atEnd()) return CSSSelectorList(); - recordSelectorStats(context, result); + parser.recordUsageAndDeprecations(result); return result; } @@ -880,4 +804,113 @@ return secondCompound; } +void CSSSelectorParser::recordUsageAndDeprecations( + const CSSSelectorList& selectorList) { + if (!m_context->useCounter()) + return; + + for (const CSSSelector* selector = selectorList.first(); selector; + selector = CSSSelectorList::next(*selector)) { + for (const CSSSelector* current = selector; current; + current = current->tagHistory()) { + UseCounter::Feature feature = UseCounter::NumberOfFeatures; + switch (current->getPseudoType()) { + case CSSSelector::PseudoAny: + feature = UseCounter::CSSSelectorPseudoAny; + break; + case CSSSelector::PseudoUnresolved: + feature = UseCounter::CSSSelectorPseudoUnresolved; + break; + case CSSSelector::PseudoDefined: + feature = UseCounter::CSSSelectorPseudoDefined; + break; + case CSSSelector::PseudoSlotted: + feature = UseCounter::CSSSelectorPseudoSlotted; + break; + case CSSSelector::PseudoContent: + feature = UseCounter::CSSSelectorPseudoContent; + break; + case CSSSelector::PseudoHost: + feature = UseCounter::CSSSelectorPseudoHost; + break; + case CSSSelector::PseudoHostContext: + feature = UseCounter::CSSSelectorPseudoHostContext; + break; + case CSSSelector::PseudoFullScreenAncestor: + feature = UseCounter::CSSSelectorPseudoFullScreenAncestor; + break; + case CSSSelector::PseudoFullScreen: + feature = UseCounter::CSSSelectorPseudoFullScreen; + break; + case CSSSelector::PseudoListBox: + if (m_context->mode() != UASheetMode) + feature = UseCounter::CSSSelectorInternalPseudoListBox; + break; + case CSSSelector::PseudoWebKitCustomElement: + if (m_context->mode() != UASheetMode) { + if (current->value() == "-internal-media-controls-cast-button") { + feature = UseCounter::CSSSelectorInternalMediaControlsCastButton; + } else if (current->value() == + "-internal-media-controls-overlay-cast-button") { + feature = + UseCounter::CSSSelectorInternalMediaControlsOverlayCastButton; + } else if (current->value() == + "-internal-media-controls-text-track-list") { + feature = + UseCounter::CSSSelectorInternalMediaControlsTextTrackList; + } else if (current->value() == + "-internal-media-controls-text-track-list-item") { + feature = + UseCounter::CSSSelectorInternalMediaControlsTextTrackListItem; + } else if (current->value() == + "-internal-media-controls-text-track-list-item-input") { + feature = UseCounter:: + CSSSelectorInternalMediaControlsTextTrackListItemInput; + } else if (current->value() == + "-internal-media-controls-text-track-list-kind-" + "captions") { + feature = UseCounter:: + CSSSelectorInternalMediaControlsTextTrackListKindCaptions; + } else if (current->value() == + "-internal-media-controls-text-track-list-kind-" + "subtitles") { + feature = UseCounter:: + CSSSelectorInternalMediaControlsTextTrackListKindSubtitles; + } + } + break; + case CSSSelector::PseudoSpatialNavigationFocus: + if (m_context->mode() != UASheetMode) { + feature = + UseCounter::CSSSelectorInternalPseudoSpatialNavigationFocus; + } + break; + case CSSSelector::PseudoReadOnly: + if (m_context->mode() != UASheetMode) + feature = UseCounter::CSSSelectorPseudoReadOnly; + break; + case CSSSelector::PseudoReadWrite: + if (m_context->mode() != UASheetMode) + feature = UseCounter::CSSSelectorPseudoReadWrite; + break; + default: + break; + } + if (feature != UseCounter::NumberOfFeatures) { + if (!Deprecation::deprecationMessage(feature).isEmpty() && + m_styleSheet->anyOwnerDocument()) { + Deprecation::countDeprecation(*m_styleSheet->anyOwnerDocument(), + feature); + } else { + m_context->useCounter()->count(feature); + } + } + if (current->relation() == CSSSelector::IndirectAdjacent) + m_context->useCounter()->count(UseCounter::CSSSelectorIndirectAdjacent); + if (current->selectorList()) + recordUsageAndDeprecations(*current->selectorList()); + } + } +} + } // namespace blink
diff --git a/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.h b/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.h index 216e262..fa3180c 100644 --- a/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.h +++ b/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.h
@@ -69,6 +69,7 @@ static std::unique_ptr<CSSParserSelector> splitCompoundAtImplicitShadowCrossingCombinator( std::unique_ptr<CSSParserSelector> compoundSelector); + void recordUsageAndDeprecations(const CSSSelectorList&); Member<const CSSParserContext> m_context; Member<StyleSheetContents> m_styleSheet; // FIXME: Should be const
diff --git a/third_party/WebKit/Source/core/dom/DOMQuad.cpp b/third_party/WebKit/Source/core/dom/DOMQuad.cpp index 21952c0..e93bce22 100644 --- a/third_party/WebKit/Source/core/dom/DOMQuad.cpp +++ b/third_party/WebKit/Source/core/dom/DOMQuad.cpp
@@ -4,6 +4,7 @@ #include "core/dom/DOMQuad.h" +#include "bindings/core/v8/V8ObjectBuilder.h" #include "core/dom/DOMPoint.h" namespace blink { @@ -24,4 +25,13 @@ m_p3(DOMPoint::fromPoint(p3)), m_p4(DOMPoint::fromPoint(p4)) {} +ScriptValue DOMQuad::toJSONForBinding(ScriptState* scriptState) const { + V8ObjectBuilder result(scriptState); + result.add("p1", p1()); + result.add("p2", p2()); + result.add("p3", p3()); + result.add("p4", p4()); + return result.scriptValue(); +} + } // namespace blink
diff --git a/third_party/WebKit/Source/core/dom/DOMQuad.h b/third_party/WebKit/Source/core/dom/DOMQuad.h index 0b5aac6..d13c4c09 100644 --- a/third_party/WebKit/Source/core/dom/DOMQuad.h +++ b/third_party/WebKit/Source/core/dom/DOMQuad.h
@@ -5,6 +5,7 @@ #ifndef DOMQuad_h #define DOMQuad_h +#include "bindings/core/v8/ScriptValue.h" #include "bindings/core/v8/ScriptWrappable.h" #include "core/CoreExport.h" @@ -28,6 +29,8 @@ DOMPoint* p3() const { return m_p3; } DOMPoint* p4() const { return m_p4; } + ScriptValue toJSONForBinding(ScriptState*) const; + DEFINE_INLINE_TRACE() { visitor->trace(m_p1); visitor->trace(m_p2);
diff --git a/third_party/WebKit/Source/core/dom/DOMQuad.idl b/third_party/WebKit/Source/core/dom/DOMQuad.idl index 9063cb1..061e9da9 100644 --- a/third_party/WebKit/Source/core/dom/DOMQuad.idl +++ b/third_party/WebKit/Source/core/dom/DOMQuad.idl
@@ -20,5 +20,5 @@ [SameObject] readonly attribute DOMPoint p4; // TODO(hs1217.lee): [NewObject] DOMRect getBounds(); - // TODO(hs1217.lee): serializer = { attribute }; + serializer = { attribute }; };
diff --git a/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.cpp b/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.cpp index 7ab3191..52676347 100644 --- a/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.cpp +++ b/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.cpp
@@ -11,16 +11,6 @@ namespace blink { -namespace { - -bool isInRemoteFrame(const Document& document) { - DCHECK(document.frame()); - Frame* mainFrame = document.frame()->tree().top(); - return !mainFrame || mainFrame->isRemoteFrame(); -} - -} // anonymous namespace - ElementVisibilityObserver::ElementVisibilityObserver( Element* element, std::unique_ptr<VisibilityCallback> callback) @@ -29,35 +19,25 @@ ElementVisibilityObserver::~ElementVisibilityObserver() = default; void ElementVisibilityObserver::start() { + DCHECK(!m_intersectionObserver); + ExecutionContext* context = m_element->getExecutionContext(); DCHECK(context->isDocument()); Document& document = toDocument(*context); - // TODO(zqzhang): IntersectionObserver does not work for RemoteFrame. - // Remove this early return when it's fixed. See https://crbug.com/615156 - if (isInRemoteFrame(document)) { - m_element.release(); - return; - } - - DCHECK(!m_intersectionObserver); m_intersectionObserver = IntersectionObserver::create( Vector<Length>(), Vector<float>({std::numeric_limits<float>::min()}), &document, WTF::bind(&ElementVisibilityObserver::onVisibilityChanged, wrapWeakPersistent(this))); DCHECK(m_intersectionObserver); + m_intersectionObserver->setInitialState( IntersectionObserver::InitialState::kAuto); m_intersectionObserver->observe(m_element.release()); } void ElementVisibilityObserver::stop() { - // TODO(zqzhang): IntersectionObserver does not work for RemoteFrame, - // so |m_intersectionObserver| may be null at this point after start(). - // Replace this early return with DCHECK when this has been fixed. See - // https://crbug.com/615156 - if (!m_intersectionObserver) - return; + DCHECK(m_intersectionObserver); m_intersectionObserver->disconnect(); m_intersectionObserver = nullptr;
diff --git a/third_party/WebKit/Source/core/dom/ElementVisibilityObserverTest.cpp b/third_party/WebKit/Source/core/dom/ElementVisibilityObserverTest.cpp index 2645259..02f03b0 100644 --- a/third_party/WebKit/Source/core/dom/ElementVisibilityObserverTest.cpp +++ b/third_party/WebKit/Source/core/dom/ElementVisibilityObserverTest.cpp
@@ -6,8 +6,10 @@ #include "core/dom/DOMImplementation.h" #include "core/dom/Document.h" +#include "core/frame/RemoteFrame.h" #include "core/html/HTMLDivElement.h" #include "core/html/HTMLDocument.h" +#include "core/loader/EmptyClients.h" #include "core/testing/DummyPageHolder.h" #include "testing/gtest/include/gtest/gtest.h" @@ -15,14 +17,47 @@ namespace { +// Stub implementation of FrameLoaderClient for the purpose of testing. It will +// alow callers to set the parent/top frames by calling |setParent|. It is used +// in ElementVisibilityObserverTest in order to mock a RemoteFrame parent of a +// LocalFrame. +class StubFrameLoaderClient final : public EmptyFrameLoaderClient { + public: + Frame* parent() const override { return m_parent; } + Frame* top() const override { return m_parent; } + + void setParent(Frame* frame) { m_parent = frame; } + + DEFINE_INLINE_VIRTUAL_TRACE() { + visitor->trace(m_parent); + EmptyFrameLoaderClient::trace(visitor); + } + + private: + WeakMember<Frame> m_parent = nullptr; +}; + class ElementVisibilityObserverTest : public ::testing::Test { protected: - void SetUp() override { m_dummyPageHolder = DummyPageHolder::create(); } + void SetUp() override { + m_frameLoaderClient = new StubFrameLoaderClient(); + m_dummyPageHolder = DummyPageHolder::create( + IntSize(), nullptr, m_frameLoaderClient, nullptr, nullptr); + } + + void TearDown() override { + m_dummyPageHolder->frame().detach(FrameDetachType::Remove); + } Document& document() { return m_dummyPageHolder->document(); } + FrameHost& frameHost() { return m_dummyPageHolder->page().frameHost(); } + StubFrameLoaderClient* frameLoaderClient() const { + return m_frameLoaderClient; + } private: std::unique_ptr<DummyPageHolder> m_dummyPageHolder; + Persistent<StubFrameLoaderClient> m_frameLoaderClient; }; TEST_F(ElementVisibilityObserverTest, ObserveElementWithoutDocumentFrame) { @@ -35,6 +70,20 @@ // It should not crash. } +TEST_F(ElementVisibilityObserverTest, ObserveElementInRemoteFrame) { + Persistent<RemoteFrame> remoteFrame = + RemoteFrame::create(new EmptyRemoteFrameClient(), &frameHost(), nullptr); + frameLoaderClient()->setParent(remoteFrame); + + Persistent<HTMLElement> element = HTMLDivElement::create(document()); + ElementVisibilityObserver* observer = + new ElementVisibilityObserver(element, WTF::bind([](bool) {})); + observer->start(); + observer->deliverObservationsForTesting(); + observer->stop(); + // It should not crash. +} + } // anonymous namespace } // blink namespace
diff --git a/third_party/WebKit/Source/core/frame/Deprecation.cpp b/third_party/WebKit/Source/core/frame/Deprecation.cpp index f826cc0..f0bbf76 100644 --- a/third_party/WebKit/Source/core/frame/Deprecation.cpp +++ b/third_party/WebKit/Source/core/frame/Deprecation.cpp
@@ -394,6 +394,24 @@ "redirected response. This will result in an error in %s.", milestoneString(M59)); + case UseCounter::CSSSelectorInternalMediaControlsCastButton: + return willBeRemoved("-internal-media-controls-cast-button selector", M59, + "5734009183141888"); + + case UseCounter::CSSSelectorInternalMediaControlsOverlayCastButton: + return willBeRemoved( + "-internal-media-controls-overlay-cast-button selector", M59, + "5714245488476160"); + + case UseCounter::CSSSelectorInternalMediaControlsTextTrackList: + case UseCounter::CSSSelectorInternalMediaControlsTextTrackListItem: + case UseCounter::CSSSelectorInternalMediaControlsTextTrackListItemInput: + case UseCounter::CSSSelectorInternalMediaControlsTextTrackListKindCaptions: + case UseCounter::CSSSelectorInternalMediaControlsTextTrackListKindSubtitles: + return willBeRemoved( + "-internal-media-controls-text-track-list* selectors", M59, + "5661431349379072"); + // Features that aren't deprecated don't have a deprecation message. default: return String();
diff --git a/third_party/WebKit/Source/core/frame/UseCounter.h b/third_party/WebKit/Source/core/frame/UseCounter.h index 2fa053d0..66c1d0b4 100644 --- a/third_party/WebKit/Source/core/frame/UseCounter.h +++ b/third_party/WebKit/Source/core/frame/UseCounter.h
@@ -1439,6 +1439,11 @@ DocumentCompleteURLHTTPContainingLessThan = 1769, DocumentCompleteURLHTTPContainingNewlineAndLessThan = 1770, DocumentCompleteURLNonHTTPContainingNewlineOrLessThan = 1771, + CSSSelectorInternalMediaControlsTextTrackList = 1772, + CSSSelectorInternalMediaControlsTextTrackListItem = 1773, + CSSSelectorInternalMediaControlsTextTrackListItemInput = 1774, + CSSSelectorInternalMediaControlsTextTrackListKindCaptions = 1775, + CSSSelectorInternalMediaControlsTextTrackListKindSubtitles = 1776, // Add new features immediately above this line. Don't change assigned // numbers of any item, and don't reuse removed slots.
diff --git a/third_party/WebKit/Source/core/loader/EmptyClients.cpp b/third_party/WebKit/Source/core/loader/EmptyClients.cpp index c9d0e5e..b3bd635 100644 --- a/third_party/WebKit/Source/core/loader/EmptyClients.cpp +++ b/third_party/WebKit/Source/core/loader/EmptyClients.cpp
@@ -196,4 +196,6 @@ return nullptr; } +EmptyRemoteFrameClient::EmptyRemoteFrameClient() = default; + } // namespace blink
diff --git a/third_party/WebKit/Source/core/loader/EmptyClients.h b/third_party/WebKit/Source/core/loader/EmptyClients.h index 416e2c1..0c02290 100644 --- a/third_party/WebKit/Source/core/loader/EmptyClients.h +++ b/third_party/WebKit/Source/core/loader/EmptyClients.h
@@ -30,6 +30,7 @@ #define EmptyClients_h #include "core/CoreExport.h" +#include "core/frame/RemoteFrameClient.h" #include "core/loader/FrameLoaderClient.h" #include "core/page/ChromeClient.h" #include "core/page/ContextMenuClient.h" @@ -423,6 +424,42 @@ void clearContextMenu() override {} }; +class CORE_EXPORT EmptyRemoteFrameClient + : NON_EXPORTED_BASE(public RemoteFrameClient) { + WTF_MAKE_NONCOPYABLE(EmptyRemoteFrameClient); + + public: + EmptyRemoteFrameClient(); + + // RemoteFrameClient implementation. + void navigate(const ResourceRequest&, + bool shouldReplaceCurrentEntry) override {} + void reload(FrameLoadType, ClientRedirectPolicy) override {} + unsigned backForwardLength() override { return 0; } + void forwardPostMessage(MessageEvent*, + PassRefPtr<SecurityOrigin> target, + LocalFrame* sourceFrame) const override {} + void forwardInputEvent(Event*) override {} + void frameRectsChanged(const IntRect& frameRect) override {} + void updateRemoteViewportIntersection( + const IntRect& viewportIntersection) override {} + void advanceFocus(WebFocusType, LocalFrame* source) override {} + void visibilityChanged(bool visible) override {} + void setHasReceivedUserGesture() override {} + + // FrameClient implementation. + bool inShadowTree() const override { return false; } + void willBeDetached() override {} + void detached(FrameDetachType) override {} + Frame* opener() const override { return nullptr; } + void setOpener(Frame*) override {} + Frame* parent() const override { return nullptr; } + Frame* top() const override { return nullptr; } + Frame* nextSibling() const override { return nullptr; } + Frame* firstChild() const override { return nullptr; } + void frameFocused() const override {} +}; + CORE_EXPORT void fillWithEmptyClients(Page::PageClients&); } // namespace blink
diff --git a/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp b/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp index c963f48..eb5a60a 100644 --- a/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp +++ b/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp
@@ -1134,13 +1134,8 @@ context, FloatPoint(localOrigin) + FloatPoint(0, lineThroughOffset), width.toFloat(), decoration, textDecorationThickness, doubleOffset, 0, antialiasDecoration); - if (skipIntercepts) { - textPainter.clipDecorationsStripe( - -baseline + decorationPainter.decorationBounds().y() - - FloatPoint(localOrigin).y(), - decorationPainter.decorationBounds().height(), - textDecorationThickness); - } + // No skip: ink for line-through, + // compare https://github.com/w3c/csswg-drafts/issues/711 decorationPainter.paint(); } }
diff --git a/third_party/WebKit/Source/platform/heap/WrapperVisitor.h b/third_party/WebKit/Source/platform/heap/WrapperVisitor.h index 13de57b..3e0c49b 100644 --- a/third_party/WebKit/Source/platform/heap/WrapperVisitor.h +++ b/third_party/WebKit/Source/platform/heap/WrapperVisitor.h
@@ -91,7 +91,7 @@ public: template <typename T> - static NEVER_INLINE void missedWriteBarrier() { + static NOINLINE void missedWriteBarrier() { NOTREACHED(); } @@ -167,7 +167,7 @@ } template <typename T> - void markAndPushToMarkingDeque(const T* traceable) const { + ALWAYS_INLINE void markAndPushToMarkingDeque(const T* traceable) const { if (pushToMarkingDeque(TraceTrait<T>::traceMarkedWrapper, TraceTrait<T>::heapObjectHeader, WrapperVisitor::missedWriteBarrier<T>, traceable)) {
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 744dac8..26a7b49 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -39181,6 +39181,18 @@ </summary> </histogram> +<histogram name="NewTabPage.ContentSuggestions.OpenedCategoryIndex" + units="index"> + <owner>vitaliii@chromium.org</owner> + <summary> + Android: The index of a category on the NTP, whose suggestion card is + clicked through to the host website of the content. This tracked index can + be different from the position observed by the user, e.g. for the user a + category may be at the top of the NTP, but with index 1, because there is an + empty category above (with index 0). + </summary> +</histogram> + <histogram name="NewTabPage.ContentSuggestions.OpenedScore" units="score"> <obsolete> Replaced by NewTabPage.ContentSuggestions.OpenedScoreNormalized. @@ -44714,6 +44726,20 @@ </summary> </histogram> +<histogram name="PasswordManager.PasswordReuse.PasswordFieldDetected" + enum="PasswordReusePasswordFieldDetected"> + <owner>dvadym@chromium.org</owner> + <owner>nparker@chromium.org</owner> + <summary> + A password reuse is an event when the user typed a string that is equal to a + saved password on another domain, and this saved password is called reused + password. + + This metric reports whether a password field had been detected on a page + when a password reuse happened. + </summary> +</histogram> + <histogram name="PasswordManager.PasswordReuse.PasswordLength" units="characters"> <owner>dvadym@chromium.org</owner> @@ -89320,6 +89346,14 @@ label="DocumentCompleteURLHTTPContainingNewlineAndLessThan"/> <int value="1771" label="DocumentCompleteURLNonHTTPContainingNewlineOrLessThan"/> + <int value="1772" label="CSSSelectorInternalMediaControlsTextTrackList"/> + <int value="1773" label="CSSSelectorInternalMediaControlsTextTrackListItem"/> + <int value="1774" + label="CSSSelectorInternalMediaControlsTextTrackListItemInput"/> + <int value="1775" + label="CSSSelectorInternalMediaControlsTextTrackListKindCaptions"/> + <int value="1776" + label="CSSSelectorInternalMediaControlsTextTrackListKindSubtitles"/> </enum> <enum name="FetchRequestMode" type="int"> @@ -94896,6 +94930,7 @@ <int value="-1203742042" label="enable-gesture-selection"/> <int value="-1201183153" label="enable-centered-app-list"/> <int value="-1184904651" label="enable-npapi"/> + <int value="-1184480269" label="LsdPermissionPrompt:enabled"/> <int value="-1177802205" label="enable-hosted-app-quit-notification"/> <int value="-1176748003" label="FramebustingNeedsSameOriginOrUserGesture:disabled"/> @@ -95605,6 +95640,7 @@ <int value="2037756154" label="enable-impl-side-painting"/> <int value="2059322877" label="new-avatar-menu"/> <int value="2063091429" label="OfflinePagesSharing:enabled"/> + <int value="2067634730" label="LsdPermissionPrompt:disabled"/> <int value="2071340353" label="progress-bar-completion"/> <int value="2071461362" label="disable-credit-card-scan"/> <int value="2076903744" label="progress-bar-animation"/> @@ -100115,6 +100151,11 @@ <int value="11" label="Clicked 'Google Smart Lock'"/> </enum> +<enum name="PasswordReusePasswordFieldDetected" type="int"> + <int value="0" label="No password field"/> + <int value="1" label="Has password field"/> +</enum> + <enum name="PasswordSubmissionEvent" type="int"> <int value="0" label="Password submission succeeded"/> <int value="1" label="Password submission failed"/> @@ -110569,6 +110610,7 @@ <affected-histogram name="NewTabPage.ContentSuggestions.OpenDisposition"/> <affected-histogram name="NewTabPage.ContentSuggestions.Opened"/> <affected-histogram name="NewTabPage.ContentSuggestions.OpenedAge"/> + <affected-histogram name="NewTabPage.ContentSuggestions.OpenedCategoryIndex"/> <affected-histogram name="NewTabPage.ContentSuggestions.OpenedScore"/> <affected-histogram name="NewTabPage.ContentSuggestions.OpenedScoreNormalized"/>
diff --git a/tools/perf/benchmarks/benchmark_smoke_unittest.py b/tools/perf/benchmarks/benchmark_smoke_unittest.py index f236444..8808a991c 100644 --- a/tools/perf/benchmarks/benchmark_smoke_unittest.py +++ b/tools/perf/benchmarks/benchmark_smoke_unittest.py
@@ -34,7 +34,13 @@ from benchmarks import v8_browsing -def SmokeTestGenerator(benchmark): +def SmokeTestGenerator(benchmark, num_pages=1): + """Generates a benchmark that includes first N pages from pageset. + + Args: + benchmark: benchmark object to make smoke test. + num_pages: use the first N pages to run smoke test. + """ # NOTE TO SHERIFFS: DO NOT DISABLE THIS TEST. # # This smoke test dynamically tests all benchmarks. So disabling it for one @@ -53,9 +59,10 @@ def CreateStorySet(self, options): # pylint: disable=super-on-old-class story_set = super(SinglePageBenchmark, self).CreateStorySet(options) + # Only smoke test the first story since smoke testing everything takes # too long. - for s in story_set.stories[1:]: + for s in story_set.stories[num_pages:]: story_set.RemoveStory(s) return story_set @@ -135,7 +142,11 @@ class BenchmarkSmokeTest(unittest.TestCase): pass - method = SmokeTestGenerator(benchmark) + # tab_switching needs more than one page to test correctly. + if 'tab_switching' in benchmark.Name(): + method = SmokeTestGenerator(benchmark, num_pages=2) + else: + method = SmokeTestGenerator(benchmark) # Make sure any decorators are propagated from the original declaration. # (access to protected members) pylint: disable=protected-access
diff --git a/tools/perf/measurements/tab_switching.py b/tools/perf/measurements/tab_switching.py index 7f71359a..dd64dfa 100644 --- a/tools/perf/measurements/tab_switching.py +++ b/tools/perf/measurements/tab_switching.py
@@ -10,6 +10,7 @@ Power usage is also measured. """ +import json import time from telemetry.core import util @@ -64,13 +65,17 @@ def ValidateAndMeasurePage(self, page, tab, results): """On the last tab, cycle through each tab that was opened and then record a single histogram for the tab switching metric.""" - if len(tab.browser.tabs) != len(page.story_set.stories): + browser = tab.browser + if len(browser.tabs) != len(page.story_set.stories): return + if browser.tabs < 2: + raise Exception('Should have at least two tabs for tab switching') + # Measure power usage of tabs after quiescence. util.WaitFor(tab.HasReachedQuiescence, 60) - if tab.browser.platform.CanMonitorPower(): + if browser.platform.CanMonitorPower(): self._power_metric.Start(page, tab) time.sleep(TabSwitching.SAMPLE_TIME) self._power_metric.Stop(page, tab) @@ -83,27 +88,32 @@ histogram_type, histogram_name, tab) prev_histogram = first_histogram - for t in tab.browser.tabs: - t.Activate() - + for tab_to_switch in browser.tabs: + tab_to_switch.Activate() def _IsDone(): + # pylint: disable=W0640 cur_histogram = histogram_util.GetHistogram( - histogram_type, histogram_name, tab) + histogram_type, histogram_name, tab_to_switch) diff_histogram = histogram_util.SubtractHistogram( cur_histogram, prev_histogram) - return diff_histogram + # TODO(deanliao): Add SubtractHistogramRawValue to process histogram + # object instead of JSON string. + diff_histogram_count = json.loads(diff_histogram).get('count', 0) + return diff_histogram_count > 0 util.WaitFor(_IsDone, 30) + + # We need to get histogram again instead of getting cur_histogram as + # variables modified inside inner function cannot be retrieved. However, + # inner function can see external scope's variables. prev_histogram = histogram_util.GetHistogram( - histogram_type, histogram_name, tab) + histogram_type, histogram_name, tab_to_switch) - last_histogram = histogram_util.GetHistogram( - histogram_type, histogram_name, tab) - diff_histogram = histogram_util.SubtractHistogram(last_histogram, - first_histogram) - + last_histogram = prev_histogram + total_diff_histogram = histogram_util.SubtractHistogram(last_histogram, + first_histogram) results.AddSummaryValue( histogram.HistogramValue(None, display_name, 'ms', - raw_value_json=diff_histogram, + raw_value_json=total_diff_histogram, important=False)) keychain_metric.KeychainMetric().AddResults(tab, results)
diff --git a/tools/perf/measurements/tab_switching_unittest.py b/tools/perf/measurements/tab_switching_unittest.py new file mode 100644 index 0000000..79efbb1 --- /dev/null +++ b/tools/perf/measurements/tab_switching_unittest.py
@@ -0,0 +1,97 @@ +# Copyright 2015 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import contextlib +from telemetry.internal.results import page_test_results +from telemetry.testing import page_test_test_case + +from measurements import tab_switching + +import mock + + +class BrowserForTest(object): + def __init__(self): + self.tabs = [] + self.platform = mock.MagicMock() + self.platform.CanMonitorPower = mock.Mock(return_value=False) + + def AddTab(self, tab): + tab.browser = self + self.tabs.append(tab) + + +class StorySetForTest(object): + def __init__(self): + self.stories = [] + + def AddStory(self, story): + story.story_set = self + self.stories.append(story) + + +class TabSwitchingUnittest(page_test_test_case.PageTestTestCase): + @staticmethod + def MakeStoryForTest(): + story = mock.MagicMock() + story.story_set = None + return story + + @staticmethod + def MakeTabForTest(): + tab = mock.MagicMock() + tab.browser = None + tab.HasReachedQuiescence = mock.Mock(return_value=True) + return tab + + def testIsDone(self): + """Tests ValidateAndMeasurePage, specifically _IsDone check.""" + measure = tab_switching.TabSwitching() + + # For sanity check: #tabs == #stories + story_set = StorySetForTest() + story_set.AddStory(self.MakeStoryForTest()) + story_set.AddStory(self.MakeStoryForTest()) + + # Set up a browser with two tabs open + browser = BrowserForTest() + tab_0 = self.MakeTabForTest() + browser.AddTab(tab_0) + tab_1 = self.MakeTabForTest() + browser.AddTab(tab_1) + + # Mock histogram result to test _IsDone really works. + expected_histogram = [ + # To get first_histogram for last tab (tab_1). + '{"count": 0, "buckets": []}', + # First _IsDone check for tab_0. Retry. + '{"count": 0, "buckets": []}', + # Second _IsDone check for tab_0. Retry. + '{"count": 0, "buckets": []}', + # Third _IsDone check for tab_0. Pass. + '{"count": 1, "buckets": [{"low": 1, "high": 2, "count": 1}]}', + # To get prev_histogram. End of tab_0 loop. + '{"count": 1, "buckets": [{"low": 1, "high": 2, "count": 1}]}', + # First _IsDone check for tab_1. Retry. + '{"count": 1, "buckets": [{"low": 1, "high": 2, "count": 1}]}', + # Second _IsDone check for tab_1. Pass. + '{"count": 2, "buckets": [{"low": 1, "high": 2, "count": 1},' + '{"low": 2, "high": 3, "count": 1}]}', + # To get prev_histogram. End of tab_1 loop. + '{"count": 2, "buckets": [{"low": 1, "high": 2, "count": 1},' + '{"low": 2, "high": 3, "count": 1}]}', + ] + mock_get_histogram = mock.MagicMock(side_effect=expected_histogram) + + with contextlib.nested( + mock.patch('telemetry.value.histogram_util.GetHistogram', + mock_get_histogram), + mock.patch('metrics.keychain_metric.KeychainMetric')): + measure.ValidateAndMeasurePage(story_set.stories[0], browser.tabs[-1], + page_test_results.PageTestResults()) + self.assertEqual(len(expected_histogram), + len(mock_get_histogram.mock_calls)) + expected_calls = [mock.call(mock.ANY, mock.ANY, t) for t in + [tab_1] + [tab_0] * 4 + [tab_1] * 3] + self.assertEqual(expected_calls, mock_get_histogram.mock_calls)
