diff --git a/DEPS b/DEPS index d628be0..261ebf4 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': '60fb0b2548c7f03b58fc52aa298b07ed137c84e3', + 'skia_revision': 'ade9f618e563cf7564f75dcfb83aa8c6a3363bc5', # 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': '959f3e87036a789d42526e703903bbab2541d7af', + 'v8_revision': '13d38e4a87153ab3b0a8d0612ea1c7ee3f664d77', # 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. @@ -235,7 +235,7 @@ Var('chromium_git') + '/native_client/src/third_party/scons-2.0.1.git' + '@' + '1c1550e17fc26355d08627fbdec13d8291227067', 'src/third_party/webrtc': - Var('chromium_git') + '/external/webrtc/trunk/webrtc.git' + '@' + '2028bbca2add17f69a792ef82f2e4167b026598d', # commit position 18594 + Var('chromium_git') + '/external/webrtc/trunk/webrtc.git' + '@' + 'c81ec9b3da5816407fe16439d26dd844a1f566da', # commit position 18614 'src/third_party/openmax_dl': Var('chromium_git') + '/external/webrtc/deps/third_party/openmax.git' + '@' + Var('openmax_dl_revision'),
diff --git a/base/android/base_jni_onload.cc b/base/android/base_jni_onload.cc index 0a82db4..170dd840 100644 --- a/base/android/base_jni_onload.cc +++ b/base/android/base_jni_onload.cc
@@ -12,10 +12,6 @@ namespace base { namespace android { -bool OnJNIOnLoadRegisterJNI(JNIEnv* env) { - return RegisterLibraryLoaderEntryHook(env); -} - bool OnJNIOnLoadInit() { InitAtExitManager(); JNIEnv* env = base::android::AttachCurrentThread();
diff --git a/base/android/base_jni_onload.h b/base/android/base_jni_onload.h index be637d5..5d7cc0a 100644 --- a/base/android/base_jni_onload.h +++ b/base/android/base_jni_onload.h
@@ -14,10 +14,6 @@ namespace base { namespace android { -// Returns whether JNI registration succeeded. -typedef base::Callback<bool(JNIEnv*)> RegisterCallback; -BASE_EXPORT bool OnJNIOnLoadRegisterJNI(JNIEnv* env); - // Returns whether initialization succeeded. BASE_EXPORT bool OnJNIOnLoadInit();
diff --git a/base/android/base_jni_registrar.cc b/base/android/base_jni_registrar.cc index 973ae16a..54b5572 100644 --- a/base/android/base_jni_registrar.cc +++ b/base/android/base_jni_registrar.cc
@@ -15,6 +15,7 @@ #include "base/android/java_handler_thread.h" #include "base/android/jni_android.h" #include "base/android/jni_registrar.h" +#include "base/android/library_loader/library_loader_hooks.h" #include "base/android/memory_pressure_listener_android.h" #include "base/android/path_service_android.h" #include "base/android/record_histogram.h" @@ -41,6 +42,7 @@ {"FieldTrialList", base::android::RegisterFieldTrialList}, {"ImportantFileWriterAndroid", base::android::RegisterImportantFileWriterAndroid}, + {"LibraryLoaderEntryHook", base::android::RegisterLibraryLoaderEntryHook}, {"MemoryPressureListenerAndroid", base::android::MemoryPressureListenerAndroid::Register}, {"JavaExceptionReporter", base::android::RegisterJavaExceptionReporterJni},
diff --git a/base/trace_event/memory_dump_manager.cc b/base/trace_event/memory_dump_manager.cc index 1ce86367..3e7f7a1e 100644 --- a/base/trace_event/memory_dump_manager.cc +++ b/base/trace_event/memory_dump_manager.cc
@@ -781,7 +781,7 @@ GetDumpsSumKb("blink_gc", process_memory_dump); FillOsDumpFromProcessMemoryDump(process_memory_dump, &result->os_dump); } else { - auto& os_dump = result->extra_processes_dump[pid]; + auto& os_dump = result->extra_processes_dumps[pid]; FillOsDumpFromProcessMemoryDump(process_memory_dump, &os_dump); } }
diff --git a/base/trace_event/memory_dump_request_args.h b/base/trace_event/memory_dump_request_args.h index 9eb6ade..3264a72f 100644 --- a/base/trace_event/memory_dump_request_args.h +++ b/base/trace_event/memory_dump_request_args.h
@@ -99,7 +99,7 @@ // In some cases, OS stats can only be dumped from a privileged process to // get around to sandboxing/selinux restrictions (see crbug.com/461788). - std::map<ProcessId, OSMemDump> extra_processes_dump; + std::map<ProcessId, OSMemDump> extra_processes_dumps; MemoryDumpCallbackResult(); MemoryDumpCallbackResult(const MemoryDumpCallbackResult&);
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/payments/ServiceWorkerPaymentAppBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/payments/ServiceWorkerPaymentAppBridge.java index fd6f120..99c8d7c 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/payments/ServiceWorkerPaymentAppBridge.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/payments/ServiceWorkerPaymentAppBridge.java
@@ -21,6 +21,8 @@ import java.util.List; import java.util.Set; +import javax.annotation.Nullable; + /** * Native bridge for interacting with service worker based payment apps. */ @@ -68,7 +70,7 @@ @CalledByNative private static void addInstrument(List<PaymentInstrument> instruments, WebContents webContents, long swRegistrationId, String instrumentId, String label, String[] methodNameArray, - Bitmap icon) { + @Nullable Bitmap icon) { Context context = ChromeActivity.fromWebContents(webContents); if (context == null) return; @@ -77,9 +79,9 @@ methodNames.add(methodNameArray[i]); } - instruments.add( - new ServiceWorkerPaymentInstrument(webContents, swRegistrationId, instrumentId, - label, methodNames, new BitmapDrawable(context.getResources(), icon))); + instruments.add(new ServiceWorkerPaymentInstrument(webContents, swRegistrationId, + instrumentId, label, methodNames, + icon == null ? null : new BitmapDrawable(context.getResources(), icon))); } @CalledByNative
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/payments/ServiceWorkerPaymentInstrument.java b/chrome/android/java/src/org/chromium/chrome/browser/payments/ServiceWorkerPaymentInstrument.java index 4579a9c..8f0cfe7 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/payments/ServiceWorkerPaymentInstrument.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/payments/ServiceWorkerPaymentInstrument.java
@@ -17,6 +17,8 @@ import java.util.Map; import java.util.Set; +import javax.annotation.Nullable; + /** * This instrument class represents a single payment instrument for a service * worker based payment app. @@ -44,7 +46,7 @@ * @param icon The drawable icon of the payment instrument. */ public ServiceWorkerPaymentInstrument(WebContents webContents, long swRegistrationId, - String instrumentId, String label, Set<String> methodNames, Drawable icon) { + String instrumentId, String label, Set<String> methodNames, @Nullable Drawable icon) { super(Long.toString(swRegistrationId) + "#" + instrumentId, label, null /* sublabel */, icon); mWebContents = webContents;
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninView.java b/chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninView.java index ab4fba7..05d495b 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninView.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninView.java
@@ -295,7 +295,12 @@ && (mAccountNames.isEmpty() || mAccountNames.get(accountToSelect) .equals(oldAccountNames.get(oldSelectedAccount))); - if (selectedAccountChanged) { + // There is a race condition where the FragmentManager can be null. This + // presumably happens once the AccountSigninView has been detached (the bug is + // triggered when Chrome is hidden). Since we are detached, the dialogs will + // have already been deleted so we don't need to cancel them. + // https://crbug.com/733117 + if (selectedAccountChanged && mDelegate.getFragmentManager() != null) { // Any dialogs that may have been showing are now invalid (they were created // for the previously selected account). ConfirmSyncDataStateMachine.cancelAllDialogs(mDelegate.getFragmentManager());
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index 269fc96c..963e110 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd
@@ -12224,11 +12224,11 @@ Chrome has blocked parts of this site to protect you from potentially deceptive content. </message> - <message name="IDS_ALWAYS_SHOW_ADS" desc="Explanation associated with a toggle to allow ads after ads have been blocked on the page. To be used on pages where the ad blocking UI is governed by a persistent permissions-based whitelist."> - Always show ads on this site + <message name="IDS_ALWAYS_ALLOW_ADS" desc="Explanation associated with a toggle to allow ads after ads have been blocked on the page. To be used on pages where the ad blocking UI is governed by a persistent permissions-based whitelist."> + Always allow ads on this site </message> - <message name="IDS_SHOW_ADS" desc="Explanation associated with a toggle to allow ads after ads have been blocked on the page. To be used on pages where the ad blocking UI is governed by a per-tab whitelist."> - Show ads on this site + <message name="IDS_ALLOW_ADS" desc="Explanation associated with a toggle to allow ads after ads have been blocked on the page. To be used on pages where the ad blocking UI is governed by a per-tab whitelist."> + Allow ads on this site </message> <message name="IDS_BLOCKED_ADS_PROMPT_TITLE" desc="Title of the prompt shown to users when Chrome has blocked ads on the site because the site tends to show intrusive ads. To be shown in an infobar / omnibox"> Ads blocked.
diff --git a/chrome/browser/android/payments/service_worker_payment_app_bridge.cc b/chrome/browser/android/payments/service_worker_payment_app_bridge.cc index c909e028..50d6745 100644 --- a/chrome/browser/android/payments/service_worker_payment_app_bridge.cc +++ b/chrome/browser/android/payments/service_worker_payment_app_bridge.cc
@@ -51,7 +51,9 @@ ConvertUTF8ToJavaString(env, instrument->instrument_key), ConvertUTF8ToJavaString(env, instrument->name), ToJavaArrayOfStrings(env, instrument->enabled_methods), - gfx::ConvertToJavaBitmap(instrument->icon.get())); + instrument->icon == nullptr + ? nullptr + : gfx::ConvertToJavaBitmap(instrument->icon.get())); } Java_ServiceWorkerPaymentAppBridge_onPaymentAppCreated( env, java_instruments, jweb_contents, jcallback);
diff --git a/chrome/browser/android/vr_shell/ui_scene_manager.cc b/chrome/browser/android/vr_shell/ui_scene_manager.cc index 598e510..9862655 100644 --- a/chrome/browser/android/vr_shell/ui_scene_manager.cc +++ b/chrome/browser/android/vr_shell/ui_scene_manager.cc
@@ -355,7 +355,8 @@ url_bar->set_hit_testable(false); url_bar->set_translation( {0, kTransientUrlBarVerticalOffset, -kTransientUrlBarDistance}); - url_bar->set_rotation({1.0, 0.0, 0.0, kUrlBarRotationRad}); + url_bar->set_rotation( + gfx::Quaternion(gfx::Vector3dF(1, 0, 0), kUrlBarRotationRad)); url_bar->set_size({kTransientUrlBarWidth, kTransientUrlBarHeight, 1}); transient_url_bar_ = url_bar.get(); scene_->AddUiElement(std::move(url_bar));
diff --git a/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc b/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc index 4c684f4..064d1bf8 100644 --- a/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc +++ b/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc
@@ -189,7 +189,7 @@ is_installable_check_complete_ = true; if (check_webapk_compatibility_) weak_observer_->OnDidDetermineWebApkCompatibility(false); - weak_observer_->OnUserTitleAvailable(base::string16()); + weak_observer_->OnUserTitleAvailable(shortcut_info_.user_title); } badge_icon_.reset();
diff --git a/chrome/browser/extensions/extension_bindings_apitest.cc b/chrome/browser/extensions/extension_bindings_apitest.cc index 930cb1a..19afb5ce 100644 --- a/chrome/browser/extensions/extension_bindings_apitest.cc +++ b/chrome/browser/extensions/extension_bindings_apitest.cc
@@ -293,5 +293,27 @@ ASSERT_TRUE(catcher.GetNextResult()) << catcher.message(); } +// crbug.com/733337 +IN_PROC_BROWSER_TEST_F(ExtensionBindingsApiTest, ValidationInterception) { + // We need to create runtime bindings in the web page. An extension that's + // externally connectable will do that for us. + ASSERT_TRUE( + LoadExtension(test_data_dir_.AppendASCII("bindings") + .AppendASCII("externally_connectable_everywhere"))); + + content::WebContents* web_contents = + browser()->tab_strip_model()->GetActiveWebContents(); + ui_test_utils::NavigateToURL( + browser(), + embedded_test_server()->GetURL( + "/extensions/api_test/bindings/validation_interception.html")); + content::WaitForLoadStop(web_contents); + ASSERT_FALSE(web_contents->IsCrashed()); + bool caught = false; + ASSERT_TRUE(content::ExecuteScriptAndExtractBool( + web_contents, "domAutomationController.send(caught)", &caught)); + EXPECT_TRUE(caught); +} + } // namespace } // namespace extensions
diff --git a/chrome/browser/ui/android/content_settings/ads_blocked_infobar_delegate.cc b/chrome/browser/ui/android/content_settings/ads_blocked_infobar_delegate.cc index 8cf8e49..f5f7bc0 100644 --- a/chrome/browser/ui/android/content_settings/ads_blocked_infobar_delegate.cc +++ b/chrome/browser/ui/android/content_settings/ads_blocked_infobar_delegate.cc
@@ -37,8 +37,8 @@ base::string16 AdsBlockedInfobarDelegate::GetToggleText() const { return base::FeatureList::IsEnabled( subresource_filter::kSafeBrowsingSubresourceFilterExperimentalUI) - ? l10n_util::GetStringUTF16(IDS_ALWAYS_SHOW_ADS) - : l10n_util::GetStringUTF16(IDS_SHOW_ADS); + ? l10n_util::GetStringUTF16(IDS_ALWAYS_ALLOW_ADS) + : l10n_util::GetStringUTF16(IDS_ALLOW_ADS); } infobars::InfoBarDelegate::InfoBarIdentifier
diff --git a/chrome/browser/ui/views/omnibox/omnibox_result_view.cc b/chrome/browser/ui/views/omnibox/omnibox_result_view.cc index 6d29e0a..e719b73 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_result_view.cc +++ b/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
@@ -50,9 +50,13 @@ namespace { -// The padding that should be placed between content and description in a -// vertical layout. -static const int kVerticalPadding = 3; +// The vertical margin that should be used above and below each suggestion. +static const int kVerticalMargin = 1; + +// The vertical padding to provide each RenderText in addition to the height of +// the font. Where possible, RenderText uses this additional space to vertically +// center the cap height of the font instead of centering the entire font. +static const int kVerticalPadding = 4; // A mapping from OmniboxResultView's ResultViewState/ColorKind types to // NativeTheme colors. @@ -303,9 +307,9 @@ gfx::Size OmniboxResultView::CalculatePreferredSize() const { int height = GetTextHeight() + (2 * GetVerticalMargin()); if (match_.answer) - height += GetAnswerHeight() + kVerticalPadding; + height += GetAnswerHeight(); else if (base::FeatureList::IsEnabled(omnibox::kUIExperimentVerticalLayout)) - height += GetTextHeight() + kVerticalPadding; + height += GetTextHeight(); return gfx::Size(0, height); } @@ -332,7 +336,7 @@ } int OmniboxResultView::GetTextHeight() const { - return font_height_; + return font_height_ + kVerticalPadding; } void OmniboxResultView::PaintMatch(const AutocompleteMatch& match, @@ -367,12 +371,15 @@ // Answers in Suggest results. if (match.answer && description_max_width != 0) { DrawRenderText(match, contents, CONTENTS, canvas, x, y, contents_max_width); - y += GetTextHeight() + kVerticalPadding; + y += GetTextHeight(); if (!answer_image_.isNull()) { - int answer_icon_size = GetAnswerHeight(); + // GetAnswerHeight includes some padding. Using that results in an image + // that's too large so we use the font height here instead. + int answer_icon_size = GetAnswerFont().GetHeight(); canvas->DrawImageInt(answer_image_, 0, 0, answer_image_.width(), - answer_image_.height(), GetMirroredXInView(x), y, - answer_icon_size, answer_icon_size, true); + answer_image_.height(), GetMirroredXInView(x), + y + (kVerticalPadding / 2), answer_icon_size, + answer_icon_size, true); // TODO(dschuyler): Perhaps this should be based on the font size // instead of hardcoded to 2 dp (e.g. by adding a space in an // appropriate font to the beginning of the description, then reducing @@ -389,12 +396,12 @@ if (base::FeatureList::IsEnabled(omnibox::kUIExperimentVerticalLayout)) { // For no description, shift down halfways to draw contents in middle. if (description_max_width == 0) - y += (GetTextHeight() + kVerticalPadding) / 2; + y += GetTextHeight() / 2; DrawRenderText(match, contents, CONTENTS, canvas, x, y, contents_max_width); if (description_max_width != 0) { - y += GetTextHeight() + kVerticalPadding; + y += GetTextHeight(); DrawRenderText(match, description, DESCRIPTION, canvas, x, y, description_max_width); } @@ -647,7 +654,7 @@ int row_height = GetTextHeight(); if (base::FeatureList::IsEnabled(omnibox::kUIExperimentVerticalLayout)) - row_height += kVerticalPadding + GetTextHeight(); + row_height += match_.answer ? GetAnswerHeight() : GetTextHeight(); const int icon_y = GetVerticalMargin() + (row_height - icon.height()) / 2; icon_bounds_.SetRect(start_x, icon_y, icon.width(), icon.height()); @@ -747,7 +754,7 @@ // If the answer specifies a maximum of 1 line we can simply return the answer // font height. if (match_.answer->second_line().num_text_lines() == 1) - return GetAnswerFont().GetHeight(); + return GetAnswerFont().GetHeight() + kVerticalPadding; // Multi-line answers require layout in order to determine the number of lines // the RenderText will use. @@ -757,7 +764,9 @@ } description_rendertext_->SetDisplayRect(gfx::Rect(text_bounds_.width(), 0)); description_rendertext_->GetStringSize(); - return GetAnswerFont().GetHeight() * description_rendertext_->GetNumLines(); + return (GetAnswerFont().GetHeight() * + description_rendertext_->GetNumLines()) + + kVerticalPadding; } int OmniboxResultView::GetVerticalMargin() const { @@ -771,7 +780,7 @@ Md::GetMode() == Md::MATERIAL_HYBRID ? 8 : 4); const int min_height = LocationBarView::kIconWidth + 2 * kIconVerticalPad; - return std::max(kVerticalPadding, (min_height - GetTextHeight()) / 2); + return std::max(kVerticalMargin, (min_height - GetTextHeight()) / 2); } std::unique_ptr<gfx::RenderText> OmniboxResultView::CreateAnswerText(
diff --git a/chrome/test/data/extensions/api_test/bindings/validation_interception.html b/chrome/test/data/extensions/api_test/bindings/validation_interception.html new file mode 100644 index 0000000..b687dcf --- /dev/null +++ b/chrome/test/data/extensions/api_test/bindings/validation_interception.html
@@ -0,0 +1,22 @@ +<html> +<script> +var v3 = {}; + +Object.prototype.__defineGetter__(1, function() { + return v3; +}); + +Object.prototype.__defineSetter__(1, function() { + this[0] = 42; +}); + +var caught = false; +try { + chrome.runtime.connect(); +} catch (e) { + caught = true; + console.warn('Message: ' + e.message); +} + +</script> +</html>
diff --git a/components/autofill/ios/browser/resources/autofill_controller.js b/components/autofill/ios/browser/resources/autofill_controller.js index 060db199..899427d 100644 --- a/components/autofill/ios/browser/resources/autofill_controller.js +++ b/components/autofill/ios/browser/resources/autofill_controller.js
@@ -286,8 +286,9 @@ * It is based on the logic in * bool ExtractFieldsFromControlElements( * const WebVector<WebFormControlElement>& control_elements, + * const FieldValueAndPropertiesMaskMap* field_value_and_properties_map, * ExtractMask extract_mask, - * ScopedVector<FormFieldData>* form_fields, + * std::vector<std::unique_ptr<FormFieldData>>* form_fields, * std::vector<bool>* fields_extracted, * std::map<WebFormControlElement, FormFieldData*>* element_map) * in chromium/src/components/autofill/content/renderer/form_autofill_util.cc
diff --git a/components/cronet/android/cronet_library_loader.cc b/components/cronet/android/cronet_library_loader.cc index 1c463d3..72c416e 100644 --- a/components/cronet/android/cronet_library_loader.cc +++ b/components/cronet/android/cronet_library_loader.cc
@@ -84,8 +84,7 @@ jint CronetOnLoad(JavaVM* vm, void* reserved) { base::android::InitVM(vm); JNIEnv* env = base::android::AttachCurrentThread(); - if (!base::android::OnJNIOnLoadRegisterJNI(env) || !RegisterJNI(env) || - !NativeInit()) { + if (!RegisterJNI(env) || !NativeInit()) { return -1; } return JNI_VERSION_1_6;
diff --git a/components/cronet/android/test/cronet_test_jni.cc b/components/cronet/android/test/cronet_test_jni.cc index 81431e6..7f4b219 100644 --- a/components/cronet/android/test/cronet_test_jni.cc +++ b/components/cronet/android/test/cronet_test_jni.cc
@@ -41,8 +41,7 @@ extern "C" jint JNI_OnLoad(JavaVM* vm, void* reserved) { base::android::InitVM(vm); JNIEnv* env = base::android::AttachCurrentThread(); - if (!base::android::OnJNIOnLoadRegisterJNI(env) || - !base::android::OnJNIOnLoadInit()) { + if (!base::android::OnJNIOnLoadInit()) { return -1; }
diff --git a/components/infobars/core/infobar_manager.h b/components/infobars/core/infobar_manager.h index 9097dfc2..fb8cbcf 100644 --- a/components/infobars/core/infobar_manager.h +++ b/components/infobars/core/infobar_manager.h
@@ -118,10 +118,10 @@ private: // InfoBars associated with this InfoBarManager. We own these pointers. - // However, this is not a ScopedVector, because we don't delete the infobars - // directly once they've been added to this; instead, when we're done with an - // infobar, we instruct it to delete itself and then orphan it. See - // RemoveInfoBarInternal(). + // However, this is not a vector of unique_ptr, because we don't delete the + // infobars directly once they've been added to this; instead, when we're + // done with an infobar, we instruct it to delete itself and then orphan it. + // See RemoveInfoBarInternal(). typedef std::vector<InfoBar*> InfoBars; void RemoveInfoBarInternal(InfoBar* infobar, bool animate);
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 8e8a0903..f0cce6dba 100644 --- a/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc +++ b/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc
@@ -331,18 +331,18 @@ notify->OnImageDataFetched(id, "1-by-1-image-data"); } -gfx::Image FetchImage(RemoteSuggestionsProviderImpl* service, +gfx::Image FetchImage(RemoteSuggestionsProviderImpl* provider, const ContentSuggestion::ID& suggestion_id) { gfx::Image result; base::RunLoop run_loop; - service->FetchSuggestionImage(suggestion_id, - base::Bind( - [](base::Closure signal, gfx::Image* output, - const gfx::Image& loaded) { - *output = loaded; - signal.Run(); - }, - run_loop.QuitClosure(), &result)); + provider->FetchSuggestionImage( + suggestion_id, base::Bind( + [](base::Closure signal, gfx::Image* output, + const gfx::Image& loaded) { + *output = loaded; + signal.Run(); + }, + run_loop.QuitClosure(), &result)); run_loop.Run(); return result; } @@ -473,13 +473,14 @@ } // TODO(vitaliii): Rewrite this function to initialize a test class member - // instead of creating a new service. + // instead of creating a new provider. std::unique_ptr<RemoteSuggestionsProviderImpl> MakeSuggestionsProvider( bool set_empty_response = true) { - auto service = MakeSuggestionsProviderWithoutInitialization( + auto provider = MakeSuggestionsProviderWithoutInitialization( /*use_mock_suggestions_fetcher=*/false); - WaitForSuggestionsProviderInitialization(service.get(), set_empty_response); - return service; + WaitForSuggestionsProviderInitialization(provider.get(), + set_empty_response); + return provider; } // TODO(vitaliii): Rewrite tests and always use mock suggestions fetcher. @@ -534,12 +535,12 @@ } void WaitForSuggestionsProviderInitialization( - RemoteSuggestionsProviderImpl* service, + RemoteSuggestionsProviderImpl* provider, bool set_empty_response) { EXPECT_EQ(RemoteSuggestionsProviderImpl::State::NOT_INITED, - service->state_); + provider->state_); - // Add an initial fetch response, as the service tries to fetch when there + // Add an initial fetch response, as the provider tries to fetch when there // is nothing in the DB. if (set_empty_response) { SetUpFetchResponse(GetTestJson(std::vector<std::string>())); @@ -548,15 +549,15 @@ // TODO(treib): Find a better way to wait for initialization to finish. base::RunLoop().RunUntilIdle(); EXPECT_NE(RemoteSuggestionsProviderImpl::State::NOT_INITED, - service->state_); + provider->state_); } void ResetSuggestionsProvider( - std::unique_ptr<RemoteSuggestionsProviderImpl>* service, + std::unique_ptr<RemoteSuggestionsProviderImpl>* provider, bool set_empty_response) { - service->reset(); + provider->reset(); observer_.reset(); - *service = MakeSuggestionsProvider(set_empty_response); + *provider = MakeSuggestionsProvider(set_empty_response); } void SetCategoryRanker(std::unique_ptr<CategoryRanker> category_ranker) { @@ -617,15 +618,16 @@ provider->FetchSuggestions(interactive_request, callback); } - void LoadFromJSONString(RemoteSuggestionsProviderImpl* service, + void LoadFromJSONString(RemoteSuggestionsProviderImpl* provider, const std::string& json) { SetUpFetchResponse(json); - service->FetchSuggestions(/*interactive_request=*/true, - RemoteSuggestionsProvider::FetchStatusCallback()); + provider->FetchSuggestions( + /*interactive_request=*/true, + RemoteSuggestionsProvider::FetchStatusCallback()); base::RunLoop().RunUntilIdle(); } - void LoadMoreFromJSONString(RemoteSuggestionsProviderImpl* service, + void LoadMoreFromJSONString(RemoteSuggestionsProviderImpl* provider, const Category& category, const std::string& json, const std::set<std::string>& known_ids, @@ -634,7 +636,7 @@ EXPECT_CALL(*scheduler(), AcquireQuotaForInteractiveFetch()) .WillOnce(Return(true)) .RetiresOnSaturation(); - service->Fetch(category, known_ids, callback); + provider->Fetch(category, known_ids, callback); base::RunLoop().RunUntilIdle(); } @@ -677,13 +679,13 @@ TEST_F(RemoteSuggestionsProviderImplTest, Full) { std::string json_str(GetTestJson({GetSuggestion()})); - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); - LoadFromJSONString(service.get(), json_str); + LoadFromJSONString(provider.get(), json_str); ASSERT_THAT(observer().SuggestionsForCategory(articles_category()), SizeIs(1)); - ASSERT_THAT(service->GetSuggestionsForTesting(articles_category()), + ASSERT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(1)); const ContentSuggestion& suggestion = @@ -703,10 +705,10 @@ // Don't send an initial response -- we want to test what happens without any // server status. - auto service = MakeSuggestionsProvider(/*set_empty_response=*/false); + auto provider = MakeSuggestionsProvider(/*set_empty_response=*/false); // The articles category should be there by default, and have a title. - CategoryInfo info_before = service->GetCategoryInfo(articles_category()); + CategoryInfo info_before = provider->GetCategoryInfo(articles_category()); ASSERT_THAT(info_before.title(), Not(IsEmpty())); ASSERT_THAT(info_before.title(), Not(Eq(test_default_title))); EXPECT_THAT(info_before.additional_action(), @@ -714,16 +716,16 @@ EXPECT_THAT(info_before.show_if_empty(), Eq(true)); std::string json_str_with_title(GetTestJson({GetSuggestion()})); - LoadFromJSONString(service.get(), json_str_with_title); + LoadFromJSONString(provider.get(), json_str_with_title); ASSERT_THAT(observer().SuggestionsForCategory(articles_category()), SizeIs(1)); - ASSERT_THAT(service->GetSuggestionsForTesting(articles_category()), + ASSERT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(1)); // The response contained a title, |kTestJsonDefaultCategoryTitle|. // Make sure we updated the title in the CategoryInfo. - CategoryInfo info_with_title = service->GetCategoryInfo(articles_category()); + CategoryInfo info_with_title = provider->GetCategoryInfo(articles_category()); EXPECT_THAT(info_before.title(), Not(Eq(info_with_title.title()))); EXPECT_THAT(test_default_title, Eq(info_with_title.title())); EXPECT_THAT(info_before.additional_action(), @@ -732,13 +734,13 @@ } TEST_F(RemoteSuggestionsProviderImplTest, MultipleCategories) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::string json_str = MultiCategoryJsonBuilder() .AddCategory({GetSuggestionN(0)}, /*remote_category_id=*/1) .AddCategory({GetSuggestionN(1)}, /*remote_category_id=*/2) .Build(); - LoadFromJSONString(service.get(), json_str); + LoadFromJSONString(provider.get(), json_str); ASSERT_THAT(observer().statuses(), Eq(std::map<Category, CategoryStatus, Category::CompareByID>{ @@ -746,9 +748,9 @@ {other_category(), CategoryStatus::AVAILABLE}, })); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(1)); - EXPECT_THAT(service->GetSuggestionsForTesting(other_category()), SizeIs(1)); + EXPECT_THAT(provider->GetSuggestionsForTesting(other_category()), SizeIs(1)); ASSERT_THAT(observer().SuggestionsForCategory(articles_category()), SizeIs(1)); @@ -780,15 +782,15 @@ } TEST_F(RemoteSuggestionsProviderImplTest, ArticleCategoryInfo) { - auto service = MakeSuggestionsProvider(); - CategoryInfo article_info = service->GetCategoryInfo(articles_category()); + auto provider = MakeSuggestionsProvider(); + CategoryInfo article_info = provider->GetCategoryInfo(articles_category()); EXPECT_THAT(article_info.additional_action(), Eq(ContentSuggestionsAdditionalAction::FETCH)); EXPECT_THAT(article_info.show_if_empty(), Eq(true)); } TEST_F(RemoteSuggestionsProviderImplTest, ExperimentalCategoryInfo) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::string json_str = MultiCategoryJsonBuilder() .AddCategory({GetSuggestionN(0)}, /*remote_category_id=*/1) @@ -796,9 +798,9 @@ .Build(); // Load data with multiple categories so that a new experimental category gets // registered. - LoadFromJSONString(service.get(), json_str); + LoadFromJSONString(provider.get(), json_str); - CategoryInfo info = service->GetCategoryInfo(unknown_category()); + CategoryInfo info = provider->GetCategoryInfo(unknown_category()); EXPECT_THAT(info.additional_action(), Eq(ContentSuggestionsAdditionalAction::NONE)); EXPECT_THAT(info.show_if_empty(), Eq(false)); @@ -825,8 +827,8 @@ EXPECT_CALL(*raw_mock_ranker, AppendCategoryIfNecessary(Category::FromRemoteCategory(12))); } - auto service = MakeSuggestionsProvider(/*set_empty_response=*/false); - LoadFromJSONString(service.get(), json_str); + auto provider = MakeSuggestionsProvider(/*set_empty_response=*/false); + LoadFromJSONString(provider.get(), json_str); } TEST_F(RemoteSuggestionsProviderImplTest, @@ -858,8 +860,8 @@ InsertCategoryAfterIfNecessary(Category::FromRemoteCategory(12), articles_category())); } - auto service = MakeSuggestionsProvider(/*set_empty_response=*/false); - LoadFromJSONString(service.get(), json_str); + auto provider = MakeSuggestionsProvider(/*set_empty_response=*/false); + LoadFromJSONString(provider.get(), json_str); } TEST_F( @@ -877,12 +879,12 @@ EXPECT_CALL(*raw_mock_ranker, InsertCategoryBeforeIfNecessary(_, _)).Times(0); EXPECT_CALL(*raw_mock_ranker, AppendCategoryIfNecessary(Category::FromRemoteCategory(11))); - auto service = MakeSuggestionsProvider(/*set_empty_response=*/false); - LoadFromJSONString(service.get(), json_str); + auto provider = MakeSuggestionsProvider(/*set_empty_response=*/false); + LoadFromJSONString(provider.get(), json_str); } TEST_F(RemoteSuggestionsProviderImplTest, PersistCategoryInfos) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); // TODO(vitaliii): Use |articles_category()| instead of constant ID below. std::string json_str = MultiCategoryJsonBuilder() @@ -891,7 +893,7 @@ .AddCategoryWithCustomTitle({GetSuggestionN(1)}, kUnknownRemoteCategoryId, "Other Things") .Build(); - LoadFromJSONString(service.get(), json_str); + LoadFromJSONString(provider.get(), json_str); ASSERT_EQ(observer().StatusForCategory(articles_category()), CategoryStatus::AVAILABLE); @@ -899,12 +901,12 @@ CategoryStatus::AVAILABLE); CategoryInfo info_articles_before = - service->GetCategoryInfo(articles_category()); + provider->GetCategoryInfo(articles_category()); CategoryInfo info_unknown_before = - service->GetCategoryInfo(unknown_category()); + provider->GetCategoryInfo(unknown_category()); - // Recreate the service to simulate a Chrome restart. - ResetSuggestionsProvider(&service, /*set_empty_response=*/true); + // Recreate the provider to simulate a Chrome restart. + ResetSuggestionsProvider(&provider, /*set_empty_response=*/true); // The categories should have been restored. ASSERT_NE(observer().StatusForCategory(articles_category()), @@ -918,26 +920,26 @@ CategoryStatus::AVAILABLE); CategoryInfo info_articles_after = - service->GetCategoryInfo(articles_category()); + provider->GetCategoryInfo(articles_category()); CategoryInfo info_unknown_after = - service->GetCategoryInfo(unknown_category()); + provider->GetCategoryInfo(unknown_category()); EXPECT_EQ(info_articles_before.title(), info_articles_after.title()); EXPECT_EQ(info_unknown_before.title(), info_unknown_after.title()); } TEST_F(RemoteSuggestionsProviderImplTest, PersistRemoteCategoryOrder) { - // We create a service with a normal ranker to store the order. + // We create a provider with a normal ranker to store the order. std::string json_str = MultiCategoryJsonBuilder() .AddCategory({GetSuggestionN(0)}, /*remote_category_id=*/11) .AddCategory({GetSuggestionN(1)}, /*remote_category_id=*/13) .AddCategory({GetSuggestionN(2)}, /*remote_category_id=*/12) .Build(); - auto service = MakeSuggestionsProvider(/*set_empty_response=*/false); - LoadFromJSONString(service.get(), json_str); + auto provider = MakeSuggestionsProvider(/*set_empty_response=*/false); + LoadFromJSONString(provider.get(), json_str); - // We manually recreate the service to simulate Chrome restart and enforce a + // We manually recreate the provider to simulate Chrome restart and enforce a // mock ranker. The response is cleared to ensure that the order is not // fetched. SetUpFetchResponse(""); @@ -959,24 +961,24 @@ EXPECT_CALL(*raw_mock_ranker, AppendCategoryIfNecessary(Category::FromRemoteCategory(12))); } - ResetSuggestionsProvider(&service, /*set_empty_response=*/false); + ResetSuggestionsProvider(&provider, /*set_empty_response=*/false); } TEST_F(RemoteSuggestionsProviderImplTest, PersistSuggestions) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::string json_str = MultiCategoryJsonBuilder() .AddCategory({GetSuggestionN(0)}, /*remote_category_id=*/1) .AddCategory({GetSuggestionN(2)}, /*remote_category_id=*/2) .Build(); - LoadFromJSONString(service.get(), json_str); + LoadFromJSONString(provider.get(), json_str); ASSERT_THAT(observer().SuggestionsForCategory(articles_category()), SizeIs(1)); ASSERT_THAT(observer().SuggestionsForCategory(other_category()), SizeIs(1)); - // Recreate the service to simulate a Chrome restart. - ResetSuggestionsProvider(&service, /*set_empty_response=*/true); + // Recreate the provider to simulate a Chrome restart. + ResetSuggestionsProvider(&provider, /*set_empty_response=*/true); // The suggestions in both categories should have been restored. EXPECT_THAT(observer().SuggestionsForCategory(articles_category()), @@ -986,29 +988,29 @@ TEST_F(RemoteSuggestionsProviderImplTest, DontNotifyIfNotAvailable) { // Get some suggestions into the database. - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::string json_str = MultiCategoryJsonBuilder() .AddCategory({GetSuggestionN(0)}, /*remote_category_id=*/1) .AddCategory({GetSuggestionN(1)}, /*remote_category_id=*/2) .Build(); - LoadFromJSONString(service.get(), json_str); + LoadFromJSONString(provider.get(), json_str); ASSERT_THAT(observer().SuggestionsForCategory(articles_category()), SizeIs(1)); ASSERT_THAT(observer().SuggestionsForCategory(other_category()), SizeIs(1)); - service.reset(); + provider.reset(); // Set the pref that disables remote suggestions. pref_service()->SetBoolean(prefs::kEnableSnippets, false); - // Recreate the service to simulate a Chrome start. - ResetSuggestionsProvider(&service, /*set_empty_response=*/true); + // Recreate the provider to simulate a Chrome start. + ResetSuggestionsProvider(&provider, /*set_empty_response=*/true); ASSERT_THAT(RemoteSuggestionsProviderImpl::State::DISABLED, - Eq(service->state_)); + Eq(provider->state_)); // Now the observer should not have received any suggestions. EXPECT_THAT(observer().SuggestionsForCategory(articles_category()), @@ -1017,41 +1019,63 @@ } TEST_F(RemoteSuggestionsProviderImplTest, Clear) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::string json_str(GetTestJson({GetSuggestion()})); - LoadFromJSONString(service.get(), json_str); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + LoadFromJSONString(provider.get(), json_str); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(1)); - service->ClearCachedSuggestions(articles_category()); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + provider->ClearCachedSuggestions(articles_category()); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), IsEmpty()); } TEST_F(RemoteSuggestionsProviderImplTest, ReplaceSuggestions) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::string first("http://first"); - LoadFromJSONString(service.get(), GetTestJson({GetSuggestionWithUrl(first)})); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + LoadFromJSONString(provider.get(), + GetTestJson({GetSuggestionWithUrl(first)})); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), ElementsAre(IdEq(first))); std::string second("http://second"); - LoadFromJSONString(service.get(), + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestionWithUrl(second)})); // The suggestions loaded last replace all that was loaded previously. - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), ElementsAre(IdEq(second))); } -TEST_F(RemoteSuggestionsProviderImplTest, LoadsAdditionalSuggestions) { - auto service = MakeSuggestionsProvider(); +TEST_F(RemoteSuggestionsProviderImplTest, + ShouldResolveFetchedSuggestionThumbnail) { + auto provider = MakeSuggestionsProvider(); - LoadFromJSONString(service.get(), + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestionWithUrl("http://first")})); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + ASSERT_THAT(provider->GetSuggestionsForTesting(articles_category()), + ElementsAre(IdEq("http://first"))); + + image_decoder()->SetDecodedImage(gfx::test::CreateImage(1, 1)); + ServeImageCallback serve_one_by_one_image_callback = + base::Bind(&ServeOneByOneImage, &provider->GetImageFetcherForTesting()); + EXPECT_CALL(*image_fetcher(), StartOrQueueNetworkRequest(_, _, _, _)) + .WillOnce(WithArgs<0, 2>( + Invoke(CreateFunctor(serve_one_by_one_image_callback)))); + + gfx::Image image = FetchImage(provider.get(), MakeArticleID("http://first")); + ASSERT_FALSE(image.IsEmpty()); + EXPECT_EQ(1, image.Width()); +} + +TEST_F(RemoteSuggestionsProviderImplTest, ShouldFetchMore) { + auto provider = MakeSuggestionsProvider(); + + LoadFromJSONString(provider.get(), + GetTestJson({GetSuggestionWithUrl("http://first")})); + ASSERT_THAT(provider->GetSuggestionsForTesting(articles_category()), ElementsAre(IdEq("http://first"))); auto expect_only_second_suggestion_received = @@ -1060,251 +1084,220 @@ EXPECT_THAT(suggestions[0].id().id_within_category(), Eq("http://second")); }); - LoadMoreFromJSONString(service.get(), articles_category(), + LoadMoreFromJSONString(provider.get(), articles_category(), GetTestJson({GetSuggestionWithUrl("http://second")}), /*known_ids=*/std::set<std::string>(), expect_only_second_suggestion_received); +} - // Verify we can resolve the image of the new suggestions. - ServeImageCallback cb = - base::Bind(&ServeOneByOneImage, &service->GetImageFetcherForTesting()); - EXPECT_CALL(*image_fetcher(), StartOrQueueNetworkRequest(_, _, _, _)) - .Times(2) - .WillRepeatedly(WithArgs<0, 2>(Invoke(CreateFunctor(cb)))); +TEST_F(RemoteSuggestionsProviderImplTest, + ShouldResolveFetchedMoreSuggestionThumbnail) { + auto provider = MakeSuggestionsProvider(); + + auto assert_only_first_suggestion_received = + base::Bind([](Status status, std::vector<ContentSuggestion> suggestions) { + ASSERT_THAT(suggestions, SizeIs(1)); + ASSERT_THAT(suggestions[0].id().id_within_category(), + Eq("http://first")); + }); + LoadMoreFromJSONString(provider.get(), articles_category(), + GetTestJson({GetSuggestionWithUrl("http://first")}), + /*known_ids=*/std::set<std::string>(), + assert_only_first_suggestion_received); + image_decoder()->SetDecodedImage(gfx::test::CreateImage(1, 1)); - gfx::Image image = FetchImage(service.get(), MakeArticleID("http://first")); - EXPECT_FALSE(image.IsEmpty()); - EXPECT_EQ(1, image.Width()); + ServeImageCallback serve_one_by_one_image_callback = + base::Bind(&ServeOneByOneImage, &provider->GetImageFetcherForTesting()); + EXPECT_CALL(*image_fetcher(), StartOrQueueNetworkRequest(_, _, _, _)) + .WillOnce(WithArgs<0, 2>( + Invoke(CreateFunctor(serve_one_by_one_image_callback)))); - image = FetchImage(service.get(), MakeArticleID("http://second")); - EXPECT_FALSE(image.IsEmpty()); + gfx::Image image = FetchImage(provider.get(), MakeArticleID("http://first")); + ASSERT_FALSE(image.IsEmpty()); EXPECT_EQ(1, image.Width()); } -// The tests TestMergingFetchedMoreSuggestionsFillup and -// TestMergingFetchedMoreSuggestionsReplaceAll simulate the following user -// story: -// 1) fetch suggestions in NTP A -// 2) fetch more suggestions in NTP A. -// 3) open new NTP B: See the first 10 results from step 1). -// 4) fetch more suggestions in NTP B. Make sure the results are independent -// from step 2) -// TODO(tschumann): Test step 4) on a higher level instead of peeking into the -// internal 'dismissed' data. The proper check is to make sure we tell the -// backend to exclude these suggestions. +// Imagine that we have surfaces A and B. The user fetches more in A, this +// should not add any suggestions to B. TEST_F(RemoteSuggestionsProviderImplTest, - FewMoreFetchedSuggestionsShouldNotInterfere) { - auto service = MakeSuggestionsProvider(/*set_empty_response=*/false); - LoadFromJSONString(service.get(), - GetTestJson({GetSuggestionWithUrl("http://id-1"), - GetSuggestionWithUrl("http://id-2"), - GetSuggestionWithUrl("http://id-3"), - GetSuggestionWithUrl("http://id-4"), - GetSuggestionWithUrl("http://id-5"), - GetSuggestionWithUrl("http://id-6"), - GetSuggestionWithUrl("http://id-7"), - GetSuggestionWithUrl("http://id-8"), - GetSuggestionWithUrl("http://id-9"), - GetSuggestionWithUrl("http://id-10")})); - EXPECT_THAT( - observer().SuggestionsForCategory(articles_category()), - ElementsAre( - IdWithinCategoryEq("http://id-1"), IdWithinCategoryEq("http://id-2"), - IdWithinCategoryEq("http://id-3"), IdWithinCategoryEq("http://id-4"), - IdWithinCategoryEq("http://id-5"), IdWithinCategoryEq("http://id-6"), - IdWithinCategoryEq("http://id-7"), IdWithinCategoryEq("http://id-8"), - IdWithinCategoryEq("http://id-9"), - IdWithinCategoryEq("http://id-10"))); + ShouldNotChangeSuggestionsInOtherSurfacesWhenFetchingMore) { + auto provider = MakeSuggestionsProviderWithoutInitialization( + /*use_mock_suggestions_fetcher=*/true); + WaitForSuggestionsProviderInitialization(provider.get(), + /*set_empty_response=*/true); - auto expect_receiving_two_new_suggestions = + // Fetch a suggestion. + auto* mock_fetcher = static_cast<StrictMock<MockRemoteSuggestionsFetcher>*>( + suggestions_fetcher()); + std::vector<FetchedCategory> fetched_categories; + fetched_categories.push_back(FetchedCategory( + articles_category(), + BuildRemoteCategoryInfo(base::UTF8ToUTF16("title"), + /*allow_fetching_more_results=*/true))); + fetched_categories[0].suggestions.push_back( + CreateTestRemoteSuggestion("http://old.com/")); + + RemoteSuggestionsFetcher::SnippetsAvailableCallback snippets_callback; + EXPECT_CALL(*mock_fetcher, FetchSnippets(_, _)) + .WillOnce(MoveSecondArgumentPointeeTo(&snippets_callback)) + .RetiresOnSaturation(); + FetchSuggestions(provider.get(), /*interactive_request=*/true, + RemoteSuggestionsProvider::FetchStatusCallback()); + std::move(snippets_callback) + .Run(Status(StatusCode::SUCCESS, "message"), + std::move(fetched_categories)); + + ASSERT_THAT(observer().SuggestionsForCategory(articles_category()), + ElementsAre(IdWithinCategoryEq("http://old.com/"))); + + // Now fetch more, but first prepare a response. + fetched_categories.push_back(FetchedCategory( + articles_category(), + BuildRemoteCategoryInfo(base::UTF8ToUTF16("title"), + /*allow_fetching_more_results=*/true))); + fetched_categories[0].suggestions.push_back( + CreateTestRemoteSuggestion("http://fetched-more.com/")); + + // The surface issuing the fetch more gets response via callback. + auto assert_receiving_one_new_suggestion = base::Bind([](Status status, std::vector<ContentSuggestion> suggestions) { - ASSERT_THAT(suggestions, SizeIs(2)); - EXPECT_THAT(suggestions[0], IdWithinCategoryEq("http://more-id-1")); - EXPECT_THAT(suggestions[1], IdWithinCategoryEq("http://more-id-2")); + ASSERT_THAT(suggestions, SizeIs(1)); + ASSERT_THAT(suggestions[0], + IdWithinCategoryEq("http://fetched-more.com/")); }); - LoadMoreFromJSONString( - service.get(), articles_category(), - GetTestJson({GetSuggestionWithUrl("http://more-id-1"), - GetSuggestionWithUrl("http://more-id-2")}), - /*known_ids=*/ - {"http://id-1", "http://id-2", "http://id-3", "http://id-4", - "http://id-5", "http://id-6", "http://id-7", "http://id-8", - "http://id-9", "http://id-10"}, - expect_receiving_two_new_suggestions); - - // Verify that the observer still has the old set. - EXPECT_THAT( - observer().SuggestionsForCategory(articles_category()), - ElementsAre( - IdWithinCategoryEq("http://id-1"), IdWithinCategoryEq("http://id-2"), - IdWithinCategoryEq("http://id-3"), IdWithinCategoryEq("http://id-4"), - IdWithinCategoryEq("http://id-5"), IdWithinCategoryEq("http://id-6"), - IdWithinCategoryEq("http://id-7"), IdWithinCategoryEq("http://id-8"), - IdWithinCategoryEq("http://id-9"), - IdWithinCategoryEq("http://id-10"))); - - // No interference from previous Fetch more: we can receive two other ones. - expect_receiving_two_new_suggestions = - base::Bind([](Status status, std::vector<ContentSuggestion> suggestions) { - ASSERT_THAT(suggestions, SizeIs(2)); - EXPECT_THAT(suggestions[0], IdWithinCategoryEq("http://more-id-3")); - EXPECT_THAT(suggestions[1], IdWithinCategoryEq("http://more-id-4")); - }); - LoadMoreFromJSONString( - service.get(), articles_category(), - GetTestJson({GetSuggestionWithUrl("http://more-id-3"), - GetSuggestionWithUrl("http://more-id-4")}), - /*known_ids=*/ - {"http://id-1", "http://id-2", "http://id-3", "http://id-4", - "http://id-5", "http://id-6", "http://id-7", "http://id-8", - "http://id-9", "http://id-10"}, - expect_receiving_two_new_suggestions); + EXPECT_CALL(*mock_fetcher, FetchSnippets(_, _)) + .WillOnce(MoveSecondArgumentPointeeTo(&snippets_callback)) + .RetiresOnSaturation(); + EXPECT_CALL(*scheduler(), AcquireQuotaForInteractiveFetch()) + .WillOnce(Return(true)) + .RetiresOnSaturation(); + provider->Fetch(articles_category(), + /*known_suggestion_ids=*/{"http://old.com/"}, + assert_receiving_one_new_suggestion); + std::move(snippets_callback) + .Run(Status(StatusCode::SUCCESS, "message"), + std::move(fetched_categories)); + // Other surfaces should remain the same. + EXPECT_THAT(observer().SuggestionsForCategory(articles_category()), + ElementsAre(IdWithinCategoryEq("http://old.com/"))); } +// Imagine that we have surfaces A and B. The user fetches more in A. This +// should not affect the next fetch more in B, i.e. assuming the same server +// response the same suggestions must be fetched in B if the user fetches more +// there as well. TEST_F(RemoteSuggestionsProviderImplTest, - TenMoreFetchedSuggestionsShouldNotInterfere) { - auto service = MakeSuggestionsProvider(/*set_empty_response=*/false); - LoadFromJSONString(service.get(), - GetTestJson({GetSuggestionWithUrl("http://id-1"), - GetSuggestionWithUrl("http://id-2"), - GetSuggestionWithUrl("http://id-3"), - GetSuggestionWithUrl("http://id-4"), - GetSuggestionWithUrl("http://id-5"), - GetSuggestionWithUrl("http://id-6"), - GetSuggestionWithUrl("http://id-7"), - GetSuggestionWithUrl("http://id-8"), - GetSuggestionWithUrl("http://id-9"), - GetSuggestionWithUrl("http://id-10")})); - EXPECT_THAT( - observer().SuggestionsForCategory(articles_category()), - ElementsAre( - IdWithinCategoryEq("http://id-1"), IdWithinCategoryEq("http://id-2"), - IdWithinCategoryEq("http://id-3"), IdWithinCategoryEq("http://id-4"), - IdWithinCategoryEq("http://id-5"), IdWithinCategoryEq("http://id-6"), - IdWithinCategoryEq("http://id-7"), IdWithinCategoryEq("http://id-8"), - IdWithinCategoryEq("http://id-9"), - IdWithinCategoryEq("http://id-10"))); + ShouldNotAffectFetchMoreInOtherSurfacesWhenFetchingMore) { + auto provider = MakeSuggestionsProviderWithoutInitialization( + /*use_mock_suggestions_fetcher=*/true); + WaitForSuggestionsProviderInitialization(provider.get(), + /*set_empty_response=*/true); + auto* mock_fetcher = static_cast<StrictMock<MockRemoteSuggestionsFetcher>*>( + suggestions_fetcher()); - auto expect_receiving_ten_new_suggestions = - base::Bind([](Status status, std::vector<ContentSuggestion> suggestions) { - EXPECT_THAT(suggestions, - ElementsAre(IdWithinCategoryEq("http://more-id-1"), - IdWithinCategoryEq("http://more-id-2"), - IdWithinCategoryEq("http://more-id-3"), - IdWithinCategoryEq("http://more-id-4"), - IdWithinCategoryEq("http://more-id-5"), - IdWithinCategoryEq("http://more-id-6"), - IdWithinCategoryEq("http://more-id-7"), - IdWithinCategoryEq("http://more-id-8"), - IdWithinCategoryEq("http://more-id-9"), - IdWithinCategoryEq("http://more-id-10"))); - }); - LoadMoreFromJSONString( - service.get(), articles_category(), - GetTestJson({GetSuggestionWithUrl("http://more-id-1"), - GetSuggestionWithUrl("http://more-id-2"), - GetSuggestionWithUrl("http://more-id-3"), - GetSuggestionWithUrl("http://more-id-4"), - GetSuggestionWithUrl("http://more-id-5"), - GetSuggestionWithUrl("http://more-id-6"), - GetSuggestionWithUrl("http://more-id-7"), - GetSuggestionWithUrl("http://more-id-8"), - GetSuggestionWithUrl("http://more-id-9"), - GetSuggestionWithUrl("http://more-id-10")}), - /*known_ids=*/ - {"http://id-1", "http://id-2", "http://id-3", "http://id-4", - "http://id-5", "http://id-6", "http://id-7", "http://id-8", - "http://id-9", "http://id-10"}, - expect_receiving_ten_new_suggestions); - EXPECT_THAT( - observer().SuggestionsForCategory(articles_category()), - ElementsAre( - IdWithinCategoryEq("http://id-1"), IdWithinCategoryEq("http://id-2"), - IdWithinCategoryEq("http://id-3"), IdWithinCategoryEq("http://id-4"), - IdWithinCategoryEq("http://id-5"), IdWithinCategoryEq("http://id-6"), - IdWithinCategoryEq("http://id-7"), IdWithinCategoryEq("http://id-8"), - IdWithinCategoryEq("http://id-9"), - IdWithinCategoryEq("http://id-10"))); + // Fetch more on the surface A. + std::vector<FetchedCategory> fetched_categories; + fetched_categories.push_back(FetchedCategory( + articles_category(), + BuildRemoteCategoryInfo(base::UTF8ToUTF16("title"), + /*allow_fetching_more_results=*/true))); + fetched_categories[0].suggestions.push_back( + CreateTestRemoteSuggestion("http://fetched-more.com/")); - // This time, test receiving the same set. - expect_receiving_ten_new_suggestions = + auto assert_receiving_one_new_suggestion = base::Bind([](Status status, std::vector<ContentSuggestion> suggestions) { - EXPECT_THAT(suggestions, - ElementsAre(IdWithinCategoryEq("http://more-id-1"), - IdWithinCategoryEq("http://more-id-2"), - IdWithinCategoryEq("http://more-id-3"), - IdWithinCategoryEq("http://more-id-4"), - IdWithinCategoryEq("http://more-id-5"), - IdWithinCategoryEq("http://more-id-6"), - IdWithinCategoryEq("http://more-id-7"), - IdWithinCategoryEq("http://more-id-8"), - IdWithinCategoryEq("http://more-id-9"), - IdWithinCategoryEq("http://more-id-10"))); + ASSERT_THAT(suggestions, SizeIs(1)); + ASSERT_THAT(suggestions[0], + IdWithinCategoryEq("http://fetched-more.com/")); }); - LoadMoreFromJSONString( - service.get(), articles_category(), - GetTestJson({GetSuggestionWithUrl("http://more-id-1"), - GetSuggestionWithUrl("http://more-id-2"), - GetSuggestionWithUrl("http://more-id-3"), - GetSuggestionWithUrl("http://more-id-4"), - GetSuggestionWithUrl("http://more-id-5"), - GetSuggestionWithUrl("http://more-id-6"), - GetSuggestionWithUrl("http://more-id-7"), - GetSuggestionWithUrl("http://more-id-8"), - GetSuggestionWithUrl("http://more-id-9"), - GetSuggestionWithUrl("http://more-id-10")}), - /*known_ids=*/ - {"http://id-1", "http://id-2", "http://id-3", "http://id-4", - "http://id-5", "http://id-6", "http://id-7", "http://id-8", - "http://id-9", "http://id-10"}, - expect_receiving_ten_new_suggestions); + RemoteSuggestionsFetcher::SnippetsAvailableCallback snippets_callback; + EXPECT_CALL(*mock_fetcher, FetchSnippets(_, _)) + .WillOnce(MoveSecondArgumentPointeeTo(&snippets_callback)) + .RetiresOnSaturation(); + EXPECT_CALL(*scheduler(), AcquireQuotaForInteractiveFetch()) + .WillOnce(Return(true)) + .RetiresOnSaturation(); + provider->Fetch(articles_category(), + /*known_suggestion_ids=*/std::set<std::string>(), + assert_receiving_one_new_suggestion); + std::move(snippets_callback) + .Run(Status(StatusCode::SUCCESS, "message"), + std::move(fetched_categories)); + + // Now fetch more on the surface B. The response is the same as before. + fetched_categories.push_back(FetchedCategory( + articles_category(), + BuildRemoteCategoryInfo(base::UTF8ToUTF16("title"), + /*allow_fetching_more_results=*/true))); + fetched_categories[0].suggestions.push_back( + CreateTestRemoteSuggestion("http://fetched-more.com/")); + + // B should receive the same suggestion as was fetched more on A. + auto expect_receiving_same_suggestion = + base::Bind([](Status status, std::vector<ContentSuggestion> suggestions) { + ASSERT_THAT(suggestions, SizeIs(1)); + EXPECT_THAT(suggestions[0], + IdWithinCategoryEq("http://fetched-more.com/")); + }); + // The provider should not ask the fetcher to exclude the suggestion fetched + // more on A. + EXPECT_CALL(*mock_fetcher, + FetchSnippets(Field(&RequestParams::excluded_ids, + Not(Contains("http://fetched-more.com/"))), + _)) + .WillOnce(MoveSecondArgumentPointeeTo(&snippets_callback)) + .RetiresOnSaturation(); + EXPECT_CALL(*scheduler(), AcquireQuotaForInteractiveFetch()) + .WillOnce(Return(true)) + .RetiresOnSaturation(); + provider->Fetch(articles_category(), + /*known_suggestion_ids=*/std::set<std::string>(), + expect_receiving_same_suggestion); + std::move(snippets_callback) + .Run(Status(StatusCode::SUCCESS, "message"), + std::move(fetched_categories)); } TEST_F(RemoteSuggestionsProviderImplTest, ClearHistoryShouldDeleteArchivedSuggestions) { - auto service = MakeSuggestionsProvider(/*set_empty_response=*/false); + auto provider = MakeSuggestionsProvider(/*set_empty_response=*/false); // First get suggestions into the archived state which happens through // subsequent fetches. Then we verify the entries are gone from the 'archived' // state by trying to load their images (and we shouldn't even know the URLs // anymore). - LoadFromJSONString(service.get(), + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestionWithUrl("http://id-1"), GetSuggestionWithUrl("http://id-2")})); - LoadFromJSONString(service.get(), + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestionWithUrl("http://new-id-1"), GetSuggestionWithUrl("http://new-id-2")})); // Make sure images of both batches are available. This is to sanity check our // assumptions for the test are right. ServeImageCallback cb = - base::Bind(&ServeOneByOneImage, &service->GetImageFetcherForTesting()); + base::Bind(&ServeOneByOneImage, &provider->GetImageFetcherForTesting()); EXPECT_CALL(*image_fetcher(), StartOrQueueNetworkRequest(_, _, _, _)) .Times(2) .WillRepeatedly(WithArgs<0, 2>(Invoke(CreateFunctor(cb)))); image_decoder()->SetDecodedImage(gfx::test::CreateImage(1, 1)); - gfx::Image image = FetchImage(service.get(), MakeArticleID("http://id-1")); + gfx::Image image = FetchImage(provider.get(), MakeArticleID("http://id-1")); ASSERT_FALSE(image.IsEmpty()); ASSERT_EQ(1, image.Width()); - image = FetchImage(service.get(), MakeArticleID("http://new-id-1")); + image = FetchImage(provider.get(), MakeArticleID("http://new-id-1")); ASSERT_FALSE(image.IsEmpty()); ASSERT_EQ(1, image.Width()); - service->ClearHistory(base::Time::UnixEpoch(), base::Time::Max(), - base::Callback<bool(const GURL& url)>()); + provider->ClearHistory(base::Time::UnixEpoch(), base::Time::Max(), + base::Callback<bool(const GURL& url)>()); // Make sure images of both batches are gone. // Verify we cannot resolve the image of the new suggestions. image_decoder()->SetDecodedImage(gfx::test::CreateImage(1, 1)); EXPECT_TRUE( - FetchImage(service.get(), MakeArticleID("http://id-1")).IsEmpty()); + FetchImage(provider.get(), MakeArticleID("http://id-1")).IsEmpty()); EXPECT_TRUE( - FetchImage(service.get(), MakeArticleID("http://new-id-1")).IsEmpty()); + FetchImage(provider.get(), MakeArticleID("http://new-id-1")).IsEmpty()); } -// TODO(tschumann): We don't have test making sure the RemoteSuggestionsFetcher -// actually gets the proper parameters. Add tests with an injected -// RemoteSuggestionsFetcher to verify the parameters, including proper handling -// of dismissed and known_ids. - namespace { // Workaround for gMock's lack of support for movable types. @@ -1318,21 +1311,21 @@ } // namespace TEST_F(RemoteSuggestionsProviderImplTest, ReturnFetchRequestEmptyBeforeInit) { - auto service = MakeSuggestionsProviderWithoutInitialization( + auto provider = MakeSuggestionsProviderWithoutInitialization( /*use_mock_suggestions_fetcher=*/false); MockFunction<void(Status, const std::vector<ContentSuggestion>&)> loaded; EXPECT_CALL(loaded, Call(HasCode(StatusCode::TEMPORARY_ERROR), IsEmpty())); - service->Fetch(articles_category(), std::set<std::string>(), - base::Bind(&SuggestionsLoaded, &loaded)); + provider->Fetch(articles_category(), std::set<std::string>(), + base::Bind(&SuggestionsLoaded, &loaded)); base::RunLoop().RunUntilIdle(); } TEST_F(RemoteSuggestionsProviderImplTest, ReturnTemporaryErrorForInvalidJson) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); MockFunction<void(Status, const std::vector<ContentSuggestion>&)> loaded; EXPECT_CALL(loaded, Call(HasCode(StatusCode::TEMPORARY_ERROR), IsEmpty())); - LoadMoreFromJSONString(service.get(), articles_category(), + LoadMoreFromJSONString(provider.get(), articles_category(), "invalid json string}]}", /*known_ids=*/std::set<std::string>(), base::Bind(&SuggestionsLoaded, &loaded)); @@ -1342,11 +1335,11 @@ TEST_F(RemoteSuggestionsProviderImplTest, ReturnTemporaryErrorForInvalidSuggestion) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); MockFunction<void(Status, const std::vector<ContentSuggestion>&)> loaded; EXPECT_CALL(loaded, Call(HasCode(StatusCode::TEMPORARY_ERROR), IsEmpty())); - LoadMoreFromJSONString(service.get(), articles_category(), + LoadMoreFromJSONString(provider.get(), articles_category(), GetTestJson({GetIncompleteSuggestion()}), /*known_ids=*/std::set<std::string>(), base::Bind(&SuggestionsLoaded, &loaded)); @@ -1357,150 +1350,150 @@ TEST_F(RemoteSuggestionsProviderImplTest, ReturnTemporaryErrorForRequestFailure) { // Created SuggestionsProvider will fail by default with unsuccessful request. - auto service = MakeSuggestionsProvider(/*set_empty_response=*/false); + auto provider = MakeSuggestionsProvider(/*set_empty_response=*/false); MockFunction<void(Status, const std::vector<ContentSuggestion>&)> loaded; EXPECT_CALL(loaded, Call(HasCode(StatusCode::TEMPORARY_ERROR), IsEmpty())); - service->Fetch(articles_category(), - /*known_ids=*/std::set<std::string>(), - base::Bind(&SuggestionsLoaded, &loaded)); + provider->Fetch(articles_category(), + /*known_ids=*/std::set<std::string>(), + base::Bind(&SuggestionsLoaded, &loaded)); base::RunLoop().RunUntilIdle(); } TEST_F(RemoteSuggestionsProviderImplTest, ReturnTemporaryErrorForHttpFailure) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); SetUpHttpError(); MockFunction<void(Status, const std::vector<ContentSuggestion>&)> loaded; EXPECT_CALL(loaded, Call(HasCode(StatusCode::TEMPORARY_ERROR), IsEmpty())); - service->Fetch(articles_category(), - /*known_ids=*/std::set<std::string>(), - base::Bind(&SuggestionsLoaded, &loaded)); + provider->Fetch(articles_category(), + /*known_ids=*/std::set<std::string>(), + base::Bind(&SuggestionsLoaded, &loaded)); base::RunLoop().RunUntilIdle(); } TEST_F(RemoteSuggestionsProviderImplTest, LoadInvalidJson) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); - LoadFromJSONString(service.get(), GetTestJson({GetInvalidSuggestion()})); + LoadFromJSONString(provider.get(), GetTestJson({GetInvalidSuggestion()})); EXPECT_THAT(suggestions_fetcher()->GetLastStatusForDebugging(), StartsWith("Received invalid JSON")); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), IsEmpty()); } TEST_F(RemoteSuggestionsProviderImplTest, LoadInvalidJsonWithExistingSuggestions) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); - LoadFromJSONString(service.get(), GetTestJson({GetSuggestion()})); - ASSERT_THAT(service->GetSuggestionsForTesting(articles_category()), + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestion()})); + ASSERT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(1)); ASSERT_EQ("OK", suggestions_fetcher()->GetLastStatusForDebugging()); - LoadFromJSONString(service.get(), GetTestJson({GetInvalidSuggestion()})); + LoadFromJSONString(provider.get(), GetTestJson({GetInvalidSuggestion()})); EXPECT_THAT(suggestions_fetcher()->GetLastStatusForDebugging(), StartsWith("Received invalid JSON")); // This should not have changed the existing suggestions. - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(1)); } TEST_F(RemoteSuggestionsProviderImplTest, LoadIncompleteJson) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); - LoadFromJSONString(service.get(), GetTestJson({GetIncompleteSuggestion()})); + LoadFromJSONString(provider.get(), GetTestJson({GetIncompleteSuggestion()})); EXPECT_EQ("Invalid / empty list.", suggestions_fetcher()->GetLastStatusForDebugging()); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), IsEmpty()); } TEST_F(RemoteSuggestionsProviderImplTest, LoadIncompleteJsonWithExistingSuggestions) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); - LoadFromJSONString(service.get(), GetTestJson({GetSuggestion()})); - ASSERT_THAT(service->GetSuggestionsForTesting(articles_category()), + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestion()})); + ASSERT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(1)); - LoadFromJSONString(service.get(), GetTestJson({GetIncompleteSuggestion()})); + LoadFromJSONString(provider.get(), GetTestJson({GetIncompleteSuggestion()})); EXPECT_EQ("Invalid / empty list.", suggestions_fetcher()->GetLastStatusForDebugging()); // This should not have changed the existing suggestions. - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(1)); } TEST_F(RemoteSuggestionsProviderImplTest, Dismiss) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::string json_str(GetTestJson( {GetSuggestionWithSources("http://site.com", "Source 1", "")})); - LoadFromJSONString(service.get(), json_str); + LoadFromJSONString(provider.get(), json_str); - ASSERT_THAT(service->GetSuggestionsForTesting(articles_category()), + ASSERT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(1)); // Load the image to store it in the database. ServeImageCallback cb = - base::Bind(&ServeOneByOneImage, &service->GetImageFetcherForTesting()); + base::Bind(&ServeOneByOneImage, &provider->GetImageFetcherForTesting()); EXPECT_CALL(*image_fetcher(), StartOrQueueNetworkRequest(_, _, _, _)) .WillOnce(WithArgs<0, 2>(Invoke(CreateFunctor(cb)))); image_decoder()->SetDecodedImage(gfx::test::CreateImage(1, 1)); - gfx::Image image = FetchImage(service.get(), MakeArticleID(kSuggestionUrl)); + gfx::Image image = FetchImage(provider.get(), MakeArticleID(kSuggestionUrl)); EXPECT_FALSE(image.IsEmpty()); EXPECT_EQ(1, image.Width()); // Dismissing a non-existent suggestion shouldn't do anything. - service->DismissSuggestion(MakeArticleID("http://othersite.com")); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + provider->DismissSuggestion(MakeArticleID("http://othersite.com")); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(1)); // Dismiss the suggestion. - service->DismissSuggestion(MakeArticleID(kSuggestionUrl)); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + provider->DismissSuggestion(MakeArticleID(kSuggestionUrl)); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), IsEmpty()); // Verify we can still load the image of the discarded suggestion (other NTPs // might still reference it). This should come from the database -- no network // fetch necessary. image_decoder()->SetDecodedImage(gfx::test::CreateImage(1, 1)); - image = FetchImage(service.get(), MakeArticleID(kSuggestionUrl)); + image = FetchImage(provider.get(), MakeArticleID(kSuggestionUrl)); EXPECT_FALSE(image.IsEmpty()); EXPECT_EQ(1, image.Width()); // Make sure that fetching the same suggestion again does not re-add it. - LoadFromJSONString(service.get(), json_str); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + LoadFromJSONString(provider.get(), json_str); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), IsEmpty()); - // The suggestion should stay dismissed even after re-creating the service. - ResetSuggestionsProvider(&service, /*set_empty_response=*/true); - LoadFromJSONString(service.get(), json_str); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + // The suggestion should stay dismissed even after re-creating the provider. + ResetSuggestionsProvider(&provider, /*set_empty_response=*/true); + LoadFromJSONString(provider.get(), json_str); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), IsEmpty()); // The suggestion can be added again after clearing dismissed suggestions. - service->ClearDismissedSuggestionsForDebugging(articles_category()); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + provider->ClearDismissedSuggestionsForDebugging(articles_category()); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), IsEmpty()); - LoadFromJSONString(service.get(), json_str); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + LoadFromJSONString(provider.get(), json_str); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(1)); } TEST_F(RemoteSuggestionsProviderImplTest, GetDismissed) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); - LoadFromJSONString(service.get(), GetTestJson({GetSuggestion()})); + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestion()})); - service->DismissSuggestion(MakeArticleID(kSuggestionUrl)); + provider->DismissSuggestion(MakeArticleID(kSuggestionUrl)); - service->GetDismissedSuggestionsForDebugging( + provider->GetDismissedSuggestionsForDebugging( articles_category(), base::Bind( - [](RemoteSuggestionsProviderImpl* service, + [](RemoteSuggestionsProviderImpl* provider, RemoteSuggestionsProviderImplTest* test, std::vector<ContentSuggestion> dismissed_suggestions) { EXPECT_EQ(1u, dismissed_suggestions.size()); @@ -1508,25 +1501,25 @@ EXPECT_EQ(test->MakeArticleID(kSuggestionUrl), suggestion.id()); } }, - service.get(), this)); + provider.get(), this)); base::RunLoop().RunUntilIdle(); // There should be no dismissed suggestion after clearing the list. - service->ClearDismissedSuggestionsForDebugging(articles_category()); - service->GetDismissedSuggestionsForDebugging( + provider->ClearDismissedSuggestionsForDebugging(articles_category()); + provider->GetDismissedSuggestionsForDebugging( articles_category(), base::Bind( - [](RemoteSuggestionsProviderImpl* service, + [](RemoteSuggestionsProviderImpl* provider, RemoteSuggestionsProviderImplTest* test, std::vector<ContentSuggestion> dismissed_suggestions) { EXPECT_EQ(0u, dismissed_suggestions.size()); }, - service.get(), this)); + provider.get(), this)); base::RunLoop().RunUntilIdle(); } TEST_F(RemoteSuggestionsProviderImplTest, CreationTimestampParseFail) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::string json = GetSuggestionWithTimes(GetDefaultCreationTime(), GetDefaultExpirationTime()); @@ -1534,66 +1527,66 @@ &json, 0, FormatTime(GetDefaultCreationTime()), "aaa1448459205"); std::string json_str(GetTestJson({json})); - LoadFromJSONString(service.get(), json_str); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + LoadFromJSONString(provider.get(), json_str); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), IsEmpty()); } TEST_F(RemoteSuggestionsProviderImplTest, RemoveExpiredDismissedContent) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::string json_str1(GetTestJson({GetExpiredSuggestion()})); // Load it. - LoadFromJSONString(service.get(), json_str1); + LoadFromJSONString(provider.get(), json_str1); // Load the image to store it in the database. // TODO(tschumann): Introduce some abstraction to nicely work with image // fetching expectations. ServeImageCallback cb = - base::Bind(&ServeOneByOneImage, &service->GetImageFetcherForTesting()); + base::Bind(&ServeOneByOneImage, &provider->GetImageFetcherForTesting()); EXPECT_CALL(*image_fetcher(), StartOrQueueNetworkRequest(_, _, _, _)) .WillOnce(WithArgs<0, 2>(Invoke(CreateFunctor(cb)))); image_decoder()->SetDecodedImage(gfx::test::CreateImage(1, 1)); - gfx::Image image = FetchImage(service.get(), MakeArticleID(kSuggestionUrl)); + gfx::Image image = FetchImage(provider.get(), MakeArticleID(kSuggestionUrl)); EXPECT_FALSE(image.IsEmpty()); EXPECT_EQ(1, image.Width()); // Dismiss the suggestion - service->DismissSuggestion( + provider->DismissSuggestion( ContentSuggestion::ID(articles_category(), kSuggestionUrl)); // Load a different suggestion - this will clear the expired dismissed ones. std::string json_str2(GetTestJson({GetSuggestionWithUrl(kSuggestionUrl2)})); - LoadFromJSONString(service.get(), json_str2); + LoadFromJSONString(provider.get(), json_str2); - EXPECT_THAT(service->GetDismissedSuggestionsForTesting(articles_category()), + EXPECT_THAT(provider->GetDismissedSuggestionsForTesting(articles_category()), IsEmpty()); // Verify the image got removed, too. EXPECT_TRUE( - FetchImage(service.get(), MakeArticleID(kSuggestionUrl)).IsEmpty()); + FetchImage(provider.get(), MakeArticleID(kSuggestionUrl)).IsEmpty()); } TEST_F(RemoteSuggestionsProviderImplTest, ExpiredContentNotRemoved) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::string json_str(GetTestJson({GetExpiredSuggestion()})); - LoadFromJSONString(service.get(), json_str); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + LoadFromJSONString(provider.get(), json_str); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(1)); } TEST_F(RemoteSuggestionsProviderImplTest, TestSingleSource) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::string json_str(GetTestJson({GetSuggestionWithSources( "http://source1.com", "Source 1", "http://source1.amp.com")})); - LoadFromJSONString(service.get(), json_str); - ASSERT_THAT(service->GetSuggestionsForTesting(articles_category()), + LoadFromJSONString(provider.get(), json_str); + ASSERT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(1)); const RemoteSuggestion& suggestion = - *service->GetSuggestionsForTesting(articles_category()).front(); + *provider->GetSuggestionsForTesting(articles_category()).front(); EXPECT_EQ(suggestion.id(), kSuggestionUrl); EXPECT_EQ(suggestion.url(), GURL("http://source1.com")); EXPECT_EQ(suggestion.publisher_name(), std::string("Source 1")); @@ -1601,32 +1594,32 @@ } TEST_F(RemoteSuggestionsProviderImplTest, TestSingleSourceWithMalformedUrl) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::string json_str(GetTestJson({GetSuggestionWithSources( "ceci n'est pas un url", "Source 1", "http://source1.amp.com")})); - LoadFromJSONString(service.get(), json_str); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + LoadFromJSONString(provider.get(), json_str); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), IsEmpty()); } TEST_F(RemoteSuggestionsProviderImplTest, TestSingleSourceWithMissingData) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::string json_str( GetTestJson({GetSuggestionWithSources("http://source1.com", "", "")})); - LoadFromJSONString(service.get(), json_str); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + LoadFromJSONString(provider.get(), json_str); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), IsEmpty()); } TEST_F(RemoteSuggestionsProviderImplTest, LogNumArticlesHistogram) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); base::HistogramTester tester; - LoadFromJSONString(service.get(), GetTestJson({GetInvalidSuggestion()})); + LoadFromJSONString(provider.get(), GetTestJson({GetInvalidSuggestion()})); EXPECT_THAT(tester.GetAllSamples("NewTabPage.Snippets.NumArticles"), ElementsAre(base::Bucket(/*min=*/0, /*count=*/1))); @@ -1636,14 +1629,14 @@ IsEmpty()); // Valid JSON with empty list. - LoadFromJSONString(service.get(), GetTestJson(std::vector<std::string>())); + LoadFromJSONString(provider.get(), GetTestJson(std::vector<std::string>())); EXPECT_THAT(tester.GetAllSamples("NewTabPage.Snippets.NumArticles"), ElementsAre(base::Bucket(/*min=*/0, /*count=*/2))); EXPECT_THAT(tester.GetAllSamples("NewTabPage.Snippets.NumArticlesFetched"), ElementsAre(base::Bucket(/*min=*/0, /*count=*/1))); // Suggestion list should be populated with size 1. - LoadFromJSONString(service.get(), GetTestJson({GetSuggestion()})); + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestion()})); EXPECT_THAT(tester.GetAllSamples("NewTabPage.Snippets.NumArticles"), ElementsAre(base::Bucket(/*min=*/0, /*count=*/2), base::Bucket(/*min=*/1, /*count=*/1))); @@ -1652,7 +1645,7 @@ base::Bucket(/*min=*/1, /*count=*/1))); // Duplicate suggestion shouldn't increase the list size. - LoadFromJSONString(service.get(), GetTestJson({GetSuggestion()})); + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestion()})); EXPECT_THAT(tester.GetAllSamples("NewTabPage.Snippets.NumArticles"), ElementsAre(base::Bucket(/*min=*/0, /*count=*/2), base::Bucket(/*min=*/1, /*count=*/2))); @@ -1665,8 +1658,8 @@ // Dismissing a suggestion should decrease the list size. This will only be // logged after the next fetch. - service->DismissSuggestion(MakeArticleID(kSuggestionUrl)); - LoadFromJSONString(service.get(), GetTestJson({GetSuggestion()})); + provider->DismissSuggestion(MakeArticleID(kSuggestionUrl)); + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestion()})); EXPECT_THAT(tester.GetAllSamples("NewTabPage.Snippets.NumArticles"), ElementsAre(base::Bucket(/*min=*/0, /*count=*/3), base::Bucket(/*min=*/1, /*count=*/2))); @@ -1680,7 +1673,7 @@ } TEST_F(RemoteSuggestionsProviderImplTest, DismissShouldRespectAllKnownUrls) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); const base::Time creation = GetDefaultCreationTime(); const base::Time expiry = GetDefaultExpirationTime(); @@ -1693,35 +1686,35 @@ "http://t2.gstatic.com/images?q=tbn:3"}; // Add the suggestion from the mashable domain. - LoadFromJSONString(service.get(), + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestionWithUrlAndTimesAndSource( source_urls, source_urls[0], creation, expiry, publishers[0], amp_urls[0])})); - ASSERT_THAT(service->GetSuggestionsForTesting(articles_category()), + ASSERT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(1)); // Dismiss the suggestion via the mashable source corpus ID. - service->DismissSuggestion(MakeArticleID(source_urls[0])); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + provider->DismissSuggestion(MakeArticleID(source_urls[0])); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), IsEmpty()); // The same article from the AOL domain should now be detected as dismissed. - LoadFromJSONString(service.get(), + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestionWithUrlAndTimesAndSource( source_urls, source_urls[1], creation, expiry, publishers[1], amp_urls[1])})); - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), IsEmpty()); } TEST_F(RemoteSuggestionsProviderImplTest, ImageReturnedWithTheSameId) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); - LoadFromJSONString(service.get(), GetTestJson({GetSuggestion()})); + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestion()})); gfx::Image image; MockFunction<void(const gfx::Image&)> image_fetched; ServeImageCallback cb = - base::Bind(&ServeOneByOneImage, &service->GetImageFetcherForTesting()); + base::Bind(&ServeOneByOneImage, &provider->GetImageFetcherForTesting()); { InSequence s; EXPECT_CALL(*image_fetcher(), StartOrQueueNetworkRequest(_, _, _, _)) @@ -1729,7 +1722,7 @@ EXPECT_CALL(image_fetched, Call(_)).WillOnce(SaveArg<0>(&image)); } - service->FetchSuggestionImage( + provider->FetchSuggestionImage( MakeArticleID(kSuggestionUrl), base::Bind(&MockFunction<void(const gfx::Image&)>::Call, base::Unretained(&image_fetched))); @@ -1739,14 +1732,14 @@ } TEST_F(RemoteSuggestionsProviderImplTest, EmptyImageReturnedForNonExistentId) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); // Create a non-empty image so that we can test the image gets updated. gfx::Image image = gfx::test::CreateImage(1, 1); MockFunction<void(const gfx::Image&)> image_fetched; EXPECT_CALL(image_fetched, Call(_)).WillOnce(SaveArg<0>(&image)); - service->FetchSuggestionImage( + provider->FetchSuggestionImage( MakeArticleID(kSuggestionUrl2), base::Bind(&MockFunction<void(const gfx::Image&)>::Call, base::Unretained(&image_fetched))); @@ -1760,7 +1753,7 @@ // Testing that the provider is not accessing the database is tricky. // Therefore, we simply put in some data making sure that if the provider asks // the database, it will get a wrong answer. - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); ContentSuggestion::ID unknown_id = MakeArticleID(kSuggestionUrl2); database()->SaveImage(unknown_id.id_within_category(), "some image blob"); @@ -1772,7 +1765,7 @@ MockFunction<void(const gfx::Image&)> image_fetched; EXPECT_CALL(image_fetched, Call(_)).WillOnce(SaveArg<0>(&image)); - service->FetchSuggestionImage( + provider->FetchSuggestionImage( MakeArticleID(kSuggestionUrl2), base::Bind(&MockFunction<void(const gfx::Image&)>::Call, base::Unretained(&image_fetched))); @@ -1782,30 +1775,30 @@ } TEST_F(RemoteSuggestionsProviderImplTest, ClearHistoryRemovesAllSuggestions) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::string first_suggestion = GetSuggestionWithUrl("http://url1.com"); std::string second_suggestion = GetSuggestionWithUrl("http://url2.com"); std::string json_str = GetTestJson({first_suggestion, second_suggestion}); - LoadFromJSONString(service.get(), json_str); - ASSERT_THAT(service->GetSuggestionsForTesting(articles_category()), + LoadFromJSONString(provider.get(), json_str); + ASSERT_THAT(provider->GetSuggestionsForTesting(articles_category()), SizeIs(2)); - service->DismissSuggestion(MakeArticleID("http://url1.com")); + provider->DismissSuggestion(MakeArticleID("http://url1.com")); ASSERT_THAT(observer().SuggestionsForCategory(articles_category()), Not(IsEmpty())); - ASSERT_THAT(service->GetDismissedSuggestionsForTesting(articles_category()), + ASSERT_THAT(provider->GetDismissedSuggestionsForTesting(articles_category()), SizeIs(1)); base::Time begin = base::Time::FromTimeT(123), end = base::Time::FromTimeT(456); base::Callback<bool(const GURL& url)> filter; - service->ClearHistory(begin, end, filter); + provider->ClearHistory(begin, end, filter); // Verify that the observer received the update with the empty data as well. EXPECT_THAT(observer().SuggestionsForCategory(articles_category()), IsEmpty()); - EXPECT_THAT(service->GetDismissedSuggestionsForTesting(articles_category()), + EXPECT_THAT(provider->GetDismissedSuggestionsForTesting(articles_category()), IsEmpty()); } @@ -1813,79 +1806,79 @@ ShouldKeepArticlesCategoryAvailableAfterClearHistory) { // If the provider marks that category as NOT_PROVIDED, then it won't be shown // at all in the UI and the user cannot load new data :-/. - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); ASSERT_THAT(observer().StatusForCategory(articles_category()), Eq(CategoryStatus::AVAILABLE)); - service->ClearHistory(base::Time::UnixEpoch(), base::Time::Max(), - base::Callback<bool(const GURL& url)>()); + provider->ClearHistory(base::Time::UnixEpoch(), base::Time::Max(), + base::Callback<bool(const GURL& url)>()); EXPECT_THAT(observer().StatusForCategory(articles_category()), Eq(CategoryStatus::AVAILABLE)); } TEST_F(RemoteSuggestionsProviderImplTest, ShouldClearOrphanedImagesOnRestart) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); - LoadFromJSONString(service.get(), GetTestJson({GetSuggestion()})); + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestion()})); ServeImageCallback cb = - base::Bind(&ServeOneByOneImage, &service->GetImageFetcherForTesting()); + base::Bind(&ServeOneByOneImage, &provider->GetImageFetcherForTesting()); EXPECT_CALL(*image_fetcher(), StartOrQueueNetworkRequest(_, _, _, _)) .WillOnce(WithArgs<0, 2>(Invoke(CreateFunctor(cb)))); image_decoder()->SetDecodedImage(gfx::test::CreateImage(1, 1)); - gfx::Image image = FetchImage(service.get(), MakeArticleID(kSuggestionUrl)); + gfx::Image image = FetchImage(provider.get(), MakeArticleID(kSuggestionUrl)); EXPECT_EQ(1, image.Width()); EXPECT_FALSE(image.IsEmpty()); // Send new suggestion which don't include the suggestion referencing the // image. - LoadFromJSONString(service.get(), + LoadFromJSONString(provider.get(), GetTestJson({GetSuggestionWithUrl( "http://something.com/pletely/unrelated")})); // The image should still be available until a restart happens. EXPECT_FALSE( - FetchImage(service.get(), MakeArticleID(kSuggestionUrl)).IsEmpty()); - ResetSuggestionsProvider(&service, /*set_empty_response=*/true); + FetchImage(provider.get(), MakeArticleID(kSuggestionUrl)).IsEmpty()); + ResetSuggestionsProvider(&provider, /*set_empty_response=*/true); // After the restart, the image should be garbage collected. EXPECT_TRUE( - FetchImage(service.get(), MakeArticleID(kSuggestionUrl)).IsEmpty()); + FetchImage(provider.get(), MakeArticleID(kSuggestionUrl)).IsEmpty()); } TEST_F(RemoteSuggestionsProviderImplTest, ShouldHandleMoreThanMaxSuggestionsInResponse) { - auto service = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider(); std::vector<std::string> suggestions; - for (int i = 0; i < service->GetMaxSuggestionCountForTesting() + 1; ++i) { + for (int i = 0; i < provider->GetMaxSuggestionCountForTesting() + 1; ++i) { suggestions.push_back(GetSuggestionWithUrl( base::StringPrintf("http://localhost/suggestion-id-%d", i))); } - LoadFromJSONString(service.get(), GetTestJson(suggestions)); + LoadFromJSONString(provider.get(), GetTestJson(suggestions)); // TODO(tschumann): We should probably trim out any additional results and // only serve the MaxSuggestionCount items. - EXPECT_THAT(service->GetSuggestionsForTesting(articles_category()), - SizeIs(service->GetMaxSuggestionCountForTesting() + 1)); + EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), + SizeIs(provider->GetMaxSuggestionCountForTesting() + 1)); } TEST_F(RemoteSuggestionsProviderImplTest, StoreLastSuccessfullBackgroundFetchTime) { // On initialization of the RemoteSuggestionsProviderImpl a background fetch - // is triggered since the suggestions DB is empty. Therefore the service must + // is triggered since the suggestions DB is empty. Therefore the provider must // not be initialized until the test clock is set. - auto service = MakeSuggestionsProviderWithoutInitialization( + auto provider = MakeSuggestionsProviderWithoutInitialization( /*use_mock_suggestions_fetcher=*/false); auto simple_test_clock = base::MakeUnique<base::SimpleTestClock>(); base::SimpleTestClock* simple_test_clock_ptr = simple_test_clock.get(); - service->SetClockForTesting(std::move(simple_test_clock)); + provider->SetClockForTesting(std::move(simple_test_clock)); // Test that the preference is correctly initialized with the default value 0. EXPECT_EQ( 0, pref_service()->GetInt64(prefs::kLastSuccessfulBackgroundFetchTime)); - WaitForSuggestionsProviderInitialization(service.get(), + WaitForSuggestionsProviderInitialization(provider.get(), /*set_empty_response=*/true); EXPECT_EQ( simple_test_clock_ptr->Now().ToInternalValue(), @@ -1895,7 +1888,7 @@ // background fetch. simple_test_clock_ptr->Advance(TimeDelta::FromHours(1)); - service->RefetchInTheBackground( + provider->RefetchInTheBackground( RemoteSuggestionsProvider::FetchStatusCallback()); base::RunLoop().RunUntilIdle(); // TODO(jkrcal): Move together with the pref storage into the scheduler. @@ -1907,26 +1900,26 @@ } TEST_F(RemoteSuggestionsProviderImplTest, CallsSchedulerWhenReady) { - auto service = + auto provider = MakeSuggestionsProviderWithoutInitializationWithStrictScheduler(); // Should be called when becoming ready. EXPECT_CALL(*scheduler(), OnProviderActivated()); - WaitForSuggestionsProviderInitialization(service.get(), + WaitForSuggestionsProviderInitialization(provider.get(), /*set_empty_response=*/true); } TEST_F(RemoteSuggestionsProviderImplTest, CallsSchedulerOnError) { - auto service = + auto provider = MakeSuggestionsProviderWithoutInitializationWithStrictScheduler(); // Should be called on error. EXPECT_CALL(*scheduler(), OnProviderDeactivated()); - service->EnterState(RemoteSuggestionsProviderImpl::State::ERROR_OCCURRED); + provider->EnterState(RemoteSuggestionsProviderImpl::State::ERROR_OCCURRED); } TEST_F(RemoteSuggestionsProviderImplTest, CallsSchedulerWhenDisabled) { - auto service = + auto provider = MakeSuggestionsProviderWithoutInitializationWithStrictScheduler(); // Should be called when becoming disabled. First deactivate and only after @@ -1934,52 +1927,52 @@ { InSequence s; EXPECT_CALL(*scheduler(), OnProviderDeactivated()); - ASSERT_THAT(service->ready(), Eq(false)); + ASSERT_THAT(provider->ready(), Eq(false)); EXPECT_CALL(*scheduler(), OnSuggestionsCleared()); } - service->EnterState(RemoteSuggestionsProviderImpl::State::DISABLED); + provider->EnterState(RemoteSuggestionsProviderImpl::State::DISABLED); } TEST_F(RemoteSuggestionsProviderImplTest, CallsSchedulerWhenHistoryCleared) { - auto service = + auto provider = MakeSuggestionsProviderWithoutInitializationWithStrictScheduler(); - // Initiate the service so that it is already READY. + // Initiate the provider so that it is already READY. EXPECT_CALL(*scheduler(), OnProviderActivated()); - WaitForSuggestionsProviderInitialization(service.get(), + WaitForSuggestionsProviderInitialization(provider.get(), /*set_empty_response=*/true); // The scheduler should be notified of clearing the history. EXPECT_CALL(*scheduler(), OnHistoryCleared()); - service->ClearHistory(GetDefaultCreationTime(), GetDefaultExpirationTime(), - base::Callback<bool(const GURL& url)>()); + provider->ClearHistory(GetDefaultCreationTime(), GetDefaultExpirationTime(), + base::Callback<bool(const GURL& url)>()); } TEST_F(RemoteSuggestionsProviderImplTest, CallsSchedulerWhenSignedIn) { - auto service = + auto provider = MakeSuggestionsProviderWithoutInitializationWithStrictScheduler(); - // Initiate the service so that it is already READY. + // Initiate the provider so that it is already READY. EXPECT_CALL(*scheduler(), OnProviderActivated()); - WaitForSuggestionsProviderInitialization(service.get(), + WaitForSuggestionsProviderInitialization(provider.get(), /*set_empty_response=*/true); // The scheduler should be notified of clearing the history. EXPECT_CALL(*scheduler(), OnSuggestionsCleared()); - service->OnStatusChanged(RemoteSuggestionsStatus::ENABLED_AND_SIGNED_IN, - RemoteSuggestionsStatus::ENABLED_AND_SIGNED_OUT); + provider->OnStatusChanged(RemoteSuggestionsStatus::ENABLED_AND_SIGNED_IN, + RemoteSuggestionsStatus::ENABLED_AND_SIGNED_OUT); } TEST_F(RemoteSuggestionsProviderImplTest, CallsSchedulerWhenSignedOut) { - auto service = + auto provider = MakeSuggestionsProviderWithoutInitializationWithStrictScheduler(); - // Initiate the service so that it is already READY. + // Initiate the provider so that it is already READY. EXPECT_CALL(*scheduler(), OnProviderActivated()); - WaitForSuggestionsProviderInitialization(service.get(), + WaitForSuggestionsProviderInitialization(provider.get(), /*set_empty_response=*/true); // The scheduler should be notified of clearing the history. EXPECT_CALL(*scheduler(), OnSuggestionsCleared()); - service->OnStatusChanged(RemoteSuggestionsStatus::ENABLED_AND_SIGNED_OUT, - RemoteSuggestionsStatus::ENABLED_AND_SIGNED_IN); + provider->OnStatusChanged(RemoteSuggestionsStatus::ENABLED_AND_SIGNED_OUT, + RemoteSuggestionsStatus::ENABLED_AND_SIGNED_IN); } TEST_F(RemoteSuggestionsProviderImplTest,
diff --git a/components/omnibox/browser/base_search_provider.h b/components/omnibox/browser/base_search_provider.h index 4a22a1e7..dd8412b 100644 --- a/components/omnibox/browser/base_search_provider.h +++ b/components/omnibox/browser/base_search_provider.h
@@ -255,8 +255,8 @@ bool field_trial_triggered_in_session_; // Each deletion handler in this vector corresponds to an outstanding request - // that a server delete a personalized suggestion. Making this a ScopedVector - // causes us to auto-cancel all such requests on shutdown. + // that a server delete a personalized suggestion. Making this a vector of + // unique_ptr causes us to auto-cancel all such requests on shutdown. SuggestionDeletionHandlers deletion_handlers_; DISALLOW_COPY_AND_ASSIGN(BaseSearchProvider);
diff --git a/content/app/android/content_jni_onload.cc b/content/app/android/content_jni_onload.cc index 15f89d2d..f43aa247 100644 --- a/content/app/android/content_jni_onload.cc +++ b/content/app/android/content_jni_onload.cc
@@ -17,9 +17,6 @@ namespace android { bool OnJNIOnLoadRegisterJNI(JNIEnv* env) { - if (!base::android::OnJNIOnLoadRegisterJNI(env)) - return false; - return content::EnsureJniRegistered(env); }
diff --git a/content/browser/renderer_host/input/wheel_scroll_latching_browsertest.cc b/content/browser/renderer_host/input/wheel_scroll_latching_browsertest.cc new file mode 100644 index 0000000..78dd9fc98 --- /dev/null +++ b/content/browser/renderer_host/input/wheel_scroll_latching_browsertest.cc
@@ -0,0 +1,239 @@ +// 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/test/scoped_feature_list.h" +#include "content/browser/renderer_host/render_widget_host_impl.h" +#include "content/browser/renderer_host/render_widget_host_input_event_router.h" +#include "content/browser/web_contents/web_contents_impl.h" +#include "content/common/input/synthetic_web_input_event_builders.h" +#include "content/public/common/content_features.h" +#include "content/public/test/browser_test_utils.h" +#include "content/public/test/content_browser_test.h" +#include "content/public/test/content_browser_test_utils.h" +#include "content/shell/browser/shell.h" +#include "ui/events/gesture_detection/gesture_configuration.h" + +#if defined(OS_ANDROID) +#include "content/browser/renderer_host/render_widget_host_view_android.h" +#endif + +using blink::WebMouseWheelEvent; + +namespace { +void GiveItSomeTime() { + base::RunLoop run_loop; + base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, run_loop.QuitClosure(), base::TimeDelta::FromMilliseconds(10)); + run_loop.Run(); +} + +const char kWheelEventLatchingDataURL[] = + "data:text/html;charset=utf-8," + "<!DOCTYPE html>" + "<style>" + "body {" + " height: 10000px;" + "}" + "#scrollableDiv {" + " position: absolute;" + " left: 100px;" + " top: 200px;" + " width: 400px;" + " height: 800px;" + " overflow: scroll;" + " background: red;" + "}" + "#nestedDiv {" + " width: 400px;" + " height: 8000px;" + " opacity: 0;" + "}" + "</style>" + "<div id='scrollableDiv'>" + " <div id='nestedDiv'></div>" + "</div>" + "<script>" + " var scrollableDiv = document.getElementById('scrollableDiv');" + " var scrollableDivWheelEventCounter = 0;" + " var documentWheelEventCounter = 0;" + " scrollableDiv.addEventListener('wheel'," + " function(e) { scrollableDivWheelEventCounter++;" + " e.stopPropagation(); });" + " document.scrollingElement.addEventListener('wheel'," + " function(e) { documentWheelEventCounter++; });" + "</script>"; +} // namespace + +namespace content { +class WheelScrollLatchingBrowserTest : public ContentBrowserTest { + public: + WheelScrollLatchingBrowserTest(bool wheel_scroll_latching_enabled = true) + : wheel_scroll_latching_enabled_(wheel_scroll_latching_enabled) { + ui::GestureConfiguration::GetInstance()->set_scroll_debounce_interval_in_ms( + 0); + + if (wheel_scroll_latching_enabled_) + EnableWheelScrollLatching(); + else + DisableWheelScrollLatching(); + } + ~WheelScrollLatchingBrowserTest() override {} + + protected: + RenderWidgetHostImpl* GetWidgetHost() { + return RenderWidgetHostImpl::From( + web_contents()->GetRenderViewHost()->GetWidget()); + } + + WebContentsImpl* web_contents() const { + return static_cast<WebContentsImpl*>(shell()->web_contents()); + } + + RenderWidgetHostInputEventRouter* GetRouter() { + return web_contents()->GetInputEventRouter(); + } + + RenderWidgetHostViewBase* GetRootView() { + return static_cast<RenderWidgetHostViewBase*>(web_contents() + ->GetFrameTree() + ->root() + ->current_frame_host() + ->GetView()); + } + + float GetPageScaleFactor() { + return GetWidgetHost()->last_frame_metadata().page_scale_factor; + } + + void LoadURL() { + const GURL data_url(kWheelEventLatchingDataURL); + NavigateToURL(shell(), data_url); + + RenderWidgetHostImpl* host = GetWidgetHost(); + host->GetView()->SetSize(gfx::Size(600, 600)); + + // The page is loaded in the renderer, wait for a new frame to arrive. + while (!host->ScheduleComposite()) + GiveItSomeTime(); + } + int ExecuteScriptAndExtractInt(const std::string& script) { + int value = 0; + EXPECT_TRUE(content::ExecuteScriptAndExtractInt( + shell(), "domAutomationController.send(" + script + ")", &value)); + return value; + } + void EnableWheelScrollLatching() { + feature_list_.InitFromCommandLine( + features::kTouchpadAndWheelScrollLatching.name, ""); + } + void DisableWheelScrollLatching() { + feature_list_.InitFromCommandLine( + "", features::kTouchpadAndWheelScrollLatching.name); + } + + void WheelEventTargetTest() { + LoadURL(); + EXPECT_EQ(0, ExecuteScriptAndExtractInt("documentWheelEventCounter")); + EXPECT_EQ(0, ExecuteScriptAndExtractInt("scrollableDivWheelEventCounter")); + + FrameWatcher frame_watcher(shell()->web_contents()); + scoped_refptr<InputMsgWatcher> input_msg_watcher(new InputMsgWatcher( + GetWidgetHost(), blink::WebInputEvent::kMouseWheel)); + float scale_factor = GetPageScaleFactor(); + + float scrollable_div_top = + ExecuteScriptAndExtractInt("scrollableDiv.getBoundingClientRect().top"); + float x = scale_factor * + (ExecuteScriptAndExtractInt( + "scrollableDiv.getBoundingClientRect().left") + + ExecuteScriptAndExtractInt( + "scrollableDiv.getBoundingClientRect().right")) / + 2; + float y = scale_factor * 0.5 * scrollable_div_top; + float delta_x = 0; + float delta_y = -0.5 * scrollable_div_top; + blink::WebMouseWheelEvent wheel_event = + SyntheticWebMouseWheelEventBuilder::Build(x, y, x, y, delta_x, delta_y, + 0, true); + + wheel_event.phase = blink::WebMouseWheelEvent::kPhaseBegan; + GetRouter()->RouteMouseWheelEvent(GetRootView(), &wheel_event, + ui::LatencyInfo()); + + // Runs until we get the InputMsgAck callback. + EXPECT_EQ(INPUT_EVENT_ACK_STATE_NOT_CONSUMED, + input_msg_watcher->WaitForAck()); + + while (ExecuteScriptAndExtractInt("document.scrollingElement.scrollTop") < + -delta_y) { + frame_watcher.WaitFrames(1); + } + + EXPECT_EQ(0, ExecuteScriptAndExtractInt("scrollableDiv.scrollTop")); + EXPECT_EQ(1, ExecuteScriptAndExtractInt("documentWheelEventCounter")); + EXPECT_EQ(0, ExecuteScriptAndExtractInt("scrollableDivWheelEventCounter")); + + wheel_event.phase = blink::WebMouseWheelEvent::kPhaseChanged; + GetRouter()->RouteMouseWheelEvent(GetRootView(), &wheel_event, + ui::LatencyInfo()); + + // Runs until we get the InputMsgAck callback. + EXPECT_EQ(INPUT_EVENT_ACK_STATE_NOT_CONSUMED, + input_msg_watcher->WaitForAck()); + + if (wheel_scroll_latching_enabled_) { + while (ExecuteScriptAndExtractInt("document.scrollingElement.scrollTop") < + -2 * delta_y) { + frame_watcher.WaitFrames(1); + } + + EXPECT_EQ(0, ExecuteScriptAndExtractInt("scrollableDiv.scrollTop")); + EXPECT_EQ(2, ExecuteScriptAndExtractInt("documentWheelEventCounter")); + EXPECT_EQ(0, + ExecuteScriptAndExtractInt("scrollableDivWheelEventCounter")); + } else { // !wheel_scroll_latching_enabled_ + while (ExecuteScriptAndExtractInt("scrollableDiv.scrollTop") < -delta_y) + frame_watcher.WaitFrames(1); + + EXPECT_EQ(1, ExecuteScriptAndExtractInt("documentWheelEventCounter")); + EXPECT_EQ(1, + ExecuteScriptAndExtractInt("scrollableDivWheelEventCounter")); + } + } + + private: + base::test::ScopedFeatureList feature_list_; + bool wheel_scroll_latching_enabled_; +}; + +class WheelScrollLatchingDisabledBrowserTest + : public WheelScrollLatchingBrowserTest { + public: + WheelScrollLatchingDisabledBrowserTest() + : WheelScrollLatchingBrowserTest(false) {} + ~WheelScrollLatchingDisabledBrowserTest() override {} +}; + +// Start scrolling by mouse wheel on the document: the wheel event will be sent +// to the document's scrolling element, the scrollable div will be under the +// cursor after applying the scrolling. Continue scrolling by mouse wheel, since +// wheel scroll latching is enabled the wheel event will be still sent to the +// document's scrolling element and the document's scrolling element will +// continue scrolling. +IN_PROC_BROWSER_TEST_F(WheelScrollLatchingBrowserTest, WheelEventTarget) { + WheelEventTargetTest(); +} + +// Start scrolling by mouse wheel on the document: the wheel event will be sent +// to the document's scrolling element, the scrollable div will be under the +// cursor after applying the scrolloffsets. Continue scrolling by mouse wheel, +// since wheel scroll latching is disabled the wheel event will be still sent to +// the scrollable div which is currently under the cursor. The div will start +// scrolling. +IN_PROC_BROWSER_TEST_F(WheelScrollLatchingDisabledBrowserTest, + WheelEventTarget) { + WheelEventTargetTest(); +} + +} // namespace content \ No newline at end of file
diff --git a/content/browser/renderer_host/render_widget_host_input_event_router.cc b/content/browser/renderer_host/render_widget_host_input_event_router.cc index fe73293b..c88f7d6 100644 --- a/content/browser/renderer_host/render_widget_host_input_event_router.cc +++ b/content/browser/renderer_host/render_widget_host_input_event_router.cc
@@ -284,7 +284,27 @@ event->PositionInWidget().y), target, &transformed_point)) return; - } else { + } else if (root_view->wheel_scroll_latching_enabled()) { + if (event->phase == blink::WebMouseWheelEvent::kPhaseBegan) { + wheel_target_.target = FindEventTarget( + root_view, + gfx::Point(event->PositionInWidget().x, event->PositionInWidget().y), + &transformed_point); + wheel_target_.delta = + transformed_point - + gfx::Point(event->PositionInWidget().x, event->PositionInWidget().y); + target = wheel_target_.target; + } else { + if (wheel_target_.target) { + target = wheel_target_.target; + transformed_point = gfx::Point(event->PositionInWidget().x, + event->PositionInWidget().y) + + wheel_target_.delta; + } + } + + } else { // !root_view->IsMouseLocked() && + // !root_view->wheel_scroll_latching_enabled() target = FindEventTarget( root_view, gfx::Point(event->PositionInWidget().x, event->PositionInWidget().y), @@ -296,6 +316,12 @@ event->SetPositionInWidget(transformed_point.x(), transformed_point.y()); target->ProcessMouseWheelEvent(*event, latency); + + DCHECK(root_view->wheel_scroll_latching_enabled() || !wheel_target_.target); + if (event->phase == blink::WebMouseWheelEvent::kPhaseEnded || + event->momentum_phase == blink::WebMouseWheelEvent::kPhaseEnded) { + wheel_target_.target = nullptr; + } } void RenderWidgetHostInputEventRouter::RouteGestureEvent(
diff --git a/content/browser/renderer_host/render_widget_host_input_event_router.h b/content/browser/renderer_host/render_widget_host_input_event_router.h index f12ba5ddc..08bb689 100644 --- a/content/browser/renderer_host/render_widget_host_input_event_router.h +++ b/content/browser/renderer_host/render_widget_host_input_event_router.h
@@ -164,6 +164,9 @@ TargetData touchpad_gesture_target_; TargetData bubbling_gesture_scroll_target_; TargetData first_bubbling_scroll_target_; + // Used to target wheel events for the duration of a scroll when wheel scroll + // latching is enabled. + TargetData wheel_target_; // Maintains the same target between mouse down and mouse up. TargetData mouse_capture_target_;
diff --git a/content/browser/service_worker/service_worker_fetch_dispatcher.cc b/content/browser/service_worker/service_worker_fetch_dispatcher.cc index 61d1ab6..b1b96204d 100644 --- a/content/browser/service_worker/service_worker_fetch_dispatcher.cc +++ b/content/browser/service_worker/service_worker_fetch_dispatcher.cc
@@ -35,6 +35,7 @@ #include "net/log/net_log.h" #include "net/log/net_log_capture_mode.h" #include "net/log/net_log_event_type.h" +#include "net/traffic_annotation/network_traffic_annotation.h" #include "net/url_request/url_request.h" namespace content { @@ -666,7 +667,8 @@ mojo::MakeRequest(&url_loader_associated_ptr), original_info->GetRouteID(), request_id, mojom::kURLLoadOptionNone, request, std::move(url_loader_client_ptr_to_pass), - net::MutableNetworkTrafficAnnotationTag(NO_TRAFFIC_ANNOTATION_YET)); + net::MutableNetworkTrafficAnnotationTag( + original_request->traffic_annotation())); std::unique_ptr<DelegatingURLLoader> url_loader( base::MakeUnique<DelegatingURLLoader>(
diff --git a/content/browser/webrtc/webrtc_audio_browsertest.cc b/content/browser/webrtc/webrtc_audio_browsertest.cc index 5b66e50..dabe2f6 100644 --- a/content/browser/webrtc/webrtc_audio_browsertest.cc +++ b/content/browser/webrtc/webrtc_audio_browsertest.cc
@@ -61,8 +61,16 @@ } }; +// Flaky on Linux. http://crbug.com/733551 +#if defined(OS_LINUX) +#define MAYBE_CanMakeVideoCallAndThenRenegotiateToAudio \ + DISABLED_CanMakeVideoCallAndThenRenegotiateToAudio +#else +#define MAYBE_CanMakeVideoCallAndThenRenegotiateToAudio \ + CanMakeVideoCallAndThenRenegotiateToAudio +#endif IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcAudioBrowserTest, - CanMakeVideoCallAndThenRenegotiateToAudio) { + MAYBE_CanMakeVideoCallAndThenRenegotiateToAudio) { MakeAudioDetectingPeerConnectionCall( "callAndRenegotiateToAudio({audio: true, video:true}, {audio: true});"); }
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn index 45103a4..45bb5a8 100644 --- a/content/test/BUILD.gn +++ b/content/test/BUILD.gn
@@ -692,6 +692,7 @@ "../browser/renderer_host/input/touch_action_browsertest.cc", "../browser/renderer_host/input/touch_input_browsertest.cc", "../browser/renderer_host/input/touch_selection_controller_client_aura_browsertest.cc", + "../browser/renderer_host/input/wheel_scroll_latching_browsertest.cc", "../browser/renderer_host/media/video_capture_browsertest.cc", "../browser/renderer_host/render_process_host_browsertest.cc", "../browser/renderer_host/render_view_host_browsertest.cc",
diff --git a/extensions/renderer/resources/binding.js b/extensions/renderer/resources/binding.js index 0b2d49ab..83e2418e 100644 --- a/extensions/renderer/resources/binding.js +++ b/extensions/renderer/resources/binding.js
@@ -405,6 +405,7 @@ mod[functionDef.name] = $Function.bind(function() { var args = $Array.slice(arguments); + $Object.setPrototypeOf(args, null); if (this.updateArgumentsPreValidate) args = $Function.apply(this.updateArgumentsPreValidate, this, args);
diff --git a/extensions/renderer/resources/schema_utils.js b/extensions/renderer/resources/schema_utils.js index 8f8e26a..c82600e 100644 --- a/extensions/renderer/resources/schema_utils.js +++ b/extensions/renderer/resources/schema_utils.js
@@ -44,7 +44,10 @@ if (parameterSchemas.length === 0) return [[]]; var signatures = []; + $Object.setPrototypeOf(signatures, null); + $Object.setPrototypeOf(parameterSchemas, null); var remaining = getSignatures($Array.slice(parameterSchemas, 1)); + $Object.setPrototypeOf(remaining, null); for (var i = 0; i < remaining.length; ++i) $Array.push(signatures, $Array.concat([parameterSchemas[0]], remaining[i])) if (parameterSchemas[0].optional) @@ -115,6 +118,7 @@ getParameterSignatureString(funDef.name, definedSignature)); validate(args, resolvedSignature); var normalizedArgs = []; + $Object.setPrototypeOf(normalizedArgs, null); var ai = 0; for (var si = 0; si < definedSignature.length; ++si) { if (definedSignature[si] === resolvedSignature[ai])
diff --git a/headless/lib/browser/devtools_api/devtools_connection.js b/headless/lib/browser/devtools_api/devtools_connection.js index 82993e0..b526908 100644 --- a/headless/lib/browser/devtools_api/devtools_connection.js +++ b/headless/lib/browser/devtools_api/devtools_connection.js
@@ -53,7 +53,7 @@ */ this.eventListenerIdToEventName_ = new Map(); - this.transport_.onmessage = this.onMessage_.bind(this); + this.transport_.onmessage = this.onJsonMessage_.bind(this); } /** @@ -118,14 +118,16 @@ }); } - /** - * @param {string} jsonMessage An object containing a DevTools protocol + * If a subclass needs to customize message handling it should override this + * method. + * + * @param {*} message A parsed DevTools protocol message. + * @param {string} jsonMessage A string containing a JSON DevTools protocol * message. - * @private + * @protected */ - onMessage_(jsonMessage) { - const message = JSON.parse(jsonMessage); + onMessage_(message, jsonMessage) { if (message.hasOwnProperty('id')) { if (!this.pendingCommands_.has(message.id)) throw new Error('Unrecognized id:' + jsonMessage); @@ -147,6 +149,16 @@ } } } + + + /** + * @param {string} jsonMessage A string containing a JSON DevTools protocol + * message. + * @private + */ + onJsonMessage_(jsonMessage) { + this.onMessage_(JSON.parse(jsonMessage), jsonMessage); + } } /**
diff --git a/ios/chrome/browser/find_in_page/BUILD.gn b/ios/chrome/browser/find_in_page/BUILD.gn index ff13a51..88e20e3 100644 --- a/ios/chrome/browser/find_in_page/BUILD.gn +++ b/ios/chrome/browser/find_in_page/BUILD.gn
@@ -37,25 +37,8 @@ testonly = true configs += [ "//build/config/compiler:enable_arc" ] sources = [ - "find_tab_helper_unittest.mm", - ] - deps = [ - ":find_in_page", - ":unit_tests_nonarc", - "//base", - "//base/test:test_support", - "//ios/chrome/browser/web:test_support", - "//ios/web", - "//ios/web/public/test", - "//ios/web/public/test/fakes", - "//testing/gtest", - ] -} - -source_set("unit_tests_nonarc") { - testonly = true - sources = [ "find_in_page_js_unittest.mm", + "find_tab_helper_unittest.mm", "js_findinpage_manager_unittest.mm", ] deps = [ @@ -64,6 +47,8 @@ "//base/test:test_support", "//ios/chrome/browser/web:test_support", "//ios/web", + "//ios/web/public/test", + "//ios/web/public/test/fakes", "//testing/gtest", ] }
diff --git a/ios/chrome/browser/find_in_page/find_in_page_js_unittest.mm b/ios/chrome/browser/find_in_page/find_in_page_js_unittest.mm index 06398da8..fc9a9f41 100644 --- a/ios/chrome/browser/find_in_page/find_in_page_js_unittest.mm +++ b/ios/chrome/browser/find_in_page/find_in_page_js_unittest.mm
@@ -5,7 +5,6 @@ #import <UIKit/UIKit.h> #include "base/mac/foundation_util.h" -#include "base/mac/scoped_nsobject.h" #include "base/strings/sys_string_conversions.h" #import "ios/chrome/browser/find_in_page/find_in_page_model.h" #import "ios/chrome/browser/find_in_page/js_findinpage_manager.h" @@ -16,6 +15,10 @@ #include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest_mac.h" +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + // Unit tests for the find_in_page.js JavaScript file. namespace { @@ -101,15 +104,15 @@ void SetUp() override { ChromeWebTest::SetUp(); - findInPageModel_.reset([[FindInPageModel alloc] init]); - findInPageJsManager_.reset([base::mac::ObjCCastStrict<JsFindinpageManager>( + findInPageModel_ = [[FindInPageModel alloc] init]; + findInPageJsManager_ = base::mac::ObjCCastStrict<JsFindinpageManager>( [web_state()->GetJSInjectionReceiver() - instanceOfClass:[JsFindinpageManager class]]) retain]); - findInPageJsManager_.get().findInPageModel = findInPageModel_; + instanceOfClass:[JsFindinpageManager class]]); + findInPageJsManager_.findInPageModel = findInPageModel_; } - base::scoped_nsobject<FindInPageModel> findInPageModel_; - base::scoped_nsobject<JsFindinpageManager> findInPageJsManager_; + FindInPageModel* findInPageModel_; + JsFindinpageManager* findInPageJsManager_; }; // Performs a search, then calls |incrementIndex| to loop through the
diff --git a/ios/chrome/browser/find_in_page/js_findinpage_manager_unittest.mm b/ios/chrome/browser/find_in_page/js_findinpage_manager_unittest.mm index ec13bec..aa74d57 100644 --- a/ios/chrome/browser/find_in_page/js_findinpage_manager_unittest.mm +++ b/ios/chrome/browser/find_in_page/js_findinpage_manager_unittest.mm
@@ -12,6 +12,10 @@ #import "ios/web/public/web_state/web_state.h" #import "testing/gtest_mac.h" +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + // Test fixture to test Find In Page JS. class JsFindinpageManagerTest : public ChromeWebTest { protected:
diff --git a/ios/chrome/browser/ui/ntp/google_landing_view_controller.mm b/ios/chrome/browser/ui/ntp/google_landing_view_controller.mm index 5eac738..4b467c11 100644 --- a/ios/chrome/browser/ui/ntp/google_landing_view_controller.mm +++ b/ios/chrome/browser/ui/ntp/google_landing_view_controller.mm
@@ -144,6 +144,8 @@ base::scoped_nsobject<NSMutableArray> _supplementaryViews; base::scoped_nsobject<NewTabPageHeaderView> _headerView; base::scoped_nsobject<WhatsNewHeaderView> _promoHeaderView; + base::WeakNSProtocol<id<GoogleLandingDataSource>> _dataSource; + base::WeakNSProtocol<id<UrlLoader, OmniboxFocuser>> _dispatcher; } // Redeclare the |view| property to be the GoogleLandingView subclass instead of @@ -234,10 +236,8 @@ @dynamic view; @synthesize logoVendor = _logoVendor; -@synthesize dataSource = _dataSource; // Property declared in NewTabPagePanelProtocol. @synthesize delegate = _delegate; -@synthesize dispatcher = _dispatcher; @synthesize isOffTheRecord = _isOffTheRecord; @synthesize logoIsShowing = _logoIsShowing; @synthesize promoText = _promoText; @@ -322,6 +322,24 @@ [super dealloc]; } +#pragma mark - Properties + +- (id<GoogleLandingDataSource>)dataSource { + return _dataSource; +} + +- (void)setDataSource:(id<GoogleLandingDataSource>)dataSource { + _dataSource.reset(dataSource); +} + +- (id<UrlLoader, OmniboxFocuser>)dispatcher { + return _dispatcher; +} + +- (void)setDispatcher:(id<UrlLoader, OmniboxFocuser>)dispatcher { + _dispatcher.reset(dispatcher); +} + #pragma mark - Private - (CGSize)mostVisitedCellSize {
diff --git a/ios/chrome/browser/ui/webui/web_ui_egtest.mm b/ios/chrome/browser/ui/webui/web_ui_egtest.mm index 661f1cb..326e586 100644 --- a/ios/chrome/browser/ui/webui/web_ui_egtest.mm +++ b/ios/chrome/browser/ui/webui/web_ui_egtest.mm
@@ -81,7 +81,8 @@ // Tests that chrome://version renders and contains correct version number and // user agent string. -- (void)testVersion { +// TODO(crbug.com/734079): Re-enable this test. +- (void)DISABLED_testVersion { LoadWebUIUrl(kChromeUIVersionHost); // Verify that app version is present on the page. @@ -95,7 +96,8 @@ } // Tests that chrome://physical-web renders and the page title is present. -- (void)testPhysicalWeb { +// TODO(crbug.com/734079): Re-enable this test. +- (void)DISABLED_testPhysicalWeb { // Enable the Physical Web via Chrome variation. base::FieldTrialList::CreateFieldTrial("PhysicalWebEnabled", "Enabled");
diff --git a/ios/web/BUILD.gn b/ios/web/BUILD.gn index 21fe3f8..7da17ca 100644 --- a/ios/web/BUILD.gn +++ b/ios/web/BUILD.gn
@@ -410,62 +410,26 @@ ] } -test("ios_web_unittests_arc") { - deps = [ - ":core", - ":ios_web_unittests_bundle_data", - ":run_all_unittests", - ":user_agent", - ":web", - "//base", - "//base/test:test_support", - "//components/payments/core", - "//components/url_formatter", - "//ios/net", - "//ios/testing:ios_test_support", - "//ios/testing:ocmock_support", - "//ios/web/public/test", - "//ios/web/public/test/fakes", - "//ios/web/test:mojo_bindings", - "//net:test_support", - "//services/service_manager/public/cpp", - "//testing/gmock", - "//testing/gtest", - "//third_party/ocmock", - "//ui/base:test_support", - ] - - sources = [ - "navigation/navigation_manager_util_unittest.mm", - "public/crw_session_certificate_policy_cache_storage_unittest.mm", - "web_state/session_certificate_policy_cache_impl_unittest.mm", - "web_state/session_certificate_policy_cache_storage_builder_unittest.mm", - ] - - configs += [ "//build/config/compiler:enable_arc" ] -} - test("ios_web_unittests") { deps = [ + # Ensure all required data are present in the bundle, and that the + # test runner is linked. + ":ios_web_unittests_bundle_data", + ":run_all_unittests", + + # Add individual test source_set targets here. ":ios_web_general_unittests", ":ios_web_navigation_unittests", ":ios_web_net_unittests", ":ios_web_public_unittests", - ":ios_web_unittests_bundle_data", ":ios_web_web_state_js_unittests", ":ios_web_web_state_ui_unittests", ":ios_web_web_state_unittests", ":ios_web_webui_unittests", - ":run_all_unittests", - ] - - public_deps = [ - ":ios_web_unittests_arc", ] assert_no_deps = ios_assert_no_deps - - allow_circular_includes_from = [ ":ios_web_unittests_arc" ] + configs += [ "//build/config/compiler:enable_arc" ] } source_set("ios_web_general_unittests") { @@ -541,7 +505,9 @@ "navigation/navigation_item_storage_test_util.h", "navigation/navigation_item_storage_test_util.mm", "navigation/navigation_manager_impl_unittest.mm", + "navigation/navigation_manager_util_unittest.mm", "navigation/nscoder_util_unittest.mm", + "navigation/serializable_user_data_manager_impl_unittest.mm", ] } @@ -583,11 +549,25 @@ ] } +source_set("test_support") { + configs += [ "//build/config/compiler:enable_arc" ] + testonly = true + sources = [ + "public/test/crw_mock_web_state_delegate.h", + "public/test/crw_mock_web_state_delegate.mm", + ] + deps = [ + "//ios/testing:ocmock_support", + "//ios/web", + ] +} + source_set("ios_web_public_unittests") { configs += [ "//build/config/compiler:enable_arc" ] testonly = true deps = [ ":core", + ":test_support", ":user_agent", ":web", "//base", @@ -611,11 +591,10 @@ ] sources = [ + "public/crw_session_certificate_policy_cache_storage_unittest.mm", "public/origin_util_unittest.mm", "public/referrer_util_unittest.cc", "public/serializable_user_data_manager_unittest.mm", - "public/test/crw_mock_web_state_delegate.h", - "public/test/crw_mock_web_state_delegate.mm", "public/user_agent_unittest.mm", "public/web_state/page_viewport_state_unittest.mm", ] @@ -626,7 +605,8 @@ testonly = true deps = [ ":core", - ":ios_web_public_unittests", + ":ios_web_web_state_unittests_arc", + ":test_support", ":user_agent", ":web", "//base", @@ -664,6 +644,23 @@ ] } +source_set("ios_web_web_state_unittests_arc") { + testonly = true + sources = [ + "web_state/session_certificate_policy_cache_impl_unittest.mm", + "web_state/session_certificate_policy_cache_storage_builder_unittest.mm", + ] + deps = [ + "//base", + "//ios/testing:ios_test_support", + "//ios/web", + "//ios/web/public/test", + "//ios/web/public/test/fakes", + "//net:test_support", + ] + configs += [ "//build/config/compiler:enable_arc" ] +} + source_set("ios_web_web_state_js_unittests") { configs += [ "//build/config/compiler:enable_arc" ] testonly = true
diff --git a/ios/web/navigation/navigation_manager_util_unittest.mm b/ios/web/navigation/navigation_manager_util_unittest.mm index 1b26ca6b..83b3b18 100644 --- a/ios/web/navigation/navigation_manager_util_unittest.mm +++ b/ios/web/navigation/navigation_manager_util_unittest.mm
@@ -36,7 +36,9 @@ // Tests GetCommittedItemWithUniqueID, GetCommittedItemIndexWithUniqueID and // GetItemWithUniqueID functions. -TEST_F(NavigationManagerUtilTest, GetCommittedItemWithUniqueID) { +// TODO(crbug.com/733658): test was incorrectly moved to a separate target +// and not run and a refactoring broke it. Disable until the issue is fixed. +TEST_F(NavigationManagerUtilTest, DISABLED_GetCommittedItemWithUniqueID) { // Start with NavigationManager that only has a pending item. manager_.AddPendingItem( GURL("http://chromium.org"), Referrer(), ui::PAGE_TRANSITION_TYPED,
diff --git a/ios/web/navigation/serializable_user_data_manager_impl.h b/ios/web/navigation/serializable_user_data_manager_impl.h index 9230a2d..f002e90 100644 --- a/ios/web/navigation/serializable_user_data_manager_impl.h +++ b/ios/web/navigation/serializable_user_data_manager_impl.h
@@ -6,6 +6,7 @@ #define IOS_WEB_NAVIGATION_SERIALIZABLE_USER_DATA_MANAGER_IMPL_H_ #import "base/mac/scoped_nsobject.h" +#include "base/macros.h" #import "ios/web/public/serializable_user_data_manager.h" namespace web { @@ -16,30 +17,34 @@ ~SerializableUserDataImpl() override; // Constructor taking the NSDictionary holding the serializable data. - explicit SerializableUserDataImpl(NSDictionary* data); + explicit SerializableUserDataImpl( + NSDictionary<NSString*, id<NSCoding>>* data); // SerializableUserData: void Encode(NSCoder* coder) override; void Decode(NSCoder* coder) override; // Returns the serializable data. - NSDictionary* data() { return data_; } + NSDictionary<NSString*, id<NSCoding>>* data() { return data_; } + + // Returns the dictionary mapping the key of value that used to be persisted + // directly in CRWSessionStorate to the corresponding key when serialised in + // SerializableUserData. + // TODO(crbug.com/691800): Remove legacy support. + static NSDictionary<NSString*, NSString*>* GetLegacyKeyConversion(); private: // Decodes the values that were previously encoded using CRWSessionStorage's // NSCoding implementation and returns an NSDictionary using the new // serialization keys. // TODO(crbug.com/691800): Remove legacy support. - NSDictionary* GetDecodedLegacyValues(NSCoder* coder); + NSDictionary<NSString*, id<NSCoding>>* GetDecodedLegacyValues(NSCoder* coder); // The dictionary passed on initialization. After calling Decode(), this will // contain the data that is decoded from the NSCoder. - base::scoped_nsobject<NSDictionary> data_; - // Some values that were previously persisted directly in CRWSessionStorage - // are now serialized using SerializableUserData, and this dictionary is used - // to decode these values. The keys are the legacy encoding keys, and the - // values are their corresponding serializable user data keys. - base::scoped_nsobject<NSDictionary> legacy_key_conversions_; + base::scoped_nsobject<NSDictionary<NSString*, id<NSCoding>>> data_; + + DISALLOW_COPY_AND_ASSIGN(SerializableUserDataImpl); }; class SerializableUserDataManagerImpl : public SerializableUserDataManager { @@ -56,7 +61,9 @@ private: // The dictionary that stores serializable user data. - base::scoped_nsobject<NSMutableDictionary> data_; + base::scoped_nsobject<NSMutableDictionary<NSString*, id<NSCoding>>> data_; + + DISALLOW_COPY_AND_ASSIGN(SerializableUserDataManagerImpl); }; } // namespace web
diff --git a/ios/web/navigation/serializable_user_data_manager_impl.mm b/ios/web/navigation/serializable_user_data_manager_impl.mm index 5a4d214..95f9638d 100644 --- a/ios/web/navigation/serializable_user_data_manager_impl.mm +++ b/ios/web/navigation/serializable_user_data_manager_impl.mm
@@ -50,22 +50,6 @@ // The SerializableUserDataManagerWrapper owned by this object. SerializableUserDataManagerImpl manager_; }; - -// Returns a dictionary mapping old CRWSessionStorage serialised properties to -// the corresponding key in the serialised user data. When adding a mapping to -// this dictionary, create a new crbug to track its removal and mark it with a -// release at least one year after the introduction of the mapping. -NSDictionary* GetLegacyKeyConversion() { - NSMutableDictionary* legacy_key_conversion = [NSMutableDictionary dictionary]; - // TODO(crbug.com/661633): those mappings where introduced between M57 and - // M58, so remove them after M67 has shipped to stable. - [legacy_key_conversion addEntriesFromDictionary:@{ - @"tabId" : @"TabID", - @"openerId" : @"OpenerID", - @"openerNavigationIndex" : @"OpenerNavigationIndex", - }]; - return [legacy_key_conversion copy]; -} } // namespace // static @@ -73,12 +57,12 @@ return base::MakeUnique<SerializableUserDataImpl>(); } -SerializableUserDataImpl::SerializableUserDataImpl() - : data_(@{}), legacy_key_conversions_(GetLegacyKeyConversion()) {} +SerializableUserDataImpl::SerializableUserDataImpl() : data_(@{}) {} SerializableUserDataImpl::~SerializableUserDataImpl() {} -SerializableUserDataImpl::SerializableUserDataImpl(NSDictionary* data) +SerializableUserDataImpl::SerializableUserDataImpl( + NSDictionary<NSString*, id<NSCoding>>* data) : data_([data copy]) { DCHECK(data_); } @@ -88,7 +72,7 @@ } void SerializableUserDataImpl::Decode(NSCoder* coder) { - NSMutableDictionary* data = + NSMutableDictionary<NSString*, id<NSCoding>>* data = [[coder decodeObjectForKey:kSerializedUserDataKey] mutableCopy]; if (!data) { // Sessions saved with version M-57 or ealier do not have a serialized @@ -102,11 +86,27 @@ DCHECK(data_); } -NSDictionary* SerializableUserDataImpl::GetDecodedLegacyValues(NSCoder* coder) { - NSMutableDictionary* legacy_values = [[NSMutableDictionary alloc] init]; - for (NSString* legacy_key in [legacy_key_conversions_ allKeys]) { +// static +NSDictionary<NSString*, NSString*>* +SerializableUserDataImpl::GetLegacyKeyConversion() { + // TODO(crbug.com/661633): those mappings where introduced between M57 and + // M58, so remove them after M67 has shipped to stable. + return @{ + @"tabId" : @"TabID", + @"openerId" : @"OpenerID", + @"openerNavigationIndex" : @"OpenerNavigationIndex", + }; +} + +NSDictionary<NSString*, id<NSCoding>>* +SerializableUserDataImpl::GetDecodedLegacyValues(NSCoder* coder) { + NSMutableDictionary<NSString*, id<NSCoding>>* legacy_values = + [[NSMutableDictionary alloc] init]; + NSDictionary<NSString*, NSString*>* legacy_key_conversion = + GetLegacyKeyConversion(); + for (NSString* legacy_key in [legacy_key_conversion allKeys]) { id<NSCoding> value = [coder decodeObjectForKey:legacy_key]; - NSString* new_key = [legacy_key_conversions_ objectForKey:legacy_key]; + NSString* new_key = [legacy_key_conversion objectForKey:legacy_key]; legacy_values[new_key] = value; } return [legacy_values copy];
diff --git a/ios/web/navigation/serializable_user_data_manager_impl_unittest.mm b/ios/web/navigation/serializable_user_data_manager_impl_unittest.mm new file mode 100644 index 0000000..bf77b79e --- /dev/null +++ b/ios/web/navigation/serializable_user_data_manager_impl_unittest.mm
@@ -0,0 +1,65 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#import "ios/web/navigation/serializable_user_data_manager_impl.h" + +#import "ios/web/public/test/fakes/test_web_state.h" +#import "testing/gtest_mac.h" +#include "testing/platform_test.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +namespace { +// User Data and Key to use for tests. +NSString* const kTestUserData = @"TestUserData"; +NSString* const kTestUserDataKey = @"TestUserDataKey"; +} // namespace + +class SerializableUserDataManagerImplTest : public PlatformTest { + protected: + // Convenience getter for the user data manager. + web::SerializableUserDataManager* manager() { + return web::SerializableUserDataManager::FromWebState(&web_state_); + } + + web::TestWebState web_state_; +}; + +// Test +TEST_F(SerializableUserDataManagerImplTest, TestLegacyKeyConversion) { + NSDictionary<NSString*, NSString*>* legacy_key_conversion = + web::SerializableUserDataImpl::GetLegacyKeyConversion(); + + // Create data mapping legacy key to itself. + NSMutableData* data = [[NSMutableData alloc] init]; + NSKeyedArchiver* archiver = + [[NSKeyedArchiver alloc] initForWritingWithMutableData:data]; + for (NSString* key : [legacy_key_conversion allKeys]) { + [archiver encodeObject:key forKey:key]; + } + [archiver finishEncoding]; + + // Decode data and check that legacy key have been converted. + std::unique_ptr<web::SerializableUserData> user_data = + web::SerializableUserData::Create(); + NSKeyedUnarchiver* unarchiver = + [[NSKeyedUnarchiver alloc] initForReadingWithData:data]; + user_data->Decode(unarchiver); + + web::TestWebState web_state; + web::SerializableUserDataManager* user_data_manager = + web::SerializableUserDataManager::FromWebState(&web_state); + user_data_manager->AddSerializableUserData(user_data.get()); + + // Check that all key have been converted. + for (NSString* key : [legacy_key_conversion allKeys]) { + id value = user_data_manager->GetValueForSerializationKey(key); + EXPECT_NSEQ(nil, value); + value = user_data_manager->GetValueForSerializationKey( + [legacy_key_conversion objectForKey:key]); + EXPECT_NSEQ(key, value); + } +}
diff --git a/ios/web/public/serializable_user_data_manager_unittest.mm b/ios/web/public/serializable_user_data_manager_unittest.mm index 11548be..b4947ac6 100644 --- a/ios/web/public/serializable_user_data_manager_unittest.mm +++ b/ios/web/public/serializable_user_data_manager_unittest.mm
@@ -65,3 +65,31 @@ decoded_manager->GetValueForSerializationKey(kTestUserDataKey); EXPECT_NSEQ(decoded_value, kTestUserData); } + +// Check that if serialized data does not include user data, then restored +// SerializableUserDataManager still allow reading and writing user data +// (see http://crbug.com/699249 for details). +TEST_F(SerializableUserDataManagerTest, DecodeNoData) { + NSMutableData* data = [[NSMutableData alloc] init]; + NSKeyedArchiver* archiver = + [[NSKeyedArchiver alloc] initForWritingWithMutableData:data]; + [archiver finishEncoding]; + + std::unique_ptr<web::SerializableUserData> user_data = + web::SerializableUserData::Create(); + NSKeyedUnarchiver* unarchiver = + [[NSKeyedUnarchiver alloc] initForReadingWithData:data]; + user_data->Decode(unarchiver); + + web::TestWebState web_state; + web::SerializableUserDataManager* user_data_manager = + web::SerializableUserDataManager::FromWebState(&web_state); + user_data_manager->AddSerializableUserData(user_data.get()); + + id value = user_data_manager->GetValueForSerializationKey(kTestUserDataKey); + EXPECT_NSEQ(nil, value); + + user_data_manager->AddSerializableData(kTestUserData, kTestUserDataKey); + value = user_data_manager->GetValueForSerializationKey(kTestUserDataKey); + EXPECT_NSEQ(kTestUserData, value); +}
diff --git a/media/audio/audio_input_controller.cc b/media/audio/audio_input_controller.cc index 7d2a2cb..996b937 100644 --- a/media/audio/audio_input_controller.cc +++ b/media/audio/audio_input_controller.cc
@@ -328,6 +328,7 @@ bool enable_agc) { DCHECK(task_runner_->BelongsToCurrentThread()); DCHECK(!stream_); + handler_->OnLog(this, "AIC::DoCreateForStream"); if (!stream_to_control) { LogCaptureStartupResult(CAPTURE_STARTUP_CREATE_STREAM_FAILED); @@ -386,7 +387,9 @@ if (!stream_) return; - // Allow calling unconditionally and bail if we don't have a stream to close. + std::string log_string; + static const char kLogStringPrefix[] = "AIC::DoClose:"; + if (audio_callback_) { stream_->Stop(); @@ -403,14 +406,15 @@ : CAPTURE_STARTUP_NEVER_GOT_DATA); LogCaptureStartupResult(capture_startup_result); LogCallbackError(); + + log_string = base::StringPrintf( + "%s stream duration=%" PRId64 " seconds%s", kLogStringPrefix, + duration.InSeconds(), + audio_callback_->received_callback() ? "" : " (no callbacks received)"); + if (type_ == LOW_LATENCY) { if (audio_callback_->received_callback()) { - // Log the total duration (since DoCreate) and update the histogram. UMA_HISTOGRAM_LONG_TIMES("Media.InputStreamDuration", duration); - const std::string log_string = base::StringPrintf( - "AIC::DoClose: stream duration=%" PRId64 " seconds", - duration.InSeconds()); - handler_->OnLog(this, log_string); } else { UMA_HISTOGRAM_LONG_TIMES("Media.InputStreamDurationWithoutCallback", duration); @@ -418,8 +422,13 @@ } audio_callback_.reset(); + } else { + log_string = + base::StringPrintf("%s recording never started", kLogStringPrefix); } + handler_->OnLog(this, log_string); + stream_->Close(); stream_ = nullptr;
diff --git a/media/audio/audio_input_controller_unittest.cc b/media/audio/audio_input_controller_unittest.cc index 4e2a714..ad3b04ef 100644 --- a/media/audio/audio_input_controller_unittest.cc +++ b/media/audio/audio_input_controller_unittest.cc
@@ -131,7 +131,7 @@ AudioDeviceDescription::kDefaultDeviceId, false); ASSERT_TRUE(controller.get()); EXPECT_CALL(event_handler, OnCreated(controller.get())).Times(Exactly(1)); - EXPECT_CALL(event_handler, OnLog(controller.get(), _)); + EXPECT_CALL(event_handler, OnLog(controller.get(), _)).Times(Exactly(3)); EXPECT_CALL(sync_writer, Close()).Times(Exactly(1)); ResumeAudioThread();
diff --git a/mojo/android/javatests/init_library.cc b/mojo/android/javatests/init_library.cc index 6d6b2861..5aa1a22 100644 --- a/mojo/android/javatests/init_library.cc +++ b/mojo/android/javatests/init_library.cc
@@ -40,8 +40,7 @@ JNI_EXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) { base::android::InitVM(vm); JNIEnv* env = base::android::AttachCurrentThread(); - if (!base::android::OnJNIOnLoadRegisterJNI(env) || !RegisterJNI(env) || - !NativeInit()) { + if (!RegisterJNI(env) || !NativeInit()) { return -1; } return JNI_VERSION_1_4;
diff --git a/net/test/android/net_test_jni_onload.cc b/net/test/android/net_test_jni_onload.cc index 0f0e5a6..e0a14f1 100644 --- a/net/test/android/net_test_jni_onload.cc +++ b/net/test/android/net_test_jni_onload.cc
@@ -23,8 +23,7 @@ } // namesapce bool OnJNIOnLoadRegisterJNI(JNIEnv* env) { - return base::android::OnJNIOnLoadRegisterJNI(env) && - base::android::RegisterJni(env) && RegisterJNI(env); + return base::android::RegisterJni(env) && RegisterJNI(env); } bool OnJNIOnLoadInit() {
diff --git a/remoting/client/jni/remoting_jni_onload.cc b/remoting/client/jni/remoting_jni_onload.cc index b4fa7c0..9db79a8 100644 --- a/remoting/client/jni/remoting_jni_onload.cc +++ b/remoting/client/jni/remoting_jni_onload.cc
@@ -32,8 +32,7 @@ JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) { base::android::InitVM(vm); JNIEnv* env = base::android::AttachCurrentThread(); - if (!base::android::OnJNIOnLoadRegisterJNI(env) || !RegisterJNI(env) || - !base::android::OnJNIOnLoadInit()) { + if (!RegisterJNI(env) || !base::android::OnJNIOnLoadInit()) { return -1; } return JNI_VERSION_1_4;
diff --git a/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc b/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc index 9d4347b..3c9b2d7 100644 --- a/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc +++ b/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc
@@ -194,7 +194,7 @@ mojom::ClientProcess* client_process, uint64_t dump_guid, bool success, - mojom::ProcessMemoryDumpPtr process_memory_dump) { + mojom::RawProcessMemoryDumpPtr process_memory_dump) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); auto it = pending_clients_for_current_dump_.find(client_process); @@ -270,13 +270,13 @@ pmd->os_dump = result.second->os_dump; } - for (auto& pair : result.second->extra_processes_dump) { + for (auto& pair : result.second->extra_processes_dumps) { const base::ProcessId extra_pid = pair.first; mojom::ProcessMemoryDumpPtr& pmd = finalized_pmds[extra_pid]; if (!pmd) pmd = mojom::ProcessMemoryDump::New(); DCHECK_EQ(0u, pmd->os_dump.resident_set_kb); - pmd->os_dump = result.second->extra_processes_dump[extra_pid]; + pmd->os_dump = result.second->extra_processes_dumps[extra_pid]; } pmd->process_type = result.second->process_type;
diff --git a/services/resource_coordinator/memory_instrumentation/coordinator_impl.h b/services/resource_coordinator/memory_instrumentation/coordinator_impl.h index 26312d08..6644bb1 100644 --- a/services/resource_coordinator/memory_instrumentation/coordinator_impl.h +++ b/services/resource_coordinator/memory_instrumentation/coordinator_impl.h
@@ -72,7 +72,7 @@ const RequestGlobalMemoryDumpCallback callback; // Collects the data received from OnProcessMemoryDumpResponse(). - std::vector<std::pair<base::ProcessId, mojom::ProcessMemoryDumpPtr>> + std::vector<std::pair<base::ProcessId, mojom::RawProcessMemoryDumpPtr>> process_memory_dumps; }; @@ -85,12 +85,12 @@ const mojom::ClientProcessPtr client; }; - // Callback of RequestProcessMemoryDump. + // Callback of RequestProcessMemoryInternalDump. void OnProcessMemoryDumpResponse( mojom::ClientProcess*, uint64_t dump_guid, bool success, - mojom::ProcessMemoryDumpPtr process_memory_dump); + mojom::RawProcessMemoryDumpPtr process_memory_dump); void PerformNextQueuedGlobalMemoryDump(); void FinalizeGlobalMemoryDumpIfAllManagersReplied();
diff --git a/services/resource_coordinator/memory_instrumentation/coordinator_impl_unittest.cc b/services/resource_coordinator/memory_instrumentation/coordinator_impl_unittest.cc index bbb6372..6e62f72 100644 --- a/services/resource_coordinator/memory_instrumentation/coordinator_impl_unittest.cc +++ b/services/resource_coordinator/memory_instrumentation/coordinator_impl_unittest.cc
@@ -92,7 +92,7 @@ const base::trace_event::MemoryDumpRequestArgs& args, const RequestProcessMemoryDumpCallback& callback) override { expected_calls_--; - callback.Run(args.dump_guid, true, mojom::ProcessMemoryDumpPtr()); + callback.Run(args.dump_guid, true, mojom::RawProcessMemoryDumpPtr()); } private:
diff --git a/services/resource_coordinator/public/cpp/memory_instrumentation/client_process_impl.cc b/services/resource_coordinator/public/cpp/memory_instrumentation/client_process_impl.cc index c923932..4a7a7c2 100644 --- a/services/resource_coordinator/public/cpp/memory_instrumentation/client_process_impl.cc +++ b/services/resource_coordinator/public/cpp/memory_instrumentation/client_process_impl.cc
@@ -69,18 +69,18 @@ uint64_t dump_guid, bool success, const base::Optional<base::trace_event::MemoryDumpCallbackResult>& result) { - mojom::ProcessMemoryDumpPtr process_memory_dump( - mojom::ProcessMemoryDump::New()); + mojom::RawProcessMemoryDumpPtr process_memory_dump( + mojom::RawProcessMemoryDump::New()); process_memory_dump->process_type = config_.process_type(); if (result) { process_memory_dump->os_dump = result->os_dump; process_memory_dump->chrome_dump = result->chrome_dump; - for (const auto& kv : result->extra_processes_dump) { + for (const auto& kv : result->extra_processes_dumps) { const base::ProcessId pid = kv.first; const base::trace_event::MemoryDumpCallbackResult::OSMemDump& os_mem_dump = kv.second; - DCHECK_EQ(0u, process_memory_dump->extra_processes_dump.count(pid)); - process_memory_dump->extra_processes_dump[pid] = os_mem_dump; + DCHECK_EQ(0u, process_memory_dump->extra_processes_dumps.count(pid)); + process_memory_dump->extra_processes_dumps[pid] = os_mem_dump; } } callback.Run(dump_guid, success, std::move(process_memory_dump));
diff --git a/services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom b/services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom index 57afe20..6f268fe 100644 --- a/services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom +++ b/services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom
@@ -19,6 +19,15 @@ DETAILED }; +enum ProcessType { + OTHER, + BROWSER, + RENDERER, + GPU, + UTILITY, + PLUGIN +}; + struct RequestArgs { uint64 dump_guid; DumpType dump_type; @@ -50,19 +59,20 @@ uint32 v8_total_kb = 0; }; -enum ProcessType { - OTHER, - BROWSER, - RENDERER, - GPU, - UTILITY, - PLUGIN +// This struct is used for internal communication between the memory +// service (Coordinator) and the client library (ClientProcessImpl). +struct RawProcessMemoryDump { + ProcessType process_type; + OSMemDump os_dump; + ChromeMemDump chrome_dump; + + // This is used only on Linux/CrOS to get around sandboxing. + // See crbug.com/461788. + map<mojo.common.mojom.ProcessId, OSMemDump> extra_processes_dumps; }; -// This struct is used both for: -// 1) The internal communication between the memory service (Coordinator) and -// the client library (ProcessLocalDumpManager). -// 2) The public-facing API Coordinator::RequestGlobalMemoryDump(). +// This struct is used for the public-facing API +// Coordinator::RequestGlobalMemoryDump(). struct ProcessMemoryDump { ProcessType process_type; OSMemDump os_dump; @@ -71,10 +81,6 @@ // This is roughly private, anonymous, non-discardable, resident or swapped // memory in kilobytes. For more details, see https://goo.gl/3kPb9S. uint32 private_footprint = 0; - - // This is used only in the use-case (1) and only on Linux/CrOS to get - // around sandboxing. See crbug.com/461788 . - map<mojo.common.mojom.ProcessId, OSMemDump> extra_processes_dump; }; @@ -92,7 +98,7 @@ // buffer of the target process and an ACK. A summary of the dump is also // returned in case of success. RequestProcessMemoryDump(RequestArgs args) => - (uint64 dump_guid, bool success, ProcessMemoryDump? process_memory_dump); + (uint64 dump_guid, bool success, RawProcessMemoryDump? process_memory_dump); }; // The memory-infra service implements this interface. There is one instance for
diff --git a/testing/android/native_test/native_test_jni_onload.cc b/testing/android/native_test/native_test_jni_onload.cc index 01c3aba..82d4e0b 100644 --- a/testing/android/native_test/native_test_jni_onload.cc +++ b/testing/android/native_test/native_test_jni_onload.cc
@@ -28,8 +28,7 @@ JNI_EXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) { base::android::InitVM(vm); JNIEnv* env = base::android::AttachCurrentThread(); - if (!base::android::OnJNIOnLoadRegisterJNI(env) || !RegisterJNI(env) || - !NativeInit()) { + if (!RegisterJNI(env) || !NativeInit()) { return -1; } return JNI_VERSION_1_4;
diff --git a/testing/variations/fieldtrial_testing_config.json b/testing/variations/fieldtrial_testing_config.json index ce3bb269..004f85a 100644 --- a/testing/variations/fieldtrial_testing_config.json +++ b/testing/variations/fieldtrial_testing_config.json
@@ -582,10 +582,19 @@ ], "experiments": [ { - "name": "Enabled_Warmup", - "params": { - "enable_warmup": "true" - } + "name": "Enabled" + } + ] + } + ], + "DataReductionProxyLowMemoryDevicePromo": [ + { + "platforms": [ + "android" + ], + "experiments": [ + { + "name": "Enabled" } ] } @@ -1646,6 +1655,18 @@ ] } ], + "NetAdaptiveProxyConnectionTimeout": [ + { + "platforms": [ + "android" + ], + "experiments": [ + { + "name": "Enabled" + } + ] + } + ], "NetDelayableH2AndQuicRequests": [ { "platforms": [
diff --git a/third_party/WebKit/LayoutTests/external/wpt/dom/events/Event-dispatch-click-expected.txt b/third_party/WebKit/LayoutTests/external/wpt/dom/events/Event-dispatch-click-expected.txt deleted file mode 100644 index 6fa10910..0000000 --- a/third_party/WebKit/LayoutTests/external/wpt/dom/events/Event-dispatch-click-expected.txt +++ /dev/null
@@ -1,17 +0,0 @@ -This is a testharness.js-based test. -PASS basic with click() -PASS basic with dispatchEvent() -PASS basic with wrong event class -PASS look at parents only when event bubbles -FAIL look at parents when event bubbles assert_true: expected true got false -FAIL pick the first with activation behavior <input type=checkbox> assert_true: child pre-click must be triggered expected true got false -PASS pick the first with activation behavior <a href> -PASS event state during post-click handling -PASS redispatch during post-click handling -PASS disabled checkbox still has activation behavior -PASS disabled checkbox still has activation behavior, part 2 -PASS disconnected checkbox should be checked -PASS disconnected radio should be checked -PASS disconnected form should not submit -Harness: the test ran to completion. -
diff --git a/third_party/WebKit/LayoutTests/external/wpt/dom/events/Event-dispatch-click.html b/third_party/WebKit/LayoutTests/external/wpt/dom/events/Event-dispatch-click.html index 760116be..29b0cae 100644 --- a/third_party/WebKit/LayoutTests/external/wpt/dom/events/Event-dispatch-click.html +++ b/third_party/WebKit/LayoutTests/external/wpt/dom/events/Event-dispatch-click.html
@@ -68,7 +68,7 @@ var child = input.appendChild(document.createElement("input")) child.type = "checkbox" child.onclick = t.step_func(function() { - assert_true(input.checked, "child pre-click must be triggered") + assert_true(child.checked, "child pre-click must be triggered") }) child.dispatchEvent(new MouseEvent("click", {bubbles:true})) t.done()
diff --git a/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-basic.html b/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-basic.html index 56a3742d..e4fbfce9 100644 --- a/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-basic.html +++ b/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-basic.html
@@ -25,7 +25,7 @@ testDiv.addEventListener('wheel', wheelHandler); if (window.eventSender) { eventSender.mouseMoveTo(testDiv.offsetLeft + 5, testDiv.offsetTop + 5); - eventSender.mouseScrollBy(-1, -2); + eventSender.mouseScrollBy(-1, -2, false, false, 0, true, 'phaseBegan'); var positive = "deltaX > 0 && deltaY > 0"; var correct = "deltaX == testDiv.scrollLeft && deltaY == testDiv.scrollTop"; shouldBecomeEqual(positive + " && " + correct , "true", finishJSTest);
diff --git a/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-ctrl.html b/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-ctrl.html index 94f1428..581c776b 100644 --- a/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-ctrl.html +++ b/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-ctrl.html
@@ -23,7 +23,7 @@ debug('With ctrl modifier set and canScroll set to be false'); wheelEventCount = 0; eventSender.mouseMoveTo(testDiv.offsetLeft + 5, testDiv.offsetTop + 5); - eventSender.mouseScrollBy(0, scrollAmount, false, true, "ctrlKey", false); + eventSender.mouseScrollBy(0, scrollAmount, false, true, "ctrlKey", false , "phaseBegan"); shouldBe("wheelEventCount", "1"); shouldEvaluateTo("deltaY", expectedDeltaY); shouldBeTrue("ctrlKey"); @@ -32,7 +32,7 @@ debug('Without ctrl and canScroll set to be default true'); wheelEventCount = 0; eventSender.mouseMoveTo(testDiv.offsetLeft + 5, testDiv.offsetTop + 5); - eventSender.mouseScrollBy(0, scrollAmount, false, true); + eventSender.mouseScrollBy(0, scrollAmount, false, true, 0, true, "phaseChanged"); shouldBe("wheelEventCount", "1"); shouldEvaluateTo("deltaY", expectedDeltaY); shouldBeFalse("ctrlKey"); @@ -45,7 +45,7 @@ debug('With ctrl modifier set and canScroll set to be false'); wheelEventCount = 0; eventSender.mouseMoveTo(testDiv.offsetLeft + 5, testDiv.offsetTop + 5); - eventSender.mouseScrollBy(0, scrollAmount, false, true, "ctrlKey", false); + eventSender.mouseScrollBy(0, scrollAmount, false, true, "ctrlKey", false, "phaseChanged"); shouldBe("wheelEventCount", "1"); shouldEvaluateTo("deltaY", expectedDeltaY); shouldBeTrue("ctrlKey"); @@ -54,7 +54,7 @@ debug('With ctrl modifier set and canScroll set to be true'); wheelEventCount = 0; eventSender.mouseMoveTo(testDiv.offsetLeft + 5, testDiv.offsetTop + 5); - eventSender.mouseScrollBy(0, scrollAmount, false, true, "ctrlKey", true); + eventSender.mouseScrollBy(0, scrollAmount, false, true, "ctrlKey", true, "phaseChanged"); shouldBe("wheelEventCount", "1"); shouldEvaluateTo("deltaY", expectedDeltaY); shouldBeTrue("ctrlKey"); @@ -63,7 +63,7 @@ debug('Now without ctrl and canScroll set to be default true'); wheelEventCount = 0; eventSender.mouseMoveTo(testDiv.offsetLeft + 5, testDiv.offsetTop + 5); - eventSender.mouseScrollBy(0, scrollAmount, false, true); + eventSender.mouseScrollBy(0, scrollAmount, false, true, 0, true, "phaseChanged"); shouldBe("wheelEventCount", "1"); shouldEvaluateTo("deltaY", expectedDeltaY); shouldBeFalse("ctrlKey");
diff --git a/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-in-scrolling-div.html b/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-in-scrolling-div.html index 3c091fc..d5a2cf7 100644 --- a/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-in-scrolling-div.html +++ b/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-in-scrolling-div.html
@@ -16,7 +16,7 @@ // EventSender uses 40px per tick. eventSender.mouseMoveTo(overflowElement.offsetLeft + 5, overflowElement.offsetTop + 5); - eventSender.mouseScrollBy(-2.5, -5); + eventSender.mouseScrollBy(-2.5, -5, false, true, 0, true, "phaseBegan"); } div = document.getElementById("overflow");
diff --git a/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-in-text-node.html b/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-in-text-node.html index bb8cb39..a54f5f9 100644 --- a/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-in-text-node.html +++ b/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-in-text-node.html
@@ -9,7 +9,7 @@ var div = document.querySelector('div'); if (window.eventSender) { eventSender.mouseMoveTo(div.offsetLeft + 5, div.offsetTop + 5); - eventSender.mouseScrollBy(0,120); + eventSender.mouseScrollBy(0, 120, false, true, 0, true, "phaseBegan"); } else { debug("FAIL: This test requires window.eventSender."); finishJSTest();
diff --git a/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-mousewheel-interaction.html b/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-mousewheel-interaction.html index ac46109..69b8b33 100644 --- a/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-mousewheel-interaction.html +++ b/third_party/WebKit/LayoutTests/fast/events/wheel/wheelevent-mousewheel-interaction.html
@@ -10,7 +10,7 @@ div.addEventListener('mousewheel', mouseWheelHandler); if (window.eventSender) { eventSender.mouseMoveTo(div.offsetLeft + 5, div.offsetTop + 5); - eventSender.mouseScrollBy(10, 20); + eventSender.mouseScrollBy(10, 20, false, true, 0, true, "phaseBegan"); finishJSTest(); } else { debug("FAIL: This test requires window.eventSender.");
diff --git a/third_party/WebKit/Source/core/dom/Node.cpp b/third_party/WebKit/Source/core/dom/Node.cpp index 61eadd4b..56f05f4 100644 --- a/third_party/WebKit/Source/core/dom/Node.cpp +++ b/third_party/WebKit/Source/core/dom/Node.cpp
@@ -2366,6 +2366,10 @@ void Node::WillCallDefaultEventHandler(const Event&) {} +bool Node::HasActivationBehavior() const { + return false; +} + bool Node::WillRespondToMouseMoveEvents() { if (IsDisabledFormControl(this)) return false;
diff --git a/third_party/WebKit/Source/core/dom/Node.h b/third_party/WebKit/Source/core/dom/Node.h index 3cbca5e..80e9e897 100644 --- a/third_party/WebKit/Source/core/dom/Node.h +++ b/third_party/WebKit/Source/core/dom/Node.h
@@ -788,6 +788,9 @@ // Perform the default action for an event. virtual void DefaultEventHandler(Event*); virtual void WillCallDefaultEventHandler(const Event&); + // Should return true if this Node has activation behavior. + // https://dom.spec.whatwg.org/#eventtarget-activation-behavior + virtual bool HasActivationBehavior() const; EventTargetData* GetEventTargetData() override; EventTargetData& EnsureEventTargetData() override;
diff --git a/third_party/WebKit/Source/core/events/EventDispatcher.cpp b/third_party/WebKit/Source/core/events/EventDispatcher.cpp index 951b3459..6e8d534 100644 --- a/third_party/WebKit/Source/core/events/EventDispatcher.cpp +++ b/third_party/WebKit/Source/core/events/EventDispatcher.cpp
@@ -126,6 +126,7 @@ nodes_dispatching_simulated_clicks.erase(&node); } +// https://dom.spec.whatwg.org/#dispatching-events DispatchEventResult EventDispatcher::Dispatch() { TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("blink.debug"), "EventDispatcher::dispatch"); @@ -141,6 +142,31 @@ } event_->GetEventPath().EnsureWindowEventContext(); + // 6. Let isActivationEvent be true, if event is a MouseEvent object and + // event's type attribute is "click", and false otherwise. + // + // We need to include non-standard textInput event for HTMLInputElement. + const bool is_activation_event = + (event_->IsMouseEvent() && event_->type() == EventTypeNames::click) || + event_->type() == EventTypeNames::textInput; + + // 7. Let activationTarget be target, if isActivationEvent is true and target + // has activation behavior, and null otherwise. + Node* activation_target = + is_activation_event && node_->HasActivationBehavior() ? node_ : nullptr; + + // A part of step 9 loop. + if (is_activation_event && !activation_target && event_->bubbles()) { + size_t size = event_->GetEventPath().size(); + for (size_t i = 1; i < size; ++i) { + Node* target = event_->GetEventPath()[i].GetNode(); + if (target->HasActivationBehavior()) { + activation_target = target; + break; + } + } + } + event_->SetTarget(EventPath::EventTargetRespectingTargetRules(*node_)); #if DCHECK_IS_ON() DCHECK(!EventDispatchForbiddenScope::IsEventDispatchForbidden()); @@ -149,14 +175,16 @@ TRACE_EVENT1("devtools.timeline", "EventDispatch", "data", InspectorEventDispatchEvent::Data(*event_)); EventDispatchHandlingState* pre_dispatch_event_handler_result = nullptr; - if (DispatchEventPreProcess(pre_dispatch_event_handler_result) == + if (DispatchEventPreProcess(activation_target, + pre_dispatch_event_handler_result) == kContinueDispatching) { if (DispatchEventAtCapturing() == kContinueDispatching) { if (DispatchEventAtTarget() == kContinueDispatching) DispatchEventAtBubbling(); } } - DispatchEventPostProcess(pre_dispatch_event_handler_result); + DispatchEventPostProcess(activation_target, + pre_dispatch_event_handler_result); // Ensure that after event dispatch, the event's target object is the // outermost shadow DOM boundary. @@ -167,11 +195,15 @@ } inline EventDispatchContinuation EventDispatcher::DispatchEventPreProcess( + Node* activation_target, EventDispatchHandlingState*& pre_dispatch_event_handler_result) { - // Give the target node a chance to do some work before DOM event handlers get - // a crack. - pre_dispatch_event_handler_result = - node_->PreDispatchEventHandler(event_.Get()); + // 11. If activationTarget is non-null and activationTarget has + // legacy-pre-activation behavior, then run activationTarget's + // legacy-pre-activation behavior. + if (activation_target) { + pre_dispatch_event_handler_result = + activation_target->PreDispatchEventHandler(event_.Get()); + } return (event_->GetEventPath().IsEmpty() || event_->PropagationStopped()) ? kDoneDispatching : kContinueDispatching; @@ -229,6 +261,7 @@ } inline void EventDispatcher::DispatchEventPostProcess( + Node* activation_target, EventDispatchHandlingState* pre_dispatch_event_handler_result) { event_->SetTarget(EventPath::EventTargetRespectingTargetRules(*node_)); // https://dom.spec.whatwg.org/#concept-event-dispatch @@ -248,15 +281,17 @@ // safe if m_event->target()->toNode() returns null. if (AXObjectCache* cache = node_->GetDocument().ExistingAXObjectCache()) cache->HandleClicked(event_->target()->ToNode()); - } - // Pass the data from the preDispatchEventHandler to the - // postDispatchEventHandler. - // This may dispatch an event, and m_node and m_event might be altered. - node_->PostDispatchEventHandler(event_.Get(), - pre_dispatch_event_handler_result); - // TODO(tkent): Is it safe to kick defaultEventHandler() with such altered - // m_event? + // Pass the data from the PreDispatchEventHandler to the + // PostDispatchEventHandler. + // This may dispatch an event, and node_ and event_ might be altered. + if (activation_target) { + activation_target->PostDispatchEventHandler( + event_.Get(), pre_dispatch_event_handler_result); + } + // TODO(tkent): Is it safe to kick DefaultEventHandler() with such altered + // event_? + } // The DOM Events spec says that events dispatched by JS (other than "click") // should not have their default handlers invoked.
diff --git a/third_party/WebKit/Source/core/events/EventDispatcher.h b/third_party/WebKit/Source/core/events/EventDispatcher.h index f2a5f1d..288ffea 100644 --- a/third_party/WebKit/Source/core/events/EventDispatcher.h +++ b/third_party/WebKit/Source/core/events/EventDispatcher.h
@@ -70,11 +70,13 @@ EventDispatcher(Node&, Event*); EventDispatchContinuation DispatchEventPreProcess( + Node* activation_target, EventDispatchHandlingState*&); EventDispatchContinuation DispatchEventAtCapturing(); EventDispatchContinuation DispatchEventAtTarget(); void DispatchEventAtBubbling(); - void DispatchEventPostProcess(EventDispatchHandlingState*); + void DispatchEventPostProcess(Node* activation_target, + EventDispatchHandlingState*); Member<Node> node_; Member<Event> event_;
diff --git a/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.cpp b/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.cpp index 33c9566..7523950 100644 --- a/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.cpp +++ b/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.cpp
@@ -47,7 +47,6 @@ #include "core/workers/ParentFrameTaskRunners.h" #include "core/workers/SharedWorkerGlobalScope.h" #include "core/workers/SharedWorkerThread.h" -#include "core/workers/WorkerClients.h" #include "core/workers/WorkerContentSettingsClient.h" #include "core/workers/WorkerGlobalScope.h" #include "core/workers/WorkerInspectorProxy.h" @@ -78,21 +77,12 @@ namespace blink { -namespace { - -// Vector of callbacks for the OnScriptLoaderFinished method. -using WorkerClientsCreatedCallbackVector = - WTF::Vector<WebSharedWorkerImpl::WorkerClientsCreatedCallback>; -WorkerClientsCreatedCallbackVector& GetWorkerClientsCreatedCallbackVector() { - DEFINE_STATIC_LOCAL(WorkerClientsCreatedCallbackVector, callback_vector, ()); - return callback_vector; -} - -} // namespace - // TODO(toyoshim): Share implementation with WebEmbeddedWorkerImpl as much as // possible. +template class CORE_TEMPLATE_EXPORT + WorkerClientsInitializer<WebSharedWorkerImpl>; + WebSharedWorkerImpl::WebSharedWorkerImpl(WebSharedWorkerClient* client) : web_view_(nullptr), main_frame_(nullptr), @@ -263,11 +253,6 @@ delete this; } -void WebSharedWorkerImpl::RegisterWorkerClientsCreatedCallback( - WorkerClientsCreatedCallback callback) { - GetWorkerClientsCreatedCallbackVector().push_back(callback); -} - void WebSharedWorkerImpl::Connect( std::unique_ptr<WebMessagePortChannel> web_channel) { DCHECK(IsMainThread()); @@ -338,10 +323,7 @@ SecurityOrigin* starter_origin = loading_document_->GetSecurityOrigin(); WorkerClients* worker_clients = WorkerClients::Create(); - DCHECK(!GetWorkerClientsCreatedCallbackVector().IsEmpty()); - for (auto& callback : GetWorkerClientsCreatedCallbackVector()) { - callback(worker_clients); - } + WorkerClientsInitializer<WebSharedWorkerImpl>::Run(worker_clients); WebSecurityOrigin web_security_origin(loading_document_->GetSecurityOrigin()); ProvideContentSettingsClientToWorker(
diff --git a/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.h b/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.h index a2b7608d..0d73710 100644 --- a/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.h +++ b/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.h
@@ -37,6 +37,7 @@ #include "core/CoreExport.h" #include "core/dom/ExecutionContext.h" #include "core/workers/SharedWorkerReportingProxy.h" +#include "core/workers/WorkerClients.h" #include "core/workers/WorkerThread.h" #include "platform/wtf/RefPtr.h" #include "public/platform/Platform.h" @@ -56,7 +57,6 @@ class WebString; class WebURL; class WebView; -class WorkerClients; class WorkerInspectorProxy; class WorkerScriptLoader; @@ -64,9 +64,10 @@ // implementation. This is basically accessed on the main thread, but some // methods must be called from a worker thread. Such methods are suffixed with // *OnWorkerThread or have header comments. -class WebSharedWorkerImpl final : public WebFrameClient, - public WebSharedWorker, - public WebDevToolsAgentClient { +class CORE_EXPORT WebSharedWorkerImpl final + : public WebFrameClient, + public WebSharedWorker, + NON_EXPORTED_BASE(public WebDevToolsAgentClient) { public: explicit WebSharedWorkerImpl(WebSharedWorkerClient*); @@ -117,14 +118,6 @@ void DidCloseWorkerGlobalScope(); void DidTerminateWorkerThread(); - using WorkerClientsCreatedCallback = void (*)(WorkerClients*); - // Allows for the registration of a callback that is invoked whenever a new - // OnScriptLoaderFinished is called. Callbacks are executed in the order that - // they were added using RegisterWorkerClientsCreatedCallback, and there are - // no checks for adding a callback multiple times. - CORE_EXPORT static void RegisterWorkerClientsCreatedCallback( - WorkerClientsCreatedCallback); - private: ~WebSharedWorkerImpl() override; @@ -170,6 +163,9 @@ WebAddressSpace creation_address_space_; }; +extern template class CORE_EXTERN_TEMPLATE_EXPORT + WorkerClientsInitializer<WebSharedWorkerImpl>; + } // namespace blink #endif // WebSharedWorkerImpl_h
diff --git a/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp b/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp index 8a230f1b..af4632e 100644 --- a/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp
@@ -171,6 +171,10 @@ HTMLElement::DefaultEventHandler(event); } +bool HTMLAnchorElement::HasActivationBehavior() const { + return true; +} + void HTMLAnchorElement::SetActive(bool down) { if (HasEditableStyle(*this)) return;
diff --git a/third_party/WebKit/Source/core/html/HTMLAnchorElement.h b/third_party/WebKit/Source/core/html/HTMLAnchorElement.h index 8804adb..a430293 100644 --- a/third_party/WebKit/Source/core/html/HTMLAnchorElement.h +++ b/third_party/WebKit/Source/core/html/HTMLAnchorElement.h
@@ -108,6 +108,7 @@ bool IsMouseFocusable() const override; bool IsKeyboardFocusable() const override; void DefaultEventHandler(Event*) final; + bool HasActivationBehavior() const override; void SetActive(bool = true) final; void AccessKeyAction(bool send_mouse_events) final; bool IsURLAttribute(const Attribute&) const final;
diff --git a/third_party/WebKit/Source/core/html/HTMLButtonElement.cpp b/third_party/WebKit/Source/core/html/HTMLButtonElement.cpp index e69bab3..05b817b 100644 --- a/third_party/WebKit/Source/core/html/HTMLButtonElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLButtonElement.cpp
@@ -148,6 +148,10 @@ HTMLFormControlElement::DefaultEventHandler(event); } +bool HTMLButtonElement::HasActivationBehavior() const { + return true; +} + bool HTMLButtonElement::WillRespondToMouseClickEvents() { if (!IsDisabledFormControl() && Form() && (type_ == SUBMIT || type_ == RESET)) return true;
diff --git a/third_party/WebKit/Source/core/html/HTMLButtonElement.h b/third_party/WebKit/Source/core/html/HTMLButtonElement.h index 59c724a..c5d7dde3 100644 --- a/third_party/WebKit/Source/core/html/HTMLButtonElement.h +++ b/third_party/WebKit/Source/core/html/HTMLButtonElement.h
@@ -56,6 +56,7 @@ void ParseAttribute(const AttributeModificationParams&) override; bool IsPresentationAttribute(const QualifiedName&) const override; void DefaultEventHandler(Event*) override; + bool HasActivationBehavior() const override; void AppendToFormData(FormData&) override;
diff --git a/third_party/WebKit/Source/core/html/HTMLInputElement.cpp b/third_party/WebKit/Source/core/html/HTMLInputElement.cpp index c946452..56d1871 100644 --- a/third_party/WebKit/Source/core/html/HTMLInputElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLInputElement.cpp
@@ -1301,6 +1301,10 @@ TextControlElement::DefaultEventHandler(evt); } +bool HTMLInputElement::HasActivationBehavior() const { + return true; +} + bool HTMLInputElement::WillRespondToMouseClickEvents() { // FIXME: Consider implementing willRespondToMouseClickEvents() in InputType // if more accurate results are necessary.
diff --git a/third_party/WebKit/Source/core/html/HTMLInputElement.h b/third_party/WebKit/Source/core/html/HTMLInputElement.h index 71982f3e..02af52c0 100644 --- a/third_party/WebKit/Source/core/html/HTMLInputElement.h +++ b/third_party/WebKit/Source/core/html/HTMLInputElement.h
@@ -312,6 +312,7 @@ InsertionNotificationRequest InsertedInto(ContainerNode*) override; void RemovedFrom(ContainerNode*) final; void DidMoveToNewDocument(Document& old_document) final; + bool HasActivationBehavior() const override; bool HasCustomFocusLogic() const final; bool IsKeyboardFocusable() const final;
diff --git a/third_party/WebKit/Source/core/html/HTMLLabelElement.cpp b/third_party/WebKit/Source/core/html/HTMLLabelElement.cpp index df06871d..6e64ac37 100644 --- a/third_party/WebKit/Source/core/html/HTMLLabelElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLLabelElement.cpp
@@ -213,6 +213,10 @@ HTMLElement::DefaultEventHandler(evt); } +bool HTMLLabelElement::HasActivationBehavior() const { + return true; +} + bool HTMLLabelElement::WillRespondToMouseClickEvents() { if (control() && control()->WillRespondToMouseClickEvents()) return true;
diff --git a/third_party/WebKit/Source/core/html/HTMLLabelElement.h b/third_party/WebKit/Source/core/html/HTMLLabelElement.h index 2e2f239a..90f1d5b 100644 --- a/third_party/WebKit/Source/core/html/HTMLLabelElement.h +++ b/third_party/WebKit/Source/core/html/HTMLLabelElement.h
@@ -54,6 +54,7 @@ // Overridden to either click() or focus() the corresponding control. void DefaultEventHandler(Event*) override; + bool HasActivationBehavior() const override; void focus(const FocusParams&) override;
diff --git a/third_party/WebKit/Source/core/html/HTMLLinkElement.cpp b/third_party/WebKit/Source/core/html/HTMLLinkElement.cpp index 8cc37030..b157134 100644 --- a/third_party/WebKit/Source/core/html/HTMLLinkElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLLinkElement.cpp
@@ -237,6 +237,11 @@ HTMLElement::FinishParsingChildren(); } +bool HTMLLinkElement::HasActivationBehavior() const { + // TODO(tkent): Implement activation behavior. crbug.com/422732. + return false; +} + bool HTMLLinkElement::StyleSheetIsLoading() const { return GetLinkStyle() && GetLinkStyle()->StyleSheetIsLoading(); }
diff --git a/third_party/WebKit/Source/core/html/HTMLLinkElement.h b/third_party/WebKit/Source/core/html/HTMLLinkElement.h index 809be07..c9aa13cf 100644 --- a/third_party/WebKit/Source/core/html/HTMLLinkElement.h +++ b/third_party/WebKit/Source/core/html/HTMLLinkElement.h
@@ -139,6 +139,7 @@ LoadedSheetErrorStatus) override; void StartLoadingDynamicSheet() override; void FinishParsingChildren() override; + bool HasActivationBehavior() const override; // From LinkLoaderClient void LinkLoaded() override;
diff --git a/third_party/WebKit/Source/core/html/HTMLSummaryElement.cpp b/third_party/WebKit/Source/core/html/HTMLSummaryElement.cpp index 2cd283d3..eb3fc7b 100644 --- a/third_party/WebKit/Source/core/html/HTMLSummaryElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLSummaryElement.cpp
@@ -139,6 +139,10 @@ HTMLElement::DefaultEventHandler(event); } +bool HTMLSummaryElement::HasActivationBehavior() const { + return true; +} + bool HTMLSummaryElement::WillRespondToMouseClickEvents() { return IsMainSummary() || HTMLElement::WillRespondToMouseClickEvents(); }
diff --git a/third_party/WebKit/Source/core/html/HTMLSummaryElement.h b/third_party/WebKit/Source/core/html/HTMLSummaryElement.h index b76d079..e778009 100644 --- a/third_party/WebKit/Source/core/html/HTMLSummaryElement.h +++ b/third_party/WebKit/Source/core/html/HTMLSummaryElement.h
@@ -40,6 +40,7 @@ LayoutObject* CreateLayoutObject(const ComputedStyle&) override; void DefaultEventHandler(Event*) override; + bool HasActivationBehavior() const override; void DidAddUserAgentShadowRoot(ShadowRoot&) override; HTMLDetailsElement* DetailsElement() const;
diff --git a/third_party/WebKit/Source/core/input/EventHandler.cpp b/third_party/WebKit/Source/core/input/EventHandler.cpp index 1b0e7390..63891ab 100644 --- a/third_party/WebKit/Source/core/input/EventHandler.cpp +++ b/third_party/WebKit/Source/core/input/EventHandler.cpp
@@ -238,6 +238,7 @@ scroll_manager_->Clear(); gesture_manager_->Clear(); mouse_event_manager_->Clear(); + mouse_wheel_event_manager_->Clear(); last_deferred_tap_element_ = nullptr; event_handler_will_reset_capturing_mouse_events_node_ = false; }
diff --git a/third_party/WebKit/Source/core/input/MouseWheelEventManager.cpp b/third_party/WebKit/Source/core/input/MouseWheelEventManager.cpp index e830907..6a8a308 100644 --- a/third_party/WebKit/Source/core/input/MouseWheelEventManager.cpp +++ b/third_party/WebKit/Source/core/input/MouseWheelEventManager.cpp
@@ -18,37 +18,100 @@ namespace blink { MouseWheelEventManager::MouseWheelEventManager(LocalFrame& frame, ScrollManager& scroll_manager) - : frame_(frame), scroll_manager_(scroll_manager) {} + : frame_(frame), wheel_target_(nullptr), scroll_manager_(scroll_manager) {} DEFINE_TRACE(MouseWheelEventManager) { visitor->Trace(frame_); + visitor->Trace(wheel_target_); visitor->Trace(scroll_manager_); } +void MouseWheelEventManager::Clear() { + wheel_target_ = nullptr; +} + WebInputEventResult MouseWheelEventManager::HandleWheelEvent( const WebMouseWheelEvent& event) { -#if OS(MACOSX) - // Filter Mac OS specific phases, usually with a zero-delta. - // https://crbug.com/553732 - // TODO(chongz): EventSender sends events with - // |WebMouseWheelEvent::PhaseNone|, - // but it shouldn't. - const int kWheelEventPhaseNoEventMask = WebMouseWheelEvent::kPhaseEnded | - WebMouseWheelEvent::kPhaseCancelled | - WebMouseWheelEvent::kPhaseMayBegin; - if ((event.phase & kWheelEventPhaseNoEventMask) || - (event.momentum_phase & kWheelEventPhaseNoEventMask)) - return WebInputEventResult::kNotHandled; -#endif - Document* doc = frame_->GetDocument(); + bool wheel_scroll_latching = + RuntimeEnabledFeatures::TouchpadAndWheelScrollLatchingEnabled(); - if (doc->GetLayoutViewItem().IsNull()) + Document* doc = frame_->GetDocument(); + if (!doc || doc->GetLayoutViewItem().IsNull()) return WebInputEventResult::kNotHandled; LocalFrameView* view = frame_->View(); if (!view) return WebInputEventResult::kNotHandled; + if (wheel_scroll_latching) { + const int kWheelEventPhaseEndedEventMask = + WebMouseWheelEvent::kPhaseEnded | WebMouseWheelEvent::kPhaseCancelled; + const int kWheelEventPhaseNoEventMask = + kWheelEventPhaseEndedEventMask | WebMouseWheelEvent::kPhaseMayBegin; + + if ((event.phase & kWheelEventPhaseEndedEventMask) || + (event.momentum_phase & kWheelEventPhaseEndedEventMask)) { + wheel_target_ = nullptr; + } + + if ((event.phase & kWheelEventPhaseNoEventMask) || + (event.momentum_phase & kWheelEventPhaseNoEventMask)) { + // Filter wheel events with zero deltas and reset the wheel_target_ node. + DCHECK(!event.delta_x && !event.delta_y); + return WebInputEventResult::kNotHandled; + } + + if (event.phase == WebMouseWheelEvent::kPhaseBegan) { + // Find and save the wheel_target_, this target will be used for the rest + // of the current scrolling sequence. + DCHECK(!wheel_target_); + wheel_target_ = FindTargetNode(event, doc, view); + } + } else { // !wheel_scroll_latching, wheel_target_ will be updated for each + // wheel event. +#if OS(MACOSX) + // Filter Mac OS specific phases, usually with a zero-delta. + // https://crbug.com/553732 + // TODO(chongz): EventSender sends events with + // |WebMouseWheelEvent::PhaseNone|, + // but it shouldn't. + const int kWheelEventPhaseNoEventMask = + WebMouseWheelEvent::kPhaseEnded | WebMouseWheelEvent::kPhaseCancelled | + WebMouseWheelEvent::kPhaseMayBegin; + if ((event.phase & kWheelEventPhaseNoEventMask) || + (event.momentum_phase & kWheelEventPhaseNoEventMask)) + return WebInputEventResult::kNotHandled; +#endif + + wheel_target_ = FindTargetNode(event, doc, view); + } + + LocalFrame* subframe = + EventHandlingUtil::SubframeForTargetNode(wheel_target_.Get()); + if (subframe) { + WebInputEventResult result = + subframe->GetEventHandler().HandleWheelEvent(event); + if (result != WebInputEventResult::kNotHandled) + scroll_manager_->SetFrameWasScrolledByUser(); + return result; + } + + if (wheel_target_) { + WheelEvent* dom_event = + WheelEvent::Create(event, wheel_target_->GetDocument().domWindow()); + DispatchEventResult dom_event_result = + wheel_target_->DispatchEvent(dom_event); + if (dom_event_result != DispatchEventResult::kNotCanceled) + return EventHandlingUtil::ToWebInputEventResult(dom_event_result); + } + + return WebInputEventResult::kNotHandled; +} + +Node* MouseWheelEventManager::FindTargetNode(const WebMouseWheelEvent& event, + const Document* doc, + const LocalFrameView* view) { + DCHECK(doc && !doc->GetLayoutViewItem().IsNull() && view); LayoutPoint v_point = view->RootFrameToContents(FlooredIntPoint(event.PositionInRootFrame())); @@ -65,24 +128,7 @@ if (!node && result.GetScrollbar()) node = doc->documentElement(); - LocalFrame* subframe = EventHandlingUtil::SubframeForTargetNode(node); - if (subframe) { - WebInputEventResult result = - subframe->GetEventHandler().HandleWheelEvent(event); - if (result != WebInputEventResult::kNotHandled) - scroll_manager_->SetFrameWasScrolledByUser(); - return result; - } - - if (node) { - WheelEvent* dom_event = - WheelEvent::Create(event, node->GetDocument().domWindow()); - DispatchEventResult dom_event_result = node->DispatchEvent(dom_event); - if (dom_event_result != DispatchEventResult::kNotCanceled) - return EventHandlingUtil::ToWebInputEventResult(dom_event_result); - } - - return WebInputEventResult::kNotHandled; + return node; } } // namespace blink
diff --git a/third_party/WebKit/Source/core/input/MouseWheelEventManager.h b/third_party/WebKit/Source/core/input/MouseWheelEventManager.h index ecf2e5ae..809ed02 100644 --- a/third_party/WebKit/Source/core/input/MouseWheelEventManager.h +++ b/third_party/WebKit/Source/core/input/MouseWheelEventManager.h
@@ -11,7 +11,10 @@ namespace blink { +class Document; class LocalFrame; +class LocalFrameView; +class Node; class ScrollManager; class WebMouseWheelEvent; @@ -23,10 +26,17 @@ explicit MouseWheelEventManager(LocalFrame&, ScrollManager&); DECLARE_TRACE(); + void Clear(); + WebInputEventResult HandleWheelEvent(const WebMouseWheelEvent&); private: + Node* FindTargetNode(const WebMouseWheelEvent&, + const Document*, + const LocalFrameView*); + const Member<LocalFrame> frame_; + Member<Node> wheel_target_; Member<ScrollManager> scroll_manager_; };
diff --git a/third_party/WebKit/Source/core/svg/SVGAElement.cpp b/third_party/WebKit/Source/core/svg/SVGAElement.cpp index 4b29c6b..ca60d7f 100644 --- a/third_party/WebKit/Source/core/svg/SVGAElement.cpp +++ b/third_party/WebKit/Source/core/svg/SVGAElement.cpp
@@ -145,6 +145,10 @@ SVGGraphicsElement::DefaultEventHandler(event); } +bool SVGAElement::HasActivationBehavior() const { + return true; +} + int SVGAElement::tabIndex() const { // Skip the supportsFocus check in SVGElement. return Element::tabIndex();
diff --git a/third_party/WebKit/Source/core/svg/SVGAElement.h b/third_party/WebKit/Source/core/svg/SVGAElement.h index 04f7d5f..5dfb265 100644 --- a/third_party/WebKit/Source/core/svg/SVGAElement.h +++ b/third_party/WebKit/Source/core/svg/SVGAElement.h
@@ -49,6 +49,7 @@ LayoutObject* CreateLayoutObject(const ComputedStyle&) override; void DefaultEventHandler(Event*) override; + bool HasActivationBehavior() const override; bool IsLiveLink() const override { return IsLink(); }
diff --git a/third_party/WebKit/Source/core/workers/BUILD.gn b/third_party/WebKit/Source/core/workers/BUILD.gn index 1dcc602..b747d62 100644 --- a/third_party/WebKit/Source/core/workers/BUILD.gn +++ b/third_party/WebKit/Source/core/workers/BUILD.gn
@@ -12,8 +12,6 @@ "DedicatedWorkerGlobalScope.h", "DedicatedWorkerMessagingProxy.cpp", "DedicatedWorkerMessagingProxy.h", - "DedicatedWorkerMessagingProxyProvider.cpp", - "DedicatedWorkerMessagingProxyProvider.h", "DedicatedWorkerThread.cpp", "DedicatedWorkerThread.h", "InProcessWorkerBase.cpp",
diff --git a/third_party/WebKit/Source/core/workers/DedicatedWorkerMessagingProxyProvider.cpp b/third_party/WebKit/Source/core/workers/DedicatedWorkerMessagingProxyProvider.cpp deleted file mode 100644 index 2d2ed9af..0000000 --- a/third_party/WebKit/Source/core/workers/DedicatedWorkerMessagingProxyProvider.cpp +++ /dev/null
@@ -1,58 +0,0 @@ -/* - * Copyright (C) 2013 Google Inc. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google Inc. nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -#include "core/workers/DedicatedWorkerMessagingProxyProvider.h" - -#include "core/page/Page.h" - -namespace blink { - -DedicatedWorkerMessagingProxyProvider* -DedicatedWorkerMessagingProxyProvider::From(Page& page) { - return static_cast<DedicatedWorkerMessagingProxyProvider*>( - Supplement<Page>::From(page, SupplementName())); -} - -const char* DedicatedWorkerMessagingProxyProvider::SupplementName() { - return "DedicatedWorkerMessagingProxyProvider"; -} - -DedicatedWorkerMessagingProxyProvider::DedicatedWorkerMessagingProxyProvider( - Page& page) - : Supplement<Page>(page) {} - -void ProvideDedicatedWorkerMessagingProxyProviderTo( - Page& page, - DedicatedWorkerMessagingProxyProvider* provider) { - Supplement<Page>::ProvideTo( - page, DedicatedWorkerMessagingProxyProvider::SupplementName(), provider); -} - -} // namespace blink
diff --git a/third_party/WebKit/Source/core/workers/DedicatedWorkerMessagingProxyProvider.h b/third_party/WebKit/Source/core/workers/DedicatedWorkerMessagingProxyProvider.h deleted file mode 100644 index fad6cb0..0000000 --- a/third_party/WebKit/Source/core/workers/DedicatedWorkerMessagingProxyProvider.h +++ /dev/null
@@ -1,67 +0,0 @@ -/* - * Copyright (C) 2013 Google Inc. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google Inc. nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -#ifndef DedicatedWorkerMessagingProxyProvider_h -#define DedicatedWorkerMessagingProxyProvider_h - -#include "core/CoreExport.h" -#include "core/page/Page.h" -#include "platform/Supplementable.h" -#include "platform/wtf/Forward.h" -#include "platform/wtf/Noncopyable.h" - -namespace blink { - -class InProcessWorkerMessagingProxy; -class Page; -class Worker; - -class CORE_EXPORT DedicatedWorkerMessagingProxyProvider - : public Supplement<Page> { - WTF_MAKE_NONCOPYABLE(DedicatedWorkerMessagingProxyProvider); - - public: - explicit DedicatedWorkerMessagingProxyProvider(Page&); - virtual ~DedicatedWorkerMessagingProxyProvider() {} - - virtual InProcessWorkerMessagingProxy* CreateWorkerMessagingProxy( - Worker*) = 0; - - static DedicatedWorkerMessagingProxyProvider* From(Page&); - static const char* SupplementName(); -}; - -CORE_EXPORT void ProvideDedicatedWorkerMessagingProxyProviderTo( - Page&, - DedicatedWorkerMessagingProxyProvider*); - -} // namespace blink - -#endif // DedicatedWorkerMessagingProxyProvider_h
diff --git a/third_party/WebKit/Source/core/workers/Worker.cpp b/third_party/WebKit/Source/core/workers/Worker.cpp index 8232da6c..9d70e84 100644 --- a/third_party/WebKit/Source/core/workers/Worker.cpp +++ b/third_party/WebKit/Source/core/workers/Worker.cpp
@@ -8,11 +8,16 @@ #include "core/dom/Document.h" #include "core/dom/ExceptionCode.h" #include "core/frame/UseCounter.h" -#include "core/workers/DedicatedWorkerMessagingProxyProvider.h" -#include "core/workers/InProcessWorkerMessagingProxy.h" +#include "core/frame/WebLocalFrameBase.h" +#include "core/workers/DedicatedWorkerMessagingProxy.h" +#include "core/workers/WorkerContentSettingsClient.h" +#include "public/platform/WebContentSettingsClient.h" +#include "public/web/WebFrameClient.h" namespace blink { +template class CORE_TEMPLATE_EXPORT WorkerClientsInitializer<Worker>; + Worker::Worker(ExecutionContext* context) : InProcessWorkerBase(context) {} Worker* Worker::Create(ExecutionContext* context, @@ -43,10 +48,14 @@ InProcessWorkerMessagingProxy* Worker::CreateInProcessWorkerMessagingProxy( ExecutionContext* context) { Document* document = ToDocument(context); - DedicatedWorkerMessagingProxyProvider* proxy_provider = - DedicatedWorkerMessagingProxyProvider::From(*document->GetPage()); - DCHECK(proxy_provider); - return proxy_provider->CreateWorkerMessagingProxy(this); + WebLocalFrameBase* web_frame = + WebLocalFrameBase::FromFrame(document->GetFrame()); + + WorkerClients* worker_clients = WorkerClients::Create(); + WorkerClientsInitializer<Worker>::Run(worker_clients); + ProvideContentSettingsClientToWorker( + worker_clients, web_frame->Client()->CreateWorkerContentSettingsClient()); + return new DedicatedWorkerMessagingProxy(this, worker_clients); } } // namespace blink
diff --git a/third_party/WebKit/Source/core/workers/Worker.h b/third_party/WebKit/Source/core/workers/Worker.h index 4450ebc..5c2f053 100644 --- a/third_party/WebKit/Source/core/workers/Worker.h +++ b/third_party/WebKit/Source/core/workers/Worker.h
@@ -7,6 +7,8 @@ #include "core/workers/InProcessWorkerBase.h" +#include "core/workers/WorkerClients.h" + namespace blink { class ExceptionState; @@ -29,6 +31,9 @@ const AtomicString& InterfaceName() const override; }; +extern template class CORE_EXTERN_TEMPLATE_EXPORT + WorkerClientsInitializer<Worker>; + } // namespace blink #endif // Worker_h
diff --git a/third_party/WebKit/Source/core/workers/WorkerClients.h b/third_party/WebKit/Source/core/workers/WorkerClients.h index 35ecdff..09abcb1 100644 --- a/third_party/WebKit/Source/core/workers/WorkerClients.h +++ b/third_party/WebKit/Source/core/workers/WorkerClients.h
@@ -34,11 +34,10 @@ #include "core/CoreExport.h" #include "platform/Supplementable.h" #include "platform/wtf/Forward.h" +#include "platform/wtf/Noncopyable.h" namespace blink { -class WorkerClients; - // This is created on the main thread, passed to the worker thread and // attached to WorkerOrWorkletGlobalScope when it is created. // This class can be used to provide "client" implementations to workers or @@ -61,6 +60,62 @@ extern template class CORE_EXTERN_TEMPLATE_EXPORT Supplement<WorkerClients>; +// Allows for the registration of a callback that is invoked whenever a new +// worker starts. Callbacks are expected to provide module clients to a given +// WorkerClients. All functions must be called on the main thread. +// +// Example: +// // In ModulesInitializer.cpp. +// WorkerClientsInitializer<CoolWorker>::Register( +// [](WorkerClients* worker_clients) { +// // Provides module clients to |worker_clients| here. +// }); +// +// // In CoolWorker.cpp. +// WorkerClients* worker_clients = WorkerClients::Create(); +// WorkerClients<CoolWorker>::Run(worker_clients); +// +template <class WorkerType> +class WorkerClientsInitializer { + WTF_MAKE_NONCOPYABLE(WorkerClientsInitializer); + static_assert(sizeof(WorkerType), "WorkerType must be a complete type."); + + public: + using Callback = void (*)(WorkerClients*); + + WorkerClientsInitializer() = default; + + static void Register(Callback callback) { + DCHECK(IsMainThread()); + if (!instance_) + instance_ = new WorkerClientsInitializer<WorkerType>; + instance_->RegisterInternal(callback); + } + + static void Run(WorkerClients* worker_clients) { + DCHECK(IsMainThread()); + DCHECK(instance_); + instance_->RunInternal(worker_clients); + } + + private: + void RegisterInternal(Callback callback) { callbacks_.push_back(callback); } + + void RunInternal(WorkerClients* worker_clients) { + DCHECK(!callbacks_.IsEmpty()); + for (auto& callback : callbacks_) + callback(worker_clients); + } + + Vector<Callback> callbacks_; + + static WorkerClientsInitializer<WorkerType>* instance_; +}; + +template <class WorkerType> +WorkerClientsInitializer<WorkerType>* + WorkerClientsInitializer<WorkerType>::instance_ = nullptr; + } // namespace blink #endif // WorkerClients_h
diff --git a/third_party/WebKit/Source/modules/ModulesInitializer.cpp b/third_party/WebKit/Source/modules/ModulesInitializer.cpp index f8919e3..d13729a 100644 --- a/third_party/WebKit/Source/modules/ModulesInitializer.cpp +++ b/third_party/WebKit/Source/modules/ModulesInitializer.cpp
@@ -15,6 +15,8 @@ #include "core/html/HTMLMediaElement.h" #include "core/offscreencanvas/OffscreenCanvas.h" #include "core/page/ChromeClient.h" +#include "core/workers/Worker.h" +#include "core/workers/WorkerClients.h" #include "core/workers/WorkerContentSettingsClient.h" #include "modules/EventModulesFactory.h" #include "modules/EventModulesNames.h" @@ -28,6 +30,7 @@ #include "modules/compositorworker/CompositorWorkerThread.h" #include "modules/csspaint/CSSPaintImageGeneratorImpl.h" #include "modules/document_metadata/CopylessPasteServer.h" +#include "modules/exported/WebEmbeddedWorkerImpl.h" #include "modules/filesystem/DraggedIsolatedFileSystemImpl.h" #include "modules/filesystem/LocalFileSystemClient.h" #include "modules/imagebitmap/ImageBitmapRenderingContext.h" @@ -140,10 +143,8 @@ InstalledAppController::ProvideTo(frame, client->GetRelatedAppsFetcher()); }); - // WebSharedWorkerImpl callbacks for modules initialization. - // TODO(nhiroki): Implement a common mechanism to set up WorkerClients - // (https://crbug.com/729500). - WebSharedWorkerImpl::RegisterWorkerClientsCreatedCallback( + // DedicatedWorker callbacks for modules initialization. + WorkerClientsInitializer<Worker>::Register( [](WorkerClients* worker_clients) { ProvideLocalFileSystemToWorker(worker_clients, LocalFileSystemClient::Create()); @@ -151,6 +152,22 @@ worker_clients, IndexedDBClientImpl::Create(*worker_clients)); }); + // SharedWorker callbacks for modules initialization. + WorkerClientsInitializer<WebSharedWorkerImpl>::Register( + [](WorkerClients* worker_clients) { + ProvideLocalFileSystemToWorker(worker_clients, + LocalFileSystemClient::Create()); + ProvideIndexedDBClientToWorker( + worker_clients, IndexedDBClientImpl::Create(*worker_clients)); + }); + + // ServiceWorker callbacks for modules initialization. + WorkerClientsInitializer<WebEmbeddedWorkerImpl>::Register( + [](WorkerClients* worker_clients) { + ProvideIndexedDBClientToWorker( + worker_clients, IndexedDBClientImpl::Create(*worker_clients)); + }); + HTMLMediaElement::RegisterMediaControlsFactory( WTF::MakeUnique<MediaControlsImpl::Factory>());
diff --git a/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.cpp b/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.cpp index bbea1b2..db94ccde 100644 --- a/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.cpp +++ b/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.cpp
@@ -46,13 +46,11 @@ #include "core/loader/WorkerFetchContext.h" #include "core/probe/CoreProbes.h" #include "core/workers/ParentFrameTaskRunners.h" -#include "core/workers/WorkerClients.h" #include "core/workers/WorkerContentSettingsClient.h" #include "core/workers/WorkerGlobalScope.h" #include "core/workers/WorkerInspectorProxy.h" #include "core/workers/WorkerScriptLoader.h" #include "core/workers/WorkerThreadStartupData.h" -#include "modules/indexeddb/IndexedDBClientImpl.h" #include "modules/serviceworkers/ServiceWorkerContainerClient.h" #include "modules/serviceworkers/ServiceWorkerGlobalScopeClient.h" #include "modules/serviceworkers/ServiceWorkerGlobalScopeProxy.h" @@ -82,6 +80,8 @@ namespace blink { +template class MODULES_EXPORT WorkerClientsInitializer<WebEmbeddedWorkerImpl>; + WebEmbeddedWorker* WebEmbeddedWorker::Create( WebServiceWorkerContextClient* client, WebContentSettingsClient* content_settings_client) { @@ -410,10 +410,10 @@ SecurityOrigin* starter_origin = document->GetSecurityOrigin(); WorkerClients* worker_clients = WorkerClients::Create(); + WorkerClientsInitializer<WebEmbeddedWorkerImpl>::Run(worker_clients); + ProvideContentSettingsClientToWorker(worker_clients, std::move(content_settings_client_)); - ProvideIndexedDBClientToWorker(worker_clients, - IndexedDBClientImpl::Create(*worker_clients)); ProvideServiceWorkerGlobalScopeClientToWorker( worker_clients, new ServiceWorkerGlobalScopeClient(*worker_context_client_));
diff --git a/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.h b/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.h index ef978c7..782b87af 100644 --- a/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.h +++ b/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.h
@@ -32,6 +32,8 @@ #define WebEmbeddedWorkerImpl_h #include <memory> +#include "core/workers/WorkerClients.h" +#include "modules/ModulesExport.h" #include "platform/heap/Handle.h" #include "public/platform/Platform.h" #include "public/platform/WebContentSecurityPolicy.h" @@ -50,9 +52,10 @@ class WorkerScriptLoader; class WorkerThread; -class WebEmbeddedWorkerImpl final : public WebEmbeddedWorker, - public WebFrameClient, - public WebDevToolsAgentClient { +class MODULES_EXPORT WebEmbeddedWorkerImpl final + : public WebEmbeddedWorker, + public WebFrameClient, + NON_EXPORTED_BASE(public WebDevToolsAgentClient) { WTF_MAKE_NONCOPYABLE(WebEmbeddedWorkerImpl); public: @@ -139,6 +142,8 @@ WaitingForDebuggerState waiting_for_debugger_state_; }; +extern template class WorkerClientsInitializer<WebEmbeddedWorkerImpl>; + } // namespace blink #endif // WebEmbeddedWorkerImpl_h
diff --git a/third_party/WebKit/Source/web/BUILD.gn b/third_party/WebKit/Source/web/BUILD.gn index 7c4a62e..8ecaafd 100644 --- a/third_party/WebKit/Source/web/BUILD.gn +++ b/third_party/WebKit/Source/web/BUILD.gn
@@ -39,8 +39,6 @@ "AssertMatchingEnums.cpp", "ChromeClientImpl.cpp", "ChromeClientImpl.h", - "DedicatedWorkerMessagingProxyProviderImpl.cpp", - "DedicatedWorkerMessagingProxyProviderImpl.h", "InspectorOverlayAgent.cpp", "InspectorOverlayAgent.h", "LocalFrameClientImpl.cpp",
diff --git a/third_party/WebKit/Source/web/DedicatedWorkerMessagingProxyProviderImpl.cpp b/third_party/WebKit/Source/web/DedicatedWorkerMessagingProxyProviderImpl.cpp deleted file mode 100644 index 7b613aa..0000000 --- a/third_party/WebKit/Source/web/DedicatedWorkerMessagingProxyProviderImpl.cpp +++ /dev/null
@@ -1,79 +0,0 @@ -/* - * Copyright (C) 2009 Google Inc. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google Inc. nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -#include "web/DedicatedWorkerMessagingProxyProviderImpl.h" - -#include "core/dom/Document.h" -#include "core/frame/Settings.h" -#include "core/frame/WebLocalFrameBase.h" -#include "core/loader/WorkerFetchContext.h" -#include "core/workers/DedicatedWorkerMessagingProxy.h" -#include "core/workers/Worker.h" -#include "core/workers/WorkerClients.h" -#include "core/workers/WorkerContentSettingsClient.h" -#include "modules/filesystem/LocalFileSystemClient.h" -#include "modules/indexeddb/IndexedDBClientImpl.h" -#include "platform/wtf/PtrUtil.h" -#include "public/platform/Platform.h" -#include "public/platform/WebContentSettingsClient.h" -#include "public/platform/WebString.h" -#include "public/web/WebFrameClient.h" - -namespace blink { - -DedicatedWorkerMessagingProxyProviderImpl:: - DedicatedWorkerMessagingProxyProviderImpl(Page& page) - : DedicatedWorkerMessagingProxyProvider(page) {} - -InProcessWorkerMessagingProxy* -DedicatedWorkerMessagingProxyProviderImpl::CreateWorkerMessagingProxy( - Worker* worker) { - if (worker->GetExecutionContext()->IsDocument()) { - Document* document = ToDocument(worker->GetExecutionContext()); - WebLocalFrameBase* web_frame = - WebLocalFrameBase::FromFrame(document->GetFrame()); - WorkerClients* worker_clients = WorkerClients::Create(); - ProvideIndexedDBClientToWorker( - worker_clients, IndexedDBClientImpl::Create(*worker_clients)); - ProvideLocalFileSystemToWorker(worker_clients, - LocalFileSystemClient::Create()); - ProvideContentSettingsClientToWorker( - worker_clients, - web_frame->Client()->CreateWorkerContentSettingsClient()); - - // FIXME: call provideServiceWorkerContainerClientToWorker here when we - // support ServiceWorker in dedicated workers (http://crbug.com/371690) - return new DedicatedWorkerMessagingProxy(worker, worker_clients); - } - NOTREACHED(); - return 0; -} - -} // namespace blink
diff --git a/third_party/WebKit/Source/web/DedicatedWorkerMessagingProxyProviderImpl.h b/third_party/WebKit/Source/web/DedicatedWorkerMessagingProxyProviderImpl.h deleted file mode 100644 index 1ec163e..0000000 --- a/third_party/WebKit/Source/web/DedicatedWorkerMessagingProxyProviderImpl.h +++ /dev/null
@@ -1,64 +0,0 @@ -/* - * Copyright (C) 2013 Google Inc. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google Inc. nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -#ifndef DedicatedWorkerMessagingProxyProviderImpl_h -#define DedicatedWorkerMessagingProxyProviderImpl_h - -#include "core/workers/DedicatedWorkerMessagingProxyProvider.h" -#include "platform/wtf/Noncopyable.h" - -namespace blink { - -class DedicatedWorkerMessagingProxyProviderImpl final - : public GarbageCollectedFinalized< - DedicatedWorkerMessagingProxyProviderImpl>, - public DedicatedWorkerMessagingProxyProvider { - USING_GARBAGE_COLLECTED_MIXIN(DedicatedWorkerMessagingProxyProviderImpl); - WTF_MAKE_NONCOPYABLE(DedicatedWorkerMessagingProxyProviderImpl); - - public: - static DedicatedWorkerMessagingProxyProviderImpl* Create(Page& page) { - return new DedicatedWorkerMessagingProxyProviderImpl(page); - } - - ~DedicatedWorkerMessagingProxyProviderImpl() override {} - InProcessWorkerMessagingProxy* CreateWorkerMessagingProxy(Worker*) override; - - DEFINE_INLINE_VIRTUAL_TRACE() { - DedicatedWorkerMessagingProxyProvider::Trace(visitor); - } - - private: - explicit DedicatedWorkerMessagingProxyProviderImpl(Page&); -}; - -} // namespace blink - -#endif // DedicatedWorkerMessagingProxyProviderImpl_h
diff --git a/third_party/WebKit/Source/web/WebViewImpl.cpp b/third_party/WebKit/Source/web/WebViewImpl.cpp index bef0c5ea..c348d41 100644 --- a/third_party/WebKit/Source/web/WebViewImpl.cpp +++ b/third_party/WebKit/Source/web/WebViewImpl.cpp
@@ -168,7 +168,6 @@ #include "public/web/WebSelection.h" #include "public/web/WebViewClient.h" #include "public/web/WebWindowFeatures.h" -#include "web/DedicatedWorkerMessagingProxyProviderImpl.h" #include "web/PageOverlay.h" #include "web/PrerendererClientImpl.h" #include "web/StorageQuotaClientImpl.h" @@ -395,8 +394,6 @@ ProvideStorageQuotaClientTo(*page_, StorageQuotaClientImpl::Create()); page_->SetValidationMessageClient(ValidationMessageClientImpl::Create(*this)); - ProvideDedicatedWorkerMessagingProxyProviderTo( - *page_, DedicatedWorkerMessagingProxyProviderImpl::Create(*page_)); StorageNamespaceController::ProvideStorageNamespaceTo(*page_, &storage_client_impl_);
diff --git a/third_party/WebKit/public/web/WebEmbeddedWorker.h b/third_party/WebKit/public/web/WebEmbeddedWorker.h index bd430c5..9a60523 100644 --- a/third_party/WebKit/public/web/WebEmbeddedWorker.h +++ b/third_party/WebKit/public/web/WebEmbeddedWorker.h
@@ -43,14 +43,14 @@ // An interface to start and terminate an embedded worker. // All methods of this class must be called on the main thread. -class WebEmbeddedWorker { +class BLINK_EXPORT WebEmbeddedWorker { public: // Invoked on the main thread to instantiate a WebEmbeddedWorker. // The given WebWorkerContextClient and WebContentSettingsClient are going to // be passed on to the worker thread and is held by a newly created // WorkerGlobalScope. - BLINK_EXPORT static WebEmbeddedWorker* Create(WebServiceWorkerContextClient*, - WebContentSettingsClient*); + static WebEmbeddedWorker* Create(WebServiceWorkerContextClient*, + WebContentSettingsClient*); virtual ~WebEmbeddedWorker() {}
diff --git a/third_party/WebKit/public/web/WebSharedWorker.h b/third_party/WebKit/public/web/WebSharedWorker.h index 8ab3a7b..3df0635 100644 --- a/third_party/WebKit/public/web/WebSharedWorker.h +++ b/third_party/WebKit/public/web/WebSharedWorker.h
@@ -43,12 +43,12 @@ class WebURL; // This is the interface to a SharedWorker thread. -class WebSharedWorker { +class BLINK_EXPORT WebSharedWorker { public: // Instantiate a WebSharedWorker that interacts with the shared worker. // WebSharedWorkerClient given here must outlive or have the identical // lifetime as this instance. - BLINK_EXPORT static WebSharedWorker* Create(WebSharedWorkerClient*); + static WebSharedWorker* Create(WebSharedWorkerClient*); virtual void StartWorkerContext(const WebURL& script_url, const WebString& name,
diff --git a/tools/binary_size/diagnose_bloat.py b/tools/binary_size/diagnose_bloat.py index c36e2ab..1492fc0bb 100755 --- a/tools/binary_size/diagnose_bloat.py +++ b/tools/binary_size/diagnose_bloat.py
@@ -32,9 +32,10 @@ _DIFF_DETAILS_LINES_THRESHOLD = 100 _SRC_ROOT = os.path.abspath( os.path.join(os.path.dirname(__file__), os.pardir, os.pardir)) -_DEFAULT_ARCHIVE_DIR = os.path.join(_SRC_ROOT, 'binary-size-bloat') -_DEFAULT_OUT_DIR = os.path.join(_SRC_ROOT, 'out', 'diagnose-apk-bloat') +_DEFAULT_ARCHIVE_DIR = os.path.join(_SRC_ROOT, 'out', 'binary-size-results') +_DEFAULT_OUT_DIR = os.path.join(_SRC_ROOT, 'out', 'binary-size-build') _DEFAULT_ANDROID_TARGET = 'monochrome_public_apk' +_BINARY_SIZE_DIR = os.path.join(_SRC_ROOT, 'tools', 'binary_size') _DiffResult = collections.namedtuple('DiffResult', ['name', 'value', 'units']) @@ -266,7 +267,8 @@ def Run(self): """Run GN gen/ninja build and return the process returncode.""" - logging.info('Building: %s (this might take a while).', self.target) + logging.info('Building %s within %s (this might take a while).', + self.target, os.path.relpath(self.output_directory)) retcode = _RunCmd( self._GenGnCmd(), verbose=True, exit_on_failure=False)[1] if retcode: @@ -371,7 +373,16 @@ with open(diff_path, 'a') as diff_file: for d in self.diffs: d.RunDiff(diff_file, before.dir, after.dir) - logging.info('See detailed diff results here: %s', diff_path) + logging.info('See detailed diff results here: %s', + os.path.relpath(diff_path)) + if len(self.build_archives) == 2: + supersize_path = os.path.join(_BINARY_SIZE_DIR, 'supersize') + size_paths = [os.path.join(a.dir, a.build.size_name) + for a in self.build_archives] + logging.info('Enter supersize console via: %s, console %s %s', + os.path.relpath(supersize_path), + os.path.relpath(size_paths[0]), + os.path.relpath(size_paths[1])) metadata.Write() self._AddDiffSummaryStat(before, after) @@ -503,7 +514,12 @@ """Sync, build and return non 0 if any commands failed.""" # Simply do a checkout if subrepo is used. retcode = 0 - if subrepo != _SRC_ROOT: + if _CurrentGitHash(subrepo) == archive.rev: + if subrepo != _SRC_ROOT: + logging.info('Skipping git checkout since already at desired rev') + else: + logging.info('Skipping gclient sync since already at desired rev') + elif subrepo != _SRC_ROOT: _GitCmd(['checkout', archive.rev], subrepo) else: # Move to a detached state since gclient sync doesn't work with local @@ -661,12 +677,16 @@ tmp_dir = tempfile.mkdtemp(dir=_SRC_ROOT) try: bs_dir = os.path.join(tmp_dir, 'binary_size') - shutil.copytree(os.path.join(_SRC_ROOT, 'tools', 'binary_size'), bs_dir) + shutil.copytree(_BINARY_SIZE_DIR, bs_dir) yield os.path.join(bs_dir, 'supersize') finally: shutil.rmtree(tmp_dir) +def _CurrentGitHash(subrepo): + return _GitCmd(['rev-parse', 'HEAD'], subrepo) + + def _SetRestoreFunc(subrepo): branch = _GitCmd(['rev-parse', '--abbrev-ref', 'HEAD'], subrepo) atexit.register(lambda: _GitCmd(['checkout', branch], subrepo))
diff --git a/ui/ozone/platform/x11/ozone_platform_x11.cc b/ui/ozone/platform/x11/ozone_platform_x11.cc index 0b6cc5b..d643c21 100644 --- a/ui/ozone/platform/x11/ozone_platform_x11.cc +++ b/ui/ozone/platform/x11/ozone_platform_x11.cc
@@ -31,11 +31,14 @@ namespace { -// Returns true if Ozone is running inside the mus process. -bool RunningInsideMus() { - bool has_channel_handle = base::CommandLine::ForCurrentProcess()->HasSwitch( - "mojo-platform-channel-handle"); - return has_channel_handle; +// Returns true if a flag is present that will cause Ozone UI and GPU to run in +// the same process. +// TODO(kylechar): Remove --mojo-platform-channel-handle when mus-ws process +// split happens. +bool HasSingleProcessFlag() { + base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); + return command_line->HasSwitch("mojo-platform-channel-handle") || + command_line->HasSwitch("single-process"); } // Singleton OzonePlatform implementation for X11 platform. @@ -100,7 +103,7 @@ // In single process mode either the UI thread will create an event source // or it's a test and an event source isn't desired. - if (!params.single_process && !RunningInsideMus()) + if (!params.single_process && !HasSingleProcessFlag()) CreatePlatformEventSource(); surface_factory_ozone_ = base::MakeUnique<X11SurfaceFactory>(); @@ -121,7 +124,7 @@ return; // In single process mode XInitThreads() must be the first Xlib call. - if (params.single_process || RunningInsideMus()) + if (params.single_process || HasSingleProcessFlag()) XInitThreads(); ui::SetDefaultX11ErrorHandlers();