diff --git a/DEPS b/DEPS index 47167b12..44ab9d73 100644 --- a/DEPS +++ b/DEPS
@@ -86,7 +86,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling ANGLE # and whatever else without interference from each other. - 'angle_revision': 'd5f44c986038fa80951834efa51123c7bf20d1d1', + 'angle_revision': 'f5be5bafa6f6376f20131c5c03b23a21b2ff6f80', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling build tools # and whatever else without interference from each other. @@ -98,7 +98,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling PDFium # and whatever else without interference from each other. - 'pdfium_revision': '0ddd5dc7969676f0c661c859512be50e379a2260', + 'pdfium_revision': 'cef20d4b2ff15d39ccfebeebd6616ee277fcd159', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling openmax_dl # and whatever else without interference from each other.
diff --git a/base/files/file.h b/base/files/file.h index 8b0581576..b01e58d 100644 --- a/base/files/file.h +++ b/base/files/file.h
@@ -334,6 +334,12 @@ static Error OSErrorToFileError(int saved_errno); #endif + // Gets the last global error (errno or GetLastError()) and converts it to the + // closest base::File::Error equivalent via OSErrorToFileError(). The returned + // value is only trustworthy immediately after another base::File method + // fails. base::File never resets the global error to zero. + static Error GetLastFileError(); + // Converts an error value to a human-readable form. Used for logging. static std::string ErrorToString(Error error);
diff --git a/base/files/file_posix.cc b/base/files/file_posix.cc index fe3a9ff..4dede57f 100644 --- a/base/files/file_posix.cc +++ b/base/files/file_posix.cc
@@ -78,7 +78,7 @@ lock.l_start = 0; lock.l_len = 0; // Lock entire file. if (HANDLE_EINTR(fcntl(file, F_SETLK, &lock)) == -1) - return File::OSErrorToFileError(errno); + return File::GetLastFileError(); return File::FILE_OK; } #endif @@ -383,7 +383,7 @@ PlatformFile other_fd = HANDLE_EINTR(dup(GetPlatformFile())); if (other_fd == -1) - return File(OSErrorToFileError(errno)); + return File(File::GetLastFileError()); File other(other_fd); if (async()) @@ -503,7 +503,7 @@ } if (descriptor < 0) { - error_details_ = File::OSErrorToFileError(errno); + error_details_ = File::GetLastFileError(); return; } @@ -539,4 +539,9 @@ file_.reset(file); } +// static +File::Error File::GetLastFileError() { + return base::File::OSErrorToFileError(errno); +} + } // namespace base
diff --git a/base/files/file_unittest.cc b/base/files/file_unittest.cc index 757e1b2..c1027f7c 100644 --- a/base/files/file_unittest.cc +++ b/base/files/file_unittest.cc
@@ -237,6 +237,25 @@ EXPECT_EQ(data_to_write[i - kOffsetBeyondEndOfFile], data_read_2[i]); } +TEST(FileTest, GetLastFileError) { +#if defined(OS_WIN) + ::SetLastError(ERROR_ACCESS_DENIED); +#else + errno = EACCES; +#endif + EXPECT_EQ(File::FILE_ERROR_ACCESS_DENIED, File::GetLastFileError()); + + base::ScopedTempDir temp_dir; + EXPECT_TRUE(temp_dir.CreateUniqueTempDir()); + + FilePath nonexistent_path(temp_dir.GetPath().AppendASCII("nonexistent")); + File file(nonexistent_path, File::FLAG_OPEN | File::FLAG_READ); + File::Error last_error = File::GetLastFileError(); + EXPECT_FALSE(file.IsValid()); + EXPECT_EQ(File::FILE_ERROR_NOT_FOUND, file.error_details()); + EXPECT_EQ(File::FILE_ERROR_NOT_FOUND, last_error); +} + TEST(FileTest, Append) { base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
diff --git a/base/files/file_util_posix.cc b/base/files/file_util_posix.cc index 5adac6b4..553192f 100644 --- a/base/files/file_util_posix.cc +++ b/base/files/file_util_posix.cc
@@ -289,7 +289,7 @@ if (rename(from_path.value().c_str(), to_path.value().c_str()) == 0) return true; if (error) - *error = File::OSErrorToFileError(errno); + *error = File::GetLastFileError(); return false; }
diff --git a/base/files/file_util_win.cc b/base/files/file_util_win.cc index 1f63211c..d484ded 100644 --- a/base/files/file_util_win.cc +++ b/base/files/file_util_win.cc
@@ -144,7 +144,7 @@ // already exist. if (::MoveFile(from_path.value().c_str(), to_path.value().c_str())) return true; - File::Error move_error = File::OSErrorToFileError(GetLastError()); + File::Error move_error = File::GetLastFileError(); // Try the full-blown replace if the move fails, as ReplaceFile will only // succeed when |to_path| does exist. When writing to a network share, we may @@ -158,7 +158,7 @@ // |to_path| does not exist. In this case, the more relevant error comes // from the call to MoveFile. if (error) { - File::Error replace_error = File::OSErrorToFileError(GetLastError()); + File::Error replace_error = File::GetLastFileError(); *error = replace_error == File::FILE_ERROR_NOT_FOUND ? move_error : replace_error; }
diff --git a/base/files/file_win.cc b/base/files/file_win.cc index fc437158..5f89794 100644 --- a/base/files/file_win.cc +++ b/base/files/file_win.cc
@@ -234,7 +234,7 @@ BOOL result = LockFile(file_.Get(), 0, 0, MAXDWORD, MAXDWORD); if (!result) - return OSErrorToFileError(GetLastError()); + return GetLastFileError(); return FILE_OK; } @@ -245,7 +245,7 @@ BOOL result = UnlockFile(file_.Get(), 0, 0, MAXDWORD, MAXDWORD); if (!result) - return OSErrorToFileError(GetLastError()); + return GetLastFileError(); return FILE_OK; } @@ -264,7 +264,7 @@ 0, // dwDesiredAccess ignored due to SAME_ACCESS FALSE, // !bInheritHandle DUPLICATE_SAME_ACCESS)) { - return File(OSErrorToFileError(GetLastError())); + return File(GetLastFileError()); } File other(other_handle); @@ -404,7 +404,7 @@ else if (flags & (FLAG_CREATE_ALWAYS | FLAG_CREATE)) created_ = true; } else { - error_details_ = OSErrorToFileError(GetLastError()); + error_details_ = GetLastFileError(); } } @@ -419,4 +419,9 @@ file_.Set(file); } +// static +File::Error File::GetLastFileError() { + return File::OSErrorToFileError(GetLastError()); +} + } // namespace base
diff --git a/base/files/important_file_writer.cc b/base/files/important_file_writer.cc index 688b6d2..64f189d8 100644 --- a/base/files/important_file_writer.cc +++ b/base/files/important_file_writer.cc
@@ -99,22 +99,12 @@ after_write_callback.Run(result); } -base::File::Error GetLastFileError() { -#if defined(OS_WIN) - return base::File::OSErrorToFileError(::GetLastError()); -#elif defined(OS_POSIX) - return base::File::OSErrorToFileError(errno); -#else - return base::File::FILE_OK; -#endif -} - void DeleteTmpFile(const FilePath& tmp_file_path, StringPiece histogram_suffix) { if (!DeleteFile(tmp_file_path, false)) { - UmaHistogramExactLinearWithSuffix("ImportantFile.FileDeleteError", - histogram_suffix, -GetLastFileError(), - -base::File::FILE_ERROR_MAX); + UmaHistogramExactLinearWithSuffix( + "ImportantFile.FileDeleteError", histogram_suffix, + -base::File::GetLastFileError(), -base::File::FILE_ERROR_MAX); } } @@ -144,9 +134,9 @@ // is securely created. FilePath tmp_file_path; if (!CreateTemporaryFileInDir(path.DirName(), &tmp_file_path)) { - UmaHistogramExactLinearWithSuffix("ImportantFile.FileCreateError", - histogram_suffix, -GetLastFileError(), - -base::File::FILE_ERROR_MAX); + UmaHistogramExactLinearWithSuffix( + "ImportantFile.FileCreateError", histogram_suffix, + -base::File::GetLastFileError(), -base::File::FILE_ERROR_MAX); LogFailure(path, histogram_suffix, FAILED_CREATING, "could not create temporary file"); return false; @@ -167,9 +157,9 @@ const int data_length = checked_cast<int32_t>(data.length()); int bytes_written = tmp_file.Write(0, data.data(), data_length); if (bytes_written < data_length) { - UmaHistogramExactLinearWithSuffix("ImportantFile.FileWriteError", - histogram_suffix, -GetLastFileError(), - -base::File::FILE_ERROR_MAX); + UmaHistogramExactLinearWithSuffix( + "ImportantFile.FileWriteError", histogram_suffix, + -base::File::GetLastFileError(), -base::File::FILE_ERROR_MAX); } bool flush_success = tmp_file.Flush(); tmp_file.Close();
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/infobar/SurveyInfoBar.java b/chrome/android/java/src/org/chromium/chrome/browser/infobar/SurveyInfoBar.java index 4a6869b5..5ab780e1 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/infobar/SurveyInfoBar.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/infobar/SurveyInfoBar.java
@@ -91,13 +91,18 @@ }); NoUnderlineClickableSpan clickableSpan = new NoUnderlineClickableSpan() { + /** Prevent double clicking on the text span. */ + private boolean mClicked; + @Override public void onClick(View widget) { + if (mClicked) return; mDelegate.onSurveyTriggered(); SurveyController.getInstance().showSurveyIfAvailable( tab.getActivity(), mSiteId, mShowAsBottomSheet, mDisplayLogoResId); onCloseButtonClicked(); + mClicked = true; } };
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc index c2aef8f..cd99f48 100644 --- a/chrome/browser/about_flags.cc +++ b/chrome/browser/about_flags.cc
@@ -3667,29 +3667,6 @@ bool SkipConditionalFeatureEntry(const FeatureEntry& entry) { version_info::Channel channel = chrome::GetChannel(); -#if defined(OS_ANDROID) - // enable-data-reduction-proxy-dev is only available for the Dev/Beta channel. - if (!strcmp("enable-data-reduction-proxy-dev", entry.internal_name) && - channel != version_info::Channel::BETA && - channel != version_info::Channel::DEV) { - return true; - } - // enable-data-reduction-proxy-alt is only available for the Dev channel. - if (!strcmp("enable-data-reduction-proxy-alt", entry.internal_name) && - channel != version_info::Channel::DEV) { - return true; - } - // enable-data-reduction-proxy-carrier-test is only available for Chromium - // builds and the Canary/Dev channel. - if (!strcmp("enable-data-reduction-proxy-carrier-test", - entry.internal_name) && - channel != version_info::Channel::DEV && - channel != version_info::Channel::CANARY && - channel != version_info::Channel::UNKNOWN) { - return true; - } -#endif // OS_ANDROID - #if defined(OS_CHROMEOS) // Don't expose --mash/--mus outside of Canary or developer builds. if (!strcmp("mus", entry.internal_name) &&
diff --git a/chrome/browser/printing/cloud_print/privet_http.h b/chrome/browser/printing/cloud_print/privet_http.h index 3bc71f6..a684e42 100644 --- a/chrome/browser/printing/cloud_print/privet_http.h +++ b/chrome/browser/printing/cloud_print/privet_http.h
@@ -168,9 +168,6 @@ // Username and jobname are for display only. virtual void SetUsername(const std::string& username) = 0; virtual void SetJobname(const std::string& jobname) = 0; - // If |offline| is true, we will indicate to the printer not to post the job - // to Google Cloud Print. - virtual void SetOffline(bool offline) = 0; // Document page size. virtual void SetPageSize(const gfx::Size& page_size) = 0;
diff --git a/chrome/browser/printing/cloud_print/privet_http_impl.cc b/chrome/browser/printing/cloud_print/privet_http_impl.cc index ceac36d..e8c7f0b 100644 --- a/chrome/browser/printing/cloud_print/privet_http_impl.cc +++ b/chrome/browser/printing/cloud_print/privet_http_impl.cc
@@ -50,8 +50,6 @@ const char kPrivetURLKeyUserName[] = "user_name"; const char kPrivetURLKeyClientName[] = "client_name"; const char kPrivetURLKeyJobname[] = "job_name"; -const char kPrivetURLKeyOffline[] = "offline"; -const char kPrivetURLValueOffline[] = "1"; const char kPrivetURLValueClientName[] = "Chrome"; const char kPrivetContentTypePDF[] = "application/pdf"; @@ -388,7 +386,6 @@ use_pdf_(false), has_extended_workflow_(false), started_(false), - offline_(false), invalid_job_retries_(0), weak_factory_(this) { } @@ -501,12 +498,6 @@ jobid_); } - if (offline_) { - url = net::AppendQueryParameter(url, - kPrivetURLKeyOffline, - kPrivetURLValueOffline); - } - url_fetcher_ = privet_client_->CreateURLFetcher(url, net::URLFetcher::POST, this); @@ -667,11 +658,6 @@ jobname_ = jobname; } -void PrivetLocalPrintOperationImpl::SetOffline(bool offline) { - DCHECK(!started_); - offline_ = offline; -} - void PrivetLocalPrintOperationImpl::SetPageSize(const gfx::Size& page_size) { DCHECK(!started_); page_size_ = page_size;
diff --git a/chrome/browser/printing/cloud_print/privet_http_impl.h b/chrome/browser/printing/cloud_print/privet_http_impl.h index d1051c6..23011b9 100644 --- a/chrome/browser/printing/cloud_print/privet_http_impl.h +++ b/chrome/browser/printing/cloud_print/privet_http_impl.h
@@ -156,30 +156,22 @@ public: PrivetLocalPrintOperationImpl(PrivetHTTPClient* privet_client, PrivetLocalPrintOperation::Delegate* delegate); - ~PrivetLocalPrintOperationImpl() override; + + // PrivetLocalPrintOperation: void Start() override; - void SetData(const scoped_refptr<base::RefCountedBytes>& data) override; - void SetCapabilities(const std::string& capabilities) override; - void SetTicket(const std::string& ticket) override; - void SetUsername(const std::string& user) override; - void SetJobname(const std::string& jobname) override; - - void SetOffline(bool offline) override; - void SetPageSize(const gfx::Size& page_size) override; - void SetPWGRasterConverterForTesting( std::unique_ptr<printing::PWGRasterConverter> pwg_raster_converter) override; - PrivetHTTPClient* GetHTTPClient() override; + // PrivetURLFetcher::Delegate: void OnError(PrivetURLFetcher* fetcher, PrivetURLFetcher::ErrorType error) override; void OnParsedJson(PrivetURLFetcher* fetcher, @@ -221,7 +213,6 @@ bool use_pdf_; bool has_extended_workflow_; bool started_; - bool offline_; gfx::Size page_size_; std::string user_;
diff --git a/chrome/browser/ssl/ssl_browsertest.cc b/chrome/browser/ssl/ssl_browsertest.cc index e2e2ce04..efe89b7a 100644 --- a/chrome/browser/ssl/ssl_browsertest.cc +++ b/chrome/browser/ssl/ssl_browsertest.cc
@@ -3901,6 +3901,7 @@ reinterpret_cast<ChromeSSLHostStateDelegate*>( profile->GetSSLHostStateDelegate()); + // First check that frame requests revoke the decision. ui_test_utils::NavigateToURL( browser(), https_server_expired_.GetURL("/ssl/google.html")); @@ -3911,6 +3912,26 @@ https_server_.GetURL("/ssl/google.html")); ASSERT_FALSE(tab->GetInterstitialPage()); EXPECT_FALSE(state->HasAllowException(https_server_host)); + + // Now check that subresource requests revoke the decision. + ui_test_utils::NavigateToURL( + browser(), https_server_expired_.GetURL("/ssl/google.html")); + + ProceedThroughInterstitial(tab); + EXPECT_TRUE(state->HasAllowException(https_server_host)); + + GURL image = https_server_.GetURL("/ssl/google_files/logo.gif"); + bool result = false; + EXPECT_TRUE(ExecuteScriptAndExtractBool( + tab, + std::string("var img = document.createElement('img');img.src ='") + + image.spec() + + "';img.onload=function() { " + "window.domAutomationController.send(true); };" + "document.body.appendChild(img);", + &result)); + EXPECT_TRUE(result); + EXPECT_FALSE(state->HasAllowException(https_server_host)); } // Tests that the SSLStatus of a navigation entry for an SSL
diff --git a/chrome/browser/ui/app_list/search/app_search_provider.cc b/chrome/browser/ui/app_list/search/app_search_provider.cc index da2099f..30320d3 100644 --- a/chrome/browser/ui/app_list/search/app_search_provider.cc +++ b/chrome/browser/ui/app_list/search/app_search_provider.cc
@@ -6,8 +6,11 @@ #include <stddef.h> +#include <algorithm> #include <map> +#include <set> #include <string> +#include <unordered_set> #include <utility> #include "ash/app_list/model/app_list_item.h" @@ -146,7 +149,6 @@ virtual std::unique_ptr<AppResult> CreateResult( const std::string& app_id, AppListControllerDelegate* list_controller, - AppListItemList* top_level_item_list, bool is_recommended) = 0; protected: @@ -184,7 +186,6 @@ std::unique_ptr<AppResult> CreateResult( const std::string& app_id, AppListControllerDelegate* list_controller, - AppListItemList* top_level_item_list, bool is_recommended) override { return base::MakeUnique<ExtensionAppResult>( profile(), app_id, list_controller, is_recommended); @@ -272,7 +273,6 @@ std::unique_ptr<AppResult> CreateResult( const std::string& app_id, AppListControllerDelegate* list_controller, - AppListItemList* top_level_item_list, bool is_recommended) override { return base::MakeUnique<ArcAppResult>(profile(), app_id, list_controller, is_recommended); @@ -354,8 +354,8 @@ id_to_app_list_index[top_level_item_list_->item_at(i)->id()] = i; for (auto& app : apps_) { - std::unique_ptr<AppResult> result = app->data_source()->CreateResult( - app->id(), list_controller_, top_level_item_list_, true); + std::unique_ptr<AppResult> result = + app->data_source()->CreateResult(app->id(), list_controller_, true); result->set_title(app->name()); // Use the app list order to tiebreak apps that have never been launched. @@ -380,8 +380,8 @@ } else { const TokenizedString query_terms(query_); for (auto& app : apps_) { - std::unique_ptr<AppResult> result = app->data_source()->CreateResult( - app->id(), list_controller_, top_level_item_list_, false); + std::unique_ptr<AppResult> result = + app->data_source()->CreateResult(app->id(), list_controller_, false); TokenizedStringMatch match; TokenizedString* indexed_name = app->GetTokenizedIndexedName(); if (!match.Calculate(query_terms, *indexed_name))
diff --git a/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc b/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc index c9d5cc8..9d3ecad 100644 --- a/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc +++ b/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
@@ -18,6 +18,7 @@ #include "base/test/simple_test_tick_clock.h" #include "base/time/time.h" #include "build/build_config.h" +#include "chrome/browser/content_settings/tab_specific_content_settings.h" #include "chrome/browser/infobars/infobar_service.h" #include "chrome/browser/ui/blocked_content/popup_tracker.h" #include "chrome/browser/ui/blocked_content/tab_under_navigation_throttle.h" @@ -37,7 +38,9 @@ #if defined(OS_ANDROID) #include "chrome/browser/ui/android/infobars/infobar_android.h" -#endif // defined(OS_ANDROID) +#else +#include "chrome/browser/ui/blocked_content/framebust_block_tab_helper.h" +#endif class InfoBarAndroid; @@ -58,6 +61,10 @@ PopupOpenerTabHelper::CreateForWebContents(web_contents(), std::move(tick_clock)); InfoBarService::CreateForWebContents(web_contents()); + TabSpecificContentSettings::CreateForWebContents(web_contents()); +#if !defined(OS_ANDROID) + FramebustBlockTabHelper::CreateForWebContents(web_contents()); +#endif // The tick clock needs to be advanced manually so it isn't set to null, // which the code uses to determine if it is set yet. @@ -344,7 +351,10 @@ void ExpectUIShown(bool shown) { #if defined(OS_ANDROID) EXPECT_EQ(shown, !!GetInfoBar()); -#endif // defined(OS_ANDROID) +#else + EXPECT_EQ(shown, FramebustBlockTabHelper::FromWebContents(web_contents()) + ->HasBlockedUrls()); +#endif } // content::WebContentsDelegate: @@ -489,7 +499,13 @@ // A subsequent navigation should be allowed, even if it is classified as a // suspicious redirect. EXPECT_TRUE(NavigateAndCommitWithoutGesture(GURL("https://example.test2/"))); +#if defined(OS_ANDROID) ExpectUIShown(false); +#else + EXPECT_EQ(1u, FramebustBlockTabHelper::FromWebContents(web_contents()) + ->blocked_urls() + .size()); +#endif } TEST_F(BlockTabUnderTest, MultipleRedirectAttempts_AreBlocked) {
diff --git a/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc b/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc index 1c41fe8..2aba694e 100644 --- a/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc +++ b/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc
@@ -15,6 +15,7 @@ #include "base/strings/stringprintf.h" #include "build/build_config.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/content_settings/tab_specific_content_settings.h" #include "chrome/browser/ui/blocked_content/popup_opener_tab_helper.h" #include "components/rappor/public/rappor_parameters.h" #include "components/rappor/public/rappor_utils.h" @@ -22,6 +23,7 @@ #include "content/public/browser/navigation_handle.h" #include "content/public/browser/render_frame_host.h" #include "content/public/browser/web_contents.h" +#include "content/public/browser/web_contents_delegate.h" #include "content/public/common/console_message_level.h" #include "services/metrics/public/cpp/ukm_builders.h" #include "services/metrics/public/cpp/ukm_recorder.h" @@ -87,8 +89,13 @@ web_contents, base::MakeUnique<FramebustBlockMessageDelegate>( web_contents, url, base::BindOnce(&LogOutcome))); #else - NOTIMPLEMENTED() << "The BlockTabUnders experiment does not currently have a " - "UI implemented on desktop platforms."; + // TODO(csharrison): Instrument click-through metrics. This may be a bit + // difficult and requires attribution since this UI surface is shared with + // framebusting. + TabSpecificContentSettings* content_settings = + TabSpecificContentSettings::FromWebContents(web_contents); + DCHECK(content_settings); + content_settings->OnFramebustBlocked(url); #endif }
diff --git a/chrome/browser/vr/BUILD.gn b/chrome/browser/vr/BUILD.gn index f1971bf..e3dcb7d 100644 --- a/chrome/browser/vr/BUILD.gn +++ b/chrome/browser/vr/BUILD.gn
@@ -84,8 +84,6 @@ "elements/text.h", "elements/text_input.cc", "elements/text_input.h", - "elements/text_texture.cc", - "elements/text_texture.h", "elements/textured_element.cc", "elements/textured_element.h", "elements/throbber.cc",
diff --git a/chrome/browser/vr/elements/text.cc b/chrome/browser/vr/elements/text.cc index a505f774..ec70ddf 100644 --- a/chrome/browser/vr/elements/text.cc +++ b/chrome/browser/vr/elements/text.cc
@@ -6,7 +6,7 @@ #include "base/memory/ptr_util.h" #include "cc/paint/skia_paint_canvas.h" -#include "chrome/browser/vr/elements/text_texture.h" +#include "chrome/browser/vr/elements/ui_texture.h" #include "ui/gfx/canvas.h" #include "ui/gfx/font_list.h" #include "ui/gfx/geometry/rect.h" @@ -15,6 +15,51 @@ namespace vr { +class TextTexture : public UiTexture { + public: + explicit TextTexture(float font_height) : font_height_(font_height) {} + ~TextTexture() override {} + + void SetText(const base::string16& text) { SetAndDirty(&text_, text); } + + void SetColor(SkColor color) { SetAndDirty(&color_, color); } + + void SetAlignment(TextAlignment alignment) { + SetAndDirty(&alignment_, alignment); + } + + void SetMultiLine(bool multiline) { SetAndDirty(&multiline_, multiline); } + + void SetTextWidth(float width) { SetAndDirty(&text_width_, width); } + + gfx::SizeF GetDrawnSize() const override { return size_; } + + // This method does all text preparation for the element other than drawing to + // the texture. This allows for deeper unit testing of the Text element + // without having to mock canvases and simulate frame rendering. The state of + // the texture is modified here. + std::vector<std::unique_ptr<gfx::RenderText>> LayOutText( + const gfx::Size& texture_size); + + private: + gfx::Size GetPreferredTextureSize(int width) const override { + return gfx::Size(width, width); + } + + void Draw(SkCanvas* sk_canvas, const gfx::Size& texture_size) override; + + gfx::SizeF size_; + base::string16 text_; + // These dimensions are in meters. + float font_height_ = 0; + float text_width_ = 0; + TextAlignment alignment_ = kTextAlignmentCenter; + bool multiline_ = true; + SkColor color_ = SK_ColorBLACK; + + DISALLOW_COPY_AND_ASSIGN(TextTexture); +}; + Text::Text(int maximum_width_pixels, float font_height_meters) : TexturedElement(maximum_width_pixels), texture_(base::MakeUnique<TextTexture>(font_height_meters)) {} @@ -40,12 +85,48 @@ texture_->SetTextWidth(size.width()); } -TextTexture* Text::GetTextureForTest() { - return texture_.get(); +std::vector<std::unique_ptr<gfx::RenderText>> Text::LayOutTextForTest( + const gfx::Size& texture_size) { + return texture_->LayOutText(texture_size); +} + +gfx::SizeF Text::GetTextureSizeForTest() const { + return texture_->GetDrawnSize(); } UiTexture* Text::GetTexture() const { return texture_.get(); } +std::vector<std::unique_ptr<gfx::RenderText>> TextTexture::LayOutText( + const gfx::Size& texture_size) { + gfx::FontList fonts; + float pixels_per_meter = texture_size.width() / text_width_; + int pixel_font_height = static_cast<int>(font_height_ * pixels_per_meter); + GetDefaultFontList(pixel_font_height, text_, &fonts); + gfx::Rect text_bounds(texture_size.width(), 0); + + std::vector<std::unique_ptr<gfx::RenderText>> lines = + // TODO(vollick): if this subsumes all text, then we should probably move + // this function into this class. + PrepareDrawStringRect( + text_, fonts, color_, &text_bounds, alignment_, + multiline_ ? kWrappingBehaviorWrap : kWrappingBehaviorNoWrap); + + // Note, there is no padding here whatsoever. + size_ = gfx::SizeF(text_bounds.size()); + + return lines; +} + +void TextTexture::Draw(SkCanvas* sk_canvas, const gfx::Size& texture_size) { + cc::SkiaPaintCanvas paint_canvas(sk_canvas); + gfx::Canvas gfx_canvas(&paint_canvas, 1.0f); + gfx::Canvas* canvas = &gfx_canvas; + + auto lines = LayOutText(texture_size); + for (auto& render_text : lines) + render_text->Draw(canvas); +} + } // namespace vr
diff --git a/chrome/browser/vr/elements/text.h b/chrome/browser/vr/elements/text.h index 7eea71f..6eb63c42 100644 --- a/chrome/browser/vr/elements/text.h +++ b/chrome/browser/vr/elements/text.h
@@ -11,6 +11,10 @@ #include "chrome/browser/vr/elements/ui_texture.h" #include "third_party/skia/include/core/SkColor.h" +namespace gfx { +class RenderText; +} + namespace vr { class TextTexture; @@ -22,12 +26,15 @@ void SetText(const base::string16& text); void SetColor(SkColor color); + void SetTextAlignment(UiTexture::TextAlignment alignment); void SetMultiLine(bool multiline); void OnSetSize(gfx::SizeF size) override; - TextTexture* GetTextureForTest(); + std::vector<std::unique_ptr<gfx::RenderText>> LayOutTextForTest( + const gfx::Size& texture_size); + gfx::SizeF GetTextureSizeForTest() const; private: UiTexture* GetTexture() const override;
diff --git a/chrome/browser/vr/elements/text_texture.cc b/chrome/browser/vr/elements/text_texture.cc deleted file mode 100644 index b3fad1d..0000000 --- a/chrome/browser/vr/elements/text_texture.cc +++ /dev/null
@@ -1,80 +0,0 @@ -// 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 "chrome/browser/vr/elements/text_texture.h" - -#include "base/memory/ptr_util.h" -#include "cc/paint/skia_paint_canvas.h" -#include "chrome/browser/vr/elements/ui_texture.h" -#include "ui/gfx/canvas.h" -#include "ui/gfx/font_list.h" -#include "ui/gfx/geometry/rect.h" -#include "ui/gfx/render_text.h" - -namespace vr { - -TextTexture::TextTexture(float font_height) : font_height_(font_height) {} - -TextTexture::~TextTexture() {} - -void TextTexture::SetText(const base::string16& text) { - SetAndDirty(&text_, text); -} - -void TextTexture::SetColor(SkColor color) { - SetAndDirty(&color_, color); -} - -void TextTexture::SetAlignment(TextAlignment alignment) { - SetAndDirty(&alignment_, alignment); -} - -void TextTexture::SetMultiLine(bool multiline) { - SetAndDirty(&multiline_, multiline); -} - -void TextTexture::SetTextWidth(float width) { - SetAndDirty(&text_width_, width); -} - -gfx::SizeF TextTexture::GetDrawnSize() const { - return size_; -} - -std::vector<std::unique_ptr<gfx::RenderText>> TextTexture::LayOutText( - const gfx::Size& texture_size) { - gfx::FontList fonts; - float pixels_per_meter = texture_size.width() / text_width_; - int pixel_font_height = static_cast<int>(font_height_ * pixels_per_meter); - GetDefaultFontList(pixel_font_height, text_, &fonts); - gfx::Rect text_bounds(texture_size.width(), 0); - - std::vector<std::unique_ptr<gfx::RenderText>> lines = - // TODO(vollick): if this subsumes all text, then we should probably move - // this function into this class. - PrepareDrawStringRect( - text_, fonts, color_, &text_bounds, alignment_, - multiline_ ? kWrappingBehaviorWrap : kWrappingBehaviorNoWrap); - - // Note, there is no padding here whatsoever. - size_ = gfx::SizeF(text_bounds.size()); - - return lines; -} - -gfx::Size TextTexture::GetPreferredTextureSize(int width) const { - return gfx::Size(width, width); -} - -void TextTexture::Draw(SkCanvas* sk_canvas, const gfx::Size& texture_size) { - cc::SkiaPaintCanvas paint_canvas(sk_canvas); - gfx::Canvas gfx_canvas(&paint_canvas, 1.0f); - gfx::Canvas* canvas = &gfx_canvas; - - auto lines = LayOutText(texture_size); - for (auto& render_text : lines) - render_text->Draw(canvas); -} - -} // namespace vr
diff --git a/chrome/browser/vr/elements/text_texture.h b/chrome/browser/vr/elements/text_texture.h deleted file mode 100644 index f0cdb50f..0000000 --- a/chrome/browser/vr/elements/text_texture.h +++ /dev/null
@@ -1,58 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef CHROME_BROWSER_VR_ELEMENTS_TEXT_TEXTURE_H_ -#define CHROME_BROWSER_VR_ELEMENTS_TEXT_TEXTURE_H_ - -#include <memory> - -#include "chrome/browser/vr/elements/textured_element.h" -#include "chrome/browser/vr/elements/ui_texture.h" -#include "third_party/skia/include/core/SkColor.h" - -namespace gfx { -class RenderText; -} - -namespace vr { - -class TextTexture : public UiTexture { - public: - explicit TextTexture(float font_height); - ~TextTexture() override; - - void SetText(const base::string16& text); - void SetColor(SkColor color); - void SetAlignment(TextAlignment alignment); - void SetMultiLine(bool multiline); - void SetTextWidth(float width); - - gfx::SizeF GetDrawnSize() const override; - - // This method does all text preparation for the element other than drawing to - // the texture. This allows for deeper unit testing of the Text element - // without having to mock canvases and simulate frame rendering. The state of - // the texture is modified here. - std::vector<std::unique_ptr<gfx::RenderText>> LayOutText( - const gfx::Size& texture_size); - - private: - gfx::Size GetPreferredTextureSize(int width) const override; - void Draw(SkCanvas* sk_canvas, const gfx::Size& texture_size) override; - - gfx::SizeF size_; - base::string16 text_; - // These dimensions are in meters. - float font_height_ = 0; - float text_width_ = 0; - TextAlignment alignment_ = kTextAlignmentCenter; - bool multiline_ = true; - SkColor color_ = SK_ColorBLACK; - - DISALLOW_COPY_AND_ASSIGN(TextTexture); -}; - -} // namespace vr - -#endif // CHROME_BROWSER_VR_ELEMENTS_TEXT_TEXTURE_H_
diff --git a/chrome/browser/vr/elements/text_unittest.cc b/chrome/browser/vr/elements/text_unittest.cc index 976cb8b..3c87cbf 100644 --- a/chrome/browser/vr/elements/text_unittest.cc +++ b/chrome/browser/vr/elements/text_unittest.cc
@@ -6,7 +6,6 @@ #include "base/memory/ptr_util.h" #include "base/strings/utf_string_conversions.h" -#include "chrome/browser/vr/elements/text_texture.h" #include "testing/gtest/include/gtest/gtest.h" #include "ui/gfx/render_text.h" @@ -21,27 +20,25 @@ text->SetSize(kInitialSize, 0); text->SetText(base::UTF8ToUTF16(std::string(1000, 'x'))); - TextTexture* texture = text->GetTextureForTest(); - // Make sure we get multiple lines of rendered text from the string. - auto layout = texture->LayOutText(texture_size); + auto layout = text->LayOutTextForTest(texture_size); size_t initial_num_lines = layout.size(); - auto initial_size = texture->GetDrawnSize(); + auto initial_size = text->GetTextureSizeForTest(); EXPECT_GT(initial_num_lines, 1u); EXPECT_GT(initial_size.height(), 0.f); // Reduce the field width, and ensure that the number of lines increases along // with the texture height. text->SetSize(kInitialSize / 2, 0); - layout = texture->LayOutText(texture_size); + layout = text->LayOutTextForTest(texture_size); EXPECT_GT(layout.size(), initial_num_lines); - EXPECT_GT(texture->GetDrawnSize().height(), initial_size.height()); + EXPECT_GT(text->GetTextureSizeForTest().height(), initial_size.height()); // Enforce single-line rendering. text->SetMultiLine(false); - layout = texture->LayOutText(texture_size); + layout = text->LayOutTextForTest(texture_size); EXPECT_EQ(layout.size(), 1u); - EXPECT_LT(texture->GetDrawnSize().height(), initial_size.height()); + EXPECT_LT(text->GetTextureSizeForTest().height(), initial_size.height()); } } // namespace vr
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc index cc1bdd2..c51f22e 100644 --- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc
@@ -12,9 +12,9 @@ #include "base/macros.h" #include "base/memory/ptr_util.h" #include "base/memory/ref_counted.h" -#include "base/message_loop/message_loop.h" #include "base/run_loop.h" #include "base/test/histogram_tester.h" +#include "base/test/scoped_task_environment.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h" @@ -113,7 +113,9 @@ class DataReductionProxyInterceptorTest : public testing::Test { public: - DataReductionProxyInterceptorTest() { + DataReductionProxyInterceptorTest() + : scoped_task_environment_( + base::test::ScopedTaskEnvironment::MainThreadType::IO) { test_context_ = DataReductionProxyTestContext::Builder() .Build(); @@ -140,7 +142,9 @@ default_context_->Init(); } - base::MessageLoopForIO message_loop_; + protected: + base::test::ScopedTaskEnvironment scoped_task_environment_; + std::unique_ptr<DataReductionProxyTestContext> test_context_; net::TestNetworkDelegate default_network_delegate_; std::unique_ptr<net::URLRequestJobFactory> job_factory_; @@ -190,7 +194,9 @@ class DataReductionProxyInterceptorWithServerTest : public testing::Test { public: DataReductionProxyInterceptorWithServerTest() - : context_(true) { + : scoped_task_environment_( + base::test::ScopedTaskEnvironment::MainThreadType::IO), + context_(true) { context_.set_network_delegate(&network_delegate_); context_.set_net_log(&net_log_); } @@ -245,8 +251,10 @@ const net::EmbeddedTestServer& direct() { return direct_; } + protected: + base::test::ScopedTaskEnvironment scoped_task_environment_; + private: - base::MessageLoopForIO message_loop_; net::TestNetLog net_log_; net::TestNetworkDelegate network_delegate_; net::TestURLRequestContext context_; @@ -289,7 +297,10 @@ class DataReductionProxyInterceptorEndToEndTest : public testing::Test { public: DataReductionProxyInterceptorEndToEndTest() - : context_(true), context_storage_(&context_) {} + : scoped_task_environment_( + base::test::ScopedTaskEnvironment::MainThreadType::IO), + context_(true), + context_storage_(&context_) {} ~DataReductionProxyInterceptorEndToEndTest() override {} @@ -338,8 +349,10 @@ return config()->test_params()->proxies_for_http().front().proxy_server(); } + protected: + base::test::ScopedTaskEnvironment scoped_task_environment_; + private: - base::MessageLoopForIO message_loop_; net::TestDelegate delegate_; net::MockClientSocketFactory mock_socket_factory_; net::TestURLRequestContext context_;
diff --git a/components/leveldb/env_mojo.cc b/components/leveldb/env_mojo.cc index c29b75a..d98c94c 100644 --- a/components/leveldb/env_mojo.cc +++ b/components/leveldb/env_mojo.cc
@@ -4,8 +4,6 @@ #include "components/leveldb/env_mojo.h" -#include <errno.h> - #include <memory> #include "base/metrics/histogram_functions.h" @@ -25,14 +23,6 @@ const base::FilePath::CharType table_extension[] = FILE_PATH_LITERAL(".ldb"); -base::File::Error LastFileError() { -#if defined(OS_WIN) - return base::File::OSErrorToFileError(GetLastError()); -#else - return base::File::OSErrorToFileError(errno); -#endif -} - Status FilesystemErrorToStatus(FileError error, const std::string& filename, leveldb_env::MethodID method) { @@ -83,7 +73,7 @@ scratch, static_cast<int>(n)); if (bytes_read == -1) { - base::File::Error error = LastFileError(); + base::File::Error error = base::File::GetLastFileError(); uma_logger_->RecordOSError(leveldb_env::kSequentialFileRead, error); return MakeIOError(filename_, base::File::ErrorToString(error), leveldb_env::kSequentialFileRead, error); @@ -96,7 +86,7 @@ Status Skip(uint64_t n) override { if (file_.Seek(base::File::FROM_CURRENT, n) == -1) { - base::File::Error error = LastFileError(); + base::File::Error error = base::File::GetLastFileError(); uma_logger_->RecordOSError(leveldb_env::kSequentialFileSkip, error); return MakeIOError(filename_, base::File::ErrorToString(error), leveldb_env::kSequentialFileSkip, error); @@ -128,7 +118,7 @@ *result = Slice(scratch, (bytes_read < 0) ? 0 : bytes_read); if (bytes_read < 0) { uma_logger_->RecordOSError(leveldb_env::kRandomAccessFileRead, - LastFileError()); + base::File::GetLastFileError()); return MakeIOError(filename_, "Could not perform read", leveldb_env::kRandomAccessFileRead); } @@ -175,7 +165,7 @@ int bytes_written = file_.WriteAtCurrentPos(data.data(), static_cast<int>(data.size())); if (bytes_written != static_cast<int>(data.size())) { - base::File::Error error = LastFileError(); + base::File::Error error = base::File::GetLastFileError(); uma_logger_->RecordOSError(leveldb_env::kWritableFileAppend, error); return MakeIOError(filename_, base::File::ErrorToString(error), leveldb_env::kWritableFileAppend, error); @@ -200,7 +190,7 @@ TRACE_EVENT0("leveldb", "MojoWritableFile::Sync"); if (!file_.Flush()) { - base::File::Error error = LastFileError(); + base::File::Error error = base::File::GetLastFileError(); uma_logger_->RecordOSError(leveldb_env::kWritableFileSync, error); return MakeIOError(filename_, base::File::ErrorToString(error), leveldb_env::kWritableFileSync, error);
diff --git a/components/offline_pages/core/model/offline_page_model_taskified.cc b/components/offline_pages/core/model/offline_page_model_taskified.cc index c383aea..6dae4b85 100644 --- a/components/offline_pages/core/model/offline_page_model_taskified.cc +++ b/components/offline_pages/core/model/offline_page_model_taskified.cc
@@ -12,13 +12,17 @@ #include "base/metrics/histogram_macros.h" #include "base/strings/string16.h" #include "base/threading/thread_task_runner_handle.h" +#include "base/time/default_clock.h" #include "base/time/time.h" #include "components/offline_pages/core/archive_manager.h" #include "components/offline_pages/core/model/add_page_task.h" +#include "components/offline_pages/core/model/clear_legacy_temporary_pages_task.h" #include "components/offline_pages/core/model/create_archive_task.h" #include "components/offline_pages/core/model/delete_page_task.h" #include "components/offline_pages/core/model/get_pages_task.h" #include "components/offline_pages/core/model/mark_page_accessed_task.h" +#include "components/offline_pages/core/model/persistent_pages_consistency_check_task.h" +#include "components/offline_pages/core/model/temporary_pages_consistency_check_task.h" #include "components/offline_pages/core/offline_page_metadata_store.h" #include "components/offline_pages/core/offline_page_metadata_store_sql.h" #include "components/offline_pages/core/offline_page_model.h" @@ -31,8 +35,7 @@ namespace { -const base::TimeDelta kClearStorageStartingDelay = - base::TimeDelta::FromSeconds(20); +const base::TimeDelta kInitialingTaskDelay = base::TimeDelta::FromSeconds(20); // The time that the storage cleanup will be triggered again since the last // one. const base::TimeDelta kClearStorageInterval = base::TimeDelta::FromMinutes(30); @@ -97,9 +100,12 @@ archive_manager_(std::move(archive_manager)), policy_controller_(new ClientPolicyController()), task_queue_(this), + clock_(new base::DefaultClock()), weak_ptr_factory_(this) { CreateArchivesDirectoryIfNeeded(); + PostClearLegacyTemporaryPagesTask(); PostClearCachedPagesTask(true /* is_initializing */); + PostCheckMetadataConsistencyTask(true /* is_initializing */); } OfflinePageModelTaskified::~OfflinePageModelTaskified() {} @@ -138,7 +144,7 @@ void OfflinePageModelTaskified::MarkPageAccessed(int64_t offline_id) { auto task = base::MakeUnique<MarkPageAccessedTask>(store_.get(), offline_id, - base::Time::Now()); + GetCurrentTime()); task_queue_.AddTask(std::move(task)); } @@ -333,23 +339,6 @@ } } -void OfflinePageModelTaskified::PostClearCachedPagesTask(bool is_initializing) { - base::TimeDelta delay; - if (is_initializing) { - delay = kClearStorageStartingDelay; - } else { - if (base::Time::Now() - last_clear_cached_pages_time_ > - kClearStorageInterval) { - delay = base::TimeDelta(); - } - } - base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( - FROM_HERE, - base::Bind(&OfflinePageModelTaskified::ClearCachedPages, - weak_ptr_factory_.GetWeakPtr()), - delay); -} - // TODO(romax): see if this method can be moved into anonymous namespace after // migrating UMAs. void OfflinePageModelTaskified::InformDeletePageDone( @@ -370,12 +359,47 @@ InformDeletePageDone(callback, result); } +void OfflinePageModelTaskified::PostClearLegacyTemporaryPagesTask() { + base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, + base::Bind(&OfflinePageModelTaskified::ClearLegacyTemporaryPages, + weak_ptr_factory_.GetWeakPtr()), + kInitialingTaskDelay); +} + +void OfflinePageModelTaskified::ClearLegacyTemporaryPages() { + // TODO(romax): When we have external directory, adding the support of getting + // 'legacy' directory and replace the persistent one here. + auto task = base::MakeUnique<ClearLegacyTemporaryPagesTask>( + store_.get(), policy_controller_.get(), + archive_manager_->GetPersistentArchivesDir()); + task_queue_.AddTask(std::move(task)); +} + +void OfflinePageModelTaskified::PostClearCachedPagesTask(bool is_initializing) { + if (is_initializing) { + base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, + base::Bind(&OfflinePageModelTaskified::PostClearCachedPagesTask, + weak_ptr_factory_.GetWeakPtr(), false), + kInitialingTaskDelay); + } + + // If not enough time has passed, do not post the task. + if (GetCurrentTime() - last_clear_cached_pages_time_ < + kClearStorageInterval) { + return; + } + + ClearCachedPages(); +} + void OfflinePageModelTaskified::ClearCachedPages() { auto task = base::MakeUnique<ClearStorageTask>( store_.get(), archive_manager_.get(), policy_controller_.get(), - base::Time::Now(), + GetCurrentTime(), base::BindOnce(&OfflinePageModelTaskified::OnClearCachedPagesDone, - weak_ptr_factory_.GetWeakPtr(), base::Time::Now())); + weak_ptr_factory_.GetWeakPtr(), GetCurrentTime())); task_queue_.AddTask(std::move(task)); } @@ -386,6 +410,35 @@ last_clear_cached_pages_time_ = start_time; } +void OfflinePageModelTaskified::PostCheckMetadataConsistencyTask( + bool is_initializing) { + if (is_initializing) { + base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, + base::Bind(&OfflinePageModelTaskified::PostCheckMetadataConsistencyTask, + weak_ptr_factory_.GetWeakPtr(), false), + kInitialingTaskDelay); + return; + } + + CheckTemporaryPagesConsistency(); + CheckPersistentPagesConsistency(); +} + +void OfflinePageModelTaskified::CheckTemporaryPagesConsistency() { + auto task = base::MakeUnique<TemporaryPagesConsistencyCheckTask>( + store_.get(), policy_controller_.get(), + archive_manager_->GetTemporaryArchivesDir()); + task_queue_.AddTask(std::move(task)); +} + +void OfflinePageModelTaskified::CheckPersistentPagesConsistency() { + auto task = base::MakeUnique<PersistentPagesConsistencyCheckTask>( + store_.get(), policy_controller_.get(), + archive_manager_->GetPersistentArchivesDir()); + task_queue_.AddTask(std::move(task)); +} + void OfflinePageModelTaskified::RemovePagesMatchingUrlAndNamespace( const OfflinePageItem& page) { auto task = DeletePageTask::CreateTaskDeletingForPageLimit( @@ -404,4 +457,9 @@ archive_manager_->EnsureArchivesDirCreated(base::Bind([]() {})); } +base::Time OfflinePageModelTaskified::GetCurrentTime() { + CHECK(clock_); + return clock_->Now(); +} + } // namespace offline_pages
diff --git a/components/offline_pages/core/model/offline_page_model_taskified.h b/components/offline_pages/core/model/offline_page_model_taskified.h index d924c06..37a3c96 100644 --- a/components/offline_pages/core/model/offline_page_model_taskified.h +++ b/components/offline_pages/core/model/offline_page_model_taskified.h
@@ -115,6 +115,7 @@ OfflinePageMetadataStoreSQL* GetStoreForTesting() { return store_.get(); } private: + // TODO(romax): https://crbug.com/791115, remove the friend class usage. friend class OfflinePageModelTaskifiedTest; // Callbacks for saving pages. @@ -148,16 +149,24 @@ DeletePageResult result, const std::vector<OfflinePageModel::DeletedPageInfo>& infos); - // Callbacks for clearing temporary pages. + // Methods for clearing temporary pages. + void PostClearLegacyTemporaryPagesTask(); + void ClearLegacyTemporaryPages(); void PostClearCachedPagesTask(bool is_initializing); void ClearCachedPages(); void OnClearCachedPagesDone(base::Time start_time, size_t deleted_page_count, ClearStorageTask::ClearStorageResult result); + // Methods for consistency check. + void PostCheckMetadataConsistencyTask(bool is_initializing); + void CheckTemporaryPagesConsistency(); + void CheckPersistentPagesConsistency(); + // Other utility methods. void RemovePagesMatchingUrlAndNamespace(const OfflinePageItem& page); void CreateArchivesDirectoryIfNeeded(); + base::Time GetCurrentTime(); // Persistent store for offline page metadata. std::unique_ptr<OfflinePageMetadataStoreSQL> store_; @@ -187,6 +196,9 @@ // not persist across Chrome restarts. base::Time last_clear_cached_pages_time_; + // Clock for testing only. + std::unique_ptr<base::Clock> clock_; + base::WeakPtrFactory<OfflinePageModelTaskified> weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(OfflinePageModelTaskified);
diff --git a/components/offline_pages/core/model/offline_page_model_taskified_unittest.cc b/components/offline_pages/core/model/offline_page_model_taskified_unittest.cc index 1ffd7fc..a703e6e5 100644 --- a/components/offline_pages/core/model/offline_page_model_taskified_unittest.cc +++ b/components/offline_pages/core/model/offline_page_model_taskified_unittest.cc
@@ -76,6 +76,8 @@ // Runs until all of the tasks that are not delayed are gone from the task // queue. void PumpLoop() { task_runner_->RunUntilIdle(); } + void BuildStore(); + void BuildModel(); void ResetResults(); // OfflinePageModel::Observer implementation. @@ -110,8 +112,12 @@ std::unique_ptr<OfflinePageTestArchiver> BuildArchiver(const GURL& url, ArchiverResult result); void CheckTaskQueueIdle(); + void SetTestingClock(std::unique_ptr<base::Clock> clock) { + model_->clock_ = std::move(clock); + } // Getters for private fields. + base::TestSimpleTaskRunner* task_runner() { return task_runner_.get(); } OfflinePageModelTaskified* model() { return model_.get(); } OfflinePageMetadataStoreSQL* store() { return store_test_util_.store(); } OfflinePageMetadataStoreTestUtil* store_test_util() { @@ -135,6 +141,9 @@ const OfflinePageModel::DeletedPageInfo& last_deleted_page_info() { return last_deleted_page_info_; } + base::Time last_clear_page_time() { + return model_->last_clear_cached_pages_time_; + } private: scoped_refptr<base::TestSimpleTaskRunner> task_runner_; @@ -160,21 +169,12 @@ OfflinePageModelTaskifiedTest::~OfflinePageModelTaskifiedTest() {} void OfflinePageModelTaskifiedTest::SetUp() { - store_test_util()->BuildStoreInMemory(); + BuildStore(); ASSERT_TRUE(temporary_dir_.CreateUniqueTempDir()); ASSERT_TRUE(persistent_dir_.CreateUniqueTempDir()); - auto archive_manager = base::MakeUnique<ArchiveManager>( - temporary_dir_path(), persistent_dir_path(), - base::ThreadTaskRunnerHandle::Get()); - model_ = base::MakeUnique<OfflinePageModelTaskified>( - store_test_util()->ReleaseStore(), std::move(archive_manager), - base::ThreadTaskRunnerHandle::Get()); - model_->AddObserver(this); - page_generator()->SetArchiveDirectory(temporary_dir_path()); - ResetResults(); + BuildModel(); PumpLoop(); CheckTaskQueueIdle(); - EXPECT_EQ(0UL, model_->pending_archivers_.size()); } void OfflinePageModelTaskifiedTest::TearDown() { @@ -194,6 +194,23 @@ PumpLoop(); } +void OfflinePageModelTaskifiedTest::BuildStore() { + store_test_util()->BuildStoreInMemory(); +} + +void OfflinePageModelTaskifiedTest::BuildModel() { + ASSERT_TRUE(store_test_util_.store()); + auto archive_manager = base::MakeUnique<ArchiveManager>( + temporary_dir_path(), persistent_dir_path(), + base::ThreadTaskRunnerHandle::Get()); + model_ = base::MakeUnique<OfflinePageModelTaskified>( + store_test_util()->ReleaseStore(), std::move(archive_manager), + base::ThreadTaskRunnerHandle::Get()); + model_->AddObserver(this); + ResetResults(); + EXPECT_EQ(0UL, model_->pending_archivers_.size()); +} + void OfflinePageModelTaskifiedTest::ResetResults() { last_path_created_by_archiver_.clear(); observer_add_page_called_ = false; @@ -454,6 +471,7 @@ TEST_F(OfflinePageModelTaskifiedTest, AddPage) { // Creates a fresh page. + page_generator()->SetArchiveDirectory(temporary_dir_path()); OfflinePageItem page = page_generator()->CreateItemWithTempFile(); base::MockCallback<AddPageCallback> callback; @@ -519,6 +537,7 @@ // should be covered in DeletePagesTaskTest. TEST_F(OfflinePageModelTaskifiedTest, DeletePagesByOfflineId) { + page_generator()->SetArchiveDirectory(temporary_dir_path()); OfflinePageItem page1 = page_generator()->CreateItemWithTempFile(); OfflinePageItem page2 = page_generator()->CreateItemWithTempFile(); InsertPageIntoStore(page1); @@ -542,6 +561,7 @@ } TEST_F(OfflinePageModelTaskifiedTest, DeletePagesByUrlPredicate) { + page_generator()->SetArchiveDirectory(temporary_dir_path()); page_generator()->SetNamespace(kDefaultNamespace); page_generator()->SetUrl(kTestUrl); OfflinePageItem page1 = page_generator()->CreateItemWithTempFile(); @@ -668,6 +688,16 @@ PumpLoop(); } +TEST_F(OfflinePageModelTaskifiedTest, CanSaveURL) { + EXPECT_TRUE(OfflinePageModel::CanSaveURL(GURL("http://foo"))); + EXPECT_TRUE(OfflinePageModel::CanSaveURL(GURL("https://foo"))); + EXPECT_FALSE(OfflinePageModel::CanSaveURL(GURL("file:///foo"))); + EXPECT_FALSE(OfflinePageModel::CanSaveURL(GURL("data:image/png;base64,ab"))); + EXPECT_FALSE(OfflinePageModel::CanSaveURL(GURL("chrome://version"))); + EXPECT_FALSE(OfflinePageModel::CanSaveURL(GURL("chrome-native://newtab/"))); + EXPECT_FALSE(OfflinePageModel::CanSaveURL(GURL("/invalid/url.mhtml"))); +} + TEST_F(OfflinePageModelTaskifiedTest, GetOfflineIdsForClientId) { page_generator()->SetNamespace(kTestClientId1.name_space); page_generator()->SetId(kTestClientId1.id); @@ -724,6 +754,7 @@ } TEST_F(OfflinePageModelTaskifiedTest, DeletePagesByClientIds) { + page_generator()->SetArchiveDirectory(temporary_dir_path()); page_generator()->SetNamespace(kTestClientId1.name_space); page_generator()->SetId(kTestClientId1.id); OfflinePageItem page1 = page_generator()->CreateItemWithTempFile(); @@ -883,6 +914,7 @@ // Add pages that have the same namespace and url directly into store, in // order to avoid triggering the removal. // The 'default' namespace has a limit of 1 per url. + page_generator()->SetArchiveDirectory(temporary_dir_path()); page_generator()->SetNamespace(kDefaultNamespace); page_generator()->SetUrl(kTestUrl); OfflinePageItem page1 = page_generator()->CreateItemWithTempFile(); @@ -923,4 +955,90 @@ PumpLoop(); } +// This test is affected by https://crbug.com/725685, which only affects windows +// platform. +#if defined(OS_WIN) +#define MAYBE_StartUp_ConsistencyCheckExecuted \ + DISABLED_StartUp_ConsistencyCheckExecuted +#else +#define MAYBE_StartUp_ConsistencyCheckExecuted StartUp_ConsistencyCheckExecuted +#endif +TEST_F(OfflinePageModelTaskifiedTest, MAYBE_StartUp_ConsistencyCheckExecuted) { + // Rebuild the store so that we can insert pages before the model constructs. + BuildStore(); + + // Insert temporary pages + page_generator()->SetArchiveDirectory(temporary_dir_path()); + page_generator()->SetNamespace(kDefaultNamespace); + // Page missing archive file in temporary directory. + OfflinePageItem temp_page1 = page_generator()->CreateItem(); + // Page missing metadata entry in database since it's not inserted into store. + OfflinePageItem temp_page2 = page_generator()->CreateItemWithTempFile(); + // Page in temporary namespace saved in persistent directory to simulate pages + // saved in legacy directory. + page_generator()->SetArchiveDirectory(persistent_dir_path()); + OfflinePageItem temp_page3 = page_generator()->CreateItemWithTempFile(); + InsertPageIntoStore(temp_page1); + InsertPageIntoStore(temp_page3); + + // Insert persistent pages. + page_generator()->SetNamespace(kDownloadNamespace); + // Page missing archive file in pesistent directory. + OfflinePageItem persistent_page1 = page_generator()->CreateItem(); + // Page missing metadata entry in database since it's not inserted into store. + OfflinePageItem persistent_page2 = page_generator()->CreateItemWithTempFile(); + // Page in persistent namespace saved in persistent directory to simulate + // pages saved in legacy directory. + OfflinePageItem persistent_page3 = page_generator()->CreateItemWithTempFile(); + InsertPageIntoStore(persistent_page1); + InsertPageIntoStore(persistent_page3); + + PumpLoop(); + + EXPECT_EQ(4LL, store_test_util()->GetPageCount()); + EXPECT_EQ(1UL, test_util::GetFileCountInDirectory(temporary_dir_path())); + EXPECT_EQ(3UL, test_util::GetFileCountInDirectory(persistent_dir_path())); + + // Rebuild the model in order to trigger consistency check. + BuildModel(); + PumpLoop(); + + EXPECT_EQ(1LL, store_test_util()->GetPageCount()); + EXPECT_EQ(0UL, test_util::GetFileCountInDirectory(temporary_dir_path())); + EXPECT_EQ(1UL, test_util::GetFileCountInDirectory(persistent_dir_path())); +} + +TEST_F(OfflinePageModelTaskifiedTest, ClearStorage) { + // Rebuilding store and model in order to set clock before executing the clear + // storage during model initialization so that we can check the time. + BuildStore(); + BuildModel(); + auto clock = base::MakeUnique<base::SimpleTestClock>(); + base::SimpleTestClock* clock_ptr = clock.get(); + clock_ptr->SetNow(base::Time::Now()); + SetTestingClock(std::move(clock)); + + PumpLoop(); + EXPECT_EQ(clock_ptr->Now(), last_clear_page_time()); + + // Only 5 minutes passed and the last clear page time should not be changed + // since the clear page will not be triggered. + clock_ptr->Advance(base::TimeDelta::FromMinutes(5)); + auto archiver = BuildArchiver(kTestUrl, ArchiverResult::SUCCESSFULLY_CREATED); + int64_t offline_id = SavePageWithExpectedResult( + kTestUrl, kTestClientId1, kTestUrl2, kEmptyRequestOrigin, + std::move(archiver), SavePageResult::SUCCESS); + PumpLoop(); + EXPECT_EQ(clock_ptr->Now() - base::TimeDelta::FromMinutes(5), + last_clear_page_time()); + + clock_ptr->Advance(base::TimeDelta::FromMinutes(30)); + archiver = BuildArchiver(kTestUrl, ArchiverResult::SUCCESSFULLY_CREATED); + offline_id = SavePageWithExpectedResult( + kTestUrl, kTestClientId1, kTestUrl2, kEmptyRequestOrigin, + std::move(archiver), SavePageResult::SUCCESS); + PumpLoop(); + EXPECT_EQ(clock_ptr->Now(), last_clear_page_time()); +} + } // namespace offline_pages
diff --git a/components/offline_pages/core/model/persistent_pages_consistency_check_task.cc b/components/offline_pages/core/model/persistent_pages_consistency_check_task.cc index cdaf304a..276fd3505 100644 --- a/components/offline_pages/core/model/persistent_pages_consistency_check_task.cc +++ b/components/offline_pages/core/model/persistent_pages_consistency_check_task.cc
@@ -101,9 +101,6 @@ if (!db) return false; - std::vector<int64_t> offline_ids_to_delete; - std::vector<base::FilePath> files_to_delete; - // One large database transaction that will: // 1. Gets persistent page infos from the database. // 2. Decide which pages to delete. @@ -114,6 +111,7 @@ auto persistent_page_infos = GetAllPersistentPageInfos(namespaces, db); + std::vector<int64_t> offline_ids_to_delete; for (const auto& page_info : persistent_page_infos) { // Get pages whose archive files does not exist and delete. if (!base::PathExists(page_info.file_path)) { @@ -135,6 +133,7 @@ // associated entries in the database. // TODO(romax): https://crbug.com/786240. std::set<base::FilePath> archive_paths = GetAllArchives(archives_dir); + std::vector<base::FilePath> files_to_delete; for (const auto& archive_path : archive_paths) { if (std::find_if(persistent_page_infos.begin(), persistent_page_infos.end(), [&archive_path](const PageInfo& page_info) -> bool {
diff --git a/components/offline_pages/core/model/persistent_pages_consistency_check_task_unittest.cc b/components/offline_pages/core/model/persistent_pages_consistency_check_task_unittest.cc index 619b7de..49afd20 100644 --- a/components/offline_pages/core/model/persistent_pages_consistency_check_task_unittest.cc +++ b/components/offline_pages/core/model/persistent_pages_consistency_check_task_unittest.cc
@@ -13,26 +13,13 @@ #include "components/offline_pages/core/client_namespace_constants.h" #include "components/offline_pages/core/client_policy_controller.h" #include "components/offline_pages/core/model/offline_page_item_generator.h" +#include "components/offline_pages/core/model/offline_page_test_util.h" #include "components/offline_pages/core/offline_page_metadata_store_test_util.h" #include "components/offline_pages/core/test_task_runner.h" #include "testing/gtest/include/gtest/gtest.h" namespace offline_pages { -namespace { - -int64_t GetFileCountInDir(const base::FilePath& dir) { - base::FileEnumerator file_enumerator(dir, false, base::FileEnumerator::FILES); - int64_t count = 0; - for (base::FilePath path = file_enumerator.Next(); !path.empty(); - path = file_enumerator.Next()) { - count++; - } - return count; -} - -} // namespace - class PersistentPagesConsistencyCheckTaskTest : public testing::Test { public: PersistentPagesConsistencyCheckTaskTest(); @@ -146,14 +133,14 @@ OfflinePageItem page2 = AddPage(kDownloadNamespace, persistent_dir()); EXPECT_EQ(1LL, store_test_util()->GetPageCount()); - EXPECT_EQ(2LL, GetFileCountInDir(persistent_dir())); + EXPECT_EQ(2UL, test_util::GetFileCountInDirectory(persistent_dir())); auto task = base::MakeUnique<PersistentPagesConsistencyCheckTask>( store(), policy_controller(), persistent_dir()); runner()->RunTask(std::move(task)); EXPECT_EQ(1LL, store_test_util()->GetPageCount()); - EXPECT_EQ(1LL, GetFileCountInDir(persistent_dir())); + EXPECT_EQ(1UL, test_util::GetFileCountInDirectory(persistent_dir())); EXPECT_FALSE(IsPageRemovedFromBothPlaces(page1)); EXPECT_TRUE(IsPageRemovedFromBothPlaces(page2)); } @@ -177,14 +164,14 @@ OfflinePageItem page2 = AddPage(kDownloadNamespace, persistent_dir()); EXPECT_EQ(2LL, store_test_util()->GetPageCount()); - EXPECT_EQ(1LL, GetFileCountInDir(persistent_dir())); + EXPECT_EQ(1UL, test_util::GetFileCountInDirectory(persistent_dir())); auto task = base::MakeUnique<PersistentPagesConsistencyCheckTask>( store(), policy_controller(), persistent_dir()); runner()->RunTask(std::move(task)); EXPECT_EQ(1LL, store_test_util()->GetPageCount()); - EXPECT_EQ(1LL, GetFileCountInDir(persistent_dir())); + EXPECT_EQ(1UL, test_util::GetFileCountInDirectory(persistent_dir())); EXPECT_FALSE(IsPageRemovedFromBothPlaces(page1)); EXPECT_TRUE(IsPageRemovedFromBothPlaces(page2)); } @@ -210,14 +197,14 @@ OfflinePageItem page3 = AddPage(kDownloadNamespace, persistent_dir()); EXPECT_EQ(2LL, store_test_util()->GetPageCount()); - EXPECT_EQ(2LL, GetFileCountInDir(persistent_dir())); + EXPECT_EQ(2UL, test_util::GetFileCountInDirectory(persistent_dir())); auto task = base::MakeUnique<PersistentPagesConsistencyCheckTask>( store(), policy_controller(), persistent_dir()); runner()->RunTask(std::move(task)); EXPECT_EQ(1LL, store_test_util()->GetPageCount()); - EXPECT_EQ(1LL, GetFileCountInDir(persistent_dir())); + EXPECT_EQ(1UL, test_util::GetFileCountInDirectory(persistent_dir())); EXPECT_FALSE(IsPageRemovedFromBothPlaces(page1)); EXPECT_TRUE(IsPageRemovedFromBothPlaces(page2)); EXPECT_TRUE(IsPageRemovedFromBothPlaces(page3)); @@ -236,14 +223,14 @@ EXPECT_TRUE(base::Move(path, mp3_path)); EXPECT_EQ(0LL, store_test_util()->GetPageCount()); - EXPECT_EQ(2LL, GetFileCountInDir(persistent_dir())); + EXPECT_EQ(2UL, test_util::GetFileCountInDirectory(persistent_dir())); auto task = base::MakeUnique<PersistentPagesConsistencyCheckTask>( store(), policy_controller(), persistent_dir()); runner()->RunTask(std::move(task)); EXPECT_EQ(0LL, store_test_util()->GetPageCount()); - EXPECT_EQ(1LL, GetFileCountInDir(persistent_dir())); + EXPECT_EQ(1UL, test_util::GetFileCountInDirectory(persistent_dir())); EXPECT_TRUE(IsPageRemovedFromBothPlaces(page1)); }
diff --git a/components/offline_pages/core/model/temporary_pages_consistency_check_task.cc b/components/offline_pages/core/model/temporary_pages_consistency_check_task.cc index b23e79a..a87b1b8 100644 --- a/components/offline_pages/core/model/temporary_pages_consistency_check_task.cc +++ b/components/offline_pages/core/model/temporary_pages_consistency_check_task.cc
@@ -25,6 +25,8 @@ namespace { +#define OFFLINE_PAGES_TABLE_NAME "offlinepages_v1" + struct PageInfo { int64_t offline_id; base::FilePath file_path; @@ -37,8 +39,7 @@ const char kSql[] = "SELECT offline_id, file_path" - " FROM offlinepages_v1" - " WHERE client_namespace = ?"; + " FROM " OFFLINE_PAGES_TABLE_NAME " WHERE client_namespace = ?"; for (const auto& temp_namespace : temp_namespaces) { sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql)); @@ -66,7 +67,8 @@ bool DeletePagesByOfflineIds(const std::vector<int64_t>& offline_ids, sql::Connection* db) { - static const char kSql[] = "DELETE FROM offlinepages_v1 WHERE offline_id = ?"; + static const char kSql[] = + "DELETE FROM " OFFLINE_PAGES_TABLE_NAME " WHERE offline_id = ?"; for (const auto& offline_id : offline_ids) { sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql));
diff --git a/components/offline_pages/core/model/temporary_pages_consistency_check_task_unittest.cc b/components/offline_pages/core/model/temporary_pages_consistency_check_task_unittest.cc index 70b1c49f..4974beb9 100644 --- a/components/offline_pages/core/model/temporary_pages_consistency_check_task_unittest.cc +++ b/components/offline_pages/core/model/temporary_pages_consistency_check_task_unittest.cc
@@ -30,8 +30,9 @@ OfflinePageItem AddPage(const std::string& name_space, const base::FilePath& archive_dir); - void SetRemoveDbEntryAndFile(bool remove_db_entry, bool remove_file); - bool IsPageRemoved(const OfflinePageItem& page); + void SetShouldCreateDbEntry(bool should_create_db_entry); + void SetShouldCreateFile(bool should_create_file); + bool IsPageRemovedFromBothPlaces(const OfflinePageItem& page); OfflinePageMetadataStoreSQL* store() { return store_test_util_.store(); } OfflinePageMetadataStoreTestUtil* store_test_util() { @@ -54,8 +55,8 @@ std::unique_ptr<ClientPolicyController> policy_controller_; base::ScopedTempDir temporary_dir_; - bool remove_db_entry_; - bool remove_file_; + bool should_create_db_entry_; + bool should_create_file_; }; TemporaryPagesConsistencyCheckTaskTest::TemporaryPagesConsistencyCheckTaskTest() @@ -63,8 +64,8 @@ task_runner_handle_(task_runner_), store_test_util_(task_runner_), runner_(task_runner_), - remove_db_entry_(false), - remove_file_(false) {} + should_create_db_entry_(false), + should_create_file_(false) {} TemporaryPagesConsistencyCheckTaskTest:: ~TemporaryPagesConsistencyCheckTaskTest() {} @@ -88,21 +89,24 @@ generator()->SetNamespace(name_space); generator()->SetArchiveDirectory(archive_dir); OfflinePageItem page = generator()->CreateItemWithTempFile(); - if (!remove_db_entry_) + if (should_create_db_entry_) store_test_util()->InsertItem(page); - if (remove_file_) + if (!should_create_file_) EXPECT_TRUE(base::DeleteFile(page.file_path, false)); return page; } -void TemporaryPagesConsistencyCheckTaskTest::SetRemoveDbEntryAndFile( - bool remove_db_entry, - bool remove_file) { - remove_db_entry_ = remove_db_entry; - remove_file_ = remove_file; +void TemporaryPagesConsistencyCheckTaskTest::SetShouldCreateDbEntry( + bool should_create_db_entry) { + should_create_db_entry_ = should_create_db_entry; } -bool TemporaryPagesConsistencyCheckTaskTest::IsPageRemoved( +void TemporaryPagesConsistencyCheckTaskTest::SetShouldCreateFile( + bool should_create_file) { + should_create_file_ = should_create_file; +} + +bool TemporaryPagesConsistencyCheckTaskTest::IsPageRemovedFromBothPlaces( const OfflinePageItem& page) { return !base::PathExists(page.file_path) && !store_test_util()->GetPageByOfflineId(page.offline_id); @@ -118,9 +122,11 @@ TEST_F(TemporaryPagesConsistencyCheckTaskTest, MAYBE_TestDeleteFileWithoutDbEntry) { // Only the file without DB entry and in temporary directory will be deleted. - SetRemoveDbEntryAndFile(false, false); + SetShouldCreateFile(true); + + SetShouldCreateDbEntry(true); OfflinePageItem page1 = AddPage(kLastNNamespace, temporary_dir()); - SetRemoveDbEntryAndFile(true, false); + SetShouldCreateDbEntry(false); OfflinePageItem page2 = AddPage(kLastNNamespace, temporary_dir()); EXPECT_EQ(1LL, store_test_util()->GetPageCount()); @@ -132,8 +138,8 @@ EXPECT_EQ(1LL, store_test_util()->GetPageCount()); EXPECT_EQ(1UL, test_util::GetFileCountInDirectory(temporary_dir())); - EXPECT_FALSE(IsPageRemoved(page1)); - EXPECT_TRUE(IsPageRemoved(page2)); + EXPECT_FALSE(IsPageRemovedFromBothPlaces(page1)); + EXPECT_TRUE(IsPageRemovedFromBothPlaces(page2)); } // This test is affected by https://crbug.com/725685, which only affects windows @@ -147,9 +153,11 @@ MAYBE_TestDeleteDbEntryWithoutFile) { // The temporary pages will be deleted from DB if their DB entries exist but // files are missing. - SetRemoveDbEntryAndFile(false, false); + SetShouldCreateDbEntry(true); + + SetShouldCreateFile(true); OfflinePageItem page1 = AddPage(kLastNNamespace, temporary_dir()); - SetRemoveDbEntryAndFile(false, true); + SetShouldCreateFile(false); OfflinePageItem page2 = AddPage(kLastNNamespace, temporary_dir()); EXPECT_EQ(2LL, store_test_util()->GetPageCount()); @@ -161,8 +169,8 @@ EXPECT_EQ(1LL, store_test_util()->GetPageCount()); EXPECT_EQ(1UL, test_util::GetFileCountInDirectory(temporary_dir())); - EXPECT_FALSE(IsPageRemoved(page1)); - EXPECT_TRUE(IsPageRemoved(page2)); + EXPECT_FALSE(IsPageRemovedFromBothPlaces(page1)); + EXPECT_TRUE(IsPageRemovedFromBothPlaces(page2)); } // This test is affected by https://crbug.com/725685, which only affects windows @@ -175,11 +183,14 @@ TEST_F(TemporaryPagesConsistencyCheckTaskTest, MAYBE_CombinedTest) { // Adding a bunch of pages with different setups. // After the consistency check, only page1 will exist. - SetRemoveDbEntryAndFile(false, false); + SetShouldCreateDbEntry(true); + SetShouldCreateFile(true); OfflinePageItem page1 = AddPage(kLastNNamespace, temporary_dir()); - SetRemoveDbEntryAndFile(true, false); + SetShouldCreateDbEntry(false); + SetShouldCreateFile(true); OfflinePageItem page2 = AddPage(kLastNNamespace, temporary_dir()); - SetRemoveDbEntryAndFile(false, true); + SetShouldCreateDbEntry(true); + SetShouldCreateFile(false); OfflinePageItem page3 = AddPage(kLastNNamespace, temporary_dir()); EXPECT_EQ(2LL, store_test_util()->GetPageCount()); @@ -191,9 +202,9 @@ EXPECT_EQ(1LL, store_test_util()->GetPageCount()); EXPECT_EQ(1UL, test_util::GetFileCountInDirectory(temporary_dir())); - EXPECT_FALSE(IsPageRemoved(page1)); - EXPECT_TRUE(IsPageRemoved(page2)); - EXPECT_TRUE(IsPageRemoved(page3)); + EXPECT_FALSE(IsPageRemovedFromBothPlaces(page1)); + EXPECT_TRUE(IsPageRemovedFromBothPlaces(page2)); + EXPECT_TRUE(IsPageRemovedFromBothPlaces(page3)); } } // namespace offline_pages
diff --git a/components/payments/content/utility/fingerprint_parser.cc b/components/payments/content/utility/fingerprint_parser.cc index 680a13a..12e0259 100644 --- a/components/payments/content/utility/fingerprint_parser.cc +++ b/components/payments/content/utility/fingerprint_parser.cc
@@ -25,6 +25,11 @@ std::vector<uint8_t> FingerprintStringToByteArray(const std::string& input) { std::vector<uint8_t> output; + if (!base::IsStringASCII(input)) { + LOG(ERROR) << "Fingerprint should be an ASCII string."; + return output; + } + const size_t kLength = 32 * 3 - 1; if (input.size() != kLength) { LOG(ERROR) << "Fingerprint \"" << input << "\" should contain exactly " @@ -32,11 +37,6 @@ return output; } - if (!base::IsStringASCII(input)) { - LOG(ERROR) << "Fingerprint \"" << input << "\" should be ASCII."; - return output; - } - for (size_t i = 0; i < input.size(); i += 3) { if (i < input.size() - 2 && input[i + 2] != ':') { LOG(ERROR) << "Bytes in fingerprint \"" << input
diff --git a/components/previews/content/previews_io_data.cc b/components/previews/content/previews_io_data.cc index d25744db..e220c6a 100644 --- a/components/previews/content/previews_io_data.cc +++ b/components/previews/content/previews_io_data.cc
@@ -153,14 +153,17 @@ previews_ui_service_, url, type, opt_out, time)); } -void PreviewsIOData::LogPreviewDecisionMade(PreviewsEligibilityReason reason, - const GURL& url, - base::Time time, - PreviewsType type) const { +void PreviewsIOData::LogPreviewDecisionMade( + PreviewsEligibilityReason reason, + const GURL& url, + base::Time time, + PreviewsType type, + std::vector<PreviewsEligibilityReason>&& passed_reasons) const { LogPreviewsEligibilityReason(reason, type); ui_task_runner_->PostTask( FROM_HERE, base::Bind(&PreviewsUIService::LogPreviewDecisionMade, - previews_ui_service_, reason, url, time, type)); + previews_ui_service_, reason, url, time, type, + base::Passed(std::move(passed_reasons)))); } void PreviewsIOData::AddPreviewNavigation(const GURL& url, @@ -204,21 +207,27 @@ // when navigating to files on disk, etc. return false; } + + std::vector<PreviewsEligibilityReason> passed_reasons; if (is_enabled_callback_.is_null() || !previews_black_list_) { LogPreviewDecisionMade(PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE, - request.url(), base::Time::Now(), type); + request.url(), base::Time::Now(), type, + std::move(passed_reasons)); return false; } + passed_reasons.push_back(PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE); + if (!is_enabled_callback_.Run(type)) return false; if (!blacklist_ignored_) { // The blacklist will disallow certain hosts for periods of time based on // user's opting out of the preview. - PreviewsEligibilityReason status = - previews_black_list_->IsLoadedAndAllowed(request.url(), type); + PreviewsEligibilityReason status = previews_black_list_->IsLoadedAndAllowed( + request.url(), type, &passed_reasons); if (status != PreviewsEligibilityReason::ALLOWED) { - LogPreviewDecisionMade(status, request.url(), base::Time::Now(), type); + LogPreviewDecisionMade(status, request.url(), base::Time::Now(), type, + std::move(passed_reasons)); return false; } } @@ -232,16 +241,20 @@ net::EFFECTIVE_CONNECTION_TYPE_OFFLINE) { LogPreviewDecisionMade( PreviewsEligibilityReason::NETWORK_QUALITY_UNAVAILABLE, request.url(), - base::Time::Now(), type); + base::Time::Now(), type, std::move(passed_reasons)); return false; } + passed_reasons.push_back( + PreviewsEligibilityReason::NETWORK_QUALITY_UNAVAILABLE); if (network_quality_estimator->GetEffectiveConnectionType() > effective_connection_type_threshold) { LogPreviewDecisionMade(PreviewsEligibilityReason::NETWORK_NOT_SLOW, - request.url(), base::Time::Now(), type); + request.url(), base::Time::Now(), type, + std::move(passed_reasons)); return false; } + passed_reasons.push_back(PreviewsEligibilityReason::NETWORK_NOT_SLOW); } // LOAD_VALIDATE_CACHE or LOAD_BYPASS_CACHE mean the user reloaded the page. @@ -250,9 +263,11 @@ request.load_flags() & (net::LOAD_VALIDATE_CACHE | net::LOAD_BYPASS_CACHE)) { LogPreviewDecisionMade(PreviewsEligibilityReason::RELOAD_DISALLOWED, - request.url(), base::Time::Now(), type); + request.url(), base::Time::Now(), type, + std::move(passed_reasons)); return false; } + passed_reasons.push_back(PreviewsEligibilityReason::RELOAD_DISALLOWED); // Check provided blacklist, if any. This type of blacklist was added for // Finch provided blacklist for Client LoFi. @@ -261,9 +276,11 @@ host_blacklist_from_server.end()) { LogPreviewDecisionMade( PreviewsEligibilityReason::HOST_BLACKLISTED_BY_SERVER, request.url(), - base::Time::Now(), type); + base::Time::Now(), type, std::move(passed_reasons)); return false; } + passed_reasons.push_back( + PreviewsEligibilityReason::HOST_BLACKLISTED_BY_SERVER); // Check whitelist from the server, if provided. if (IsServerWhitelistedType(type)) { @@ -273,21 +290,23 @@ !previews_opt_guide_->IsWhitelisted(request, type)) { LogPreviewDecisionMade( PreviewsEligibilityReason::HOST_NOT_WHITELISTED_BY_SERVER, - request.url(), base::Time::Now(), type); + request.url(), base::Time::Now(), type, std::move(passed_reasons)); return false; } + passed_reasons.push_back( + PreviewsEligibilityReason::HOST_NOT_WHITELISTED_BY_SERVER); } else { // Since server optimization guidance not configure, allow the preview // but with qualified eligibility reason. LogPreviewDecisionMade( PreviewsEligibilityReason::ALLOWED_WITHOUT_OPTIMIZATION_HINTS, - request.url(), base::Time::Now(), type); + request.url(), base::Time::Now(), type, std::move(passed_reasons)); return true; } } LogPreviewDecisionMade(PreviewsEligibilityReason::ALLOWED, request.url(), - base::Time::Now(), type); + base::Time::Now(), type, std::move(passed_reasons)); return true; }
diff --git a/components/previews/content/previews_io_data.h b/components/previews/content/previews_io_data.h index c66eaab..d361c4fe 100644 --- a/components/previews/content/previews_io_data.h +++ b/components/previews/content/previews_io_data.h
@@ -67,11 +67,15 @@ PreviewsType type, base::Time time) const; - // Adds log message of preview decision made asynchronously. - void LogPreviewDecisionMade(PreviewsEligibilityReason reason, - const GURL& url, - base::Time time, - PreviewsType type) const; + // Adds log message of preview decision made asynchronously. |passed_reasons| + // are PreviewsEligibilityReasons that got passed the decision before + // |reason|. The method takes ownership of |passed_reasons|. + void LogPreviewDecisionMade( + PreviewsEligibilityReason reason, + const GURL& url, + base::Time time, + PreviewsType type, + std::vector<PreviewsEligibilityReason>&& passed_reasons) const; // Adds a navigation to |url| to the black list with result |opt_out|. void AddPreviewNavigation(const GURL& url, bool opt_out, PreviewsType type);
diff --git a/components/previews/content/previews_io_data_unittest.cc b/components/previews/content/previews_io_data_unittest.cc index cf064ff..c0e4040 100644 --- a/components/previews/content/previews_io_data_unittest.cc +++ b/components/previews/content/previews_io_data_unittest.cc
@@ -85,7 +85,23 @@ // PreviewsBlackList: PreviewsEligibilityReason IsLoadedAndAllowed( const GURL& url, - PreviewsType type) const override { + PreviewsType type, + std::vector<PreviewsEligibilityReason>* passed_reasons) const override { + PreviewsEligibilityReason ordered_reasons[] = { + PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, + PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + PreviewsEligibilityReason::USER_BLACKLISTED, + PreviewsEligibilityReason::HOST_BLACKLISTED, + PreviewsEligibilityReason::ALLOWED, + }; + + for (auto reason : ordered_reasons) { + if (status_ == reason) { + return status_; + } + passed_reasons->push_back(reason); + } + NOTREACHED(); return status_; } @@ -157,6 +173,11 @@ return decision_times_; } + const std::vector<std::vector<PreviewsEligibilityReason>>& + decision_passed_reasons() const { + return decision_passed_reasons_; + } + // Expose passed in LogPreviewsNavigation parameters. const std::vector<GURL>& navigation_urls() const { return navigation_urls_; } const std::vector<bool>& navigation_opt_outs() const { @@ -190,14 +211,17 @@ navigation_times_.push_back(time); } - void LogPreviewDecisionMade(PreviewsEligibilityReason reason, - const GURL& url, - base::Time time, - PreviewsType type) override { + void LogPreviewDecisionMade( + PreviewsEligibilityReason reason, + const GURL& url, + base::Time time, + PreviewsType type, + std::vector<PreviewsEligibilityReason>&& passed_reasons) override { decision_reasons_.push_back(reason); decision_urls_.push_back(GURL(url)); decision_times_.push_back(time); decision_types_.push_back(type); + decision_passed_reasons_.push_back(std::move(passed_reasons)); } // Passed in params for blacklist status events. @@ -211,6 +235,7 @@ std::vector<GURL> decision_urls_; std::vector<PreviewsType> decision_types_; std::vector<base::Time> decision_times_; + std::vector<std::vector<PreviewsEligibilityReason>> decision_passed_reasons_; // Passed in LogPreviewsNavigation parameters. std::vector<GURL> navigation_urls_; @@ -741,26 +766,38 @@ GURL url("http://www.url_a.com/url_a"); base::Time time = base::Time::Now(); PreviewsType type = PreviewsType::OFFLINE; + std::vector<PreviewsEligibilityReason> passed_reasons = { + PreviewsEligibilityReason::NETWORK_NOT_SLOW, + PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + PreviewsEligibilityReason::RELOAD_DISALLOWED, + }; + const std::vector<PreviewsEligibilityReason> expected_passed_reasons( + passed_reasons); - io_data()->LogPreviewDecisionMade(reason, url, time, type); + io_data()->LogPreviewDecisionMade(reason, url, time, type, + std::move(passed_reasons)); base::RunLoop().RunUntilIdle(); EXPECT_THAT(ui_service()->decision_reasons(), ::testing::ElementsAre(reason)); EXPECT_THAT(ui_service()->decision_urls(), ::testing::ElementsAre(url)); EXPECT_THAT(ui_service()->decision_types(), ::testing::ElementsAre(type)); EXPECT_THAT(ui_service()->decision_times(), ::testing::ElementsAre(time)); -} -TEST_F(PreviewsIODataTest, BlacklistNotAvailableLogDecisionMade) { + auto actual_passed_reasons = ui_service()->decision_passed_reasons(); + EXPECT_EQ(1UL, actual_passed_reasons.size()); + EXPECT_EQ(expected_passed_reasons.size(), actual_passed_reasons[0].size()); + for (size_t i = 0; i < actual_passed_reasons[0].size(); i++) { + EXPECT_EQ(expected_passed_reasons[i], actual_passed_reasons[0][i]); + } +} // namespace + +TEST_F(PreviewsIODataTest, LogDecisionMadeBlacklistNotAvailable) { InitializeUIService(); CreateFieldTrialWithParams("PreviewsClientLoFi", "Enabled", {}); auto expected_reason = PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE; auto expected_type = PreviewsType::LOFI; - std::unique_ptr<TestPreviewsBlackList> blacklist = - base::MakeUnique<TestPreviewsBlackList>(expected_reason, io_data()); - io_data()->InjectTestBlacklist(std::move(blacklist)); - + io_data()->InjectTestBlacklist(nullptr /* blacklist */); io_data()->ShouldAllowPreviewAtECT(*CreateRequest(), expected_type, net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN, {}); @@ -772,7 +809,7 @@ ::testing::Contains(expected_type)); } -TEST_F(PreviewsIODataTest, BlacklistStatusesLogDecisionMadeDefault) { +TEST_F(PreviewsIODataTest, LogDecisionMadeBlacklistStatusesDefault) { InitializeUIService(); CreateFieldTrialWithParams("PreviewsClientLoFi", "Enabled", {}); @@ -784,8 +821,11 @@ }; auto expected_type = PreviewsType::LOFI; + const size_t reasons_size = 4; - for (auto expected_reason : expected_reasons) { + for (size_t i = 0; i < reasons_size; i++) { + auto expected_reason = expected_reasons[i]; + std::unique_ptr<TestPreviewsBlackList> blacklist = base::MakeUnique<TestPreviewsBlackList>(expected_reason, io_data()); io_data()->InjectTestBlacklist(std::move(blacklist)); @@ -795,14 +835,17 @@ {}); base::RunLoop().RunUntilIdle(); // Testing correct log method is called. - EXPECT_THAT(ui_service()->decision_reasons(), - ::testing::Contains(expected_reason)); + // Check for all decision upto current decision is logged. + for (size_t j = 0; j <= i; j++) { + EXPECT_THAT(ui_service()->decision_reasons(), + ::testing::Contains(expected_reasons[j])); + } EXPECT_THAT(ui_service()->decision_types(), ::testing::Contains(expected_type)); } } -TEST_F(PreviewsIODataTest, BlacklistStatusesLogDecisionMadeIgnore) { +TEST_F(PreviewsIODataTest, LogDecisionMadeBlacklistStatusesIgnore) { InitializeUIService(); CreateFieldTrialWithParams("PreviewsClientLoFi", "Enabled", {}); network_quality_estimator()->set_effective_connection_type( @@ -838,7 +881,7 @@ } } -TEST_F(PreviewsIODataTest, NetworkQualityNotAvailableCallsLogDecisionMade) { +TEST_F(PreviewsIODataTest, LogDecisionMadeNetworkQualityNotAvailable) { InitializeUIService(); CreateFieldTrialWithParams("PreviewsClientLoFi", "Enabled", {}); std::unique_ptr<TestPreviewsBlackList> blacklist = @@ -849,6 +892,14 @@ auto expected_reason = PreviewsEligibilityReason::NETWORK_QUALITY_UNAVAILABLE; auto expected_type = PreviewsType::LOFI; + std::vector<PreviewsEligibilityReason> checked_decisions = { + PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE, + PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, + PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + PreviewsEligibilityReason::USER_BLACKLISTED, + PreviewsEligibilityReason::HOST_BLACKLISTED, + }; + network_quality_estimator()->set_effective_connection_type( net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN); @@ -863,9 +914,17 @@ ::testing::Contains(expected_reason)); EXPECT_THAT(ui_service()->decision_types(), ::testing::Contains(expected_type)); + + EXPECT_EQ(1UL, ui_service()->decision_passed_reasons().size()); + auto actual_passed_reasons = ui_service()->decision_passed_reasons()[0]; + EXPECT_EQ(checked_decisions.size(), actual_passed_reasons.size()); + EXPECT_EQ(checked_decisions.size(), actual_passed_reasons.size()); + for (size_t i = 0; i < actual_passed_reasons.size(); i++) { + EXPECT_EQ(checked_decisions[i], actual_passed_reasons[i]); + } } -TEST_F(PreviewsIODataTest, NetworkNotSlowLogDecisionMade) { +TEST_F(PreviewsIODataTest, LogDecisionMadeNetworkNotSlow) { InitializeUIService(); CreateFieldTrialWithParams("PreviewsClientLoFi", "Enabled", {}); std::unique_ptr<TestPreviewsBlackList> blacklist = @@ -879,6 +938,15 @@ auto expected_reason = PreviewsEligibilityReason::NETWORK_NOT_SLOW; auto expected_type = PreviewsType::LOFI; + std::vector<PreviewsEligibilityReason> checked_decisions = { + PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE, + PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, + PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + PreviewsEligibilityReason::USER_BLACKLISTED, + PreviewsEligibilityReason::HOST_BLACKLISTED, + PreviewsEligibilityReason::NETWORK_QUALITY_UNAVAILABLE, + }; + io_data()->ShouldAllowPreviewAtECT( *CreateRequest(), expected_type, net::EFFECTIVE_CONNECTION_TYPE_2G /* threshold */, {}); @@ -888,9 +956,17 @@ ::testing::Contains(expected_reason)); EXPECT_THAT(ui_service()->decision_types(), ::testing::Contains(expected_type)); + + EXPECT_EQ(1UL, ui_service()->decision_passed_reasons().size()); + auto actual_passed_reasons = ui_service()->decision_passed_reasons()[0]; + EXPECT_EQ(checked_decisions.size(), actual_passed_reasons.size()); + EXPECT_EQ(checked_decisions.size(), actual_passed_reasons.size()); + for (size_t i = 0; i < actual_passed_reasons.size(); i++) { + EXPECT_EQ(checked_decisions[i], actual_passed_reasons[i]); + } } -TEST_F(PreviewsIODataTest, HostBlacklistedLogDecisionMade) { +TEST_F(PreviewsIODataTest, LogDecisionMadeHostBlacklisted) { InitializeUIService(); CreateFieldTrialWithParams("PreviewsClientLoFi", "Enabled", {{"short_host_blacklist", "example.com"}}); @@ -905,6 +981,17 @@ auto expected_reason = PreviewsEligibilityReason::HOST_BLACKLISTED_BY_SERVER; auto expected_type = PreviewsType::LOFI; + std::vector<PreviewsEligibilityReason> checked_decisions = { + PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE, + PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, + PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + PreviewsEligibilityReason::USER_BLACKLISTED, + PreviewsEligibilityReason::HOST_BLACKLISTED, + PreviewsEligibilityReason::NETWORK_QUALITY_UNAVAILABLE, + PreviewsEligibilityReason::NETWORK_NOT_SLOW, + PreviewsEligibilityReason::RELOAD_DISALLOWED, + }; + io_data()->ShouldAllowPreviewAtECT( *CreateRequest(), expected_type, params::EffectiveConnectionTypeThresholdForClientLoFi(), @@ -916,9 +1003,17 @@ ::testing::Contains(expected_reason)); EXPECT_THAT(ui_service()->decision_types(), ::testing::Contains(expected_type)); + + EXPECT_EQ(1UL, ui_service()->decision_passed_reasons().size()); + auto actual_passed_reasons = ui_service()->decision_passed_reasons()[0]; + EXPECT_EQ(checked_decisions.size(), actual_passed_reasons.size()); + EXPECT_EQ(checked_decisions.size(), actual_passed_reasons.size()); + for (size_t i = 0; i < actual_passed_reasons.size(); i++) { + EXPECT_EQ(checked_decisions[i], actual_passed_reasons[i]); + } } -TEST_F(PreviewsIODataTest, ReloadDisallowedLogDecisionMade) { +TEST_F(PreviewsIODataTest, LogDecisionMadeReloadDisallowed) { InitializeUIService(); std::unique_ptr<TestPreviewsBlackList> blacklist = base::MakeUnique<TestPreviewsBlackList>( @@ -933,6 +1028,16 @@ auto expected_reason = PreviewsEligibilityReason::RELOAD_DISALLOWED; auto expected_type = PreviewsType::OFFLINE; + std::vector<PreviewsEligibilityReason> checked_decisions = { + PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE, + PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, + PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + PreviewsEligibilityReason::USER_BLACKLISTED, + PreviewsEligibilityReason::HOST_BLACKLISTED, + PreviewsEligibilityReason::NETWORK_QUALITY_UNAVAILABLE, + PreviewsEligibilityReason::NETWORK_NOT_SLOW, + }; + io_data()->ShouldAllowPreviewAtECT( *request, expected_type, params::EffectiveConnectionTypeThresholdForClientLoFi(), @@ -944,9 +1049,17 @@ ::testing::Contains(expected_reason)); EXPECT_THAT(ui_service()->decision_types(), ::testing::Contains(expected_type)); + + EXPECT_EQ(1UL, ui_service()->decision_passed_reasons().size()); + auto actual_passed_reasons = ui_service()->decision_passed_reasons()[0]; + EXPECT_EQ(checked_decisions.size(), actual_passed_reasons.size()); + EXPECT_EQ(checked_decisions.size(), actual_passed_reasons.size()); + for (size_t i = 0; i < actual_passed_reasons.size(); i++) { + EXPECT_EQ(checked_decisions[i], actual_passed_reasons[i]); + } } -TEST_F(PreviewsIODataTest, AllowPreviewsOnECTLogDecisionMade) { +TEST_F(PreviewsIODataTest, LogDecisionMadeAllowPreviewsOnECT) { InitializeUIService(); CreateFieldTrialWithParams("PreviewsClientLoFi", "Enabled", {}); @@ -962,6 +1075,18 @@ auto expected_reason = PreviewsEligibilityReason::ALLOWED; auto expected_type = PreviewsType::LOFI; + std::vector<PreviewsEligibilityReason> checked_decisions = { + PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE, + PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, + PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + PreviewsEligibilityReason::USER_BLACKLISTED, + PreviewsEligibilityReason::HOST_BLACKLISTED, + PreviewsEligibilityReason::NETWORK_QUALITY_UNAVAILABLE, + PreviewsEligibilityReason::NETWORK_NOT_SLOW, + PreviewsEligibilityReason::RELOAD_DISALLOWED, + PreviewsEligibilityReason::HOST_BLACKLISTED_BY_SERVER, + }; + io_data()->ShouldAllowPreviewAtECT( *CreateRequest(), expected_type, params::EffectiveConnectionTypeThresholdForClientLoFi(), @@ -973,6 +1098,14 @@ ::testing::Contains(expected_reason)); EXPECT_THAT(ui_service()->decision_types(), ::testing::Contains(expected_type)); + + EXPECT_EQ(1UL, ui_service()->decision_passed_reasons().size()); + auto actual_passed_reasons = ui_service()->decision_passed_reasons()[0]; + EXPECT_EQ(checked_decisions.size(), actual_passed_reasons.size()); + EXPECT_EQ(checked_decisions.size(), actual_passed_reasons.size()); + for (size_t i = 0; i < actual_passed_reasons.size(); i++) { + EXPECT_EQ(checked_decisions[i], actual_passed_reasons[i]); + } } TEST_F(PreviewsIODataTest, OnNewBlacklistedHostCallsUIMethodCorrectly) {
diff --git a/components/previews/content/previews_ui_service.cc b/components/previews/content/previews_ui_service.cc index 147b91c..3753aa1 100644 --- a/components/previews/content/previews_ui_service.cc +++ b/components/previews/content/previews_ui_service.cc
@@ -52,12 +52,15 @@ logger_->LogPreviewNavigation(url, type, opt_out, time); } -void PreviewsUIService::LogPreviewDecisionMade(PreviewsEligibilityReason reason, - const GURL& url, - base::Time time, - PreviewsType type) { +void PreviewsUIService::LogPreviewDecisionMade( + PreviewsEligibilityReason reason, + const GURL& url, + base::Time time, + PreviewsType type, + std::vector<PreviewsEligibilityReason>&& passed_reasons) { DCHECK(thread_checker_.CalledOnValidThread()); - logger_->LogPreviewDecisionMade(reason, url, time, type); + logger_->LogPreviewDecisionMade(reason, url, time, type, + std::move(passed_reasons)); } void PreviewsUIService::OnNewBlacklistedHost(const std::string& host,
diff --git a/components/previews/content/previews_ui_service.h b/components/previews/content/previews_ui_service.h index 4a79757..7215ec2 100644 --- a/components/previews/content/previews_ui_service.h +++ b/components/previews/content/previews_ui_service.h
@@ -81,12 +81,15 @@ bool opt_out, base::Time time); - // Log the made decision of previews to PreviewsLogger. Virtualized in - // testing. - virtual void LogPreviewDecisionMade(PreviewsEligibilityReason reason, - const GURL& url, - base::Time time, - PreviewsType type); + // Log the made decision of previews to PreviewsLogger. |passed_reasons| is a + // collection of PreviewsEligibilityReasons passed the checks before |reason|. + // The method takes ownership of |passed_reasons|. Virtualized in testing. + virtual void LogPreviewDecisionMade( + PreviewsEligibilityReason reason, + const GURL& url, + base::Time time, + PreviewsType type, + std::vector<PreviewsEligibilityReason>&& passed_reasons); // Expose the pointer to PreviewsLogger to extract logging messages. This // pointer's life time is the same as of |this|, and it is guaranteed to not
diff --git a/components/previews/content/previews_ui_service_unittest.cc b/components/previews/content/previews_ui_service_unittest.cc index 381010be..87082be 100644 --- a/components/previews/content/previews_ui_service_unittest.cc +++ b/components/previews/content/previews_ui_service_unittest.cc
@@ -71,14 +71,17 @@ navigation_time_ = base::Time(time); } - void LogPreviewDecisionMade(PreviewsEligibilityReason reason, - const GURL& url, - base::Time time, - PreviewsType type) override { + void LogPreviewDecisionMade( + PreviewsEligibilityReason reason, + const GURL& url, + base::Time time, + PreviewsType type, + std::vector<PreviewsEligibilityReason>&& passed_reasons) override { decision_reason_ = reason; decision_url_ = GURL(url); decision_time_ = time; decision_type_ = type; + decision_passed_reasons_ = std::move(passed_reasons); } void OnNewBlacklistedHost(const std::string& host, base::Time time) override { @@ -103,6 +106,10 @@ GURL decision_url() const { return decision_url_; } PreviewsType decision_type() const { return decision_type_; } base::Time decision_time() const { return decision_time_; } + const std::vector<PreviewsEligibilityReason>& decision_passed_reasons() + const { + return decision_passed_reasons_; + } // Return the passed in LogPreviewNavigation parameters. GURL navigation_url() const { return navigation_url_; } @@ -125,6 +132,7 @@ GURL decision_url_; PreviewsType decision_type_; base::Time decision_time_; + std::vector<PreviewsEligibilityReason> decision_passed_reasons_; // Passed in LogPreviewsNavigation parameters. GURL navigation_url_; @@ -238,26 +246,53 @@ const GURL url_a("http://www.url_a.com/url_a"); const base::Time time_a = base::Time::Now(); PreviewsType type_a = PreviewsType::OFFLINE; + std::vector<PreviewsEligibilityReason> passed_reasons_a = { + PreviewsEligibilityReason::NETWORK_NOT_SLOW, + PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + PreviewsEligibilityReason::RELOAD_DISALLOWED, + }; + const std::vector<PreviewsEligibilityReason> expected_passed_reasons_a( + passed_reasons_a); - ui_service()->LogPreviewDecisionMade(reason_a, url_a, time_a, type_a); + ui_service()->LogPreviewDecisionMade(reason_a, url_a, time_a, type_a, + std::move(passed_reasons_a)); EXPECT_EQ(reason_a, logger_ptr_->decision_reason()); EXPECT_EQ(url_a, logger_ptr_->decision_url()); EXPECT_EQ(time_a, logger_ptr_->decision_time()); EXPECT_EQ(type_a, logger_ptr_->decision_type()); + auto actual_passed_reasons_a = logger_ptr_->decision_passed_reasons(); + EXPECT_EQ(3UL, actual_passed_reasons_a.size()); + for (size_t i = 0; i < actual_passed_reasons_a.size(); i++) { + EXPECT_EQ(expected_passed_reasons_a[i], actual_passed_reasons_a[i]); + } + PreviewsEligibilityReason reason_b = PreviewsEligibilityReason::NETWORK_NOT_SLOW; const GURL url_b("http://www.url_b.com/url_b"); const base::Time time_b = base::Time::Now(); PreviewsType type_b = PreviewsType::LOFI; + std::vector<PreviewsEligibilityReason> passed_reasons_b = { + PreviewsEligibilityReason::HOST_NOT_WHITELISTED_BY_SERVER, + PreviewsEligibilityReason::NETWORK_QUALITY_UNAVAILABLE, + }; + const std::vector<PreviewsEligibilityReason> expected_passed_reasons_b( + passed_reasons_b); - ui_service()->LogPreviewDecisionMade(reason_b, url_b, time_b, type_b); + ui_service()->LogPreviewDecisionMade(reason_b, url_b, time_b, type_b, + std::move(passed_reasons_b)); EXPECT_EQ(reason_b, logger_ptr_->decision_reason()); EXPECT_EQ(url_b, logger_ptr_->decision_url()); EXPECT_EQ(type_b, logger_ptr_->decision_type()); EXPECT_EQ(time_b, logger_ptr_->decision_time()); + + auto actual_passed_reasons_b = logger_ptr_->decision_passed_reasons(); + EXPECT_EQ(2UL, actual_passed_reasons_b.size()); + for (size_t i = 0; i < actual_passed_reasons_b.size(); i++) { + EXPECT_EQ(expected_passed_reasons_b[i], actual_passed_reasons_b[i]); + } } TEST_F(PreviewsUIServiceTest, TestOnNewBlacklistedHostPassesCorrectParams) {
diff --git a/components/previews/core/previews_black_list.cc b/components/previews/core/previews_black_list.cc index e4edfd0a..e929192 100644 --- a/components/previews/core/previews_black_list.cc +++ b/components/previews/core/previews_black_list.cc
@@ -29,9 +29,8 @@ item_to_delete = iter; break; } - if (!oldest_opt_out || - iter->second->most_recent_opt_out_time().value() < - oldest_opt_out.value()) { + if (!oldest_opt_out || iter->second->most_recent_opt_out_time().value() < + oldest_opt_out.value()) { oldest_opt_out = iter->second->most_recent_opt_out_time().value(); item_to_delete = iter; } @@ -147,23 +146,29 @@ PreviewsEligibilityReason PreviewsBlackList::IsLoadedAndAllowed( const GURL& url, - PreviewsType type) const { + PreviewsType type, + std::vector<PreviewsEligibilityReason>* passed_reasons) const { DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(url.has_host()); if (!loaded_) return PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED; + passed_reasons->push_back( + PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED); DCHECK(black_list_item_map_); if (last_opt_out_time_ && clock_->Now() < last_opt_out_time_.value() + params::SingleOptOutDuration()) { return PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT; } + passed_reasons->push_back(PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT); if (host_indifferent_black_list_item_->IsBlackListed(clock_->Now())) return PreviewsEligibilityReason::USER_BLACKLISTED; + passed_reasons->push_back(PreviewsEligibilityReason::USER_BLACKLISTED); PreviewsBlackListItem* black_list_item = GetBlackListItemFromMap(*black_list_item_map_, url.host()); if (black_list_item && black_list_item->IsBlackListed(clock_->Now())) return PreviewsEligibilityReason::HOST_BLACKLISTED; + passed_reasons->push_back(PreviewsEligibilityReason::HOST_BLACKLISTED); return PreviewsEligibilityReason::ALLOWED; }
diff --git a/components/previews/core/previews_black_list.h b/components/previews/core/previews_black_list.h index 75a5d46..2860f7793 100644 --- a/components/previews/core/previews_black_list.h +++ b/components/previews/core/previews_black_list.h
@@ -99,10 +99,12 @@ // Synchronously determines if |host_name| should be allowed to show previews. // Returns the reason the blacklist disallowed the preview, or - // PreviewsEligibilityReason::ALLOWED if the preview is allowed. Virtualized - // in testing. - virtual PreviewsEligibilityReason IsLoadedAndAllowed(const GURL& url, - PreviewsType type) const; + // PreviewsEligibilityReason::ALLOWED if the preview is allowed. Record + // checked reasons in |passed_reasons|. Virtualized in testing. + virtual PreviewsEligibilityReason IsLoadedAndAllowed( + const GURL& url, + PreviewsType type, + std::vector<PreviewsEligibilityReason>* passed_reasons) const; // Asynchronously deletes all entries in the in-memory black list. Informs // the backing store to delete entries between |begin_time| and |end_time|,
diff --git a/components/previews/core/previews_black_list_unittest.cc b/components/previews/core/previews_black_list_unittest.cc index a2871caa..b7c2784 100644 --- a/components/previews/core/previews_black_list_unittest.cc +++ b/components/previews/core/previews_black_list_unittest.cc
@@ -144,7 +144,7 @@ class PreviewsBlackListTest : public testing::Test { public: - PreviewsBlackListTest() : field_trial_list_(nullptr) {} + PreviewsBlackListTest() : field_trial_list_(nullptr), passed_reasons_({}) {} ~PreviewsBlackListTest() override {} void TearDown() override { variations::testing::ClearAllVariationParams(); } @@ -163,6 +163,8 @@ black_list_ = base::MakeUnique<PreviewsBlackList>( std::move(opt_out_store), &test_clock_, &blacklist_delegate_); start_ = test_clock_.Now(); + + passed_reasons_ = {}; } void SetHostHistoryParam(size_t host_history) { @@ -236,6 +238,7 @@ base::FieldTrialList field_trial_list_; std::unique_ptr<PreviewsBlackList> black_list_; + std::vector<PreviewsEligibilityReason> passed_reasons_; private: DISALLOW_COPY_AND_ASSIGN(PreviewsBlackListTest); @@ -262,9 +265,11 @@ test_clock_.Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); black_list_->AddPreviewNavigation(url_a, true, PreviewsType::OFFLINE); test_clock_.Advance(base::TimeDelta::FromSeconds(1)); @@ -272,9 +277,11 @@ test_clock_.Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); black_list_->AddPreviewNavigation(url_b, true, PreviewsType::OFFLINE); test_clock_.Advance(base::TimeDelta::FromSeconds(1)); @@ -282,9 +289,11 @@ test_clock_.Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); black_list_->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); test_clock_.Advance(base::TimeDelta::FromSeconds(1)); @@ -294,16 +303,20 @@ test_clock_.Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); black_list_->ClearBlackList(start_, test_clock_.Now()); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); } TEST_F(PreviewsBlackListTest, PerHostBlackListWithStore) { @@ -328,20 +341,26 @@ test_clock_.Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); base::RunLoop().RunUntilIdle(); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); black_list_->AddPreviewNavigation(url_a1, true, PreviewsType::OFFLINE); test_clock_.Advance(base::TimeDelta::FromSeconds(1)); @@ -349,11 +368,14 @@ test_clock_.Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); black_list_->AddPreviewNavigation(url_b, true, PreviewsType::OFFLINE); test_clock_.Advance(base::TimeDelta::FromSeconds(1)); @@ -361,11 +383,14 @@ test_clock_.Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); black_list_->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); test_clock_.Advance(base::TimeDelta::FromSeconds(1)); @@ -375,32 +400,41 @@ test_clock_.Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(0, opt_out_store_->clear_blacklist_count()); black_list_->ClearBlackList(start_, base::Time::Now()); EXPECT_EQ(1, opt_out_store_->clear_blacklist_count()); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); base::RunLoop().RunUntilIdle(); EXPECT_EQ(1, opt_out_store_->clear_blacklist_count()); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); } TEST_F(PreviewsBlackListTest, HostIndifferentBlackList) { @@ -426,43 +460,56 @@ test_clock_.Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(urls[1], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[1], PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(urls[2], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[2], PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(urls[3], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[3], PreviewsType::OFFLINE, + &passed_reasons_)); for (size_t i = 0; i < host_indifferent_threshold; i++) { black_list_->AddPreviewNavigation(urls[i], true, PreviewsType::OFFLINE); EXPECT_EQ(i != 3 ? PreviewsEligibilityReason::ALLOWED : PreviewsEligibilityReason::USER_BLACKLISTED, - black_list_->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE, + &passed_reasons_)); test_clock_.Advance(base::TimeDelta::FromSeconds(1)); } EXPECT_EQ(PreviewsEligibilityReason::USER_BLACKLISTED, - black_list_->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::USER_BLACKLISTED, - black_list_->IsLoadedAndAllowed(urls[1], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[1], PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::USER_BLACKLISTED, - black_list_->IsLoadedAndAllowed(urls[2], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[2], PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::USER_BLACKLISTED, - black_list_->IsLoadedAndAllowed(urls[3], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[3], PreviewsType::OFFLINE, + &passed_reasons_)); black_list_->AddPreviewNavigation(urls[3], false, PreviewsType::OFFLINE); test_clock_.Advance(base::TimeDelta::FromSeconds(1)); // New non-opt-out entry will cause these to be allowed now. EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(urls[1], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[1], PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(urls[2], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[2], PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(urls[3], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[3], PreviewsType::OFFLINE, + &passed_reasons_)); } TEST_F(PreviewsBlackListTest, QueueBehavior) { @@ -487,17 +534,20 @@ StartTest(false /* null_opt_out */); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE, + &passed_reasons_)); black_list_->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE); test_clock_.Advance(base::TimeDelta::FromSeconds(1)); black_list_->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE); test_clock_.Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE, + &passed_reasons_)); base::RunLoop().RunUntilIdle(); EXPECT_EQ(opt_out ? PreviewsEligibilityReason::HOST_BLACKLISTED : PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE, + &passed_reasons_)); black_list_->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE); test_clock_.Advance(base::TimeDelta::FromSeconds(1)); black_list_->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE); @@ -514,10 +564,12 @@ EXPECT_EQ(1, opt_out_store_->clear_blacklist_count()); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(opt_out ? PreviewsEligibilityReason::HOST_BLACKLISTED : PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url2, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url2, PreviewsType::OFFLINE, + &passed_reasons_)); } } @@ -552,11 +604,14 @@ black_list_->AddPreviewNavigation(url_c, false, PreviewsType::OFFLINE); // url_a should stay in the map, since it has an opt out time. EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE, + &passed_reasons_)); test_clock_.Advance(base::TimeDelta::FromSeconds(1)); black_list_->AddPreviewNavigation(url_d, true, PreviewsType::OFFLINE); @@ -564,11 +619,14 @@ black_list_->AddPreviewNavigation(url_e, true, PreviewsType::OFFLINE); // url_d and url_e should remain in the map, but url_a should be evicted. EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list_->IsLoadedAndAllowed(url_d, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_d, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list_->IsLoadedAndAllowed(url_e, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_e, PreviewsType::OFFLINE, + &passed_reasons_)); } TEST_F(PreviewsBlackListTest, SingleOptOut) { @@ -594,34 +652,42 @@ black_list_->AddPreviewNavigation(url_a, false, PreviewsType::OFFLINE); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE, + &passed_reasons_)); test_clock_.Advance( base::TimeDelta::FromSeconds(single_opt_out_duration + 1)); black_list_->AddPreviewNavigation(url_b, true, PreviewsType::OFFLINE); EXPECT_EQ(PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, - black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE, + &passed_reasons_)); test_clock_.Advance( base::TimeDelta::FromSeconds(single_opt_out_duration - 1)); EXPECT_EQ(PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, - black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE, + &passed_reasons_)); test_clock_.Advance( base::TimeDelta::FromSeconds(single_opt_out_duration + 1)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE, + &passed_reasons_)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE, + &passed_reasons_)); } TEST_F(PreviewsBlackListTest, AddPreviewUMA) { @@ -646,7 +712,8 @@ const GURL url("http://www.url.com"); RunClearingBlackListTest(url, true /* short_time */); EXPECT_EQ(PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, - black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE, + &passed_reasons_)); } TEST_F(PreviewsBlackListTest, ClearingBlackListClearsRecentNavigation) { @@ -658,7 +725,8 @@ RunClearingBlackListTest(url, false /* short_time */); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE, + &passed_reasons_)); } TEST_F(PreviewsBlackListTest, ObserverIsNotifiedOnHostBlacklisted) { @@ -679,7 +747,8 @@ StartTest(true /* null_opt_out */); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list_->IsLoadedAndAllowed(url_, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_, PreviewsType::OFFLINE, + &passed_reasons_)); // Observer is not notified as blacklisted when the threshold does not met. test_clock_.Advance(base::TimeDelta::FromSeconds(1)); @@ -860,6 +929,133 @@ blacklist_delegate_.blacklisted_hosts().find(url_a.host())->second); } +TEST_F(PreviewsBlackListTest, PassedReasonsWhenBlacklistDataNotLoaded) { + // Test that IsLoadedAndAllow, push checked PreviewsEligibilityReasons to the + // |passed_reasons| vector. + const GURL url("http://www.url_.com/"); + StartTest(false /* null_opt_out */); + + EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE, + &passed_reasons_)); + + EXPECT_EQ(0UL, passed_reasons_.size()); +} + +TEST_F(PreviewsBlackListTest, PassedReasonsWhenUserRecentlyOptedOut) { + // Test that IsLoadedAndAllow, push checked PreviewsEligibilityReasons to the + // |passed_reasons| vector. + const GURL url("http://www.url_.com/"); + StartTest(true /* null_opt_out */); + + black_list_->AddPreviewNavigation(url, true, PreviewsType::OFFLINE); + EXPECT_EQ(PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE, + &passed_reasons_)); + EXPECT_EQ(1UL, passed_reasons_.size()); + EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, + passed_reasons_[0]); +} + +TEST_F(PreviewsBlackListTest, PassedReasonsWhenUserBlacklisted) { + // Test that IsLoadedAndAllow, push checked PreviewsEligibilityReasons to the + // |passed_reasons| vector. + const GURL urls[] = { + GURL("http://www.url_0.com"), GURL("http://www.url_1.com"), + GURL("http://www.url_2.com"), GURL("http://www.url_3.com"), + }; + + // Per host blacklisting should have no effect with the following params. + const size_t per_host_history = 1; + const size_t host_indifferent_history = 4; + const size_t host_indifferent_threshold = host_indifferent_history; + SetHostHistoryParam(per_host_history); + SetHostIndifferentHistoryParam(host_indifferent_history); + SetHostThresholdParam(per_host_history + 1); + SetHostIndifferentThresholdParam(host_indifferent_threshold); + SetHostDurationParam(365); + // Disable single opt out by setting duration to 0. + SetSingleOptOutDurationParam(0); + + StartTest(true /* null_opt_out */); + test_clock_.Advance(base::TimeDelta::FromSeconds(1)); + + for (auto url : urls) { + black_list_->AddPreviewNavigation(url, true, PreviewsType::OFFLINE); + } + + EXPECT_EQ(PreviewsEligibilityReason::USER_BLACKLISTED, + black_list_->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE, + &passed_reasons_)); + + PreviewsEligibilityReason expected_reasons[] = { + PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, + PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + }; + EXPECT_EQ(2UL, passed_reasons_.size()); + for (size_t i = 0; i < passed_reasons_.size(); i++) { + EXPECT_EQ(expected_reasons[i], passed_reasons_[i]); + } +} + +TEST_F(PreviewsBlackListTest, PassedReasonsWhenHostBlacklisted) { + // Test that IsLoadedAndAllow, push checked PreviewsEligibilityReasons to the + // |passed_reasons| vector. + const GURL url("http://www.url_a.com"); + + // Host indifferent blacklisting should have no effect with the following + // params. + const size_t host_indifferent_history = 1; + SetHostHistoryParam(4); + SetHostIndifferentHistoryParam(host_indifferent_history); + SetHostThresholdParam(2); + SetHostIndifferentThresholdParam(host_indifferent_history + 1); + SetHostDurationParam(365); + // Disable single opt out by setting duration to 0. + SetSingleOptOutDurationParam(0); + + StartTest(true /* null_opt_out */); + + black_list_->AddPreviewNavigation(url, true, PreviewsType::OFFLINE); + black_list_->AddPreviewNavigation(url, true, PreviewsType::OFFLINE); + + EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE, + &passed_reasons_)); + + PreviewsEligibilityReason expected_reasons[] = { + PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, + PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + PreviewsEligibilityReason::USER_BLACKLISTED, + }; + EXPECT_EQ(3UL, passed_reasons_.size()); + for (size_t i = 0; i < passed_reasons_.size(); i++) { + EXPECT_EQ(expected_reasons[i], passed_reasons_[i]); + } +} + +TEST_F(PreviewsBlackListTest, PassedReasonsWhenAllowed) { + // Test that IsLoadedAndAllow, push checked PreviewsEligibilityReasons to the + // |passed_reasons| vector. + const GURL url("http://www.url.com"); + StartTest(true /* null_opt_out */); + + EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE, + &passed_reasons_)); + + PreviewsEligibilityReason expected_reasons[] = { + PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, + PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + PreviewsEligibilityReason::USER_BLACKLISTED, + PreviewsEligibilityReason::HOST_BLACKLISTED, + }; + EXPECT_EQ(4UL, passed_reasons_.size()); + for (size_t i = 0; i < passed_reasons_.size(); i++) { + EXPECT_EQ(expected_reasons[i], passed_reasons_[i]); + } +} + } // namespace } // namespace previews
diff --git a/components/previews/core/previews_logger.cc b/components/previews/core/previews_logger.cc index 57ff34c..7cfb8544 100644 --- a/components/previews/core/previews_logger.cc +++ b/components/previews/core/previews_logger.cc
@@ -29,42 +29,56 @@ opt_out ? "True" : "False"); } -std::string GetReasonDescription(PreviewsEligibilityReason reason) { +std::string GetReasonDescription(PreviewsEligibilityReason reason, + bool final_reason) { switch (reason) { case PreviewsEligibilityReason::ALLOWED: + DCHECK(final_reason); return "Allowed"; case PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE: - return "Blacklist failed to be created"; + return final_reason ? "Blacklist failed to be created" + : "Blacklist not null"; case PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED: - return "Blacklist not loaded from disk yet"; + return final_reason ? "Blacklist not loaded from disk yet" + : "Blacklist loaded from disk"; case PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT: - return "User recently opted out"; + return final_reason ? "User recently opted out" + : "User did not opt out recently"; case PreviewsEligibilityReason::USER_BLACKLISTED: - return "All previews are blacklisted"; + return final_reason ? "All previews are blacklisted" + : "Not all previews are blacklisted"; case PreviewsEligibilityReason::HOST_BLACKLISTED: - return "All previews on this host are blacklisted"; + return final_reason ? "All previews on this host are blacklisted" + : "Host is not blacklisted on all previews"; case PreviewsEligibilityReason::NETWORK_QUALITY_UNAVAILABLE: - return "Network quality unavailable"; + return final_reason ? "Network quality unavailable" + : "Network quality available"; case PreviewsEligibilityReason::NETWORK_NOT_SLOW: - return "Network not slow"; + return final_reason ? "Network not slow" : "Network is slow"; case PreviewsEligibilityReason::RELOAD_DISALLOWED: - return "Page reloads do not show previews for this preview type"; + return final_reason + ? "Page reloads do not show previews for this preview type" + : "Page reloads allowed"; case PreviewsEligibilityReason::HOST_BLACKLISTED_BY_SERVER: - return "Host blacklisted by server rules"; + return final_reason ? "Host blacklisted by server rules" + : "Host not blacklisted by server rules"; case PreviewsEligibilityReason::HOST_NOT_WHITELISTED_BY_SERVER: - return "Host not whitelisted by server rules"; + return final_reason ? "Host not whitelisted by server rules" + : "Host whitelisted by server rules"; case PreviewsEligibilityReason::ALLOWED_WITHOUT_OPTIMIZATION_HINTS: - return "Allowed (but without server rule check)"; + return final_reason ? "Allowed (but without server rule check)" + : "Not allowed (without server rule check)"; } NOTREACHED(); return ""; } std::string GetDescriptionForPreviewsDecision(PreviewsEligibilityReason reason, - PreviewsType type) { + PreviewsType type, + bool final_reason) { return base::StringPrintf("%s preview - %s", GetStringNameForType(type).c_str(), - GetReasonDescription(reason).c_str()); + GetReasonDescription(reason, final_reason).c_str()); } } // namespace @@ -167,18 +181,30 @@ time); } -void PreviewsLogger::LogPreviewDecisionMade(PreviewsEligibilityReason reason, - const GURL& url, - base::Time time, - PreviewsType type) { +void PreviewsLogger::LogPreviewDecisionMade( + PreviewsEligibilityReason reason, + const GURL& url, + base::Time time, + PreviewsType type, + std::vector<PreviewsEligibilityReason>&& passed_reasons) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_GE(kMaximumDecisionLogs, decisions_logs_.size()); - std::string description = GetDescriptionForPreviewsDecision(reason, type); + // Logs all passed decisions messages. + for (auto decision : passed_reasons) { + std::string decision_description = GetDescriptionForPreviewsDecision( + decision, type, false /* final_reason */); + LogMessage(kPreviewDecisionMadeEventType, decision_description, url, time); + decisions_logs_.emplace_back(kPreviewDecisionMadeEventType, + decision_description, url, time); + } + + std::string description = + GetDescriptionForPreviewsDecision(reason, type, true /* final_reason */); LogMessage(kPreviewDecisionMadeEventType, description, url, time); - // Pop out the oldest message when the list is full. - if (decisions_logs_.size() >= kMaximumDecisionLogs) { + // Pop out the older messages when the list is full. + while (decisions_logs_.size() >= kMaximumDecisionLogs) { decisions_logs_.pop_front(); }
diff --git a/components/previews/core/previews_logger.h b/components/previews/core/previews_logger.h index 18f3417..03b033b 100644 --- a/components/previews/core/previews_logger.h +++ b/components/previews/core/previews_logger.h
@@ -10,6 +10,7 @@ #include <unordered_map> #include "base/macros.h" +#include "base/memory/ptr_util.h" #include "base/observer_list.h" #include "base/sequence_checker.h" #include "base/time/time.h" @@ -81,11 +82,15 @@ base::Time time); // Add a MessageLog for the a decision that was made about the state of - // previews and blacklist. Virtualized in testing. - virtual void LogPreviewDecisionMade(PreviewsEligibilityReason reason, - const GURL& url, - base::Time time, - PreviewsType type); + // previews and blacklist. |passed_reasons| is an ordered list of + // PreviewsEligibilityReasons that got pass the decision. The method takes + // ownership of |passed_reasons|. Virtualized in testing. + virtual void LogPreviewDecisionMade( + PreviewsEligibilityReason reason, + const GURL& url, + base::Time time, + PreviewsType type, + std::vector<PreviewsEligibilityReason>&& passed_reasons); // Notify observers that |host| is blacklisted at |time|. Virtualized in // testing.
diff --git a/components/previews/core/previews_logger_unittest.cc b/components/previews/core/previews_logger_unittest.cc index c165964..f02e45a 100644 --- a/components/previews/core/previews_logger_unittest.cc +++ b/components/previews/core/previews_logger_unittest.cc
@@ -117,17 +117,26 @@ void SetUp() override { logger_ = base::MakeUnique<PreviewsLogger>(); } std::string LogPreviewDecisionAndGetReasonDescription( - PreviewsEligibilityReason reason) { + PreviewsEligibilityReason reason, + bool final_reason) { const base::Time time = base::Time::Now(); PreviewsType type = PreviewsType::OFFLINE; const GURL url("http://www.url_a.com/url"); TestPreviewsLoggerObserver observer; logger_->AddAndNotifyObserver(&observer); - logger_->LogPreviewDecisionMade(reason, url, time, type); + if (final_reason) { + std::vector<PreviewsEligibilityReason> passed_reasons = {}; + logger_->LogPreviewDecisionMade(reason, url, time, type, + std::move(passed_reasons)); + } else { + std::vector<PreviewsEligibilityReason> passed_reasons = {reason}; + logger_->LogPreviewDecisionMade(PreviewsEligibilityReason::ALLOWED, url, + time, type, std::move(passed_reasons)); + } auto actual = observer.messages(); - const size_t expected_size = 1; + const size_t expected_size = final_reason ? 1 : 2; EXPECT_EQ(expected_size, actual.size()); std::vector<std::string> description_parts = base::SplitStringUsingSubstr( @@ -147,19 +156,26 @@ PreviewsType type_b = PreviewsType::LOFI; PreviewsEligibilityReason reason_a = PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE; + std::vector<PreviewsEligibilityReason> passed_reasons_a = {}; PreviewsEligibilityReason reason_b = PreviewsEligibilityReason::NETWORK_NOT_SLOW; + std::vector<PreviewsEligibilityReason> passed_reasons_b = { + PreviewsEligibilityReason::NETWORK_QUALITY_UNAVAILABLE, + PreviewsEligibilityReason::RELOAD_DISALLOWED, + }; const GURL url_a("http://www.url_a.com/url_a"); const GURL url_b("http://www.url_b.com/url_b"); TestPreviewsLoggerObserver observer; logger_->AddAndNotifyObserver(&observer); - logger_->LogPreviewDecisionMade(reason_a, url_a, time, type_a); - logger_->LogPreviewDecisionMade(reason_b, url_b, time, type_b); + logger_->LogPreviewDecisionMade(reason_a, url_a, time, type_a, + std::move(passed_reasons_a)); + logger_->LogPreviewDecisionMade(reason_b, url_b, time, type_b, + std::move(passed_reasons_b)); auto actual = observer.messages(); - const size_t expected_size = 2; + const size_t expected_size = 4; // reason_a, reason_b, and passed_reasons_b EXPECT_EQ(expected_size, actual.size()); std::string expected_description_a = @@ -169,11 +185,23 @@ EXPECT_EQ(url_a, actual[0].url); EXPECT_EQ(time, actual[0].time); - std::string expected_description_b = "LoFi preview - Network not slow"; + std::string expected_passed_0 = "LoFi preview - Network quality available"; EXPECT_EQ(kPreviewsDecisionMadeEventType, actual[1].event_type); - EXPECT_EQ(expected_description_b, actual[1].event_description); + EXPECT_EQ(expected_passed_0, actual[1].event_description); EXPECT_EQ(url_b, actual[1].url); EXPECT_EQ(time, actual[1].time); + + std::string expected_passed_1 = "LoFi preview - Page reloads allowed"; + EXPECT_EQ(kPreviewsDecisionMadeEventType, actual[2].event_type); + EXPECT_EQ(expected_passed_1, actual[2].event_description); + EXPECT_EQ(url_b, actual[2].url); + EXPECT_EQ(time, actual[2].time); + + std::string expected_description_b = "LoFi preview - Network not slow"; + EXPECT_EQ(kPreviewsDecisionMadeEventType, actual[3].event_type); + EXPECT_EQ(expected_description_b, actual[3].event_description); + EXPECT_EQ(url_b, actual[3].url); + EXPECT_EQ(time, actual[3].time); } TEST_F(PreviewsLoggerTest, LogPreviewNavigationLogMessage) { @@ -216,7 +244,9 @@ const GURL url("http://www.url_.com/url_"); for (size_t i = 0; i < 2 * kMaximumDecisionLogs; i++) { - logger_->LogPreviewDecisionMade(reason, url, time, type); + std::vector<PreviewsEligibilityReason> passed_reasons = {}; + logger_->LogPreviewDecisionMade(reason, url, time, type, + std::move(passed_reasons)); } TestPreviewsLoggerObserver observer; @@ -239,28 +269,32 @@ EXPECT_EQ(kMaximumNavigationLogs, observer.messages().size()); } -TEST_F(PreviewsLoggerTest, ObserverIsNotifiedOfHistoricalNavigationsWhenAdded) { +TEST_F(PreviewsLoggerTest, + ObserverIsNotifiedOfHistoricalNavigationsAndDecisionsWhenAdded) { // Non historical log event. logger_->LogMessage("Event_", "Some description_", GURL("http://www.url_.com/url_"), base::Time::Now()); - PreviewsEligibilityReason reason = - PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE; PreviewsType type = PreviewsType::LOFI; + PreviewsEligibilityReason final_reason = + PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE; + std::vector<PreviewsEligibilityReason> passed_reasons = { + PreviewsEligibilityReason::NETWORK_NOT_SLOW}; const GURL urls[] = { - GURL("http://www.url_0.com/url_0"), GURL("http://www.url_1.com/url_1"), - GURL("http://www.url_2.com/url_2"), + GURL("http://www.url_0.com/url_0"), // Decision event. + GURL("http://www.url_0.com/url_0"), // Decision event. + GURL("http://www.url_1.com/url_1"), // Navigation event. }; const base::Time times[] = { base::Time::FromJsTime(-413696806000), // Nov 21 1956 20:13:14 UTC + base::Time::FromJsTime(-413696806000), // Same as above. base::Time::FromJsTime(758620800000), // Jan 15 1994 08:00:00 UTC - base::Time::FromJsTime(1581696550000), // Feb 14 2020 16:09:10 UTC }; - // Logging decisions and navigations events in mixed orders. - logger_->LogPreviewDecisionMade(reason, urls[0], times[0], type); - logger_->LogPreviewNavigation(urls[1], type, true /* opt_out */, times[1]); - logger_->LogPreviewDecisionMade(reason, urls[2], times[2], type); + // Logging decisions and navigations events. + logger_->LogPreviewDecisionMade(final_reason, urls[0], times[0], type, + std::move(passed_reasons)); + logger_->LogPreviewNavigation(urls[2], type, true /* opt_out */, times[2]); TestPreviewsLoggerObserver observer; logger_->AddAndNotifyObserver(&observer); @@ -272,8 +306,8 @@ EXPECT_EQ(expected_size, received_messages.size()); const std::string expected_types[] = { - kPreviewsDecisionMadeEventType, kPreviewsNavigationEventType, - kPreviewsDecisionMadeEventType, + kPreviewsDecisionMadeEventType, kPreviewsDecisionMadeEventType, + kPreviewsNavigationEventType, }; for (size_t i = 0; i < expected_size; i++) { @@ -337,94 +371,198 @@ } } -TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionAllowed) { +TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionAllowedChecked) { std::string actual_description = LogPreviewDecisionAndGetReasonDescription( - PreviewsEligibilityReason::ALLOWED); + PreviewsEligibilityReason::ALLOWED, true /* final_reason */); std::string expected_description = "Allowed"; EXPECT_EQ(expected_description, actual_description); } -TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionUnavailabe) { +TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionUnavailabeFailed) { std::string actual_description = LogPreviewDecisionAndGetReasonDescription( - PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE); + PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE, + true /* final_reason */); std::string expected_description = "Blacklist failed to be created"; EXPECT_EQ(expected_description, actual_description); } -TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionNotLoaded) { +TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionUnavailabeChecked) { std::string actual_description = LogPreviewDecisionAndGetReasonDescription( - PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED); + PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE, + false /* final_reason */); + std::string expected_description = "Blacklist not null"; + EXPECT_EQ(expected_description, actual_description); +} + +TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionNotLoadedFailed) { + std::string actual_description = LogPreviewDecisionAndGetReasonDescription( + PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, + true /* final_reason */); std::string expected_description = "Blacklist not loaded from disk yet"; EXPECT_EQ(expected_description, actual_description); } -TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionRecentlyOptedOut) { +TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionNotLoadedChecked) { std::string actual_description = LogPreviewDecisionAndGetReasonDescription( - PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT); + PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, + false /* final_reason */); + std::string expected_description = "Blacklist loaded from disk"; + EXPECT_EQ(expected_description, actual_description); +} + +TEST_F(PreviewsLoggerTest, + LogPreviewDecisionDescriptionRecentlyOptedOutFailed) { + std::string actual_description = LogPreviewDecisionAndGetReasonDescription( + PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + true /* final_reason */); std::string expected_description = "User recently opted out"; EXPECT_EQ(expected_description, actual_description); } -TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionBlacklisted) { +TEST_F(PreviewsLoggerTest, + LogPreviewDecisionDescriptionRecentlyOptedOutChecked) { std::string actual_description = LogPreviewDecisionAndGetReasonDescription( - PreviewsEligibilityReason::USER_BLACKLISTED); + PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + false /* final_reason */); + std::string expected_description = "User did not opt out recently"; + EXPECT_EQ(expected_description, actual_description); +} + +TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionBlacklistedFailed) { + std::string actual_description = LogPreviewDecisionAndGetReasonDescription( + PreviewsEligibilityReason::USER_BLACKLISTED, true /* final_reason */); std::string expected_description = "All previews are blacklisted"; EXPECT_EQ(expected_description, actual_description); } -TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionHostBlacklisted) { +TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionBlacklistedChecked) { std::string actual_description = LogPreviewDecisionAndGetReasonDescription( - PreviewsEligibilityReason::HOST_BLACKLISTED); + PreviewsEligibilityReason::USER_BLACKLISTED, false /* final_reason */); + std::string expected_description = "Not all previews are blacklisted"; + EXPECT_EQ(expected_description, actual_description); +} + +TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionHostBlacklistedFailed) { + std::string actual_description = LogPreviewDecisionAndGetReasonDescription( + PreviewsEligibilityReason::HOST_BLACKLISTED, true /* final_reason */); std::string expected_description = "All previews on this host are blacklisted"; EXPECT_EQ(expected_description, actual_description); } -TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionNetworkUnavailable) { +TEST_F(PreviewsLoggerTest, + LogPreviewDecisionDescriptionHostBlacklistedChecked) { std::string actual_description = LogPreviewDecisionAndGetReasonDescription( - PreviewsEligibilityReason::NETWORK_QUALITY_UNAVAILABLE); + PreviewsEligibilityReason::HOST_BLACKLISTED, false /* final_reason */); + std::string expected_description = "Host is not blacklisted on all previews"; + EXPECT_EQ(expected_description, actual_description); +} + +TEST_F(PreviewsLoggerTest, + LogPreviewDecisionDescriptionNetworkUnavailableFailed) { + std::string actual_description = LogPreviewDecisionAndGetReasonDescription( + PreviewsEligibilityReason::NETWORK_QUALITY_UNAVAILABLE, + true /* final_reason */); std::string expected_description = "Network quality unavailable"; EXPECT_EQ(expected_description, actual_description); } -TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionNetworkNotSlow) { +TEST_F(PreviewsLoggerTest, + LogPreviewDecisionDescriptionNetworkUnavailableChecked) { std::string actual_description = LogPreviewDecisionAndGetReasonDescription( - PreviewsEligibilityReason::NETWORK_NOT_SLOW); + PreviewsEligibilityReason::NETWORK_QUALITY_UNAVAILABLE, + false /* final_reason */); + std::string expected_description = "Network quality available"; + EXPECT_EQ(expected_description, actual_description); +} + +TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionNetworkNotSlowFailed) { + std::string actual_description = LogPreviewDecisionAndGetReasonDescription( + PreviewsEligibilityReason::NETWORK_NOT_SLOW, true /* final_reason */); std::string expected_description = "Network not slow"; EXPECT_EQ(expected_description, actual_description); } -TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionReloadDisallowed) { +TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionNetworkNotSlowChecked) { std::string actual_description = LogPreviewDecisionAndGetReasonDescription( - PreviewsEligibilityReason::RELOAD_DISALLOWED); + PreviewsEligibilityReason::NETWORK_NOT_SLOW, false /* final_reason */); + std::string expected_description = "Network is slow"; + + EXPECT_EQ(expected_description, actual_description); +} + +TEST_F(PreviewsLoggerTest, + LogPreviewDecisionDescriptionReloadDisallowedFailed) { + std::string actual_description = LogPreviewDecisionAndGetReasonDescription( + PreviewsEligibilityReason::RELOAD_DISALLOWED, true /* final_reason */); std::string expected_description = "Page reloads do not show previews for this preview type"; EXPECT_EQ(expected_description, actual_description); } -TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionServerRules) { +TEST_F(PreviewsLoggerTest, + LogPreviewDecisionDescriptionReloadDisallowedChecked) { std::string actual_description = LogPreviewDecisionAndGetReasonDescription( - PreviewsEligibilityReason::HOST_BLACKLISTED_BY_SERVER); + PreviewsEligibilityReason::RELOAD_DISALLOWED, false /* final_reason */); + std::string expected_description = "Page reloads allowed"; + EXPECT_EQ(expected_description, actual_description); +} + +TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionServerRulesFailed) { + std::string actual_description = LogPreviewDecisionAndGetReasonDescription( + PreviewsEligibilityReason::HOST_BLACKLISTED_BY_SERVER, + true /* final_reason */); std::string expected_description = "Host blacklisted by server rules"; EXPECT_EQ(expected_description, actual_description); } -TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionNotWhitelisedByServer) { +TEST_F(PreviewsLoggerTest, LogPreviewDecisionDescriptionServerRulesChecked) { std::string actual_description = LogPreviewDecisionAndGetReasonDescription( - PreviewsEligibilityReason::HOST_NOT_WHITELISTED_BY_SERVER); + PreviewsEligibilityReason::HOST_BLACKLISTED_BY_SERVER, + false /* final_reason */); + std::string expected_description = "Host not blacklisted by server rules"; + EXPECT_EQ(expected_description, actual_description); +} + +TEST_F(PreviewsLoggerTest, + LogPreviewDecisionDescriptionNotWhitelisedByServerFailed) { + std::string actual_description = LogPreviewDecisionAndGetReasonDescription( + PreviewsEligibilityReason::HOST_NOT_WHITELISTED_BY_SERVER, + true /* final_reason */); std::string expected_description = "Host not whitelisted by server rules"; EXPECT_EQ(expected_description, actual_description); } TEST_F(PreviewsLoggerTest, - LogPreviewDecisionDescriptionAllowedWithoutServerOptimizationHints) { + LogPreviewDecisionDescriptionNotWhitelisedByServerChecked) { std::string actual_description = LogPreviewDecisionAndGetReasonDescription( - PreviewsEligibilityReason::ALLOWED_WITHOUT_OPTIMIZATION_HINTS); + PreviewsEligibilityReason::HOST_NOT_WHITELISTED_BY_SERVER, + false /* final_reason */); + std::string expected_description = "Host whitelisted by server rules"; + EXPECT_EQ(expected_description, actual_description); +} + +TEST_F( + PreviewsLoggerTest, + LogPreviewDecisionDescriptionAllowedWithoutServerOptimizationHintsFailed) { + std::string actual_description = LogPreviewDecisionAndGetReasonDescription( + PreviewsEligibilityReason::ALLOWED_WITHOUT_OPTIMIZATION_HINTS, + true /* final_reason */); std::string expected_description = "Allowed (but without server rule check)"; EXPECT_EQ(expected_description, actual_description); } +TEST_F( + PreviewsLoggerTest, + LogPreviewDecisionDescriptionAllowedWithoutServerOptimizationHintsChecked) { + std::string actual_description = LogPreviewDecisionAndGetReasonDescription( + PreviewsEligibilityReason::ALLOWED_WITHOUT_OPTIMIZATION_HINTS, + false /* final_reason */); + std::string expected_description = "Not allowed (without server rule check)"; + EXPECT_EQ(expected_description, actual_description); +} + TEST_F(PreviewsLoggerTest, NotifyObserversOfNewBlacklistedHost) { TestPreviewsLoggerObserver observers[3];
diff --git a/content/browser/accessibility/accessibility_win_browsertest.cc b/content/browser/accessibility/accessibility_win_browsertest.cc index bbf0fd10..03cd1af 100644 --- a/content/browser/accessibility/accessibility_win_browsertest.cc +++ b/content/browser/accessibility/accessibility_win_browsertest.cc
@@ -2063,4 +2063,77 @@ accessible_cell.Reset(); } +IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest, TestScrollTo) { + LoadInitialAccessibilityTreeFromHtml( + "<!DOCTYPE html><html><body>" + "<div style='height: 5000px;'></div>" + "<img src='#' alt='Target1'>" + "<div style='height: 5000px;'></div>" + "<img src='#' alt='Target2'>" + "<div style='height: 5000px;'></div>" + "</body></html>"); + + // Retrieve the IAccessible interface for the document node. + Microsoft::WRL::ComPtr<IAccessible> document(GetRendererAccessible()); + + // Get the dimensions of the document. + LONG doc_x, doc_y, doc_width, doc_height; + base::win::ScopedVariant childid_self(CHILDID_SELF); + ASSERT_HRESULT_SUCCEEDED(document->accLocation(&doc_x, &doc_y, &doc_width, + &doc_height, childid_self)); + + // The document should only have two children, both with a role of GRAPHIC. + std::vector<base::win::ScopedVariant> document_children = + GetAllAccessibleChildren(document.Get()); + ASSERT_EQ(2u, document_children.size()); + Microsoft::WRL::ComPtr<IAccessible2> target; + ASSERT_HRESULT_SUCCEEDED(QueryIAccessible2( + GetAccessibleFromVariant(document.Get(), document_children[0].AsInput()) + .Get(), + target.GetAddressOf())); + LONG target_role = 0; + ASSERT_HRESULT_SUCCEEDED(target->role(&target_role)); + ASSERT_EQ(ROLE_SYSTEM_GRAPHIC, target_role); + Microsoft::WRL::ComPtr<IAccessible2> target2; + ASSERT_HRESULT_SUCCEEDED(QueryIAccessible2( + GetAccessibleFromVariant(document.Get(), document_children[1].AsInput()) + .Get(), + target2.GetAddressOf())); + LONG target2_role = 0; + ASSERT_HRESULT_SUCCEEDED(target2->role(&target2_role)); + ASSERT_EQ(ROLE_SYSTEM_GRAPHIC, target2_role); + + // Call scrollTo on the first target. Ensure it ends up very near the + // center of the window. + AccessibilityNotificationWaiter waiter(shell()->web_contents(), + ui::kAXModeComplete, + ui::AX_EVENT_SCROLL_POSITION_CHANGED); + ASSERT_HRESULT_SUCCEEDED(target->scrollTo(IA2_SCROLL_TYPE_ANYWHERE)); + waiter.WaitForNotification(); + + // Don't assume anything about the font size or the exact centering + // behavior, just assert that the object is (roughly) centered by + // checking that its top coordinate is between 40% and 60% of the + // document's height. + LONG x, y, width, height; + ASSERT_HRESULT_SUCCEEDED( + target->accLocation(&x, &y, &width, &height, childid_self)); + EXPECT_GT(y + height / 2, doc_y + 0.4 * doc_height); + EXPECT_LT(y + height / 2, doc_y + 0.6 * doc_height); + + // Now call scrollTo on the second target. Ensure it ends up very near the + // center of the window. + AccessibilityNotificationWaiter waiter2(shell()->web_contents(), + ui::kAXModeComplete, + ui::AX_EVENT_SCROLL_POSITION_CHANGED); + ASSERT_HRESULT_SUCCEEDED(target2->scrollTo(IA2_SCROLL_TYPE_ANYWHERE)); + waiter2.WaitForNotification(); + + // Same as above, make sure it's roughly centered. + ASSERT_HRESULT_SUCCEEDED( + target2->accLocation(&x, &y, &width, &height, childid_self)); + EXPECT_GT(y + height / 2, doc_y + 0.4 * doc_height); + EXPECT_LT(y + height / 2, doc_y + 0.6 * doc_height); +} + } // namespace content
diff --git a/content/browser/accessibility/browser_accessibility.cc b/content/browser/accessibility/browser_accessibility.cc index eb0ea60..2da5ca8 100644 --- a/content/browser/accessibility/browser_accessibility.cc +++ b/content/browser/accessibility/browser_accessibility.cc
@@ -985,13 +985,7 @@ } if (data.action == ui::AX_ACTION_SCROLL_TO_MAKE_VISIBLE) { - // target_rect is in screen coordinates. We need to convert this to frame - // coordinates because that's what BrowserAccessiblity cares about. - gfx::Rect target = - data.target_rect - - manager_->GetRootManager()->GetViewBounds().OffsetFromOrigin(); - - manager_->ScrollToMakeVisible(*this, target); + manager_->ScrollToMakeVisible(*this, data.target_rect); return true; }
diff --git a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc index 3dd4390..c6886b6 100644 --- a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc +++ b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
@@ -582,7 +582,7 @@ EXPECT_EQ(3u, result_ids_.size()); } -IN_PROC_BROWSER_TEST_F(SyntheticMouseEventTest, DISABLED_MouseEventAck) { +IN_PROC_BROWSER_TEST_F(SyntheticMouseEventTest, MouseEventAck) { NavigateToURLBlockUntilNavigationsComplete(shell(), GURL("about:blank"), 1); Attach(); ASSERT_TRUE(content::ExecuteScript(
diff --git a/content/browser/devtools/protocol/input_handler.cc b/content/browser/devtools/protocol/input_handler.cc index c8a746b7..af425ccb 100644 --- a/content/browser/devtools/protocol/input_handler.cc +++ b/content/browser/devtools/protocol/input_handler.cc
@@ -66,7 +66,7 @@ bool auto_repeat, bool is_keypad, int location) { - int result = 0; + int result = blink::WebInputEvent::kFromDebugger; if (auto_repeat) result |= blink::WebInputEvent::kIsAutoRepeat; if (is_keypad) @@ -293,6 +293,9 @@ void InputHandler::OnInputEventAck(InputEventAckSource source, InputEventAckState state, const blink::WebInputEvent& event) { + if ((event.GetModifiers() & blink::WebInputEvent::kFromDebugger) == 0) + return; + if (blink::WebInputEvent::IsKeyboardEventType(event.GetType()) && !pending_key_callbacks_.empty()) { pending_key_callbacks_.front()->sendSuccess();
diff --git a/content/browser/frame_host/render_frame_host_delegate.h b/content/browser/frame_host/render_frame_host_delegate.h index a654997..2244cd5 100644 --- a/content/browser/frame_host/render_frame_host_delegate.h +++ b/content/browser/frame_host/render_frame_host_delegate.h
@@ -18,8 +18,10 @@ #include "content/public/browser/site_instance.h" #include "content/public/common/javascript_dialog_type.h" #include "content/public/common/media_stream_request.h" +#include "content/public/common/resource_type.h" #include "device/geolocation/public/interfaces/geolocation_context.mojom.h" #include "mojo/public/cpp/bindings/scoped_interface_endpoint_handle.h" +#include "net/cert/cert_status_flags.h" #include "net/http/http_response_headers.h" #include "services/device/public/interfaces/wake_lock.mojom.h" #include "ui/base/window_open_disposition.h" @@ -337,6 +339,14 @@ // should not be asked to create a RenderFrame. virtual bool IsBeingDestroyed() const; + // Notifies that the render frame started loading a subresource. + virtual void SubresourceResponseStarted(const GURL& url, + const GURL& referrer, + const std::string& method, + ResourceType resource_type, + const std::string& ip, + net::CertStatus cert_status) {} + protected: virtual ~RenderFrameHostDelegate() {} };
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc index b7509cd..5202167e 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -2981,6 +2981,16 @@ frame_tree_node(), validated_params, std::move(begin_params)); } +void RenderFrameHostImpl::SubresourceResponseStarted(const GURL& url, + const GURL& referrer, + const std::string& method, + ResourceType resource_type, + const std::string& ip, + uint32_t cert_status) { + delegate_->SubresourceResponseStarted(url, referrer, method, resource_type, + ip, cert_status); +} + namespace { void GetRestrictedCookieManager(
diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h index 2f1613d..5c92e60b 100644 --- a/content/browser/frame_host/render_frame_host_impl.h +++ b/content/browser/frame_host/render_frame_host_impl.h
@@ -885,6 +885,12 @@ validated_params) override; void BeginNavigation(const CommonNavigationParams& common_params, mojom::BeginNavigationParamsPtr begin_params) override; + void SubresourceResponseStarted(const GURL& url, + const GURL& referrer, + const std::string& method, + ResourceType resource_type, + const std::string& ip, + uint32_t cert_status) override; // Registers Mojo interfaces that this frame host makes available. void RegisterMojoInterfaces();
diff --git a/content/browser/ssl/ssl_manager.cc b/content/browser/ssl/ssl_manager.cc index 1bc61f58..2cfa9ac 100644 --- a/content/browser/ssl/ssl_manager.cc +++ b/content/browser/ssl/ssl_manager.cc
@@ -305,28 +305,22 @@ } void SSLManager::DidStartResourceResponse(const GURL& url, - bool has_certificate, - net::CertStatus ssl_cert_status, - ResourceType resource_type) { - if (has_certificate && url.SchemeIsCryptographic() && - !net::IsCertStatusError(ssl_cert_status)) { - // If the scheme is https: or wss: *and* the security info for the - // cert has been set (i.e. the cert id is not 0) and the cert did - // not have any errors, revoke any previous decisions that - // have occurred. If the cert info has not been set, do nothing since it - // isn't known if the connection was actually a valid connection or if it - // had a cert error. - if (ssl_host_state_delegate_ && - ssl_host_state_delegate_->HasAllowException(url.host())) { - // If there's no certificate error, a good certificate has been seen, so - // clear out any exceptions that were made by the user for bad - // certificates. This intentionally does not apply to cached resources - // (see https://crbug.com/634553 for an explanation). - UMA_HISTOGRAM_BOOLEAN("interstitial.ssl.good_cert_seen_type_is_frame", - IsResourceTypeFrame(resource_type)); - ssl_host_state_delegate_->RevokeUserAllowExceptions(url.host()); - } + bool has_certificate_errors) { + if (!url.SchemeIsCryptographic() || has_certificate_errors) + return; + + // If the scheme is https: or wss and the cert did not have any errors, revoke + // any previous decisions that have occurred. + if (!ssl_host_state_delegate_ || + !ssl_host_state_delegate_->HasAllowException(url.host())) { + return; } + + // If there's no certificate error, a good certificate has been seen, so + // clear out any exceptions that were made by the user for bad + // certificates. This intentionally does not apply to cached resources + // (see https://crbug.com/634553 for an explanation). + ssl_host_state_delegate_->RevokeUserAllowExceptions(url.host()); } void SSLManager::OnCertErrorInternal(std::unique_ptr<SSLErrorHandler> handler,
diff --git a/content/browser/ssl/ssl_manager.h b/content/browser/ssl/ssl_manager.h index d4d846f7..f7d35e8 100644 --- a/content/browser/ssl/ssl_manager.h +++ b/content/browser/ssl/ssl_manager.h
@@ -74,10 +74,7 @@ NavigationControllerImpl* controller() { return controller_; } void DidCommitProvisionalLoad(const LoadCommittedDetails& details); - void DidStartResourceResponse(const GURL& url, - bool has_certificate, - net::CertStatus ssl_cert_status, - ResourceType resource_type); + void DidStartResourceResponse(const GURL& url, bool has_certificate_errors); // The following methods are called when a page includes insecure // content. These methods update the SSLStatus on the NavigationEntry
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index c1ae2a9..a62db74 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc
@@ -3423,11 +3423,8 @@ } void WebContentsImpl::DidGetResourceResponseStart( - const ResourceRequestDetails& details) { + const ResourceRequestDetails& details) { SetNotWaitingForResponse(); - controller_.ssl_manager()->DidStartResourceResponse( - details.url, details.has_certificate, details.ssl_cert_status, - details.resource_type); for (auto& observer : observers_) observer.DidGetResourceResponseStart(details); @@ -3698,6 +3695,10 @@ NavigationHandle* navigation_handle) { for (auto& observer : observers_) observer.ReadyToCommitNavigation(navigation_handle); + + controller_.ssl_manager()->DidStartResourceResponse( + navigation_handle->GetURL(), + net::IsCertStatusError(navigation_handle->GetSSLInfo().cert_status)); } void WebContentsImpl::DidFinishNavigation(NavigationHandle* navigation_handle) { @@ -4000,6 +4001,15 @@ // AddNewContents method call. } +void WebContentsImpl::SubresourceResponseStarted(const GURL& url, + const GURL& referrer, + const std::string& method, + ResourceType resource_type, + const std::string& ip, + net::CertStatus cert_status) { + controller_.ssl_manager()->DidStartResourceResponse(url, cert_status); +} + #if defined(OS_ANDROID) base::android::ScopedJavaLocalRef<jobject> WebContentsImpl::GetJavaRenderFrameHostDelegate() {
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h index a8ac12f..781f815 100644 --- a/content/browser/web_contents/web_contents_impl.h +++ b/content/browser/web_contents/web_contents_impl.h
@@ -566,6 +566,12 @@ base::android::ScopedJavaLocalRef<jobject> GetJavaRenderFrameHostDelegate() override; #endif + void SubresourceResponseStarted(const GURL& url, + const GURL& referrer, + const std::string& method, + ResourceType resource_type, + const std::string& ip, + net::CertStatus cert_status) override; // RenderViewHostDelegate ---------------------------------------------------- RenderViewHostDelegateView* GetDelegateView() override;
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc index baf27d6..e5d23f1 100644 --- a/content/browser/web_contents/web_contents_impl_unittest.cc +++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -3471,30 +3471,6 @@ EXPECT_EQ(SK_ColorGREEN, observer.last_theme_color()); } -// Test that if a resource is loaded with empty security info, the SSLManager -// does not mistakenly think it has seen a good certificate and thus forget any -// user exceptions for that host. See https://crbug.com/516808. -TEST_F(WebContentsImplTest, LoadResourceWithEmptySecurityInfo) { - WebContentsImplTestBrowserClient browser_client; - SetBrowserClientForTesting(&browser_client); - - scoped_refptr<net::X509Certificate> cert = - net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem"); - const GURL test_url("https://example.test"); - - SSLHostStateDelegate* state_delegate = - contents()->controller_.GetBrowserContext()->GetSSLHostStateDelegate(); - ASSERT_TRUE(state_delegate); - state_delegate->AllowCert(test_url.host(), *cert.get(), 1); - EXPECT_TRUE(state_delegate->HasAllowException(test_url.host())); - contents()->controller_.ssl_manager()->DidStartResourceResponse( - test_url, false, 0, RESOURCE_TYPE_MAIN_FRAME); - - EXPECT_TRUE(state_delegate->HasAllowException(test_url.host())); - - DeleteContents(); -} - TEST_F(WebContentsImplTest, ParseDownloadHeaders) { DownloadUrlParameters::RequestHeadersType request_headers = WebContentsImpl::ParseDownloadHeaders("A: 1\r\nB: 2\r\nC: 3\r\n\r\n");
diff --git a/content/common/frame.mojom b/content/common/frame.mojom index 8ae578c5..46064ca 100644 --- a/content/common/frame.mojom +++ b/content/common/frame.mojom
@@ -6,6 +6,7 @@ import "content/common/navigation_params.mojom"; import "content/common/url_loader_factory_bundle.mojom"; +import "content/public/common/resource_type.mojom"; import "content/public/common/url_loader.mojom"; import "content/public/common/window_container_type.mojom"; import "mojo/common/unguessable_token.mojom"; @@ -176,5 +177,14 @@ BeginNavigation( CommonNavigationParams common_params, BeginNavigationParams begin_params); + + // Sent when a subresource response has started. + SubresourceResponseStarted( + url.mojom.Url url, + url.mojom.Url referrer, + string method, + ResourceType resource_type, + string ip, + uint32 cert_status); };
diff --git a/content/renderer/loader/resource_dispatcher.cc b/content/renderer/loader/resource_dispatcher.cc index 0fc8b684..225c22d3 100644 --- a/content/renderer/loader/resource_dispatcher.cc +++ b/content/renderer/loader/resource_dispatcher.cc
@@ -40,6 +40,7 @@ #include "content/renderer/loader/sync_load_context.h" #include "content/renderer/loader/sync_load_response.h" #include "content/renderer/loader/url_loader_client_impl.h" +#include "content/renderer/render_frame_impl.h" #include "net/base/net_errors.h" #include "net/base/request_priority.h" #include "net/http/http_response_headers.h" @@ -79,6 +80,35 @@ } } +void NotifySubresourceStarted( + scoped_refptr<base::SingleThreadTaskRunner> thread_task_runner, + int render_frame_id, + const GURL& url, + const GURL& referrer, + const std::string& method, + ResourceType resource_type, + const std::string& ip, + uint32_t cert_status) { + if (!thread_task_runner) + return; + + if (!thread_task_runner->BelongsToCurrentThread()) { + thread_task_runner->PostTask( + FROM_HERE, base::BindOnce(NotifySubresourceStarted, thread_task_runner, + render_frame_id, url, referrer, method, + resource_type, ip, cert_status)); + return; + } + + RenderFrameImpl* render_frame = + RenderFrameImpl::FromRoutingID(render_frame_id); + if (!render_frame) + return; + + render_frame->GetFrameHost()->SubresourceResponseStarted( + url, referrer, method, resource_type, ip, cert_status); +} + } // namespace // static @@ -183,6 +213,14 @@ request_info->peer = std::move(new_peer); } + if (!IsResourceTypeFrame(request_info->resource_type)) { + NotifySubresourceStarted( + RenderThreadImpl::GetMainTaskRunner(), request_info->render_frame_id, + request_info->response_url, request_info->response_referrer, + request_info->response_method, request_info->resource_type, + response_head.socket_address.host(), response_head.cert_status); + } + ResourceResponseInfo renderer_response_info; ToResourceResponseInfo(*request_info, response_head, &renderer_response_info); request_info->site_isolation_metadata =
diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h index 01a93e2..7f45eace 100644 --- a/content/renderer/render_frame_impl.h +++ b/content/renderer/render_frame_impl.h
@@ -1525,7 +1525,6 @@ AssociatedInterfaceRegistryImpl associated_interfaces_; std::unique_ptr<AssociatedInterfaceProviderImpl> remote_associated_interfaces_; - mojom::FrameHostAssociatedPtr frame_host_; // TODO(dcheng): Remove these members. bool committed_first_load_ = false;
diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc index 5ee8a2b..907ee755 100644 --- a/content/renderer/render_thread_impl.cc +++ b/content/renderer/render_thread_impl.cc
@@ -280,6 +280,9 @@ base::LazyInstance<base::ThreadLocalPointer<RenderThreadImpl>>::DestructorAtExit lazy_tls = LAZY_INSTANCE_INITIALIZER; +base::LazyInstance<scoped_refptr<base::SingleThreadTaskRunner>>:: + DestructorAtExit g_main_task_runner = LAZY_INSTANCE_INITIALIZER; + // v8::MemoryPressureLevel should correspond to base::MemoryPressureListener. static_assert(static_cast<v8::MemoryPressureLevel>( base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE) == @@ -613,6 +616,12 @@ g_current_blink_platform_impl_for_testing = blink_platform_impl; } +// static +scoped_refptr<base::SingleThreadTaskRunner> +RenderThreadImpl::GetMainTaskRunner() { + return g_main_task_runner.Get(); +} + // In single-process mode used for debugging, we don't pass a renderer client // ID via command line because RenderThreadImpl lives in the same process as // the browser @@ -675,6 +684,7 @@ #endif lazy_tls.Pointer()->Set(this); + g_main_task_runner.Get() = base::MessageLoop::current()->task_runner(); // Register this object as the main thread. ChildProcess::current()->set_main_thread(this); @@ -1005,7 +1015,9 @@ } } -RenderThreadImpl::~RenderThreadImpl() = default; +RenderThreadImpl::~RenderThreadImpl() { + g_main_task_runner.Get() = nullptr; +} void RenderThreadImpl::Shutdown() { ChildThreadImpl::Shutdown();
diff --git a/content/renderer/render_thread_impl.h b/content/renderer/render_thread_impl.h index 5222956..3fbced7a 100644 --- a/content/renderer/render_thread_impl.h +++ b/content/renderer/render_thread_impl.h
@@ -197,6 +197,9 @@ static void SetRendererBlinkPlatformImplForTesting( RendererBlinkPlatformImpl* blink_platform_impl); + // Returns the task runner for the main thread where the RenderThread lives. + static scoped_refptr<base::SingleThreadTaskRunner> GetMainTaskRunner(); + ~RenderThreadImpl() override; void Shutdown() override; bool ShouldBeDestroyed() override;
diff --git a/content/test/test_render_frame.cc b/content/test/test_render_frame.cc index 04eab0a6..424e39c 100644 --- a/content/test/test_render_frame.cc +++ b/content/test/test_render_frame.cc
@@ -57,6 +57,13 @@ void BeginNavigation(const CommonNavigationParams& common_params, mojom::BeginNavigationParamsPtr begin_params) override {} + void SubresourceResponseStarted(const GURL& url, + const GURL& referrer, + const std::string& method, + ResourceType resource_type, + const std::string& ip, + uint32_t cert_status) override {} + private: std::unique_ptr<FrameHostMsg_DidCommitProvisionalLoad_Params> last_commit_params_;
diff --git a/docs/security/mojo.md b/docs/security/mojo.md index 7d46ed4..e4cde6e 100644 --- a/docs/security/mojo.md +++ b/docs/security/mojo.md
@@ -554,5 +554,32 @@ interface pointer is probably a good idea. +## Ensure An Explicit Grant For WebUI Bindings + +WebUI renderers sometimes need to call special, powerful IPC endpoints in a +privileged process. It is important to enforce the constraint that the +privileged callee previously created and blessed the calling process as a WebUI +process, and not as a (potentially compromised) web renderer or other +low-privilege process. + +* Use the standard pattern for instantiating `MojoWebUIController`. WebUI +Mojo interfaces must only be exposed through a `MojoWebUIController` subclass. +* If there is external functionality that the WebUI needs, make sure to route +it through the Mojo interfaces implemented by the `MojoWebUIController`, to +avoid circumventing access checks. + + +## Not-Yet-Shipped Features Should Be Feature-Checked On The Privileged Side + +Sometimes, there will be powerful new features that are not yet turned on by +default, such as behind a flag, Finch trial, or [origin +trial](https://www.chromium.org/blink/origin-trials). It is not safe to check +for the feature's availability on the renderer side (or in another low-privilege +process type). Instead, ensure that the check is done in the process that has +power to actually enact the feature. Otherwise, a compromised renderer could opt +itself in to the feature! If the feature might not yet be fully developed and +safe, vulnerabilities could arise. + + [security-tips-for-ipc]: https://www.chromium.org/Home/chromium-security/education/security-tips-for-ipc [NfcTypeConverter.java]: https://chromium.googlesource.com/chromium/src/+/e97442ee6e8c4cf6bcf7f5623c6fb2cc8cce92ac/services/device/nfc/android/java/src/org/chromium/device/nfc/NfcTypeConverter.java
diff --git a/ios/chrome/browser/metrics/tab_usage_recorder_unittest.mm b/ios/chrome/browser/metrics/tab_usage_recorder_unittest.mm index b7c4aeb..bb836d3 100644 --- a/ios/chrome/browser/metrics/tab_usage_recorder_unittest.mm +++ b/ios/chrome/browser/metrics/tab_usage_recorder_unittest.mm
@@ -2,13 +2,16 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#import "ios/chrome/browser/metrics/tab_usage_recorder.h" + +#import <UIKit/UIKit.h> + #include <memory> #include "base/metrics/histogram_samples.h" #include "base/test/histogram_tester.h" #include "base/test/scoped_task_environment.h" #import "ios/chrome/browser/metrics/previous_session_info.h" -#include "ios/chrome/browser/metrics/tab_usage_recorder.h" #import "ios/chrome/browser/web_state_list/fake_web_state_list_delegate.h" #import "ios/chrome/browser/web_state_list/web_state_list.h" #import "ios/chrome/browser/web_state_list/web_state_opener.h" @@ -16,6 +19,7 @@ #import "ios/web/public/test/fakes/test_web_state.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" +#import "third_party/ocmock/OCMock/OCMock.h" #if !defined(__has_feature) || !__has_feature(objc_arc) #error "This file requires ARC support." @@ -34,15 +38,25 @@ const char kURL[] = "http://www.chromium.org"; const char kNativeURL[] = "chrome://version"; +// Option to InsertTestWebState() to create the WebState for a tab that is in +// memory or not. +enum WebStateInMemoryOption { NOT_IN_MEMORY = 0, IN_MEMORY }; + } // namespace class TabUsageRecorderTest : public PlatformTest { protected: TabUsageRecorderTest() : web_state_list_(&web_state_list_delegate_), - tab_usage_recorder_(&web_state_list_, nullptr) {} + tab_usage_recorder_(&web_state_list_, nullptr), + application_(OCMClassMock([UIApplication class])) { + OCMStub([application_ sharedApplication]).andReturn(application_); + } - web::WebState* InsertTestWebState(const char* url, bool in_memory) { + ~TabUsageRecorderTest() override { [application_ stopMocking]; } + + web::TestWebState* InsertTestWebState(const char* url, + WebStateInMemoryOption in_memory) { auto test_navigation_manager = std::make_unique<web::TestNavigationManager>(); test_navigation_manager->AddItem(GURL(url), ui::PAGE_TRANSITION_LINK); @@ -52,13 +66,14 @@ auto test_web_state = std::make_unique<web::TestWebState>(); test_web_state->SetNavigationManager(std::move(test_navigation_manager)); - test_web_state->SetIsEvicted(!in_memory); + test_web_state->SetIsEvicted(in_memory == NOT_IN_MEMORY); const int insertion_index = web_state_list_.InsertWebState( WebStateList::kInvalidIndex, std::move(test_web_state), WebStateList::INSERT_NO_FLAGS, WebStateOpener()); - return web_state_list_.GetWebStateAt(insertion_index); + return static_cast<web::TestWebState*>( + web_state_list_.GetWebStateAt(insertion_index)); } void AddTimeToDequeInTabUsageRecorder(base::TimeTicks time) { @@ -70,11 +85,12 @@ WebStateList web_state_list_; base::HistogramTester histogram_tester_; TabUsageRecorder tab_usage_recorder_; + id application_; }; TEST_F(TabUsageRecorderTest, SwitchBetweenInMemoryTabs) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, true); - web::WebState* mock_tab_b = InsertTestWebState(kURL, true); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, IN_MEMORY); tab_usage_recorder_.RecordTabSwitched(mock_tab_a, mock_tab_b); histogram_tester_.ExpectUniqueSample(kSelectedTabHistogramName, @@ -82,8 +98,8 @@ } TEST_F(TabUsageRecorderTest, SwitchToEvictedTab) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, true); - web::WebState* mock_tab_b = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY); tab_usage_recorder_.RecordTabSwitched(mock_tab_a, mock_tab_b); histogram_tester_.ExpectUniqueSample(kSelectedTabHistogramName, @@ -91,8 +107,8 @@ } TEST_F(TabUsageRecorderTest, SwitchFromEvictedTab) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, false); - web::WebState* mock_tab_b = InsertTestWebState(kURL, true); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, NOT_IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, IN_MEMORY); tab_usage_recorder_.RecordTabSwitched(mock_tab_a, mock_tab_b); histogram_tester_.ExpectUniqueSample(kSelectedTabHistogramName, @@ -100,8 +116,8 @@ } TEST_F(TabUsageRecorderTest, SwitchBetweenEvictedTabs) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, false); - web::WebState* mock_tab_b = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, NOT_IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY); tab_usage_recorder_.RecordTabSwitched(mock_tab_a, mock_tab_b); histogram_tester_.ExpectUniqueSample(kSelectedTabHistogramName, @@ -109,8 +125,8 @@ } TEST_F(TabUsageRecorderTest, CountPageLoadsBeforeEvictedTab) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, true); - web::WebState* mock_tab_b = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY); // Call reload an arbitrary number of times. const int kNumReloads = 4; @@ -123,8 +139,8 @@ } TEST_F(TabUsageRecorderTest, CountNativePageLoadsBeforeEvictedTab) { - web::WebState* mock_tab_a = InsertTestWebState(kNativeURL, true); - web::WebState* mock_tab_b = InsertTestWebState(kNativeURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kNativeURL, IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kNativeURL, NOT_IN_MEMORY); // Call reload an arbitrary number of times. const int kNumReloads = 4; @@ -136,9 +152,9 @@ } TEST_F(TabUsageRecorderTest, TestColdStartTabs) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, false); - web::WebState* mock_tab_b = InsertTestWebState(kURL, false); - web::WebState* mock_tab_c = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, NOT_IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY); + web::TestWebState* mock_tab_c = InsertTestWebState(kURL, NOT_IN_MEMORY); // Set A and B as cold-start evicted tabs. Leave C just evicted. std::vector<web::WebState*> cold_start_web_states = { mock_tab_a, mock_tab_b, @@ -158,9 +174,9 @@ } TEST_F(TabUsageRecorderTest, TestSwitchedModeTabs) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, false); - web::WebState* mock_tab_b = InsertTestWebState(kURL, false); - web::WebState* mock_tab_c = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, NOT_IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY); + web::TestWebState* mock_tab_c = InsertTestWebState(kURL, NOT_IN_MEMORY); tab_usage_recorder_.RecordPrimaryTabModelChange(false, nullptr); // Switch from A (incognito evicted) to B (incognito evicted). @@ -175,8 +191,8 @@ } TEST_F(TabUsageRecorderTest, TestEvictedTabReloadTime) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, true); - web::WebState* mock_tab_b = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY); tab_usage_recorder_.RecordTabSwitched(mock_tab_a, mock_tab_b); tab_usage_recorder_.RecordPageLoadStart(mock_tab_b); tab_usage_recorder_.RecordPageLoadDone(mock_tab_b, true); @@ -184,8 +200,8 @@ } TEST_F(TabUsageRecorderTest, TestEvictedTabReloadSuccess) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, true); - web::WebState* mock_tab_b = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY); tab_usage_recorder_.RecordTabSwitched(mock_tab_a, mock_tab_b); tab_usage_recorder_.RecordPageLoadStart(mock_tab_b); tab_usage_recorder_.RecordPageLoadDone(mock_tab_b, true); @@ -194,8 +210,8 @@ } TEST_F(TabUsageRecorderTest, TestEvictedTabReloadFailure) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, true); - web::WebState* mock_tab_b = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY); tab_usage_recorder_.RecordTabSwitched(mock_tab_a, mock_tab_b); tab_usage_recorder_.RecordPageLoadStart(mock_tab_b); tab_usage_recorder_.RecordPageLoadDone(mock_tab_b, false); @@ -204,8 +220,8 @@ } TEST_F(TabUsageRecorderTest, TestUserWaitedForEvictedTabLoad) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, true); - web::WebState* mock_tab_b = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY); tab_usage_recorder_.RecordTabSwitched(mock_tab_a, mock_tab_b); tab_usage_recorder_.RecordPageLoadStart(mock_tab_b); tab_usage_recorder_.RecordPageLoadDone(mock_tab_b, true); @@ -215,8 +231,8 @@ } TEST_F(TabUsageRecorderTest, TestUserDidNotWaitForEvictedTabLoad) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, true); - web::WebState* mock_tab_b = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY); tab_usage_recorder_.RecordTabSwitched(mock_tab_a, mock_tab_b); tab_usage_recorder_.RecordPageLoadStart(mock_tab_b); tab_usage_recorder_.RecordTabSwitched(mock_tab_b, mock_tab_a); @@ -225,8 +241,8 @@ } TEST_F(TabUsageRecorderTest, TestUserBackgroundedDuringEvictedTabLoad) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, true); - web::WebState* mock_tab_b = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY); tab_usage_recorder_.RecordTabSwitched(mock_tab_a, mock_tab_b); tab_usage_recorder_.RecordPageLoadStart(mock_tab_b); tab_usage_recorder_.AppDidEnterBackground(); @@ -235,8 +251,8 @@ } TEST_F(TabUsageRecorderTest, TestTimeBetweenRestores) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, false); - web::WebState* mock_tab_b = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, NOT_IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY); tab_usage_recorder_.RecordTabSwitched(mock_tab_a, mock_tab_b); // Should record the time since launch until this page load begins. tab_usage_recorder_.RecordPageLoadStart(mock_tab_b); @@ -247,8 +263,8 @@ } TEST_F(TabUsageRecorderTest, TestTimeAfterLastRestore) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, false); - web::WebState* mock_tab_b = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, NOT_IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY); // Should record time since launch until background. tab_usage_recorder_.AppDidEnterBackground(); tab_usage_recorder_.AppWillEnterForeground(); @@ -260,13 +276,14 @@ // Verifies that metrics are recorded correctly when a renderer terminates. TEST_F(TabUsageRecorderTest, RendererTerminated) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, NOT_IN_MEMORY); + OCMStub([application_ applicationState]).andReturn(UIApplicationStateActive); // Add some extra WebStates that are not considered evicted so that // TabUsageRecorder count kAliveTabsCountAtRendererTermination tabs // as alive when mock_tab_a is evicted. for (int ii = 0; ii < kAliveTabsCountAtRendererTermination; ++ii) { - ignore_result(InsertTestWebState(kURL, true)); + ignore_result(InsertTestWebState(kURL, IN_MEMORY)); } base::TimeTicks now = base::TimeTicks::Now(); @@ -282,7 +299,7 @@ now - base::TimeDelta::FromSeconds(kSecondsBeforeRendererTermination / 2); AddTimeToDequeInTabUsageRecorder(recent_time); - tab_usage_recorder_.RendererTerminated(mock_tab_a, false, true); + mock_tab_a->OnRenderProcessGone(); NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults]; BOOL saw_memory_warning = @@ -303,10 +320,11 @@ // Verifies that metrics are recorded correctly when a renderer terminated tab // is switched to and reloaded. TEST_F(TabUsageRecorderTest, SwitchToRendererTerminatedTab) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, true); - web::WebState* mock_tab_b = InsertTestWebState(kURL, false); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY); + OCMStub([application_ applicationState]).andReturn(UIApplicationStateActive); - tab_usage_recorder_.RendererTerminated(mock_tab_b, false, true); + mock_tab_b->OnRenderProcessGone(); tab_usage_recorder_.RecordTabSwitched(mock_tab_a, mock_tab_b); histogram_tester_.ExpectUniqueSample( @@ -317,15 +335,17 @@ // Verifies that Tab.StateAtRendererTermination metric is correctly reported // when the application is in the foreground. TEST_F(TabUsageRecorderTest, StateAtRendererTerminationForeground) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, true); - web::WebState* mock_tab_b = InsertTestWebState(kURL, true); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, IN_MEMORY); + OCMStub([application_ applicationState]).andReturn(UIApplicationStateActive); - tab_usage_recorder_.RendererTerminated(mock_tab_a, true, true); + mock_tab_a->WasShown(); + mock_tab_a->OnRenderProcessGone(); histogram_tester_.ExpectBucketCount( kRendererTerminationStateHistogram, TabUsageRecorder::FOREGROUND_TAB_FOREGROUND_APP, 1); - tab_usage_recorder_.RendererTerminated(mock_tab_b, false, true); + mock_tab_b->OnRenderProcessGone(); histogram_tester_.ExpectBucketCount( kRendererTerminationStateHistogram, TabUsageRecorder::BACKGROUND_TAB_FOREGROUND_APP, 1); @@ -334,15 +354,38 @@ // Verifies that Tab.StateAtRendererTermination metric is correctly reported // when the application is in the background. TEST_F(TabUsageRecorderTest, StateAtRendererTerminationBackground) { - web::WebState* mock_tab_a = InsertTestWebState(kURL, true); - web::WebState* mock_tab_b = InsertTestWebState(kURL, true); + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, IN_MEMORY); + OCMStub([application_ applicationState]) + .andReturn(UIApplicationStateBackground); - tab_usage_recorder_.RendererTerminated(mock_tab_a, true, false); + mock_tab_a->WasShown(); + mock_tab_a->OnRenderProcessGone(); histogram_tester_.ExpectBucketCount( kRendererTerminationStateHistogram, TabUsageRecorder::FOREGROUND_TAB_BACKGROUND_APP, 1); - tab_usage_recorder_.RendererTerminated(mock_tab_b, false, false); + mock_tab_b->OnRenderProcessGone(); + histogram_tester_.ExpectBucketCount( + kRendererTerminationStateHistogram, + TabUsageRecorder::BACKGROUND_TAB_BACKGROUND_APP, 1); +} + +// Verifies that Tab.StateAtRendererTermination metric is correctly reported +// when the application is in the inactive state. +TEST_F(TabUsageRecorderTest, StateAtRendererTerminationInactive) { + web::TestWebState* mock_tab_a = InsertTestWebState(kURL, IN_MEMORY); + web::TestWebState* mock_tab_b = InsertTestWebState(kURL, IN_MEMORY); + OCMStub([application_ applicationState]) + .andReturn(UIApplicationStateInactive); + + mock_tab_a->WasShown(); + mock_tab_a->OnRenderProcessGone(); + histogram_tester_.ExpectBucketCount( + kRendererTerminationStateHistogram, + TabUsageRecorder::FOREGROUND_TAB_BACKGROUND_APP, 1); + + mock_tab_b->OnRenderProcessGone(); histogram_tester_.ExpectBucketCount( kRendererTerminationStateHistogram, TabUsageRecorder::BACKGROUND_TAB_BACKGROUND_APP, 1);
diff --git a/media/test/pipeline_integration_fuzzertest.cc b/media/test/pipeline_integration_fuzzertest.cc index 4550616..d2f01da 100644 --- a/media/test/pipeline_integration_fuzzertest.cc +++ b/media/test/pipeline_integration_fuzzertest.cc
@@ -8,9 +8,12 @@ #include "base/at_exit.h" #include "base/bind.h" +#include "base/bind_helpers.h" #include "base/command_line.h" +#include "base/location.h" #include "base/logging.h" #include "base/metrics/statistics_recorder.h" +#include "base/threading/thread_task_runner_handle.h" #include "media/base/bind_to_current_loop.h" #include "media/base/eme_constants.h" #include "media/base/media.h" @@ -112,15 +115,26 @@ // we will start demuxing the data but media pipeline will wait for a CDM to // be available to start initialization, which will not happen in this case. // To prevent the test timeout, we'll just fail the test immediately here. + // Note: Since the callback is on the media task runner but the test is on + // the main task runner, this must be posted. // TODO(xhwang): Support encrypted media in this fuzzer test. - test->FailTest(media::PIPELINE_ERROR_INITIALIZATION_FAILED); + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::BindOnce(&PipelineIntegrationTestBase::FailTest, + base::Unretained(test), + media::PIPELINE_ERROR_INITIALIZATION_FAILED)); } void OnAudioPlayDelay(media::PipelineIntegrationTestBase* test, base::TimeDelta play_delay) { CHECK_GT(play_delay, base::TimeDelta()); - if (play_delay > kMaxPlayDelay) - test->FailTest(media::PIPELINE_ERROR_INITIALIZATION_FAILED); + if (play_delay > kMaxPlayDelay) { + // Note: Since the callback is on the media task runner but the test is on + // the main task runner, this must be posted. + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::BindOnce(&PipelineIntegrationTestBase::FailTest, + base::Unretained(test), + media::PIPELINE_ERROR_INITIALIZATION_FAILED)); + } } class ProgressivePipelineIntegrationFuzzerTest
diff --git a/net/http/http_stream_factory_impl_job_controller.cc b/net/http/http_stream_factory_impl_job_controller.cc index 262b34ed..e5631fe 100644 --- a/net/http/http_stream_factory_impl_job_controller.cc +++ b/net/http/http_stream_factory_impl_job_controller.cc
@@ -559,17 +559,19 @@ const base::TimeDelta& delay) { net_log_.AddEvent(NetLogEventType::HTTP_STREAM_JOB_DELAYED, NetLog::Int64Callback("delay", delay.InMilliseconds())); + resume_main_job_callback_.Reset( + base::BindOnce(&HttpStreamFactoryImpl::JobController::ResumeMainJob, + ptr_factory_.GetWeakPtr())); base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( - FROM_HERE, - base::Bind(&HttpStreamFactoryImpl::JobController::ResumeMainJob, - ptr_factory_.GetWeakPtr()), - delay); + FROM_HERE, resume_main_job_callback_.callback(), delay); } void HttpStreamFactoryImpl::JobController::ResumeMainJob() { - if (main_job_is_resumed_ || !main_job_) - return; + DCHECK(main_job_); + if (main_job_is_resumed_) { + return; + } main_job_is_resumed_ = true; main_job_->net_log().AddEvent( NetLogEventType::HTTP_STREAM_JOB_RESUMED, @@ -1316,10 +1318,15 @@ // Abandon all Jobs and start over. job_bound_ = false; bound_job_ = nullptr; - main_job_is_resumed_ = false; - main_job_is_blocked_ = false; alternative_job_.reset(); main_job_.reset(); + // Also resets states that related to the old main job. In particular, + // cancels |resume_main_job_callback_| so there won't be any delayed + // ResumeMainJob() left in the task queue. + resume_main_job_callback_.Cancel(); + main_job_is_resumed_ = false; + main_job_is_blocked_ = false; + next_state_ = STATE_RESOLVE_PROXY_COMPLETE; } else { // If ReconsiderProxyAfterError() failed synchronously, it means
diff --git a/net/http/http_stream_factory_impl_job_controller.h b/net/http/http_stream_factory_impl_job_controller.h index f64cfbcb..ddd4dc0 100644 --- a/net/http/http_stream_factory_impl_job_controller.h +++ b/net/http/http_stream_factory_impl_job_controller.h
@@ -7,6 +7,7 @@ #include <memory> +#include "base/cancelable_callback.h" #include "net/base/host_port_pair.h" #include "net/base/privacy_mode.h" #include "net/http/http_stream_factory_impl_job.h" @@ -356,6 +357,8 @@ // job must not create a connection until it is resumed. bool main_job_is_blocked_; + // Handle for cancelling any posted delayed ResumeMainJob() task. + base::CancelableOnceClosure resume_main_job_callback_; // True if the main job was blocked and has been resumed in ResumeMainJob(). bool main_job_is_resumed_;
diff --git a/net/http/http_stream_factory_impl_job_controller_unittest.cc b/net/http/http_stream_factory_impl_job_controller_unittest.cc index ac8124de..5553121 100644 --- a/net/http/http_stream_factory_impl_job_controller_unittest.cc +++ b/net/http/http_stream_factory_impl_job_controller_unittest.cc
@@ -14,6 +14,7 @@ #include "base/test/histogram_tester.h" #include "base/test/scoped_feature_list.h" #include "base/test/scoped_mock_time_message_loop_task_runner.h" +#include "base/test/scoped_task_environment.h" #include "base/threading/platform_thread.h" #include "net/base/test_proxy_delegate.h" #include "net/dns/mock_host_resolver.h" @@ -39,6 +40,7 @@ #include "net/quic/test_tools/mock_random.h" #include "net/socket/socket_test_util.h" #include "net/spdy/chromium/spdy_test_util_common.h" +#include "net/test/net_test_suite.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock_mutant.h" #include "testing/gtest/include/gtest/gtest.h" @@ -150,6 +152,10 @@ const HttpStreamFactoryImpl::Job* job) { return job->spdy_session_key_; } + + static void SetShouldReconsiderProxy(HttpStreamFactoryImpl::Job* job) { + job->should_reconsider_proxy_ = true; + } }; class JobControllerPeer { @@ -1427,6 +1433,82 @@ EXPECT_FALSE(job_controller_->alternative_job()); } +// Regression test for crbug.com/789560. +TEST_F(HttpStreamFactoryImplJobControllerTest, ResumeMainJobLaterCanceled) { + NetTestSuite::SetScopedTaskEnvironment( + base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME); + std::unique_ptr<ProxyService> proxy_service = ProxyService::CreateDirect(); + ProxyService* proxy_service_raw = proxy_service.get(); + session_deps_.proxy_service = std::move(proxy_service); + + // Using hanging resolver will cause the alternative job to hang indefinitely. + session_deps_.host_resolver = std::make_unique<HangingResolver>(); + + HttpRequestInfo request_info; + request_info.method = "GET"; + request_info.url = GURL("https://www.google.com"); + + Initialize(request_info); + + // Enable delayed TCP and set time delay for waiting job. + QuicStreamFactory* quic_stream_factory = session_->quic_stream_factory(); + quic_stream_factory->set_require_confirmation(false); + ServerNetworkStats stats1; + stats1.srtt = base::TimeDelta::FromMicroseconds(10); + session_->http_server_properties()->SetServerNetworkStats( + url::SchemeHostPort(GURL("https://www.google.com")), stats1); + + url::SchemeHostPort server(request_info.url); + AlternativeService alternative_service(kProtoQUIC, server.host(), 443); + SetAlternativeService(request_info, alternative_service); + + request_ = + job_controller_->Start(&request_delegate_, nullptr, net_log_.bound(), + HttpStreamRequest::HTTP_STREAM, DEFAULT_PRIORITY); + EXPECT_TRUE(job_controller_->main_job()); + EXPECT_TRUE(job_controller_->alternative_job()); + EXPECT_TRUE(job_controller_->main_job()->is_waiting()); + + base::RunLoop run_loop; + // The main job should be resumed without delay when alt job fails. + EXPECT_CALL(*job_factory_.main_job(), Resume()) + .Times(1) + .WillOnce(Invoke([&run_loop]() { run_loop.Quit(); })); + job_controller_->OnStreamFailed(job_factory_.alternative_job(), + ERR_QUIC_PROTOCOL_ERROR, SSLConfig()); + NetTestSuite::GetScopedTaskEnvironment()->FastForwardBy( + base::TimeDelta::FromMicroseconds(0)); + run_loop.Run(); + EXPECT_FALSE(job_controller_->alternative_job()); + + // Calling ForceReloadProxyConfig will cause the proxy configuration to + // change. It will still be the direct connection but the configuration + // version will be bumped. That is enough for the job controller to restart + // the jobs. + proxy_service_raw->ForceReloadProxyConfig(); + HttpStreamFactoryImplJobPeer::SetShouldReconsiderProxy( + job_factory_.main_job()); + // Now the alt service is marked as broken (e.g. through a different request), + // so only non-alt job is restarted. + session_->http_server_properties()->MarkAlternativeServiceBroken( + alternative_service); + + job_controller_->OnStreamFailed(job_factory_.main_job(), ERR_FAILED, + SSLConfig()); + // Jobs are restarted. + EXPECT_TRUE(job_controller_->main_job()); + EXPECT_FALSE(job_controller_->alternative_job()); + + // There shouldn't be any ResumeMainJobLater() delayed tasks. + // This EXPECT_CALL will fail before crbug.com/789560 fix. + EXPECT_CALL(*job_factory_.main_job(), Resume()).Times(0); + NetTestSuite::GetScopedTaskEnvironment()->FastForwardBy( + base::TimeDelta::FromMicroseconds(15)); + + EXPECT_TRUE(job_controller_->main_job()); + request_.reset(); +} + // Test that main job is blocked for kMaxDelayTimeForMainJob(3s) if // http_server_properties cached an inappropriate large srtt for the server, // which would potentially delay the main job for a extremely long time in
diff --git a/net/test/net_test_suite.cc b/net/test/net_test_suite.cc index 3d131dd..8b1a668 100644 --- a/net/test/net_test_suite.cc +++ b/net/test/net_test_suite.cc
@@ -5,7 +5,6 @@ #include "net/test/net_test_suite.h" #include "base/logging.h" -#include "base/test/scoped_task_environment.h" #include "net/base/network_change_notifier.h" #include "net/http/http_stream_factory.h" #include "net/spdy/chromium/spdy_session.h" @@ -52,6 +51,13 @@ return g_current_net_test_suite->scoped_task_environment_.get(); } +void NetTestSuite::SetScopedTaskEnvironment( + base::test::ScopedTaskEnvironment::MainThreadType type) { + g_current_net_test_suite->scoped_task_environment_ = nullptr; + g_current_net_test_suite->scoped_task_environment_ = + std::make_unique<base::test::ScopedTaskEnvironment>(type); +} + void NetTestSuite::InitializeTestThread() { network_change_notifier_.reset(net::NetworkChangeNotifier::CreateMock());
diff --git a/net/test/net_test_suite.h b/net/test/net_test_suite.h index a495609..9cb4e35 100644 --- a/net/test/net_test_suite.h +++ b/net/test/net_test_suite.h
@@ -8,6 +8,7 @@ #include <memory> #include "base/memory/ref_counted.h" +#include "base/test/scoped_task_environment.h" #include "base/test/test_suite.h" #include "build/build_config.h" #include "net/dns/mock_host_resolver.h" @@ -35,6 +36,12 @@ // NetTestSuite. static base::test::ScopedTaskEnvironment* GetScopedTaskEnvironment(); + // Sets the global ScopedTaskEnvironment with a new environment of main thread + // type, |type|. For example, one might want to use a MOCK_TIME + // ScopedTaskEnvironment. + static void SetScopedTaskEnvironment( + base::test::ScopedTaskEnvironment::MainThreadType type); + protected: // Called from within Initialize(), but separate so that derived classes // can initialize the NetTestSuite instance only and not
diff --git a/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter b/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter index f150633..bdaae62f0 100644 --- a/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter +++ b/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
@@ -198,7 +198,6 @@ -SSLUIMITMSoftwareEnabledTest.OverridableError_NoMITMSoftwareInterstitial -SSLUIMITMSoftwareEnabledTest.TwoCertErrors_NoMITMSoftwareInterstitial -SSLUIMITMSoftwareEnabledTest.WrongCertError_NoMITMSoftwareInterstitial --SSLUITest.BadCertFollowedByGoodCert -SSLUITest.TestBadHTTPSDownload -SSLUITest.TestHTTPSOCSPOk -SSLUITest.TestHTTPSOCSPRevoked
diff --git a/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG b/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG index 8de633a..c4754b3 100644 --- a/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG +++ b/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
@@ -6253,8 +6253,8 @@ crbug.com/591099 http/tests/devtools/elements/highlight/highlight-node-scaled.js [ Failure ] crbug.com/591099 http/tests/devtools/elements/highlight/highlight-node-scroll.js [ Failure ] crbug.com/591099 http/tests/devtools/elements/highlight/highlight-node.js [ Failure ] -crbug.com/591099 http/tests/devtools/elements/styles-2/add-import-rule.html [ Failure ] -crbug.com/591099 http/tests/devtools/elements/styles-3/style-rule-from-imported-stylesheet.html [ Failure ] +crbug.com/591099 http/tests/devtools/elements/styles-2/add-import-rule.js [ Failure ] +crbug.com/591099 http/tests/devtools/elements/styles-3/style-rule-from-imported-stylesheet.js [ Failure ] crbug.com/591099 http/tests/devtools/elements/styles-3/styles-add-new-rule-tab.js [ Failure Pass ] crbug.com/591099 http/tests/devtools/elements/styles-3/styles-add-new-rule.js [ Failure Pass ] crbug.com/591099 http/tests/devtools/elements/styles-3/styles-change-node-while-editing.js [ Failure ]
diff --git a/third_party/WebKit/LayoutTests/SlowTests b/third_party/WebKit/LayoutTests/SlowTests index 4a4dec19..97307ec 100644 --- a/third_party/WebKit/LayoutTests/SlowTests +++ b/third_party/WebKit/LayoutTests/SlowTests
@@ -234,7 +234,7 @@ crbug.com/459009 crypto/subtle/ [ Slow ] -crbug.com/528419 http/tests/devtools/elements/styles-2/pseudo-elements.html [ Slow ] +crbug.com/528419 http/tests/devtools/elements/styles-2/pseudo-elements.js [ Slow ] crbug.com/529345 [ Win10 ] paint/masks/fieldset-mask.html [ Slow ] crbug.com/552556 [ Win Linux ] virtual/threaded/fast/scroll-behavior/overflow-scroll-root-frame-animates.html [ Slow ]
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index cb04fa8..2882769 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -2384,7 +2384,6 @@ crbug.com/788337 external/wpt/css/css-multicol/multicol-block-no-clip-002.xht [ Failure ] crbug.com/788337 external/wpt/css/css-multicol/multicol-count-computed-003.xht [ Failure ] crbug.com/788337 external/wpt/css/css-multicol/multicol-count-computed-005.xht [ Failure ] -crbug.com/788337 external/wpt/css/css-multicol/multicol-fill-auto-block-children-002.xht [ Failure ] crbug.com/788337 external/wpt/css/css-multicol/multicol-fill-auto.xht [ Failure ] crbug.com/788337 external/wpt/css/css-multicol/multicol-height-block-child-001.xht [ Failure ] crbug.com/788337 external/wpt/css/css-multicol/multicol-inherit-004.xht [ Failure ]
diff --git a/third_party/WebKit/LayoutTests/external/wpt/bluetooth/idl-Bluetooth.html b/third_party/WebKit/LayoutTests/external/wpt/bluetooth/idl/idl-Bluetooth.html similarity index 100% rename from third_party/WebKit/LayoutTests/external/wpt/bluetooth/idl-Bluetooth.html rename to third_party/WebKit/LayoutTests/external/wpt/bluetooth/idl/idl-Bluetooth.html
diff --git a/third_party/WebKit/LayoutTests/bluetooth/idl/idl-BluetoothUUID.html b/third_party/WebKit/LayoutTests/external/wpt/bluetooth/idl/idl-BluetoothUUID.html similarity index 98% rename from third_party/WebKit/LayoutTests/bluetooth/idl/idl-BluetoothUUID.html rename to third_party/WebKit/LayoutTests/external/wpt/bluetooth/idl/idl-BluetoothUUID.html index 79e35e8..efebb15 100644 --- a/third_party/WebKit/LayoutTests/bluetooth/idl/idl-BluetoothUUID.html +++ b/third_party/WebKit/LayoutTests/external/wpt/bluetooth/idl/idl-BluetoothUUID.html
@@ -1,6 +1,6 @@ <!DOCTYPE html> -<script src="../../resources/testharness.js"></script> -<script src="../../resources/testharnessreport.js"></script> +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> <script> 'use strict'
diff --git a/third_party/WebKit/LayoutTests/bluetooth/idl/idl-NavigatorBluetooth.html b/third_party/WebKit/LayoutTests/external/wpt/bluetooth/idl/idl-NavigatorBluetooth.html similarity index 72% rename from third_party/WebKit/LayoutTests/bluetooth/idl/idl-NavigatorBluetooth.html rename to third_party/WebKit/LayoutTests/external/wpt/bluetooth/idl/idl-NavigatorBluetooth.html index 40bf9ea..9449cf1 100644 --- a/third_party/WebKit/LayoutTests/bluetooth/idl/idl-NavigatorBluetooth.html +++ b/third_party/WebKit/LayoutTests/external/wpt/bluetooth/idl/idl-NavigatorBluetooth.html
@@ -1,6 +1,6 @@ <!DOCTYPE html> -<script src="../../resources/testharness.js"></script> -<script src="../../resources/testharnessreport.js"></script> +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> <script> 'use strict';
diff --git a/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-fill-auto-block-children-002-ref.xht b/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-fill-auto-block-children-002-ref.xht index f57fa43..3cec0f5 100644 --- a/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-fill-auto-block-children-002-ref.xht +++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-fill-auto-block-children-002-ref.xht
@@ -6,13 +6,15 @@ <link rel="reviewer" title="Gérard Talbot" href="http://www.gtalbot.org/BrowserBugsSection/css21testsuite/" /> <!-- 2013-08-16 --> <meta name="flags" content="" /> <style type="text/css"><![CDATA[ - body {margin: 8px;} + body {margin:0; margin-top:8px;} div { + float: left; background-color: blue; height: 200px; - width: 680px; + width: 60%; + margin-left: 8px; } h1 @@ -20,51 +22,19 @@ color: white; font-size: 2em; line-height: 1.25; /* or 1.21875 to achieve a 39px tall line box */ - margin-top: 8px; /* The margin-top of body and h1 will collapse into an 8px gap */ - margin-bottom: 21px; - padding-top: 21px; + margin: 21px 0em; } span#pass { + float: left; + margin-left: 10px; color: blue; font-size: 1.5em; font-weight: bolder; - left: 698px; - - /* - - Expected result: - - 8px 688px - v v - ************************************************ - * * - * <h1>Test passes if the word "PASS!" is<br />* 1st line box - * on the right ↘</h1> * 2nd line box - * * - ************************************************ - * * - * <h2>nbsp;<h2> <h2>nbsp;<h2> <h2>nbsp;<h2> * <h2>PASS!</h2> - * * - ************************************************ - ^ ^ - 228px 458px - - */ line-height: 1; - position: absolute; - top: 130px; - - /* - 8px : margin-top of body element - 21px : margin-top of h1 element which must not collapse with body's margin-top - 80px : content height: 2 line boxes required to render the "Test passes if ..." sentence - 21px : margin-bottom of h1 element - ==================================== - 130px : top position of span#pass in document box - */ + margin-top: 122px; } ]]></style> </head> @@ -72,8 +42,8 @@ <div> <h1>Test passes if "PASS!" is<br />on the right ↘</h1> - <span id="pass">PASS!</span> </div> + <span id="pass">PASS!</span> </body> </html>
diff --git a/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-fill-auto-block-children-002.xht b/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-fill-auto-block-children-002.xht index 9097e3c..d79fa95 100644 --- a/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-fill-auto-block-children-002.xht +++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-fill-auto-block-children-002.xht
@@ -18,16 +18,11 @@ background-color: blue; height: 200px; margin: 8px; - width: 680px; + width: 60%; column-count: 3; column-fill: auto; column-gap: 10px; - - /* - So, each column box should be - [680px minus (2 mult 10px)] divided by 3 == 220px wide - */ } h1 @@ -75,25 +70,5 @@ <h2>PASS!</h2> - <!-- - - Expected result: - - 8px 688px - v v - ************************************************ - * * - * <h1>Test passes if the word "PASS!" is<br />* 1st line box - * on the right ↘</h1> * 2nd line box - * * - ************************************************ - * * - * <h2>nbsp;<h2> <h2>nbsp;<h2> <h2>nbsp;<h2> * <h2>PASS!</h2> - * * - ************************************************ - ^ ^ - 228px 458px - --> - </body> </html>
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/add-import-rule-expected.txt b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/add-import-rule-expected.txt index 27c79d9..2ad324c 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/add-import-rule-expected.txt +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/add-import-rule-expected.txt
@@ -1,11 +1,13 @@ - +Tests that adding an @import with data URI does not lead to stylesheet collection crbug.com/644719 + + == Matched rules before @import added == [expanded] element.style { () [expanded] -span { (add-import-rule.html:4 -> add-import-rule.html:4:14) +span { (<style>…</style>) color: red;
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/add-import-rule.html b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/add-import-rule.js similarity index 62% rename from third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/add-import-rule.html rename to third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/add-import-rule.js index 52ec8b5e..e482ca7 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/add-import-rule.html +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/add-import-rule.js
@@ -1,9 +1,17 @@ -<!DOCTYPE html> -<script src="../../../inspector/inspector-test.js"></script> -<script src="../../../inspector/elements-test.js"></script> -<style>span { color: red }</style> -<script> -function test() { +// 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. + +(async function() { + TestRunner.addResult(`Tests that adding an @import with data URI does not lead to stylesheet collection crbug.com/644719\n`); + await TestRunner.loadModule('elements_test_runner'); + await TestRunner.showPanel('elements'); + await TestRunner.loadHTML(` + <!DOCTYPE html> + <style>span { color: red }</style> + <span id="styled-span"></span> + `); + var nodeId; var sheetId; @@ -30,7 +38,4 @@ ElementsTestRunner.dumpSelectedElementStyles(true); TestRunner.completeTest(); } -} -</script> -<body onload="runTest()"> -<span id="styled-span"></span> +})();
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/force-pseudo-state-expected.txt b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/force-pseudo-state-expected.txt index a956dec..4b2c55d3 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/force-pseudo-state-expected.txt +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/force-pseudo-state-expected.txt
@@ -1,17 +1,16 @@ Tests that forced element state is reflected in the DOM tree and Styles pane. -Test text DIV with :hover and :active [expanded] element.style { () [expanded] -div:active, a:active { (force-pseudo-state.html:12 -> force-pseudo-state.html:12:23) +div:active, a:active { (<style>…</style>) font-weight: bold; [expanded] -div:hover, a:hover { (force-pseudo-state.html:4 -> force-pseudo-state.html:4:21) +div:hover, a:hover { (<style>…</style>) color: red; [expanded] @@ -25,8 +24,7 @@ - <html> [subtreeMarkerCount:1] + <head>…</head> - - <body id="mainBody" class="main1 main2 mainpage" onload="runTest()" style="font-weight: normal; width: 85%; background-image: url(bar.png)"> [subtreeMarkerCount:1] - <p>\nTests that forced element state is reflected in the DOM tree and Styles pane.\n</p> + - <body id="mainBody" class="main1 main2 mainpage" style="font-weight: normal; width: 85%; background-image: url(bar.png)"> [subtreeMarkerCount:1] <div id="div">Test text</div> [markers:[pseudo-state-marker=hover,active], subtreeMarkerCount:1] </body> </html> @@ -36,11 +34,11 @@ element.style { () [expanded] -div:active, a:active { (force-pseudo-state.html:12 -> force-pseudo-state.html:12:23) +div:active, a:active { (<style>…</style>) font-weight: bold; [expanded] -div:focus, a:focus { (force-pseudo-state.html:8 -> force-pseudo-state.html:8:21) +div:focus, a:focus { (<style>…</style>) border: 1px solid green; border-top-color: green; border-top-style: solid; @@ -78,8 +76,7 @@ - <html> [subtreeMarkerCount:1] + <head>…</head> - - <body id="mainBody" class="main1 main2 mainpage" onload="runTest()" style="font-weight: normal; width: 85%; background-image: url(bar.png)"> [subtreeMarkerCount:1] - <p>\nTests that forced element state is reflected in the DOM tree and Styles pane.\n</p> + - <body id="mainBody" class="main1 main2 mainpage" style="font-weight: normal; width: 85%; background-image: url(bar.png)"> [subtreeMarkerCount:1] <div id="div">Test text</div> [markers:[pseudo-state-marker=active,focus], subtreeMarkerCount:1] </body> </html> @@ -99,8 +96,7 @@ - <html> + <head>…</head> - - <body id="mainBody" class="main1 main2 mainpage" onload="runTest()" style="font-weight: normal; width: 85%; background-image: url(bar.png)"> - <p>\nTests that forced element state is reflected in the DOM tree and Styles pane.\n</p> + - <body id="mainBody" class="main1 main2 mainpage" style="font-weight: normal; width: 85%; background-image: url(bar.png)"> <div id="div">Test text</div> </body> </html>
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/force-pseudo-state.html b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/force-pseudo-state.js similarity index 62% rename from third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/force-pseudo-state.html rename to third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/force-pseudo-state.js index 3b04365ae..a44ecbe0 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/force-pseudo-state.html +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/force-pseudo-state.js
@@ -1,24 +1,33 @@ -<html> -<head> -<style> -div:hover, a:hover { - color: red; -} +// 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. -div:focus, a:focus { - border: 1px solid green; -} +(async function() { + TestRunner.addResult(`Tests that forced element state is reflected in the DOM tree and Styles pane.\n`); + await TestRunner.loadModule('elements_test_runner'); + await TestRunner.showPanel('elements'); + await TestRunner.loadHTML(` + <head> + <style> + div:hover, a:hover { + color: red; + } -div:active, a:active { - font-weight: bold; -} + div:focus, a:focus { + border: 1px solid green; + } -</style> -<script src="../../../inspector/inspector-test.js"></script> -<script src="../../../inspector/elements-test.js"></script> -<script> + div:active, a:active { + font-weight: bold; + } -function test() { + </style> + </head> + <body id="mainBody" class="main1 main2 mainpage" style="font-weight: normal; width: 85%; background-image: url(bar.png)"> + <div id="div">Test text</div> + </body> + `); + ElementsTestRunner.nodeWithId('div', foundDiv); var divNode; @@ -67,13 +76,4 @@ dumpData(); TestRunner.completeTest(); } -} -</script> -</head> -<body id="mainBody" class="main1 main2 mainpage" onload="runTest()" style="font-weight: normal; width: 85%; background-image: url(bar.png)"> -<p> -Tests that forced element state is reflected in the DOM tree and Styles pane. -</p> -<div id="div">Test text</div> -</body> -</html> +})();
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/pseudo-elements-expected.txt b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/pseudo-elements-expected.txt index 4fd3308..2cc90ff 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/pseudo-elements-expected.txt +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/pseudo-elements-expected.txt
@@ -25,24 +25,24 @@ ======== Pseudo ::before element ======== [expanded] -[$#inspected:before, $].some-other-selector { (pseudo-elements.html:4 -> pseudo-elements.html:4:42) +[$#inspected:before, $].some-other-selector { (<style>…</style>) content: "BEFORE"; ======== Pseudo ::after element ======== [expanded] -[$#inspected:after$] { (pseudo-elements.html:8 -> pseudo-elements.html:8:19) +[$#inspected:after$] { (<style>…</style>) content: "AFTER"; Running: dumpBeforeStyles [expanded] -[$#inspected:before, $].some-other-selector { (pseudo-elements.html:4 -> pseudo-elements.html:4:42) +[$#inspected:before, $].some-other-selector { (<style>…</style>) content: "BEFORE"; Running: dumpAfterStyles [expanded] -[$#inspected:after$] { (pseudo-elements.html:8 -> pseudo-elements.html:8:19) +[$#inspected:after$] { (<style>…</style>) content: "AFTER";
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/pseudo-elements.html b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/pseudo-elements.js similarity index 68% rename from third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/pseudo-elements.html rename to third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/pseudo-elements.js index a3a1300..0d328fb 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/pseudo-elements.html +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/pseudo-elements.js
@@ -1,58 +1,67 @@ -<html> -<head> -<style> -#inspected:before, .some-other-selector { - content: "BEFORE"; -} +// 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. -#inspected:after { - content: "AFTER"; -} -</style> -<style> -#empty::before { - content: "EmptyBefore"; -} +(async function() { + TestRunner.addResult(`Tests that pseudo elements and their styles are handled properly.\n`); + await TestRunner.loadModule('elements_test_runner'); + await TestRunner.showPanel('elements'); + await TestRunner.loadHTML(` + <style> + #inspected:before, .some-other-selector { + content: "BEFORE"; + } -#empty::after { - content: "EmptyAfter"; -} -</style> -<script src="../../../inspector/inspector-test.js"></script> -<script src="../../../inspector/elements-test.js"></script> -<script> + #inspected:after { + content: "AFTER"; + } + </style> + <style> + #empty::before { + content: "EmptyBefore"; + } -function removeLastRule() -{ - document.styleSheets[0].deleteRule(document.styleSheets[0].cssRules.length - 1); -} + #empty::after { + content: "EmptyAfter"; + } + </style> + <div id="container"> + <div id="inspected">Text</div> + <div id="empty"></div> + </div> + `); + await TestRunner.evaluateInPagePromise(` + function removeLastRule() + { + document.styleSheets[0].deleteRule(document.styleSheets[0].cssRules.length - 1); + } -function addAfterRule() -{ - document.styleSheets[0].addRule("#inspected:after", "content: \"AFTER\""); -} + function addAfterRule() + { + document.styleSheets[0].addRule("#inspected:after", "content: \\"AFTER\\""); + } -function addBeforeRule() -{ - document.styleSheets[0].addRule("#inspected:before", "content: \"BEFORE\""); -} + function addBeforeRule() + { + document.styleSheets[0].addRule("#inspected:before", "content: \\"BEFORE\\""); + } -function modifyTextContent() -{ - document.getElementById("inspected").textContent = "bar"; -} + function modifyTextContent() + { + document.getElementById("inspected").textContent = "bar"; + } -function clearTextContent() -{ - document.getElementById("inspected").textContent = ""; -} + function clearTextContent() + { + document.getElementById("inspected").textContent = ""; + } -function removeNode() -{ - document.getElementById("inspected").remove(); -} + function removeNode() + { + document.getElementById("inspected").remove(); + } + `); -function test() { var containerNode; var inspectedNode; @@ -157,20 +166,4 @@ callback(); } } -} - -</script> -</head> - -<body onload="runTest()"> -<p> -Tests that pseudo elements and their styles are handled properly. -</p> - -<div id="container"> - <div id="inspected">Text</div> - <div id="empty"></div> -</div> - -</body> -</html> +})();
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/style-rule-from-imported-stylesheet-expected.txt b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/style-rule-from-imported-stylesheet-expected.txt index 83e111e..84d991e 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/style-rule-from-imported-stylesheet-expected.txt +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/style-rule-from-imported-stylesheet-expected.txt
@@ -1,7 +1,6 @@ -CONSOLE WARNING: Styling master document from stylesheets defined in HTML Imports is deprecated, and is planned to be removed in M65, around March 2018. Please refer to https://goo.gl/EGXzpw for possible migration paths. Tests that rules from imported stylesheets are correctly shown and are editable in inspector. - Rules before toggling: +Rules before toggling: [expanded] element.style { ()
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/style-rule-from-imported-stylesheet.html b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/style-rule-from-imported-stylesheet.html deleted file mode 100644 index 945b02d..0000000 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/style-rule-from-imported-stylesheet.html +++ /dev/null
@@ -1,31 +0,0 @@ -<html> -<head> -<link rel="import" href="../styles/resources/imported-stylesheet.html"/> -<script src="../../../inspector/inspector-test.js"></script> -<script src="../../../inspector/elements-test.js"></script> -<script> - -function test() { - ElementsTestRunner.selectNodeAndWaitForStyles('square', step1); - - function step1() { - TestRunner.addResult('Rules before toggling:'); - ElementsTestRunner.dumpSelectedElementStyles(true, false, true); - ElementsTestRunner.waitForStyleApplied(step2); - ElementsTestRunner.toggleMatchedStyleProperty('background-color', false); - } - - function step2() { - TestRunner.addResult('Rules after toggling:'); - ElementsTestRunner.dumpSelectedElementStyles(true, false, true); - TestRunner.completeTest(); - } -} -</script> -</head> - -<body onload="runTest()"> -<p>Tests that rules from imported stylesheets are correctly shown and are editable in inspector.</p> -<div id="square" class="square"></div> -</body> -</html>
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/style-rule-from-imported-stylesheet.js b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/style-rule-from-imported-stylesheet.js new file mode 100644 index 0000000..a43f6da --- /dev/null +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/style-rule-from-imported-stylesheet.js
@@ -0,0 +1,29 @@ +// 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. + +(async function() { + TestRunner.addResult( + `Tests that rules from imported stylesheets are correctly shown and are editable in inspector.\n`); + await TestRunner.loadModule('elements_test_runner'); + await TestRunner.showPanel('elements'); + await TestRunner.loadHTML(` + <div id="square" class="square"></div> + `); + await TestRunner.addHTMLImport('../styles/resources/imported-stylesheet.html'); + + ElementsTestRunner.selectNodeAndWaitForStyles('square', step1); + + function step1() { + TestRunner.addResult('Rules before toggling:'); + ElementsTestRunner.dumpSelectedElementStyles(true, false, true); + ElementsTestRunner.waitForStyleApplied(step2); + ElementsTestRunner.toggleMatchedStyleProperty('background-color', false); + } + + function step2() { + TestRunner.addResult('Rules after toggling:'); + ElementsTestRunner.dumpSelectedElementStyles(true, false, true); + TestRunner.completeTest(); + } +})();
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/styles-disable-then-enable-overriden-ua.html b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/styles-disable-then-enable-overriden-ua.html deleted file mode 100644 index 783f9dbc..0000000 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/styles-disable-then-enable-overriden-ua.html +++ /dev/null
@@ -1,41 +0,0 @@ -<!DOCTYPE html> -<html> -<head> -<script src="../../../inspector/inspector-test.js"></script> -<script src="../../../inspector/elements-test.js"></script> - -<script> - -function test() { - ElementsTestRunner.selectNodeAndWaitForStyles('body-id', step1); - - function step1() { - TestRunner.addResult('Before disable'); - ElementsTestRunner.dumpSelectedElementStyles(true, false, true); - ElementsTestRunner.toggleStyleProperty('margin', false); - ElementsTestRunner.waitForStyles('body-id', step2); - } - - function step2() { - TestRunner.addResult('After disable'); - ElementsTestRunner.dumpSelectedElementStyles(true, false, true); - ElementsTestRunner.toggleStyleProperty('margin', true); - ElementsTestRunner.waitForStyles('body-id', step3); - } - - function step3() { - TestRunner.addResult('After enable'); - ElementsTestRunner.dumpSelectedElementStyles(true, false, true); - TestRunner.completeTest(); - } -} -</script> -</head> - -<body onload="runTest()" id="body-id" style="margin: 10px"> -<p> -Tests that disabling shorthand removes the "overriden" mark from the UA shorthand it overrides. -</p> - -</body> -</html>
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/styles-disable-then-enable-overriden-ua.js b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/styles-disable-then-enable-overriden-ua.js new file mode 100644 index 0000000..9178776 --- /dev/null +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-3/styles-disable-then-enable-overriden-ua.js
@@ -0,0 +1,35 @@ +// 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. + +(async function() { + TestRunner.addResult( + `Tests that disabling shorthand removes the "overriden" mark from the UA shorthand it overrides.\n`); + await TestRunner.loadModule('elements_test_runner'); + await TestRunner.showPanel('elements'); + await TestRunner.loadHTML(` + <body id="body-id" style="margin: 10px"> + `); + + ElementsTestRunner.selectNodeAndWaitForStyles('body-id', step1); + + function step1() { + TestRunner.addResult('Before disable'); + ElementsTestRunner.dumpSelectedElementStyles(true, false, true); + ElementsTestRunner.toggleStyleProperty('margin', false); + ElementsTestRunner.waitForStyles('body-id', step2); + } + + function step2() { + TestRunner.addResult('After disable'); + ElementsTestRunner.dumpSelectedElementStyles(true, false, true); + ElementsTestRunner.toggleStyleProperty('margin', true); + ElementsTestRunner.waitForStyles('body-id', step3); + } + + function step3() { + TestRunner.addResult('After enable'); + ElementsTestRunner.dumpSelectedElementStyles(true, false, true); + TestRunner.completeTest(); + } +})();
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/inline-style-sourcemap-expected.txt b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/inline-style-sourcemap-expected.txt index 024098c..4a4e87cd 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/inline-style-sourcemap-expected.txt +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/inline-style-sourcemap-expected.txt
@@ -1,7 +1,5 @@ Verify that inline style sourceMappingURL is resolved properly. -.red,body{color:red}body{background-color:red} -/*# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIm1peGluLmxlc3MiLCJ0ZXN0Lmxlc3MiXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IkFBQUEsS0NJQSxLREhFLE1BQUEsSUNHRixLQUVFLGlCQUFBIn0=*/ [expanded] element.style { ()
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/inline-style-sourcemap.html b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/inline-style-sourcemap.html deleted file mode 100644 index b8c51fca..0000000 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/inline-style-sourcemap.html +++ /dev/null
@@ -1,39 +0,0 @@ -<html> -<head> -<script src="../../../inspector/inspector-test.js"></script> -<script src="../../../inspector/elements-test.js"></script> -<script> - -function embedInlineStyleSheet() -{ - var style = document.createElement("style"); - style.type = "text/css"; - style.textContent = document.querySelector(".stylesheet-text").textContent; - document.head.appendChild(style); -} - -function test() { - SDK.targetManager.addModelListener(SDK.CSSModel, SDK.CSSModel.Events.StyleSheetAdded, function() {}); - TestRunner.evaluateInPage('embedInlineStyleSheet()', onEvaluated); - - function onEvaluated() { - ElementsTestRunner.selectNodeAndWaitForStyles('inspect', onSelected); - } - - function onSelected() { - ElementsTestRunner.dumpSelectedElementStyles(true, false, false); - TestRunner.completeTest(); - } -}; - -</script> - -</head> - -<body id="inspect" onload="runTest()"> -<p>Verify that inline style sourceMappingURL is resolved properly.</p> -<pre class="stylesheet-text">.red,body{color:red}body{background-color:red} -/*# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIm1peGluLmxlc3MiLCJ0ZXN0Lmxlc3MiXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IkFBQUEsS0NJQSxLREhFLE1BQUEsSUNHRixLQUVFLGlCQUFBIn0=*/</pre> - -</body> -</html>
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/inline-style-sourcemap.js b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/inline-style-sourcemap.js new file mode 100644 index 0000000..eb5b84a --- /dev/null +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/inline-style-sourcemap.js
@@ -0,0 +1,36 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +(async function() { + TestRunner.addResult(`Verify that inline style sourceMappingURL is resolved properly.\n`); + await TestRunner.loadModule('elements_test_runner'); + await TestRunner.showPanel('elements'); + await TestRunner.loadHTML(` + <body id="inspect"> + <pre class="stylesheet-text">.red,body{color:red}body{background-color:red} + /*# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIm1peGluLmxlc3MiLCJ0ZXN0Lmxlc3MiXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IkFBQUEsS0NJQSxLREhFLE1BQUEsSUNHRixLQUVFLGlCQUFBIn0=*/</pre> + </body> + `); + await TestRunner.evaluateInPagePromise(` + function embedInlineStyleSheet() + { + var style = document.createElement("style"); + style.type = "text/css"; + style.textContent = document.querySelector(".stylesheet-text").textContent; + document.head.appendChild(style); + } + `); + + SDK.targetManager.addModelListener(SDK.CSSModel, SDK.CSSModel.Events.StyleSheetAdded, function() {}); + TestRunner.evaluateInPage('embedInlineStyleSheet()', onEvaluated); + + function onEvaluated() { + ElementsTestRunner.selectNodeAndWaitForStyles('inspect', onSelected); + } + + function onSelected() { + ElementsTestRunner.dumpSelectedElementStyles(true, false, false); + TestRunner.completeTest(); + } +})();
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/resources/stylesheet-source-url-comment.html b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/resources/stylesheet-source-url-comment.html new file mode 100644 index 0000000..d7197c38 --- /dev/null +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/resources/stylesheet-source-url-comment.html
@@ -0,0 +1,36 @@ +<style> +body { + color: green; +} + +/*# sourceURL=inlineStyleSheet.css */ +</style> +<script> +function addInlineStyleSheet() +{ + var styleElement = document.createElement("style"); + styleElement.textContent = "body { color: black; }\n/*# sourceURL=css/addedInlineStylesheet.css */"; + document.head.appendChild(styleElement); +} + +function addInlineStyleSheetNonRelative() +{ + var styleElement = document.createElement("style"); + styleElement.textContent = "body { color: red; }\n/*# sourceURL=css/nonRelativeInlineStylesheet.css */"; + document.head.appendChild(styleElement); +} + +function addInlineStyleSheetMultiple() +{ + var styleElement = document.createElement("style"); + styleElement.textContent = "\n/*# sourceURL=1.css */\nbody { color: red; }\n/*# sourceURL=2.css*/\n/*# sourceURL=css/addedInlineStylesheetMultiple.css */"; + document.head.appendChild(styleElement); +} + +function addInlineStyleSheetDeprecated() +{ + var styleElement = document.createElement("style"); + styleElement.textContent = "body { color: black; }\n/*@ sourceURL=css/addedInlineStylesheetDeprecated.css */"; + document.head.appendChild(styleElement); +} +</script> \ No newline at end of file
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-new-API-expected.txt b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-new-API-expected.txt index bf918b2..56dca65 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-new-API-expected.txt +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-new-API-expected.txt
@@ -1,6 +1,5 @@ Tests that InspectorCSSAgent API methods work as expected. -H1 Running: test_styles
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-new-API.html b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-new-API.js similarity index 91% rename from third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-new-API.html rename to third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-new-API.js index 2a80162..63fd963 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-new-API.html +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-new-API.js
@@ -1,13 +1,39 @@ -<html> +// 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. + +(async function() { + TestRunner.addResult(`Tests that InspectorCSSAgent API methods work as expected.\n`); + await TestRunner.loadModule('elements_test_runner'); + await TestRunner.showPanel('elements'); + await TestRunner.loadHTML(` <head> - <link rel="stylesheet" href="resources/styles-new-API.css"> +<style> -<script src="../../../inspector/inspector-test.js"></script> -<script src="../../../inspector/elements-test.js"></script> -<script> +/* An inline stylesheet */ +body.mainpage { + text-decoration: none; /* at least one valid property is necessary for WebCore to match a rule */ + ;badproperty: 1badvalue1; +} -function test() { +body.mainpage { + prop1: val1; + prop2: val2; +} + +body:hover { + color: #CDE; +} +</style> +</head> +<body id="mainBody" class="main1 main2 mainpage" style="font-weight: normal; width: 85%; background-image: url(bar.png)"> + <table width="50%" id="thetable"> + </table> + <h1 id="toggle">H1</h1> +</body> + `); + var bodyId; TestRunner.runTestSuite([ function test_styles(next) { @@ -184,35 +210,4 @@ next(); }, ]); -} - -</script> - -<style> - -/* An inline stylesheet */ -body.mainpage { - text-decoration: none; /* at least one valid property is necessary for WebCore to match a rule */ - ;badproperty: 1badvalue1; -} - -body.mainpage { - prop1: val1; - prop2: val2; -} - -body:hover { - color: #CDE; -} -</style> -</head> - -<body id="mainBody" class="main1 main2 mainpage" onload="runTest()" style="font-weight: normal; width: 85%; background-image: url(bar.png)"> -<p> -Tests that InspectorCSSAgent API methods work as expected. -</p> -<table width="50%" id="thetable"> -</table> -<h1 id="toggle">H1</h1> -</body> -</html> +})();
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-rerequest-sourcemap-on-watchdog.html b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-rerequest-sourcemap-on-watchdog.html deleted file mode 100644 index 8c7d734..0000000 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-rerequest-sourcemap-on-watchdog.html +++ /dev/null
@@ -1,42 +0,0 @@ -<html> -<head> -<script type="text/javascript" src="../../../inspector/inspector-test.js"></script> -<script type="text/javascript" src="../../../inspector/debugger-test.js"></script> -<link rel="stylesheet"> -<script> - -function addStyleSheet() -{ - var link = document.querySelector("link"); - link.setAttribute("href", "./resources/styles-rerequest-sourcemap-on-watchdog.css"); -} - -function test() { - TestRunner.cssModel.sourceMapManager().addEventListener( - SDK.SourceMapManager.Events.SourceMapAttached, onInitialSourceMap); - - TestRunner.evaluateInPagePromise('addStyleSheet()'); - - function onInitialSourceMap() { - TestRunner.cssModel.removeEventListener(SDK.SourceMapManager.Events.SourceMapAttached, onInitialSourceMap); - SourcesTestRunner.waitForScriptSource('styles-rerequest-sourcemap-on-watchdog.css', onCSSFile); - } - - function onCSSFile(uiSourceCode) { - TestRunner.addSniffer(SDK.SourceMapManager.prototype, '_sourceMapLoadedForTest', onSourceMapRerequested); - uiSourceCode.addRevision( - 'div { color: blue; } /*# sourceMappingURL=styles-rerequest-sourcemap-on-watchdog.css.map */'); - } - - function onSourceMapRerequested() { - TestRunner.addResult('SourceMap successfully re-requested.'); - TestRunner.completeTest(); - } -} -</script> -</head> -<body onLoad="runTest();"> -<p>Verifies that the sourceMap is in fact re-requested from network as SASS watchdog updates the CSS file.</p> - -</body> -</html>
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-rerequest-sourcemap-on-watchdog.js b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-rerequest-sourcemap-on-watchdog.js new file mode 100644 index 0000000..85adbb3 --- /dev/null +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-rerequest-sourcemap-on-watchdog.js
@@ -0,0 +1,40 @@ +// 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. + +(async function() { + TestRunner.addResult( + `Verifies that the sourceMap is in fact re-requested from network as SASS watchdog updates the CSS file.\n`); + await TestRunner.loadModule('sources_test_runner'); + await TestRunner.showPanel('sources'); + await TestRunner.showPanel('elements'); + await TestRunner.loadHTML(`<link rel="stylesheet">`); + await TestRunner.evaluateInPagePromise(` + function addStyleSheet() + { + var link = document.querySelector("link"); + link.setAttribute("href", "./resources/styles-rerequest-sourcemap-on-watchdog.css"); + } + `); + + TestRunner.cssModel.sourceMapManager().addEventListener( + SDK.SourceMapManager.Events.SourceMapAttached, onInitialSourceMap); + + TestRunner.evaluateInPagePromise('addStyleSheet()'); + + function onInitialSourceMap() { + TestRunner.cssModel.removeEventListener(SDK.SourceMapManager.Events.SourceMapAttached, onInitialSourceMap); + SourcesTestRunner.waitForScriptSource('styles-rerequest-sourcemap-on-watchdog.css', onCSSFile); + } + + function onCSSFile(uiSourceCode) { + TestRunner.addSniffer(SDK.SourceMapManager.prototype, '_sourceMapLoadedForTest', onSourceMapRerequested); + uiSourceCode.addRevision( + 'div { color: blue; } /*# sourceMappingURL=styles-rerequest-sourcemap-on-watchdog.css.map */'); + } + + function onSourceMapRerequested() { + TestRunner.addResult('SourceMap successfully re-requested.'); + TestRunner.completeTest(); + } +})();
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-source-offsets.html b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-source-offsets.js similarity index 77% rename from third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-source-offsets.html rename to third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-source-offsets.js index 8e1ccaa..98363b01 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-source-offsets.html +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/styles-source-offsets.js
@@ -1,13 +1,28 @@ -<html> +// 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. + +(async function() { + TestRunner.addResult( + `Tests that proper data and start/end offset positions are reported for CSS style declarations and properties.\n`); + await TestRunner.loadModule('elements_test_runner'); + await TestRunner.showPanel('elements'); + await TestRunner.loadHTML(` <head> - <link rel="stylesheet" href="../styles/resources/styles-source-offsets.css"> +<style> -<script src="../../../inspector/inspector-test.js"></script> -<script src="../../../inspector/elements-test.js"></script> -<script> +body.mainpage { + text-decoration: none; /* at least one valid property is necessary for WebCore to match a rule */ + badproperty: 1badvalue1; +} -function test() { +</style> +</head> +<body id="mainBody" class="main1 main2 mainpage" style="font-weight: normal; width: 80%"> +</body> + `); + function dumpStyleData(ruleOrStyle) { var isRule = !!(ruleOrStyle.style); var style; @@ -53,24 +68,4 @@ dumpStyleData(response.inlineStyle); TestRunner.completeTest(); } -} - -</script> - -<style> - -body.mainpage { - text-decoration: none; /* at least one valid property is necessary for WebCore to match a rule */ - badproperty: 1badvalue1; -} - -</style> -</head> - -<body id="mainBody" class="main1 main2 mainpage" onload="runTest()" style="font-weight: normal; width: 80%"> -<p> -Tests that proper data and start/end offset positions are reported for CSS style declarations and properties. -</p> - -</body> -</html> +})();
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/stylesheet-source-url-comment.html b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/stylesheet-source-url-comment.js similarity index 64% rename from third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/stylesheet-source-url-comment.html rename to third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/stylesheet-source-url-comment.js index 91bb7dad..53c2393 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/stylesheet-source-url-comment.html +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/stylesheet-source-url-comment.js
@@ -1,44 +1,14 @@ -<html> -<head> -<script src="../../../inspector/inspector-test.js"></script> -<script src="../../../inspector/debugger-test.js"></script> -<style> -body { - color: green; -} +// 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. -/*# sourceURL=inlineStyleSheet.css */ -</style> -<script> -function addInlineStyleSheet() -{ - var styleElement = document.createElement("style"); - styleElement.textContent = "body { color: black; }\n/*# sourceURL=css/addedInlineStylesheet.css */"; - document.head.appendChild(styleElement); -} +(async function() { + TestRunner.addResult(`Tests that stylesheets with sourceURL comment are shown in the Sources panel.\n`); + await TestRunner.loadModule('sources_test_runner'); + await TestRunner.showPanel('sources'); + await TestRunner.showPanel('elements'); + await TestRunner.navigatePromise('resources/stylesheet-source-url-comment.html'); -function addInlineStyleSheetNonRelative() -{ - var styleElement = document.createElement("style"); - styleElement.textContent = "body { color: red; }\n/*# sourceURL=css/nonRelativeInlineStylesheet.css */"; - document.head.appendChild(styleElement); -} - -function addInlineStyleSheetMultiple() -{ - var styleElement = document.createElement("style"); - styleElement.textContent = "\n/*# sourceURL=1.css */\nbody { color: red; }\n/*# sourceURL=2.css*/\n/*# sourceURL=css/addedInlineStylesheetMultiple.css */"; - document.head.appendChild(styleElement); -} - -function addInlineStyleSheetDeprecated() -{ - var styleElement = document.createElement("style"); - styleElement.textContent = "body { color: black; }\n/*@ sourceURL=css/addedInlineStylesheetDeprecated.css */"; - document.head.appendChild(styleElement); -} - -function test() { function forEachHeaderMatchingURL(url, handler) { var headers = TestRunner.cssModel.styleSheetHeaders(); for (var i = 0; i < headers.length; ++i) { @@ -109,13 +79,4 @@ } } ]); -}; - -</script> - -</head> - -<body onload="runTest()"> -<p>Tests that stylesheets with sourceURL comment are shown in the Sources panel.</p> -</body> -</html> +})();
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/supports-rule-after-invalid-selector-rule-crash-expected.txt b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/supports-rule-after-invalid-selector-rule-crash-expected.txt index 730ebf6..d9e8ba62 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/supports-rule-after-invalid-selector-rule-crash-expected.txt +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/supports-rule-after-invalid-selector-rule-crash-expected.txt
@@ -1 +1,3 @@ -This test passes if it doesn't crash. +This test passes if it doesn't crash. crbug.com/789263 + +
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/supports-rule-after-invalid-selector-rule-crash.html b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/supports-rule-after-invalid-selector-rule-crash.html deleted file mode 100644 index dbafe15e..0000000 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/supports-rule-after-invalid-selector-rule-crash.html +++ /dev/null
@@ -1,15 +0,0 @@ -<!DOCTYPE html> -<!-- Test for crbug.com/789263 --> -<style> -** { } -@supports (display: flex) { } -</style> -<script src="../../../inspector/inspector-test.js"></script> -<script> -function test() { - TestRunner.completeTest(); -} -</script> -<body onload="runTest()"> - This test passes if it doesn't crash. -</body>
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/supports-rule-after-invalid-selector-rule-crash.js b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/supports-rule-after-invalid-selector-rule-crash.js new file mode 100644 index 0000000..11ec99c --- /dev/null +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-4/supports-rule-after-invalid-selector-rule-crash.js
@@ -0,0 +1,18 @@ +// 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. + +(async function() { + TestRunner.addResult(`This test passes if it doesn't crash. crbug.com/789263\n`); + await TestRunner.showPanel('elements'); + await TestRunner.loadHTML(` + <!DOCTYPE html> + <style> + ** { } + @supports (display: flex) { } + </style> + This test passes if it doesn't crash. + `); + + TestRunner.completeTest(); +})();
diff --git a/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp b/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp index 62dfce3..8a143df 100644 --- a/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp +++ b/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
@@ -710,10 +710,10 @@ static CSSValue* ValueForPosition(const LengthPoint& position, const ComputedStyle& style) { - DCHECK((position.X() == kAuto) == (position.Y() == kAuto)); - if (position.X() == kAuto) { + DCHECK_EQ(position.X().IsAuto(), position.Y().IsAuto()); + if (position.X().IsAuto()) return CSSIdentifierValue::Create(CSSValueAuto); - } + CSSValueList* list = CSSValueList::CreateSpaceSeparated(); list->Append(*ZoomAdjustedPixelValueForLength(position.X(), style)); list->Append(*ZoomAdjustedPixelValueForLength(position.Y(), style));
diff --git a/third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp b/third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp index 455757dd..7a41509 100644 --- a/third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp +++ b/third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp
@@ -1500,11 +1500,12 @@ return current; } -static LayoutUnit GetBPMWidth(LayoutUnit child_value, Length css_unit) { - if (css_unit.GetType() != kAuto) - return (css_unit.IsFixed() ? static_cast<LayoutUnit>(css_unit.Value()) - : child_value); - return LayoutUnit(); +static LayoutUnit GetBPMWidth(LayoutUnit child_value, const Length& css_unit) { + if (css_unit.IsFixed()) + return LayoutUnit(css_unit.Value()); + if (css_unit.IsAuto()) + return LayoutUnit(); + return child_value; } static LayoutUnit GetBorderPaddingMargin(const LayoutBoxModelObject& child,
diff --git a/third_party/WebKit/Source/core/layout/LayoutDeprecatedFlexibleBox.cpp b/third_party/WebKit/Source/core/layout/LayoutDeprecatedFlexibleBox.cpp index 7ef70bb..cae0ef44 100644 --- a/third_party/WebKit/Source/core/layout/LayoutDeprecatedFlexibleBox.cpp +++ b/third_party/WebKit/Source/core/layout/LayoutDeprecatedFlexibleBox.cpp
@@ -1356,17 +1356,18 @@ if (IsHorizontal()) { LayoutUnit min_width = child->MinPreferredLogicalWidth(); LayoutUnit width = ContentWidthForChild(child); - if (child->Style()->MinWidth().IsFixed()) - min_width = LayoutUnit(child->Style()->MinWidth().Value()); - else if (child->Style()->MinWidth().GetType() == kAuto) + const Length& min_width_length = child->Style()->MinWidth(); + if (min_width_length.IsFixed()) + min_width = LayoutUnit(min_width_length.Value()); + else if (min_width_length.IsAuto()) min_width = LayoutUnit(); LayoutUnit allowed_shrinkage = (min_width - width).ClampPositiveToZero(); return allowed_shrinkage; } - Length min_height = child->Style()->MinHeight(); - if (min_height.IsFixed() || min_height.IsAuto()) { - LayoutUnit min_height(child->Style()->MinHeight().Value()); + const Length& min_height_length = child->Style()->MinHeight(); + if (min_height_length.IsFixed() || min_height_length.IsAuto()) { + LayoutUnit min_height(min_height_length.Value()); LayoutUnit height = ContentHeightForChild(child); LayoutUnit allowed_shrinkage = (min_height - height).ClampPositiveToZero(); return allowed_shrinkage;
diff --git a/third_party/WebKit/Source/core/layout/TableLayoutAlgorithmFixed.cpp b/third_party/WebKit/Source/core/layout/TableLayoutAlgorithmFixed.cpp index 2d4391f..8c68cf9 100644 --- a/third_party/WebKit/Source/core/layout/TableLayoutAlgorithmFixed.cpp +++ b/third_party/WebKit/Source/core/layout/TableLayoutAlgorithmFixed.cpp
@@ -167,7 +167,7 @@ while (used_span < span && current_column < n_eff_cols) { float e_span = table_->SpanOfEffectiveColumn(current_column); // Only set if no col element has already set it. - if (width_[current_column].IsAuto() && logical_width.GetType() != kAuto) { + if (width_[current_column].IsAuto() && !logical_width.IsAuto()) { width_[current_column] = logical_width; width_[current_column] *= e_span / span; used_width += fixed_border_box_logical_width * e_span / span;
diff --git a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp index cbeb5f2..cb20338 100644 --- a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp +++ b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
@@ -53,7 +53,6 @@ #include "platform/loader/fetch/FetchUtils.h" #include "platform/loader/fetch/Resource.h" #include "platform/loader/fetch/ResourceFetcher.h" -#include "platform/loader/fetch/ResourceLoader.h" #include "platform/loader/fetch/ResourceLoaderOptions.h" #include "platform/loader/fetch/ResourceRequest.h" #include "platform/weborigin/SchemeRegistry.h" @@ -537,7 +536,7 @@ DocumentThreadableLoader::~DocumentThreadableLoader() { CHECK(!client_); - DCHECK(!GetResource()); + DCHECK(!resource_); } void DocumentThreadableLoader::OverrideTimeout( @@ -586,16 +585,14 @@ } void DocumentThreadableLoader::SetDefersLoading(bool value) { - if (GetResource() && GetResource()->Loader()) - GetResource()->Loader()->SetDefersLoading(value); + if (GetResource()) + GetResource()->SetDefersLoading(value); } void DocumentThreadableLoader::Clear() { client_ = nullptr; timeout_timer_.Stop(); request_started_seconds_ = 0.0; - if (GetResource()) - checker_.WillRemoveClient(); ClearResource(); } @@ -737,8 +734,6 @@ // FIXME: consider combining this with CORS redirect handling performed by // CrossOriginAccessControl::handleRedirect(). - if (GetResource()) - checker_.WillRemoveClient(); ClearResource(); // If @@ -1106,8 +1101,6 @@ } void DocumentThreadableLoader::LoadFallbackRequestForServiceWorker() { - if (GetResource()) - checker_.WillRemoveClient(); ClearResource(); ResourceRequest fallback_request(fallback_request_for_service_worker_); fallback_request_for_service_worker_ = ResourceRequest(); @@ -1123,8 +1116,6 @@ actual_request_ = ResourceRequest(); actual_options_ = ResourceLoaderOptions(); - if (GetResource()) - checker_.WillRemoveClient(); ClearResource(); PrepareCrossOriginRequest(actual_request); @@ -1209,8 +1200,6 @@ } else { SetResource(RawResource::Fetch(new_params, fetcher)); } - if (GetResource()) - checker_.WillAddClient(); if (!GetResource()) { probe::documentThreadableLoaderFailedToStartLoadingForClient( @@ -1376,6 +1365,7 @@ } void DocumentThreadableLoader::Trace(blink::Visitor* visitor) { + visitor->Trace(resource_); visitor->Trace(loading_context_); ThreadableLoader::Trace(visitor); RawResourceClient::Trace(visitor);
diff --git a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h index 73cadb9..89b55c0 100644 --- a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h +++ b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h
@@ -57,9 +57,8 @@ // TODO(horo): We are using this class not only in documents, but also in // workers. We should change the name to ThreadableLoaderImpl. -class CORE_EXPORT DocumentThreadableLoader final - : public ThreadableLoader, - private ResourceOwner<RawResource> { +class CORE_EXPORT DocumentThreadableLoader final : public ThreadableLoader, + private RawResourceClient { USING_GARBAGE_COLLECTED_MIXIN(DocumentThreadableLoader); public: @@ -204,6 +203,32 @@ void LoadRequest(ResourceRequest&, ResourceLoaderOptions); bool IsAllowedRedirect(network::mojom::FetchRequestMode, const KURL&) const; + // TODO(hiroshige): After crbug.com/633696 is fixed, + // - Remove RawResourceClientStateChecker logic, + // - Make DocumentThreadableLoader to be a ResourceOwner and remove this + // re-implementation of ResourceOwner, and + // - Consider re-applying RawResourceClientStateChecker in a more + // general fashion (crbug.com/640291). + RawResource* GetResource() const { return resource_.Get(); } + void ClearResource() { SetResource(nullptr); } + void SetResource(RawResource* new_resource) { + if (new_resource == resource_) + return; + + if (RawResource* old_resource = resource_.Release()) { + checker_.WillRemoveClient(); + old_resource->RemoveClient(this); + } + + if (new_resource) { + resource_ = new_resource; + checker_.WillAddClient(); + resource_->AddClient(this); + } + } + Member<RawResource> resource_; + // End of ResourceOwner re-implementation, see above. + SecurityOrigin* GetSecurityOrigin() const; // Returns null if the loader is not associated with Document.
diff --git a/third_party/WebKit/Source/core/style/ComputedStyle.cpp b/third_party/WebKit/Source/core/style/ComputedStyle.cpp index b9211f25..f3ea190 100644 --- a/third_party/WebKit/Source/core/style/ComputedStyle.cpp +++ b/third_party/WebKit/Source/core/style/ComputedStyle.cpp
@@ -1011,9 +1011,9 @@ float origin_shift_x = 0; float origin_shift_y = 0; - // If offset-Position and offset-anchor properties are not yet enabled, + // If the offset-position and offset-anchor properties are not yet enabled, // they will have the default value, auto. - if (position.X() != Length(kAuto) || anchor.X() != Length(kAuto)) { + if (!position.X().IsAuto() || !anchor.X().IsAuto()) { // Shift the origin from transform-origin to offset-anchor. origin_shift_x = FloatValueForLength(anchor.X(), bounding_box.Width()) - @@ -1027,7 +1027,7 @@ point.Y() - origin_y + origin_shift_y); transform.Rotate(angle + rotate.angle); - if (position.X() != Length(kAuto) || anchor.X() != Length(kAuto)) + if (!position.X().IsAuto() || !anchor.X().IsAuto()) // Shift the origin back to transform-origin. transform.Translate(-origin_shift_x, -origin_shift_y); }
diff --git a/third_party/WebKit/Source/core/style/ComputedStyle.h b/third_party/WebKit/Source/core/style/ComputedStyle.h index 8b90963..eea9544f 100644 --- a/third_party/WebKit/Source/core/style/ComputedStyle.h +++ b/third_party/WebKit/Source/core/style/ComputedStyle.h
@@ -1705,7 +1705,7 @@ // Motion utility functions. bool HasOffset() const { - return (OffsetPosition().X() != Length(kAuto)) || OffsetPath(); + return !OffsetPosition().X().IsAuto() || OffsetPath(); } // Direction utility functions.
diff --git a/third_party/WebKit/Source/devtools/front_end/test_runner/TestRunner.js b/third_party/WebKit/Source/devtools/front_end/test_runner/TestRunner.js index e54bae2..9b044d4 100644 --- a/third_party/WebKit/Source/devtools/front_end/test_runner/TestRunner.js +++ b/third_party/WebKit/Source/devtools/front_end/test_runner/TestRunner.js
@@ -560,6 +560,24 @@ })(); `); }; + +/** + * @param {string} path + * @return {!Promise<*>} + */ +TestRunner.addHTMLImport = function(path) { + return TestRunner.evaluateInPageAsync(` + (function(){ + var link = document.createElement('link'); + link.rel = 'import'; + link.href = '${path}'; + var promise = new Promise(r => link.onload = r); + document.body.append(link); + return promise; + })(); + `); +}; + /** * @param {string} path * @param {!Object|undefined} options
diff --git a/third_party/WebKit/Source/platform/graphics/GpuMemoryBufferImageCopy.cpp b/third_party/WebKit/Source/platform/graphics/GpuMemoryBufferImageCopy.cpp index 65ec0839..6861ad56 100644 --- a/third_party/WebKit/Source/platform/graphics/GpuMemoryBufferImageCopy.cpp +++ b/third_party/WebKit/Source/platform/graphics/GpuMemoryBufferImageCopy.cpp
@@ -33,7 +33,7 @@ return false; gpu_memory_buffer_ = gpu_memory_buffer_manager->CreateGpuMemoryBuffer( - gfx::Size(width, height), gfx::BufferFormat::RGBA_8888, + gfx::Size(width, height), gfx::BufferFormat::RGBX_8888, gfx::BufferUsage::SCANOUT, gpu::kNullSurfaceHandle); if (!gpu_memory_buffer_) return false; @@ -57,7 +57,7 @@ // Bind the write framebuffer to our memory buffer. GLuint image_id = gl_->CreateImageCHROMIUM( - gpu_memory_buffer_->AsClientBuffer(), width, height, GL_RGBA); + gpu_memory_buffer_->AsClientBuffer(), width, height, GL_RGB); if (!image_id) return nullptr; GLuint dest_texture_id; @@ -95,7 +95,7 @@ // Copy the read framebuffer to the draw framebuffer. gl_->BlitFramebufferCHROMIUM(0, 0, width, height, 0, 0, width, height, - GL_COLOR_BUFFER_BIT, GL_LINEAR); + GL_COLOR_BUFFER_BIT, GL_NEAREST); // Cleanup the read framebuffer, associated image and texture. gl_->BindFramebuffer(GL_FRAMEBUFFER, 0);
diff --git a/third_party/WebKit/Source/platform/graphics/gpu/SharedContextRateLimiter.cpp b/third_party/WebKit/Source/platform/graphics/gpu/SharedContextRateLimiter.cpp index 3f0820f..af49d21 100644 --- a/third_party/WebKit/Source/platform/graphics/gpu/SharedContextRateLimiter.cpp +++ b/third_party/WebKit/Source/platform/graphics/gpu/SharedContextRateLimiter.cpp
@@ -46,9 +46,8 @@ return; queries_.push_back(0); - if (can_use_sync_queries_) - gl->GenQueriesEXT(1, &queries_.back()); if (can_use_sync_queries_) { + gl->GenQueriesEXT(1, &queries_.back()); gl->BeginQueryEXT(GL_COMMANDS_COMPLETED_CHROMIUM, queries_.back()); gl->EndQueryEXT(GL_COMMANDS_COMPLETED_CHROMIUM); } @@ -70,7 +69,8 @@ return; gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL(); - if (gl && gl->GetGraphicsResetStatusKHR() == GL_NO_ERROR) { + if (can_use_sync_queries_ && gl && + gl->GetGraphicsResetStatusKHR() == GL_NO_ERROR) { while (queries_.size() > 0) { gl->DeleteQueriesEXT(1, &queries_.front()); queries_.pop_front();
diff --git a/third_party/WebKit/Source/platform/loader/fetch/RawResource.cpp b/third_party/WebKit/Source/platform/loader/fetch/RawResource.cpp index f06f0904..7b63439 100644 --- a/third_party/WebKit/Source/platform/loader/fetch/RawResource.cpp +++ b/third_party/WebKit/Source/platform/loader/fetch/RawResource.cpp
@@ -31,6 +31,7 @@ #include "platform/loader/fetch/MemoryCache.h" #include "platform/loader/fetch/ResourceClientWalker.h" #include "platform/loader/fetch/ResourceFetcher.h" +#include "platform/loader/fetch/ResourceLoader.h" #include "platform/network/http_names.h" #include "platform/scheduler/child/web_scheduler.h" #include "public/platform/Platform.h" @@ -295,6 +296,11 @@ Resource::NotifyFinished(); } +void RawResource::SetDefersLoading(bool defers) { + if (Loader()) + Loader()->SetDefersLoading(defers); +} + static bool ShouldIgnoreHeaderForCacheReuse(AtomicString header_name) { // FIXME: This list of headers that don't affect cache policy almost certainly // isn't complete.
diff --git a/third_party/WebKit/Source/platform/loader/fetch/RawResource.h b/third_party/WebKit/Source/platform/loader/fetch/RawResource.h index b81d484..8d59b043 100644 --- a/third_party/WebKit/Source/platform/loader/fetch/RawResource.h +++ b/third_party/WebKit/Source/platform/loader/fetch/RawResource.h
@@ -66,6 +66,12 @@ return CreateForTest(KURL(url), type); } + // FIXME: AssociatedURLLoader shouldn't be a DocumentThreadableLoader and + // therefore shouldn't use RawResource. However, it is, and it needs to be + // able to defer loading. This can be fixed by splitting CORS preflighting out + // of DocumentThreadableLoader. + void SetDefersLoading(bool); + // Resource implementation bool CanReuse(const FetchParameters&) const override; bool WillFollowRedirect(const ResourceRequest&,
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot.py b/third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot.py index 98afa57..3079a79 100644 --- a/third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot.py +++ b/third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot.py
@@ -140,7 +140,8 @@ build_number = host.environ.get('BUILDBOT_BUILDNUMBER') if not (master_name and builder_name and build_number): return None - return 'https://build.chromium.org/p/%s/builders/%s/builds/%s' % (master_name, builder_name, build_number) + return ('https://ci.chromium.org/buildbot/%s/%s/%s' % + (master_name, builder_name, build_number)) def filter_latest_builds(builds):
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer_unittest.py b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer_unittest.py index 723f19b..3c5a478 100644 --- a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer_unittest.py +++ b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer_unittest.py
@@ -346,7 +346,7 @@ importer.host.environ['BUILDBOT_BUILDNUMBER'] = '123' description = importer._cl_description(directory_owners={}) self.assertIn( - 'Build: https://build.chromium.org/p/my.master/builders/b/builds/123\n\n', + 'Build: https://ci.chromium.org/buildbot/my.master/b/123\n\n', description) self.assertEqual(host.executive.calls, [['git', 'log', '-1', '--format=%B']])
diff --git a/third_party/WebKit/public/platform/WebInputEvent.h b/third_party/WebKit/public/platform/WebInputEvent.h index 0335e45..986b7042 100644 --- a/third_party/WebKit/public/platform/WebInputEvent.h +++ b/third_party/WebKit/public/platform/WebInputEvent.h
@@ -246,6 +246,11 @@ // not actual physical movement of the pointer kRelativeMotionEvent = 1 << 22, + // Indication this event was injected by the devtools. + // TODO(dtapuska): Remove this flag once we are able to bind callbacks + // in event sending. + kFromDebugger = 1 << 23, + // The set of non-stateful modifiers that specifically change the // interpretation of the key being pressed. For example; IsLeft, // IsRight, IsComposing don't change the meaning of the key
diff --git a/third_party/leveldatabase/env_chromium.cc b/third_party/leveldatabase/env_chromium.cc index 5f549e7177..762dffc 100644 --- a/third_party/leveldatabase/env_chromium.cc +++ b/third_party/leveldatabase/env_chromium.cc
@@ -101,14 +101,6 @@ static const FilePath::CharType kLevelDBTestDirectoryPrefix[] = FILE_PATH_LITERAL("leveldb-test-"); -static base::File::Error LastFileError() { -#if defined(OS_WIN) - return base::File::OSErrorToFileError(GetLastError()); -#else - return base::File::OSErrorToFileError(errno); -#endif -} - // Making direct platform in lieu of using base::FileEnumerator because the // latter can fail quietly without return an error result. static base::File::Error GetDirectoryEntries(const FilePath& dir_param, @@ -249,7 +241,7 @@ TRACE_EVENT1("leveldb", "ChromiumSequentialFile::Read", "size", n); int bytes_read = file_.ReadAtCurrentPosNoBestEffort(scratch, n); if (bytes_read == -1) { - base::File::Error error = LastFileError(); + base::File::Error error = base::File::GetLastFileError(); uma_logger_->RecordErrorAt(kSequentialFileRead); return MakeIOError(filename_, base::File::ErrorToString(error), kSequentialFileRead, error); @@ -262,7 +254,7 @@ Status Skip(uint64_t n) override { if (file_.Seek(base::File::FROM_CURRENT, n) == -1) { - base::File::Error error = LastFileError(); + base::File::Error error = base::File::GetLastFileError(); uma_logger_->RecordErrorAt(kSequentialFileSkip); return MakeIOError(filename_, base::File::ErrorToString(error), kSequentialFileSkip, error); @@ -389,7 +381,7 @@ f.error_details()); } if (!f.Flush()) { - base::File::Error error = LastFileError(); + base::File::Error error = base::File::GetLastFileError(); uma_logger_->RecordOSError(kSyncParent, error); return MakeIOError(parent_dir_, base::File::ErrorToString(error), kSyncParent, error); @@ -403,7 +395,7 @@ DCHECK(uma_logger_); int bytes_written = file_.WriteAtCurrentPos(data.data(), data.size()); if (static_cast<size_t>(bytes_written) != data.size()) { - base::File::Error error = LastFileError(); + base::File::Error error = base::File::GetLastFileError(); uma_logger_->RecordOSError(kWritableFileAppend, error); return MakeIOError(filename_, base::File::ErrorToString(error), kWritableFileAppend, error); @@ -428,7 +420,7 @@ TRACE_EVENT0("leveldb", "WritableFile::Sync"); if (!file_.Flush()) { - base::File::Error error = LastFileError(); + base::File::Error error = base::File::GetLastFileError(); uma_logger_->RecordErrorAt(kWritableFileSync); return MakeIOError(filename_, base::File::ErrorToString(error), kWritableFileSync, error);
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index df47145..49581f64 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -30509,6 +30509,9 @@ </histogram> <histogram name="interstitial.ssl.good_cert_seen_type_is_frame" enum="Boolean"> + <obsolete> + Deprecated December 2017 (M65). + </obsolete> <owner>jam@chromium.org</owner> <summary> Whether the resource type for a request that succeeded with a good cert and
diff --git a/ui/accessibility/ax_enums.idl b/ui/accessibility/ax_enums.idl index 332f1800..5036668 100644 --- a/ui/accessibility/ax_enums.idl +++ b/ui/accessibility/ax_enums.idl
@@ -275,7 +275,7 @@ // Scroll any scrollable containers to make the target object visible // on the screen. Optionally pass a subfocus rect in - // AXActionData.target_rect. + // AXActionData.target_rect, in node-local coordinates. scroll_to_make_visible, // Scroll the given object to a specified point on the screen in
diff --git a/ui/accessibility/platform/ax_platform_node_win.cc b/ui/accessibility/platform/ax_platform_node_win.cc index 6865f75..1c5003e 100644 --- a/ui/accessibility/platform/ax_platform_node_win.cc +++ b/ui/accessibility/platform/ax_platform_node_win.cc
@@ -1366,7 +1366,9 @@ COM_OBJECT_VALIDATE(); WIN_ACCESSIBILITY_API_HISTOGRAM(UMA_API_IA2_SCROLL_TO); - gfx::Rect r = delegate_->GetScreenBoundsRect(); + // AX_ACTION_SCROLL_TO_MAKE_VISIBLE wants a target rect in *local* coords. + gfx::Rect r = gfx::ToEnclosingRect(GetData().location); + r.Offset(-r.OffsetFromOrigin()); switch (scroll_type) { case IA2_SCROLL_TYPE_TOP_LEFT: r = gfx::Rect(r.x(), r.y(), 0, 0);
diff --git a/ui/accessibility/platform/ax_platform_node_win_unittest.cc b/ui/accessibility/platform/ax_platform_node_win_unittest.cc index 32b67e0..eef013f 100644 --- a/ui/accessibility/platform/ax_platform_node_win_unittest.cc +++ b/ui/accessibility/platform/ax_platform_node_win_unittest.cc
@@ -1027,7 +1027,7 @@ EXPECT_EQ(S_OK, ax_child1->accLocation(&x_left, &y_top, &width, &height, SELF)); EXPECT_EQ(610, x_left); - EXPECT_EQ(610, y_top); + EXPECT_EQ(660, y_top); EXPECT_EQ(10, width); EXPECT_EQ(10, height); @@ -1077,8 +1077,8 @@ EXPECT_EQ(S_OK, ax_child1->accLocation(&x_left, &y_top, &width, &height, SELF)); - EXPECT_EQ(20, x_left); - EXPECT_EQ(20, y_top); + EXPECT_EQ(0, x_left); + EXPECT_EQ(0, y_top); EXPECT_EQ(10, width); EXPECT_EQ(10, height); }
diff --git a/ui/accessibility/platform/test_ax_node_wrapper.cc b/ui/accessibility/platform/test_ax_node_wrapper.cc index a78075f..e71022a 100644 --- a/ui/accessibility/platform/test_ax_node_wrapper.cc +++ b/ui/accessibility/platform/test_ax_node_wrapper.cc
@@ -201,12 +201,13 @@ bool TestAXNodeWrapper::AccessibilityPerformAction( const ui::AXActionData& data) { if (data.action == ui::AX_ACTION_SCROLL_TO_POINT) { - g_offset = gfx::Vector2d(data.target_point.x(), data.target_point.x()); + g_offset = gfx::Vector2d(data.target_point.x(), data.target_point.y()); return true; } if (data.action == ui::AX_ACTION_SCROLL_TO_MAKE_VISIBLE) { - g_offset = gfx::Vector2d(data.target_rect.x(), data.target_rect.x()); + auto offset = node_->data().location.OffsetFromOrigin(); + g_offset = gfx::Vector2d(-offset.x(), -offset.y()); return true; }
diff --git a/ui/compositor/compositor.cc b/ui/compositor/compositor.cc index 71eb2bf..beab73d 100644 --- a/ui/compositor/compositor.cc +++ b/ui/compositor/compositor.cc
@@ -365,7 +365,11 @@ void Compositor::SetDisplayColorSpace(const gfx::ColorSpace& color_space) { output_color_space_ = color_space; blending_color_space_ = output_color_space_.GetBlendingColorSpace(); - host_->SetRasterColorSpace(output_color_space_.GetRasterColorSpace()); + // Do all ui::Compositor rasterization to sRGB because UI resources will not + // have their color conversion results cached, and will suffer repeated + // image color conversions. + // https://crbug.com/769677 + host_->SetRasterColorSpace(gfx::ColorSpace::CreateSRGB()); // Color space is reset when the output surface is lost, so this must also be // updated then. // TODO(fsamuel): Get rid of this.