diff --git a/DEPS b/DEPS index eb94556e..bb2b558 100644 --- a/DEPS +++ b/DEPS
@@ -63,7 +63,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other. - 'v8_revision': 'f39879061b8e7f3ee08b98b53dc6c986536e16c3', + 'v8_revision': '51e240033771b0e8f504d3eb401c781d5908fe39', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling swarming_client # and whatever else without interference from each other. @@ -71,7 +71,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': 'c1f14fbea92bcf267d39cbce937b993913dc6578', + 'angle_revision': 'e88ec8eed5a448b9fc470a6684c93f24404c7e89', # 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. @@ -1436,7 +1436,7 @@ 'action': [ 'python', 'src/build/fuchsia/update_sdk.py', - 'f6dffb2fee82a21900fc3a00261dc5844901ea9e', + 'ff6b8e980b4e5b0c898341e8a467b9c751857e5d', ], },
diff --git a/base/test/launcher/test_launcher.cc b/base/test/launcher/test_launcher.cc index 941da33c..a77302aa 100644 --- a/base/test/launcher/test_launcher.cc +++ b/base/test/launcher/test_launcher.cc
@@ -44,7 +44,6 @@ #include "base/test/test_timeouts.h" #include "base/threading/sequenced_worker_pool.h" #include "base/threading/thread.h" -#include "base/threading/thread_checker.h" #include "base/threading/thread_task_runner_handle.h" #include "base/time/time.h" #include "build/build_config.h" @@ -251,13 +250,12 @@ // Launches a child process using |command_line|. If the child process is still // running after |timeout|, it is terminated and |*was_timeout| is set to true. // Returns exit code of the process. -int LaunchChildTestProcessWithOptions( - const CommandLine& command_line, - const LaunchOptions& options, - int flags, - TimeDelta timeout, - const TestLauncher::GTestProcessLaunchedCallback& launched_callback, - bool* was_timeout) { +int LaunchChildTestProcessWithOptions(const CommandLine& command_line, + const LaunchOptions& options, + int flags, + TimeDelta timeout, + ProcessLifetimeObserver* observer, + bool* was_timeout) { TimeTicks start_time(TimeTicks::Now()); #if defined(OS_FUCHSIA) // TODO(scottmg): https://crbug.com/755282 const bool kOnBot = getenv("CHROME_HEADLESS") != nullptr; @@ -347,11 +345,14 @@ GetLiveProcesses()->insert(std::make_pair(process.Handle(), command_line)); } - if (!launched_callback.is_null()) - launched_callback.Run(process.Handle(), process.Pid()); + if (observer) + observer->OnLaunched(process.Handle(), process.Pid()); int exit_code = 0; if (!process.WaitForExitWithTimeout(timeout, &exit_code)) { + if (observer) + observer->OnTimedOut(command_line); + *was_timeout = true; exit_code = -1; // Set a non-zero exit code to signal a failure. @@ -402,22 +403,13 @@ return exit_code; } -void RunCallback(const TestLauncher::GTestProcessCompletedCallback& callback, - int exit_code, - const TimeDelta& elapsed_time, - bool was_timeout, - const std::string& output) { - callback.Run(exit_code, elapsed_time, was_timeout, output); -} - void DoLaunchChildTestProcess( const CommandLine& command_line, TimeDelta timeout, const TestLauncher::LaunchOptions& test_launch_options, bool redirect_stdio, SingleThreadTaskRunner* task_runner, - const TestLauncher::GTestProcessCompletedCallback& completed_callback, - const TestLauncher::GTestProcessLaunchedCallback& launched_callback) { + std::unique_ptr<ProcessLifetimeObserver> observer) { TimeTicks start_time = TimeTicks::Now(); ScopedFILE output_file; @@ -466,8 +458,8 @@ bool was_timeout = false; int exit_code = LaunchChildTestProcessWithOptions( - command_line, options, test_launch_options.flags, timeout, - launched_callback, &was_timeout); + command_line, options, test_launch_options.flags, timeout, observer.get(), + &was_timeout); std::string output_file_contents; if (redirect_stdio) { @@ -481,12 +473,13 @@ } } - // Run target callback on the thread it was originating from, not on - // a worker pool thread. - task_runner->PostTask(FROM_HERE, - BindOnce(&RunCallback, completed_callback, exit_code, - TimeTicks::Now() - start_time, was_timeout, - output_file_contents)); + // Invoke OnCompleted on the thread it was originating from, not on a worker + // pool thread. + task_runner->PostTask( + FROM_HERE, + BindOnce(&ProcessLifetimeObserver::OnCompleted, std::move(observer), + exit_code, TimeTicks::Now() - start_time, was_timeout, + output_file_contents)); } } // namespace @@ -578,8 +571,7 @@ const std::string& wrapper, TimeDelta timeout, const LaunchOptions& options, - const GTestProcessCompletedCallback& completed_callback, - const GTestProcessLaunchedCallback& launched_callback) { + std::unique_ptr<ProcessLifetimeObserver> observer) { DCHECK(thread_checker_.CalledOnValidThread()); // Record the exact command line used to launch the child. @@ -595,9 +587,7 @@ FROM_HERE, BindOnce(&DoLaunchChildTestProcess, new_command_line, timeout, options, redirect_stdio, RetainedRef(ThreadTaskRunnerHandle::Get()), - Bind(&TestLauncher::OnLaunchTestProcessFinished, - Unretained(this), completed_callback), - launched_callback)); + base::Passed(std::move(observer)))); } void TestLauncher::OnTestFinished(const TestResult& original_result) { @@ -1174,17 +1164,6 @@ } } -void TestLauncher::OnLaunchTestProcessFinished( - const GTestProcessCompletedCallback& callback, - int exit_code, - const TimeDelta& elapsed_time, - bool was_timeout, - const std::string& output) { - DCHECK(thread_checker_.CalledOnValidThread()); - - callback.Run(exit_code, elapsed_time, was_timeout, output); -} - void TestLauncher::OnTestIterationFinished() { TestResultsTracker::TestStatusMap tests_by_status( results_tracker_.GetTestStatusMapForCurrentIteration());
diff --git a/base/test/launcher/test_launcher.h b/base/test/launcher/test_launcher.h index 3043de0..eed9f069 100644 --- a/base/test/launcher/test_launcher.h +++ b/base/test/launcher/test_launcher.h
@@ -11,13 +11,13 @@ #include <set> #include <string> -#include "base/callback_forward.h" #include "base/compiler_specific.h" #include "base/macros.h" #include "base/process/launch.h" #include "base/test/gtest_util.h" #include "base/test/launcher/test_result.h" #include "base/test/launcher/test_results_tracker.h" +#include "base/threading/thread_checker.h" #include "base/time/time.h" #include "base/timer/timer.h" #include "build/build_config.h" @@ -74,6 +74,40 @@ virtual ~TestLauncherDelegate(); }; +// An observer of child process lifetime events generated by +// LaunchChildGTestProcess. +class ProcessLifetimeObserver { + public: + virtual ~ProcessLifetimeObserver() {} + + // Invoked once the child process is started. |handle| is a handle to the + // child process and |id| is its pid. NOTE: this method is invoked on the + // thread the process is launched on immediately after it is launched. The + // caller owns the ProcessHandle. + virtual void OnLaunched(ProcessHandle handle, ProcessId id) {} + + // Invoked when a test process exceeds its runtime, immediately before it is + // terminated. |command_line| is the command line used to launch the process. + // NOTE: this method is invoked on the thread the process is launched on. + virtual void OnTimedOut(const CommandLine& command_line) {} + + // Invoked after a child process finishes, reporting the process |exit_code|, + // child process |elapsed_time|, whether or not the process was terminated as + // a result of a timeout, and the output of the child (stdout and stderr + // together). NOTE: this method is invoked on the same thread as + // LaunchChildGTestProcess. + virtual void OnCompleted(int exit_code, + TimeDelta elapsed_time, + bool was_timeout, + const std::string& output) {} + + protected: + ProcessLifetimeObserver() = default; + + private: + DISALLOW_COPY_AND_ASSIGN(ProcessLifetimeObserver); +}; + // Launches tests using a TestLauncherDelegate. class TestLauncher { public: @@ -113,32 +147,17 @@ // Runs the launcher. Must be called at most once. bool Run() WARN_UNUSED_RESULT; - // Callback called after a child process finishes. First argument is the exit - // code, second one is child process elapsed time, third one is true if - // the child process was terminated because of a timeout, and fourth one - // contains output of the child (stdout and stderr together). - // NOTE: this callback happens on the originating thread. - using GTestProcessCompletedCallback = - Callback<void(int, const TimeDelta&, bool, const std::string&)>; - - // Once the test process is started this callback is run with the handle and - // pid of the process. This callback is run on the thread the process is - // launched on and immediately after the process is launched. The caller - // owns the ProcessHandle. - using GTestProcessLaunchedCallback = Callback<void(ProcessHandle, ProcessId)>; - // Launches a child process (assumed to be gtest-based binary) using // |command_line|. If |wrapper| is not empty, it is prepended to the final - // command line. If the child process is still running after |timeout|, it - // is terminated. After the child process finishes |callback| is called - // on the same thread this method was called. + // command line. |observer|, if not null, is used to convey process lifetime + // events to the caller. |observer| is destroyed after its OnCompleted + // method is invoked. void LaunchChildGTestProcess( const CommandLine& command_line, const std::string& wrapper, - base::TimeDelta timeout, + TimeDelta timeout, const LaunchOptions& options, - const GTestProcessCompletedCallback& completed_callback, - const GTestProcessLaunchedCallback& launched_callback); + std::unique_ptr<ProcessLifetimeObserver> observer); // Called when a test has finished running. void OnTestFinished(const TestResult& result); @@ -146,7 +165,7 @@ private: bool Init() WARN_UNUSED_RESULT; - // Runs all tests in current iteration. Uses callbacks to communicate success. + // Runs all tests in current iteration. void RunTests(); void CombinePositiveTestFilters(std::vector<std::string> filter_a, @@ -161,14 +180,6 @@ // Saves test results summary as JSON if requested from command line. void MaybeSaveSummaryAsJSON(const std::vector<std::string>& additional_tags); - // Called on a worker thread after a child process finishes. - void OnLaunchTestProcessFinished( - const GTestProcessCompletedCallback& callback, - int exit_code, - const TimeDelta& elapsed_time, - bool was_timeout, - const std::string& output); - // Called when a test iteration is finished. void OnTestIterationFinished(); @@ -180,10 +191,9 @@ scoped_refptr<TaskRunner> GetTaskRunner(); // Make sure we don't accidentally call the wrong methods e.g. on the worker - // pool thread. With lots of callbacks used this is non-trivial. - // Should be the first member so that it's destroyed last: when destroying - // other members, especially the worker pool, we may check the code is running - // on the correct thread. + // pool thread. Should be the first member so that it's destroyed last: when + // destroying other members, especially the worker pool, we may check the code + // is running on the correct thread. ThreadChecker thread_checker_; TestLauncherDelegate* launcher_delegate_;
diff --git a/base/test/launcher/unit_test_launcher.cc b/base/test/launcher/unit_test_launcher.cc index 8d4c520..cdbc65c7 100644 --- a/base/test/launcher/unit_test_launcher.cc +++ b/base/test/launcher/unit_test_launcher.cc
@@ -16,6 +16,7 @@ #include "base/location.h" #include "base/macros.h" #include "base/message_loop/message_loop.h" +#include "base/sequence_checker.h" #include "base/single_thread_task_runner.h" #include "base/stl_util.h" #include "base/strings/string_number_conversions.h" @@ -402,49 +403,128 @@ return called_any_callback; } -// TODO(phajdan.jr): Pass parameters directly with C++11 variadic templates. -struct GTestCallbackState { - TestLauncher* test_launcher; - UnitTestPlatformDelegate* platform_delegate; - std::vector<std::string> test_names; - int launch_flags; - FilePath output_file; +class UnitTestProcessLifetimeObserver : public ProcessLifetimeObserver { + public: + ~UnitTestProcessLifetimeObserver() override { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + } + + TestLauncher* test_launcher() { return test_launcher_; } + UnitTestPlatformDelegate* platform_delegate() { return platform_delegate_; } + const std::vector<std::string>& test_names() { return test_names_; } + int launch_flags() { return launch_flags_; } + const FilePath& output_file() { return output_file_; } + + protected: + UnitTestProcessLifetimeObserver(TestLauncher* test_launcher, + UnitTestPlatformDelegate* platform_delegate, + const std::vector<std::string>& test_names, + int launch_flags, + const FilePath& output_file) + : ProcessLifetimeObserver(), + test_launcher_(test_launcher), + platform_delegate_(platform_delegate), + test_names_(test_names), + launch_flags_(launch_flags), + output_file_(output_file) {} + + SEQUENCE_CHECKER(sequence_checker_); + + private: + TestLauncher* const test_launcher_; + UnitTestPlatformDelegate* const platform_delegate_; + const std::vector<std::string> test_names_; + const int launch_flags_; + const FilePath output_file_; + + DISALLOW_COPY_AND_ASSIGN(UnitTestProcessLifetimeObserver); }; -void GTestCallback( - const GTestCallbackState& callback_state, +class ParallelUnitTestProcessLifetimeObserver + : public UnitTestProcessLifetimeObserver { + public: + ParallelUnitTestProcessLifetimeObserver( + TestLauncher* test_launcher, + UnitTestPlatformDelegate* platform_delegate, + const std::vector<std::string>& test_names, + int launch_flags, + const FilePath& output_file) + : UnitTestProcessLifetimeObserver(test_launcher, + platform_delegate, + test_names, + launch_flags, + output_file) {} + ~ParallelUnitTestProcessLifetimeObserver() override = default; + + private: + // ProcessLifetimeObserver: + void OnCompleted(int exit_code, + TimeDelta elapsed_time, + bool was_timeout, + const std::string& output) override; + + DISALLOW_COPY_AND_ASSIGN(ParallelUnitTestProcessLifetimeObserver); +}; + +void ParallelUnitTestProcessLifetimeObserver::OnCompleted( int exit_code, - const TimeDelta& elapsed_time, + TimeDelta elapsed_time, bool was_timeout, const std::string& output) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); std::vector<std::string> tests_to_relaunch; - ProcessTestResults(callback_state.test_launcher, callback_state.test_names, - callback_state.output_file, output, exit_code, was_timeout, - &tests_to_relaunch); + ProcessTestResults(test_launcher(), test_names(), output_file(), output, + exit_code, was_timeout, &tests_to_relaunch); if (!tests_to_relaunch.empty()) { - callback_state.platform_delegate->RelaunchTests( - callback_state.test_launcher, - tests_to_relaunch, - callback_state.launch_flags); + platform_delegate()->RelaunchTests(test_launcher(), tests_to_relaunch, + launch_flags()); } // The temporary file's directory is also temporary. - DeleteFile(callback_state.output_file.DirName(), true); + DeleteFile(output_file().DirName(), true); } -void SerialGTestCallback( - const GTestCallbackState& callback_state, - const std::vector<std::string>& test_names, +class SerialUnitTestProcessLifetimeObserver + : public UnitTestProcessLifetimeObserver { + public: + SerialUnitTestProcessLifetimeObserver( + TestLauncher* test_launcher, + UnitTestPlatformDelegate* platform_delegate, + const std::vector<std::string>& test_names, + int launch_flags, + const FilePath& output_file, + std::vector<std::string>&& next_test_names) + : UnitTestProcessLifetimeObserver(test_launcher, + platform_delegate, + test_names, + launch_flags, + output_file), + next_test_names_(std::move(next_test_names)) {} + ~SerialUnitTestProcessLifetimeObserver() override = default; + + private: + // ProcessLifetimeObserver: + void OnCompleted(int exit_code, + TimeDelta elapsed_time, + bool was_timeout, + const std::string& output) override; + + std::vector<std::string> next_test_names_; + + DISALLOW_COPY_AND_ASSIGN(SerialUnitTestProcessLifetimeObserver); +}; + +void SerialUnitTestProcessLifetimeObserver::OnCompleted( int exit_code, - const TimeDelta& elapsed_time, + TimeDelta elapsed_time, bool was_timeout, const std::string& output) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); std::vector<std::string> tests_to_relaunch; bool called_any_callbacks = - ProcessTestResults(callback_state.test_launcher, - callback_state.test_names, callback_state.output_file, - output, exit_code, was_timeout, &tests_to_relaunch); + ProcessTestResults(test_launcher(), test_names(), output_file(), output, + exit_code, was_timeout, &tests_to_relaunch); // There is only one test, there cannot be other tests to relaunch // due to a crash. @@ -454,12 +534,12 @@ DCHECK(called_any_callbacks); // The temporary file's directory is also temporary. - DeleteFile(callback_state.output_file.DirName(), true); + DeleteFile(output_file().DirName(), true); ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, BindOnce(&RunUnitTestsSerially, callback_state.test_launcher, - callback_state.platform_delegate, test_names, - callback_state.launch_flags)); + FROM_HERE, + BindOnce(&RunUnitTestsSerially, test_launcher(), platform_delegate(), + std::move(next_test_names_), launch_flags())); } } // namespace @@ -522,35 +602,26 @@ if (test_names.empty()) return; - std::vector<std::string> new_test_names(test_names); - std::string test_name(new_test_names.back()); - new_test_names.pop_back(); - // Create a dedicated temporary directory to store the xml result data // per run to ensure clean state and make it possible to launch multiple // processes in parallel. base::FilePath output_file; CHECK(platform_delegate->CreateTemporaryFile(&output_file)); - std::vector<std::string> current_test_names; - current_test_names.push_back(test_name); - CommandLine cmd_line(platform_delegate->GetCommandLineForChildGTestProcess( - current_test_names, output_file)); + auto observer = std::make_unique<SerialUnitTestProcessLifetimeObserver>( + test_launcher, platform_delegate, + std::vector<std::string>(1, test_names.back()), launch_flags, output_file, + std::vector<std::string>(test_names.begin(), test_names.end() - 1)); - GTestCallbackState callback_state; - callback_state.test_launcher = test_launcher; - callback_state.platform_delegate = platform_delegate; - callback_state.test_names = current_test_names; - callback_state.launch_flags = launch_flags; - callback_state.output_file = output_file; + CommandLine cmd_line(platform_delegate->GetCommandLineForChildGTestProcess( + observer->test_names(), output_file)); TestLauncher::LaunchOptions launch_options; launch_options.flags = launch_flags; test_launcher->LaunchChildGTestProcess( cmd_line, platform_delegate->GetWrapperForChildGTestProcess(), TestTimeouts::test_launcher_timeout(), launch_options, - Bind(&SerialGTestCallback, callback_state, new_test_names), - TestLauncher::GTestProcessLaunchedCallback()); + std::move(observer)); } void RunUnitTestsBatch( @@ -567,6 +638,9 @@ base::FilePath output_file; CHECK(platform_delegate->CreateTemporaryFile(&output_file)); + auto observer = std::make_unique<ParallelUnitTestProcessLifetimeObserver>( + test_launcher, platform_delegate, test_names, launch_flags, output_file); + CommandLine cmd_line(platform_delegate->GetCommandLineForChildGTestProcess( test_names, output_file)); @@ -579,19 +653,11 @@ base::TimeDelta timeout = test_names.size() * TestTimeouts::test_launcher_timeout(); - GTestCallbackState callback_state; - callback_state.test_launcher = test_launcher; - callback_state.platform_delegate = platform_delegate; - callback_state.test_names = test_names; - callback_state.launch_flags = launch_flags; - callback_state.output_file = output_file; - TestLauncher::LaunchOptions options; options.flags = launch_flags; test_launcher->LaunchChildGTestProcess( cmd_line, platform_delegate->GetWrapperForChildGTestProcess(), timeout, - options, Bind(>estCallback, callback_state), - TestLauncher::GTestProcessLaunchedCallback()); + options, std::move(observer)); } UnitTestLauncherDelegate::UnitTestLauncherDelegate(
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 6997770..2c7641c0e 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn
@@ -493,6 +493,10 @@ "base/interactive_test_utils_mac.mm", "base/interactive_test_utils_win.cc", "base/interactive_ui_tests_main.cc", + "base/process_inspector_win.cc", + "base/process_inspector_win.h", + "base/save_desktop_snapshot_win.cc", + "base/save_desktop_snapshot_win.h", "base/view_event_test_platform_part.h", "base/view_event_test_platform_part_chromeos.cc", "base/view_event_test_platform_part_default.cc",
diff --git a/chrome/test/base/always_on_top_window_killer_win.cc b/chrome/test/base/always_on_top_window_killer_win.cc index cedab4ee..298e843 100644 --- a/chrome/test/base/always_on_top_window_killer_win.cc +++ b/chrome/test/base/always_on_top_window_killer_win.cc
@@ -11,180 +11,57 @@ #include <vector> #include "base/command_line.h" -#include "base/files/file.h" #include "base/files/file_path.h" #include "base/logging.h" #include "base/macros.h" -#include "base/message_loop/message_loop.h" -#include "base/numerics/safe_conversions.h" #include "base/process/process.h" -#include "base/run_loop.h" -#include "base/strings/stringprintf.h" -#include "base/time/time.h" -#include "third_party/skia/include/core/SkBitmap.h" -#include "third_party/webrtc/modules/desktop_capture/desktop_capture_options.h" -#include "third_party/webrtc/modules/desktop_capture/desktop_capturer.h" -#include "third_party/webrtc/modules/desktop_capture/desktop_frame.h" +#include "chrome/test/base/process_inspector_win.h" +#include "chrome/test/base/save_desktop_snapshot_win.h" #include "ui/display/win/screen_win.h" -#include "ui/gfx/codec/png_codec.h" namespace { constexpr char kDialogFoundBeforeTest[] = "There is an always on top dialog on the desktop. This was most likely " "caused by a previous test and may cause this test to fail. Trying to " - "close it."; + "close it;"; constexpr char kDialogFoundPostTest[] = - "There is an always on top dialog on the desktop after running this test. " - "This was most likely caused by this test and may cause future tests to " - "fail, trying to close it."; + "There is an always on top dialog on the desktop after this test timed " + "out. This was most likely caused by this test and may cause future tests " + "to fail, trying to close it;"; constexpr char kWindowFoundBeforeTest[] = - "There is an always on top window on the desktop before running the test. " - "This may have been caused by a previous test and may cause this test to " - "fail, class-name="; + "There is an always on top window on the desktop. This may have been " + "caused by a previous test and may cause this test to fail;"; constexpr char kWindowFoundPostTest[] = - "There is an always on top window on the desktop after running the test. " - "This may have been caused by this test or a previous test and may cause " - "flake, class-name="; + "There is an always on top window on the desktop after this test timed " + "out. This may have been caused by this test or a previous test and may " + "cause flakes;"; // A command line switch to specify the output directory into which snapshots // are to be saved in case an always-on-top window is found. constexpr char kSnapshotOutputDir[] = "snapshot-output-dir"; -// A worker that captures a single frame from a webrtc::DesktopCapturer and then -// runs a callback when done. -class CaptureWorker : public webrtc::DesktopCapturer::Callback { - public: - CaptureWorker(std::unique_ptr<webrtc::DesktopCapturer> capturer, - base::Closure on_done) - : capturer_(std::move(capturer)), on_done_(std::move(on_done)) { - capturer_->Start(this); - capturer_->CaptureFrame(); - } - - // Returns the frame that was captured or null in case of failure. - std::unique_ptr<webrtc::DesktopFrame> TakeFrame() { - return std::move(frame_); - } - - private: - // webrtc::DesktopCapturer::Callback: - void OnCaptureResult(webrtc::DesktopCapturer::Result result, - std::unique_ptr<webrtc::DesktopFrame> frame) override { - if (result == webrtc::DesktopCapturer::Result::SUCCESS) - frame_ = std::move(frame); - on_done_.Run(); - } - - std::unique_ptr<webrtc::DesktopCapturer> capturer_; - base::Closure on_done_; - std::unique_ptr<webrtc::DesktopFrame> frame_; - DISALLOW_COPY_AND_ASSIGN(CaptureWorker); -}; - -// Captures and returns a snapshot of the screen, or an empty bitmap in case of -// error. -SkBitmap CaptureScreen() { - auto options = webrtc::DesktopCaptureOptions::CreateDefault(); - options.set_disable_effects(false); - options.set_allow_directx_capturer(true); - options.set_allow_use_magnification_api(false); - std::unique_ptr<webrtc::DesktopCapturer> capturer( - webrtc::DesktopCapturer::CreateScreenCapturer(options)); - - // Grab a single frame. - std::unique_ptr<webrtc::DesktopFrame> frame; - { - // While webrtc::DesktopCapturer seems to be synchronous, comments in its - // implementation seem to indicate that it may require a UI message loop on - // its thread. - base::MessageLoopForUI message_loop; - base::RunLoop run_loop; - CaptureWorker worker(std::move(capturer), run_loop.QuitClosure()); - run_loop.Run(); - frame = worker.TakeFrame(); - } - - if (!frame) - return SkBitmap(); - - // Create an image from the frame. - SkBitmap result; - result.allocN32Pixels(frame->size().width(), frame->size().height(), true); - memcpy(result.getAddr32(0, 0), frame->data(), - frame->size().width() * frame->size().height() * - webrtc::DesktopFrame::kBytesPerPixel); - return result; -} - -// Saves a snapshot of the screen to a file in |output_dir|, returning the path -// to the file if created. An empty path is returned if no new snapshot is -// created. -base::FilePath SaveSnapshot(const base::FilePath& output_dir) { - // Create the output file. - base::Time::Exploded exploded; - base::Time::Now().LocalExplode(&exploded); - base::FilePath output_path( - output_dir.Append(base::FilePath(base::StringPrintf( - L"ss_%4d%02d%02d%02d%02d%02d_%03d.png", exploded.year, exploded.month, - exploded.day_of_month, exploded.hour, exploded.minute, - exploded.second, exploded.millisecond)))); - base::File file(output_path, base::File::FLAG_CREATE | - base::File::FLAG_WRITE | - base::File::FLAG_SHARE_DELETE | - base::File::FLAG_CAN_DELETE_ON_CLOSE); - if (!file.IsValid()) { - if (file.error_details() == base::File::FILE_ERROR_EXISTS) { - LOG(INFO) << "Skipping screen snapshot since it is already present: " - << output_path.BaseName(); - } else { - LOG(ERROR) << "Failed to create snapshot output file \"" << output_path - << "\" with error " << file.error_details(); - } - return base::FilePath(); - } - - // Delete the output file in case of any error. - file.DeleteOnClose(true); - - // Take the snapshot and encode it. - SkBitmap screen = CaptureScreen(); - if (screen.drawsNothing()) { - LOG(ERROR) << "Failed to capture a frame of the screen for a snapshot."; - return base::FilePath(); - } - - std::vector<unsigned char> encoded; - if (!gfx::PNGCodec::EncodeBGRASkBitmap(CaptureScreen(), false, &encoded)) { - LOG(ERROR) << "Failed to PNG encode screen snapshot."; - return base::FilePath(); - } - - // Write it to disk. - const int to_write = base::checked_cast<int>(encoded.size()); - int written = - file.WriteAtCurrentPos(reinterpret_cast<char*>(encoded.data()), to_write); - if (written != to_write) { - LOG(ERROR) << "Failed to write entire snapshot to file"; - return base::FilePath(); - } - - // Keep the output file. - file.DeleteOnClose(false); - return output_path; -} - // A window enumerator that searches for always-on-top windows. A snapshot of // the screen is saved if any unexpected on-top windows are found. class WindowEnumerator { public: - explicit WindowEnumerator(RunType run_type); + // |run_type| influences which log message is used. |child_command_line|, only + // specified when |run_type| is AFTER_TEST_TIMEOUT, is the command line of the + // child process that timed out. + WindowEnumerator(RunType run_type, + const base::CommandLine* child_command_line); void Run(); private: + // Properies of a running process. + struct ProcessProperties { + DWORD process_id; + base::string16 command_line; + }; + // An EnumWindowsProc invoked by EnumWindows once for each window. static BOOL CALLBACK OnWindowProc(HWND hwnd, LPARAM l_param); @@ -201,19 +78,31 @@ // shell. static bool IsShellWindowClass(const base::string16& class_name); + // Returns the lineage of |process_id|; specifically, the pid and command line + // of |process_id| and each of its ancestors that are still running. Due to + // aggressive pid reuse on Windows, it's possible that the returned collection + // may contain misleading information. Since the goal of this method is to get + // useful data in the aggregate, some misleading info here and there is + // tolerable. If it proves to be intolerable, additional checks can be added + // to be sure that each ancestor is older that its child. + static std::vector<ProcessProperties> GetProcessLineage(DWORD process_id); + // Main processing function run for each window. BOOL OnWindow(HWND hwnd); const base::FilePath output_dir_; const RunType run_type_; + const base::CommandLine* const child_command_line_; bool saved_snapshot_; DISALLOW_COPY_AND_ASSIGN(WindowEnumerator); }; -WindowEnumerator::WindowEnumerator(RunType run_type) +WindowEnumerator::WindowEnumerator(RunType run_type, + const base::CommandLine* child_command_line) : output_dir_(base::CommandLine::ForCurrentProcess()->GetSwitchValuePath( kSnapshotOutputDir)), run_type_(run_type), + child_command_line_(child_command_line), saved_snapshot_(false) {} void WindowEnumerator::Run() { @@ -254,6 +143,34 @@ class_name == L"Shell_SecondaryTrayWnd"; } +// static +std::vector<WindowEnumerator::ProcessProperties> +WindowEnumerator::GetProcessLineage(DWORD process_id) { + std::vector<ProcessProperties> properties; + + while (true) { + base::Process process = base::Process::OpenWithAccess( + process_id, PROCESS_QUERY_INFORMATION | SYNCHRONIZE | PROCESS_VM_READ); + if (!process.IsValid()) + break; + + auto inspector = ProcessInspector::Create(process); + if (!inspector) + break; + + // If PID reuse proves to be a problem, this would be a good point to add + // extra checks that |process| is older than the previously inspected + // process. + + properties.push_back({process_id, inspector->command_line()}); + DWORD parent_pid = inspector->GetParentPid(); + if (process_id == parent_pid) + break; + process_id = parent_pid; + } + return properties; +} + BOOL WindowEnumerator::OnWindow(HWND hwnd) { const BOOL kContinueIterating = TRUE; @@ -268,62 +185,80 @@ if (IsShellWindowClass(class_name)) return kContinueIterating; - // Something unexpected was found. Save a snapshot of the screen if one hasn't - // already been saved and an output directory was specified. - if (!saved_snapshot_ && !output_dir_.empty()) { - base::FilePath snapshot_file = SaveSnapshot(output_dir_); - if (!snapshot_file.empty()) { - saved_snapshot_ = true; - LOG(ERROR) << "Wrote snapshot to file " << snapshot_file; + // All other always-on-top windows may be problematic, but in theory tests + // should not be creating an always on top window that outlives the test. + // Prepare details of the command line of the test that timed out (if + // provided), the process owning the window, and the location of a snapshot + // taken of the screen. + base::string16 details; + if (LOG_IS_ON(ERROR)) { + std::wostringstream sstream; + + if (!IsSystemDialogClass(class_name)) + sstream << " window class name: " << class_name << ";"; + + if (child_command_line_) { + sstream << " subprocess command line: \"" + << child_command_line_->GetCommandLineString() << "\";"; } + + // Save a snapshot of the screen if one hasn't already been saved and an + // output directory was specified. + base::FilePath snapshot_file; + if (!saved_snapshot_ && !output_dir_.empty()) { + snapshot_file = SaveDesktopSnapshot(output_dir_); + if (!snapshot_file.empty()) + saved_snapshot_ = true; + } + + DWORD process_id = 0; + GetWindowThreadProcessId(hwnd, &process_id); + std::vector<ProcessProperties> process_properties = + GetProcessLineage(process_id); + if (!process_properties.empty()) { + sstream << " owning process lineage: "; + base::string16 sep; + for (const auto& prop : process_properties) { + sstream << sep << L"(process_id: " << prop.process_id + << L", command_line: \"" << prop.command_line << "\")"; + if (sep.empty()) + sep = L", "; + } + sstream << ";"; + } + + if (!snapshot_file.empty()) { + sstream << " screen snapshot saved to file: \"" << snapshot_file.value() + << "\";"; + } + + details = sstream.str(); } // System dialogs may be present if a child process triggers an assert(), for // example. if (IsSystemDialogClass(class_name)) { - LOG(ERROR) << (run_type_ == RunType::BEFORE_TEST ? kDialogFoundBeforeTest - : kDialogFoundPostTest); + LOG(ERROR) << (run_type_ == RunType::BEFORE_SHARD ? kDialogFoundBeforeTest + : kDialogFoundPostTest) + << details; // We don't own the dialog, so we can't destroy it. CloseWindow() // results in iconifying the window. An alternative may be to focus it, // then send return and wait for close. As we reboot machines running // interactive ui tests at least every 12 hours we're going with the // simple for now. CloseWindow(hwnd); - return kContinueIterating; + } else { + LOG(ERROR) << (run_type_ == RunType::BEFORE_SHARD ? kWindowFoundBeforeTest + : kWindowFoundPostTest) + << details; } - // All other always-on-top windows may be problematic, but in theory tests - // should not be creating an always on top window that outlives the test. Log - // attributes of the window in case there are problems. - DWORD process_id = 0; - DWORD thread_id = GetWindowThreadProcessId(hwnd, &process_id); - - base::Process process = base::Process::Open(process_id); - base::string16 process_path(MAX_PATH, L'\0'); - if (process.IsValid()) { - // It's possible that the actual process owning |hwnd| has gone away and - // that a new process using the same PID has appeared. If this turns out to - // be an issue, we could fetch the process start time here and compare it - // with the time just before getting |thread_id| above. This is likely - // overkill for diagnostic purposes. - DWORD str_len = process_path.size(); - if (!::QueryFullProcessImageName(process.Handle(), 0, &process_path[0], - &str_len) || - str_len >= MAX_PATH) { - str_len = 0; - } - process_path.resize(str_len); - } - LOG(ERROR) << (run_type_ == RunType::BEFORE_TEST ? kWindowFoundBeforeTest - : kWindowFoundPostTest) - << class_name << " process_id=" << process_id - << " thread_id=" << thread_id << " process_path=" << process_path; - return kContinueIterating; } } // namespace -void KillAlwaysOnTopWindows(RunType run_type) { - WindowEnumerator(run_type).Run(); +void KillAlwaysOnTopWindows(RunType run_type, + const base::CommandLine* child_command_line) { + WindowEnumerator(run_type, child_command_line).Run(); }
diff --git a/chrome/test/base/always_on_top_window_killer_win.h b/chrome/test/base/always_on_top_window_killer_win.h index 7310fb52..a428a1f1 100644 --- a/chrome/test/base/always_on_top_window_killer_win.h +++ b/chrome/test/base/always_on_top_window_killer_win.h
@@ -5,16 +5,23 @@ #ifndef CHROME_TEST_BASE_ALWAYS_ON_TOP_WINDOW_KILLER_WIN_H_ #define CHROME_TEST_BASE_ALWAYS_ON_TOP_WINDOW_KILLER_WIN_H_ -enum class RunType { - // Indicates cleanup is happening before the test run. - BEFORE_TEST, +namespace base { +class CommandLine; +} - // Indicates cleanup is happening after the test run. - AFTER_TEST, +enum class RunType { + // Indicates cleanup is happening before sharded tests are run. + BEFORE_SHARD, + + // Indicates cleanup is happening after a test subprocess has timed out. + AFTER_TEST_TIMEOUT, }; // Logs if there are any always on top windows, and if one is a system dialog -// closes it. -void KillAlwaysOnTopWindows(RunType run_type); +// closes it. |child_command_line|, if non-null, is the command line of the +// test subprocess that timed out. +void KillAlwaysOnTopWindows( + RunType run_type, + const base::CommandLine* child_command_line = nullptr); #endif // CHROME_TEST_BASE_ALWAYS_ON_TOP_WINDOW_KILLER_WIN_H_
diff --git a/chrome/test/base/interactive_ui_tests_main.cc b/chrome/test/base/interactive_ui_tests_main.cc index 0989be9..a1bf525 100644 --- a/chrome/test/base/interactive_ui_tests_main.cc +++ b/chrome/test/base/interactive_ui_tests_main.cc
@@ -51,7 +51,6 @@ #elif defined(USE_AURA) #if defined(OS_WIN) com_initializer_.reset(new base::win::ScopedCOMInitializer()); - KillAlwaysOnTopWindows(RunType::BEFORE_TEST); #endif #if defined(OS_LINUX) @@ -70,7 +69,6 @@ void Shutdown() override { #if defined(OS_WIN) - KillAlwaysOnTopWindows(RunType::AFTER_TEST); com_initializer_.reset(); #endif } @@ -81,6 +79,37 @@ #endif }; +class InteractiveUITestLauncherDelegate : public ChromeTestLauncherDelegate { + public: + explicit InteractiveUITestLauncherDelegate(ChromeTestSuiteRunner* runner) + : ChromeTestLauncherDelegate(runner) {} + + protected: + // content::TestLauncherDelegate: + void PreSharding() override { + ChromeTestLauncherDelegate::PreSharding(); +#if defined(OS_WIN) + // Check for any always-on-top windows present before any tests are run. + // Take a snapshot if any are found and attempt to close any that are system + // dialogs. + KillAlwaysOnTopWindows(RunType::BEFORE_SHARD); +#endif + } + + void OnTestTimedOut(const base::CommandLine& command_line) override { +#if defined(OS_WIN) + // Check for any always-on-top windows present before terminating the test. + // Take a snapshot if any are found and attempt to close any that are system + // dialogs. + KillAlwaysOnTopWindows(RunType::AFTER_TEST_TIMEOUT, &command_line); +#endif + ChromeTestLauncherDelegate::OnTestTimedOut(command_line); + } + + private: + DISALLOW_COPY_AND_ASSIGN(InteractiveUITestLauncherDelegate); +}; + class InteractiveUITestSuiteRunner : public ChromeTestSuiteRunner { public: int RunTestSuite(int argc, char** argv) override { @@ -106,6 +135,6 @@ size_t parallel_jobs = 1U; InteractiveUITestSuiteRunner runner; - ChromeTestLauncherDelegate delegate(&runner); + InteractiveUITestLauncherDelegate delegate(&runner); return LaunchChromeTests(parallel_jobs, &delegate, argc, argv); }
diff --git a/chrome/test/base/process_inspector_win.cc b/chrome/test/base/process_inspector_win.cc new file mode 100644 index 0000000..de059b7 --- /dev/null +++ b/chrome/test/base/process_inspector_win.cc
@@ -0,0 +1,267 @@ +// 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/test/base/process_inspector_win.h" + +#include <winternl.h> + +#include "base/process/process.h" +#include "base/win/windows_version.h" + +namespace { + +// Certain Windows types that depend on the word size of the OS (rather than the +// size of the current process) are defined here. + +// PROCESS_BASIC_INFORMATION. +template <class Traits> +struct ProcessInformation { + using RemotePointer = typename Traits::RemotePointer; + + DWORD exit_status() const { return static_cast<DWORD>(exit_status_); } + RemotePointer peb_base_address() const { return peb_base_address_; } + RemotePointer affinity_mask() const { return affinity_mask_; } + int base_priority() const { return static_cast<int>(base_priority_); } + DWORD unique_process_id() const { return static_cast<DWORD>(unique_pid_); } + DWORD inherited_from_unique_process_id() const { + return static_cast<DWORD>(inherited_from_unique_process_id_); + } + + private: + RemotePointer exit_status_; + RemotePointer peb_base_address_; + RemotePointer affinity_mask_; + RemotePointer base_priority_; + RemotePointer unique_pid_; + RemotePointer inherited_from_unique_process_id_; +}; + +// A subset of a process's environment block. +template <class Traits> +struct ProcessExecutionBlock { + using RemotePointer = typename Traits::RemotePointer; + + uint8_t InheritedAddressSpace; + uint8_t ReadImageFileExecOptions; + uint8_t BeingDebugged; + uint8_t ProcessFlags; + uint8_t Padding[4]; + RemotePointer Mutant; + RemotePointer ImageBaseAddress; + RemotePointer Ldr; + RemotePointer ProcessParameters; // RtlUserProcessParameters +}; + +// UNICODE_STRING. +template <class Traits> +struct UnicodeString { + using RemotePointer = typename Traits::RemotePointer; + + uint16_t Length; + uint16_t MaximumLength; + RemotePointer Buffer; +}; + +// CURDIR. +template <class Traits> +struct CurDir { + using RemotePointer = typename Traits::RemotePointer; + + UnicodeString<Traits> DosPath; + RemotePointer Handle; +}; + +// RTL_USER_PROCESS_PARAMETERS. +template <class Traits> +struct RtlUserProcessParameters { + using RemotePointer = typename Traits::RemotePointer; + + uint32_t MaximumLength; + uint32_t Length; + uint32_t Flags; + uint32_t DebugFlags; + RemotePointer ConsoleHandle; + uint32_t ConsoleFlags; + RemotePointer StandardInput; + RemotePointer StandardOutput; + RemotePointer StandardError; + CurDir<Traits> CurrentDirectory; + UnicodeString<Traits> DllPath; + UnicodeString<Traits> ImagePathName; + UnicodeString<Traits> CommandLine; +}; + +// A concrete ProcessInspector that can read from another process based on the +// architecture. |Traits| specifies traits based on the OS architecture. +template <class Traits> +class Inspector : public ProcessInspector { + public: + Inspector(); + + // ProcessInspector: + DWORD GetParentPid() const override; + const base::string16& command_line() const override; + + private: + // ProcessInspector: + bool Inspect(const base::Process& process) override; + + ProcessInformation<Traits> process_basic_information_; + ProcessExecutionBlock<Traits> peb_; + RtlUserProcessParameters<Traits> process_parameters_; + base::string16 command_line_; + + DISALLOW_COPY_AND_ASSIGN(Inspector); +}; + +#if !defined(_WIN64) +// Traits for a 32-bit process running in WoW. +struct Wow64Traits { + // The name of the ntdll function to query process information. + static const char kQueryProcessInformationFunctionName[]; + + // The type of a pointer to the read process memory function. + using ReadMemoryFn = + NTSTATUS(NTAPI*)(HANDLE, uint64_t, void*, uint64_t, uint64_t*); + + // An unsigned integer type matching the size of a pointer in the remote + // process. + using RemotePointer = uint64_t; + + // Returns the function to read memory from a remote process. + static ReadMemoryFn GetReadMemoryFn() { + return reinterpret_cast<ReadMemoryFn>(::GetProcAddress( + ::GetModuleHandle(L"ntdll.dll"), "NtWow64ReadVirtualMemory64")); + } + + // Reads |buffer_size| bytes from |handle|'s process at |address| into + // |buffer| using |fn|. Returns true on success. + static bool ReadMemory(ReadMemoryFn fn, + HANDLE handle, + RemotePointer address, + void* buffer, + RemotePointer buffer_size) { + NTSTATUS status = fn(handle, address, buffer, buffer_size, nullptr); + if (NT_SUCCESS(status)) + return true; + LOG(ERROR) << "Failed to read process memory with status " << std::hex + << status << "."; + return false; + } +}; + +// static +constexpr char Wow64Traits::kQueryProcessInformationFunctionName[] = + "NtWow64QueryInformationProcess64"; + +#endif + +// Traits for a 32-bit process running on 32-bit Windows, or a 64-bit process +// running on 64-bit Windows. +struct NormalTraits { + // The name of the ntdll function to query process information. + static const char kQueryProcessInformationFunctionName[]; + + // The type of a pointer to the read process memory function. + using ReadMemoryFn = decltype(&::ReadProcessMemory); + + // An unsigned integer type matching the size of a pointer in the remote + // process. + using RemotePointer = uintptr_t; + + // Returns the function to read memory from a remote process. + static ReadMemoryFn GetReadMemoryFn() { return &::ReadProcessMemory; } + + // Reads |buffer_size| bytes from |handle|'s process at |address| into + // |buffer| using |fn|. Returns true on success. + static bool ReadMemory(ReadMemoryFn fn, + HANDLE handle, + RemotePointer address, + void* buffer, + RemotePointer buffer_size) { + BOOL result = fn(handle, reinterpret_cast<const void*>(address), buffer, + buffer_size, nullptr); + if (result) + return true; + PLOG(ERROR) << "Failed to read process memory"; + return false; + } +}; + +// static +constexpr char NormalTraits::kQueryProcessInformationFunctionName[] = + "NtQueryInformationProcess"; + +template <class Traits> +Inspector<Traits>::Inspector() = default; + +template <class Traits> +DWORD Inspector<Traits>::GetParentPid() const { + return process_basic_information_.inherited_from_unique_process_id(); +} + +template <class Traits> +const base::string16& Inspector<Traits>::command_line() const { + return command_line_; +} + +template <class Traits> +bool Inspector<Traits>::Inspect(const base::Process& process) { + auto query_information_process_fn = + reinterpret_cast<decltype(&::NtQueryInformationProcess)>( + ::GetProcAddress(::GetModuleHandle(L"ntdll.dll"), + Traits::kQueryProcessInformationFunctionName)); + typename Traits::ReadMemoryFn read_memory_fn = Traits::GetReadMemoryFn(); + + if (!query_information_process_fn) + return false; + ULONG in_len = sizeof(process_basic_information_); + ULONG out_len = 0; + NTSTATUS status = query_information_process_fn( + process.Handle(), ProcessBasicInformation, &process_basic_information_, + in_len, &out_len); + if (NT_ERROR(status) || out_len != in_len) + return false; + + if (!Traits::ReadMemory(read_memory_fn, process.Handle(), + process_basic_information_.peb_base_address(), &peb_, + sizeof(peb_))) { + return false; + } + if (!Traits::ReadMemory(read_memory_fn, process.Handle(), + peb_.ProcessParameters, &process_parameters_, + sizeof(process_parameters_))) { + return false; + } + if (process_parameters_.CommandLine.Length) { + command_line_.resize(process_parameters_.CommandLine.Length / + sizeof(wchar_t)); + if (!Traits::ReadMemory(read_memory_fn, process.Handle(), + process_parameters_.CommandLine.Buffer, + &command_line_[0], + process_parameters_.CommandLine.Length)) { + command_line_.clear(); + return false; + } + } + return true; +} + +} // namespace + +// static +std::unique_ptr<ProcessInspector> ProcessInspector::Create( + const base::Process& process) { + std::unique_ptr<ProcessInspector> inspector; +#if !defined(_WIN64) + using base::win::OSInfo; + if (OSInfo::GetInstance()->wow64_status() == OSInfo::WOW64_ENABLED) + inspector = std::make_unique<Inspector<Wow64Traits>>(); +#endif + if (!inspector) + inspector = std::make_unique<Inspector<NormalTraits>>(); + if (!inspector->Inspect(process)) + inspector.reset(); + return inspector; +}
diff --git a/chrome/test/base/process_inspector_win.h b/chrome/test/base/process_inspector_win.h new file mode 100644 index 0000000..26fbb00 --- /dev/null +++ b/chrome/test/base/process_inspector_win.h
@@ -0,0 +1,45 @@ +// 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_TEST_BASE_PROCESS_INSPECTOR_WIN_H_ +#define CHROME_TEST_BASE_PROCESS_INSPECTOR_WIN_H_ + +#include <windows.h> + +#include <memory> + +#include "base/macros.h" +#include "base/strings/string16.h" + +namespace base { +class Process; +} + +// An inspector that can read properties of a remote process. +class ProcessInspector { + public: + // Returns an instance that reads data from |process|, which must have been + // opened with at least PROCESS_VM_READ access rights. Returns null in case of + // any error. + static std::unique_ptr<ProcessInspector> Create(const base::Process& process); + + virtual ~ProcessInspector() = default; + + // Returns the parent process PID of the process. + virtual DWORD GetParentPid() const = 0; + + // Returns the command line of the process. + virtual const base::string16& command_line() const = 0; + + protected: + ProcessInspector() = default; + + private: + // Inspects |process|, returning true if all inspections succeed. + virtual bool Inspect(const base::Process& process) = 0; + + DISALLOW_COPY_AND_ASSIGN(ProcessInspector); +}; + +#endif // CHROME_TEST_BASE_PROCESS_INSPECTOR_WIN_H_
diff --git a/chrome/test/base/save_desktop_snapshot_win.cc b/chrome/test/base/save_desktop_snapshot_win.cc new file mode 100644 index 0000000..2120d60 --- /dev/null +++ b/chrome/test/base/save_desktop_snapshot_win.cc
@@ -0,0 +1,146 @@ +// 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/test/base/save_desktop_snapshot_win.h" + +#include <memory> +#include <utility> +#include <vector> + +#include "base/callback.h" +#include "base/files/file.h" +#include "base/logging.h" +#include "base/macros.h" +#include "base/numerics/safe_conversions.h" +#include "base/run_loop.h" +#include "base/strings/stringprintf.h" +#include "base/time/time.h" +#include "third_party/skia/include/core/SkBitmap.h" +#include "third_party/webrtc/modules/desktop_capture/desktop_capture_options.h" +#include "third_party/webrtc/modules/desktop_capture/desktop_capturer.h" +#include "third_party/webrtc/modules/desktop_capture/desktop_frame.h" +#include "ui/gfx/codec/png_codec.h" + +namespace { + +// A worker that captures a single frame from a webrtc::DesktopCapturer and then +// runs a callback when done. +class CaptureWorker : public webrtc::DesktopCapturer::Callback { + public: + CaptureWorker(std::unique_ptr<webrtc::DesktopCapturer> capturer, + base::Closure on_done) + : capturer_(std::move(capturer)), on_done_(std::move(on_done)) { + capturer_->Start(this); + capturer_->CaptureFrame(); + } + + // Returns the frame that was captured or null in case of failure. + std::unique_ptr<webrtc::DesktopFrame> TakeFrame() { + return std::move(frame_); + } + + private: + // webrtc::DesktopCapturer::Callback: + void OnCaptureResult(webrtc::DesktopCapturer::Result result, + std::unique_ptr<webrtc::DesktopFrame> frame) override { + if (result == webrtc::DesktopCapturer::Result::SUCCESS) + frame_ = std::move(frame); + on_done_.Run(); + } + + std::unique_ptr<webrtc::DesktopCapturer> capturer_; + base::Closure on_done_; + std::unique_ptr<webrtc::DesktopFrame> frame_; + DISALLOW_COPY_AND_ASSIGN(CaptureWorker); +}; + +// Captures and returns a snapshot of the screen, or an empty bitmap in case of +// error. +SkBitmap CaptureScreen() { + auto options = webrtc::DesktopCaptureOptions::CreateDefault(); + options.set_disable_effects(false); + options.set_allow_directx_capturer(true); + options.set_allow_use_magnification_api(false); + std::unique_ptr<webrtc::DesktopCapturer> capturer( + webrtc::DesktopCapturer::CreateScreenCapturer(options)); + + // Grab a single frame. + std::unique_ptr<webrtc::DesktopFrame> frame; + { + // While webrtc::DesktopCapturer seems to be synchronous, comments in its + // implementation seem to indicate that it may require a UI message loop on + // its thread. + base::RunLoop run_loop; + CaptureWorker worker(std::move(capturer), run_loop.QuitClosure()); + run_loop.Run(); + frame = worker.TakeFrame(); + } + + if (!frame) + return SkBitmap(); + + // Create an image from the frame. + SkBitmap result; + result.allocN32Pixels(frame->size().width(), frame->size().height(), true); + memcpy(result.getAddr32(0, 0), frame->data(), + frame->size().width() * frame->size().height() * + webrtc::DesktopFrame::kBytesPerPixel); + return result; +} + +} // namespace + +base::FilePath SaveDesktopSnapshot(const base::FilePath& output_dir) { + // Create the output file. + base::Time::Exploded exploded; + base::Time::Now().LocalExplode(&exploded); + base::FilePath output_path( + output_dir.Append(base::FilePath(base::StringPrintf( + L"ss_%4d%02d%02d%02d%02d%02d_%03d.png", exploded.year, exploded.month, + exploded.day_of_month, exploded.hour, exploded.minute, + exploded.second, exploded.millisecond)))); + base::File file(output_path, base::File::FLAG_CREATE | + base::File::FLAG_WRITE | + base::File::FLAG_SHARE_DELETE | + base::File::FLAG_CAN_DELETE_ON_CLOSE); + if (!file.IsValid()) { + if (file.error_details() == base::File::FILE_ERROR_EXISTS) { + LOG(INFO) << "Skipping screen snapshot since it is already present: " + << output_path.BaseName(); + } else { + LOG(ERROR) << "Failed to create snapshot output file \"" << output_path + << "\" with error " << file.error_details(); + } + return base::FilePath(); + } + + // Delete the output file in case of any error. + file.DeleteOnClose(true); + + // Take the snapshot and encode it. + SkBitmap screen = CaptureScreen(); + if (screen.drawsNothing()) { + LOG(ERROR) << "Failed to capture a frame of the screen for a snapshot."; + return base::FilePath(); + } + + std::vector<unsigned char> encoded; + if (!gfx::PNGCodec::EncodeBGRASkBitmap(CaptureScreen(), false, &encoded)) { + LOG(ERROR) << "Failed to PNG encode screen snapshot."; + return base::FilePath(); + } + + // Write it to disk. + const int to_write = base::checked_cast<int>(encoded.size()); + int written = + file.WriteAtCurrentPos(reinterpret_cast<char*>(encoded.data()), to_write); + if (written != to_write) { + LOG(ERROR) << "Failed to write entire snapshot to file"; + return base::FilePath(); + } + + // Keep the output file. + file.DeleteOnClose(false); + return output_path; +}
diff --git a/chrome/test/base/save_desktop_snapshot_win.h b/chrome/test/base/save_desktop_snapshot_win.h new file mode 100644 index 0000000..ef26f7c --- /dev/null +++ b/chrome/test/base/save_desktop_snapshot_win.h
@@ -0,0 +1,15 @@ +// 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_TEST_BASE_SAVE_DESKTOP_SNAPSHOT_WIN_H_ +#define CHROME_TEST_BASE_SAVE_DESKTOP_SNAPSHOT_WIN_H_ + +#include "base/files/file_path.h" + +// Saves a snapshot of the desktop to a file in |output_dir|, returning the path +// to the file if created. An empty path is returned if no new snapshot is +// created. +base::FilePath SaveDesktopSnapshot(const base::FilePath& output_dir); + +#endif // CHROME_TEST_BASE_SAVE_DESKTOP_SNAPSHOT_WIN_H_
diff --git a/content/public/test/test_launcher.cc b/content/public/test/test_launcher.cc index 862fcba..cfaa8bc 100644 --- a/content/public/test/test_launcher.cc +++ b/content/public/test/test_launcher.cc
@@ -21,6 +21,7 @@ #include "base/logging.h" #include "base/macros.h" #include "base/message_loop/message_loop.h" +#include "base/sequence_checker.h" #include "base/stl_util.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" @@ -120,13 +121,6 @@ " Sets the shard index to run to N (from 0 to TOTAL - 1).\n"); } -void CallChildProcessLaunched(TestState* test_state, - base::ProcessHandle handle, - base::ProcessId pid) { - if (test_state) - test_state->ChildProcessLaunched(handle, pid); -} - // Implementation of base::TestLauncherDelegate. This is also a test launcher, // wrapping a lower-level test launcher with content-specific code. class WrapperTestLauncherDelegate : public base::TestLauncherDelegate { @@ -147,6 +141,58 @@ const std::vector<std::string>& test_names) override; private: + class ChildProcessLifetimeObserver : public base::ProcessLifetimeObserver { + public: + ChildProcessLifetimeObserver( + WrapperTestLauncherDelegate* test_launcher_delegate, + base::TestLauncher* test_launcher, + std::vector<std::string>&& next_test_names, + const std::string& test_name, + const base::FilePath& output_file, + std::unique_ptr<TestState> test_state) + : base::ProcessLifetimeObserver(), + test_launcher_delegate_(test_launcher_delegate), + test_launcher_(test_launcher), + next_test_names_(std::move(next_test_names)), + test_name_(test_name), + output_file_(output_file), + test_state_(std::move(test_state)) {} + ~ChildProcessLifetimeObserver() override { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + } + + private: + // base::ProcessLifetimeObserver: + void OnLaunched(base::ProcessHandle handle, base::ProcessId id) override { + if (test_state_) + test_state_->ChildProcessLaunched(handle, id); + } + + void OnTimedOut(const base::CommandLine& command_line) override { + test_launcher_delegate_->OnTestTimedOut(command_line); + } + + void OnCompleted(int exit_code, + base::TimeDelta elapsed_time, + bool was_timeout, + const std::string& output) override { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + test_launcher_delegate_->GTestCallback( + test_launcher_, next_test_names_, test_name_, output_file_, + std::move(test_state_), exit_code, elapsed_time, was_timeout, output); + } + + SEQUENCE_CHECKER(sequence_checker_); + WrapperTestLauncherDelegate* test_launcher_delegate_; + base::TestLauncher* test_launcher_; + std::vector<std::string> next_test_names_; + std::string test_name_; + base::FilePath output_file_; + std::unique_ptr<TestState> test_state_; + + DISALLOW_COPY_AND_ASSIGN(ChildProcessLifetimeObserver); + }; + void DoRunTests(base::TestLauncher* test_launcher, const std::vector<std::string>& test_names); @@ -156,6 +202,10 @@ const std::string test_name, const base::TestResult& pre_test_result); + // Relays timeout notification from the TestLauncher (by way of a + // ProcessLifetimeObserver) to the caller's content::TestLauncherDelegate. + void OnTestTimedOut(const base::CommandLine& command_line); + // Callback to receive result of a test. // |output_file| is a path to xml file written by test-launcher // child process. It contains information about test and failed @@ -398,14 +448,13 @@ char* browser_wrapper = getenv("BROWSER_WRAPPER"); - TestState* test_state = test_state_ptr.get(); + auto observer = std::make_unique<ChildProcessLifetimeObserver>( + this, test_launcher, std::move(test_names_copy), test_name, output_file, + std::move(test_state_ptr)); test_launcher->LaunchChildGTestProcess( new_cmd_line, browser_wrapper ? browser_wrapper : std::string(), TestTimeouts::action_max_timeout(), test_launch_options, - base::Bind(&WrapperTestLauncherDelegate::GTestCallback, - base::Unretained(this), test_launcher, test_names_copy, - test_name, output_file, base::Passed(&test_state_ptr)), - base::Bind(&CallChildProcessLaunched, base::Unretained(test_state))); + std::move(observer)); } void WrapperTestLauncherDelegate::RunDependentTest( @@ -432,6 +481,11 @@ } } +void WrapperTestLauncherDelegate::OnTestTimedOut( + const base::CommandLine& command_line) { + launcher_delegate_->OnTestTimedOut(command_line); +} + void WrapperTestLauncherDelegate::GTestCallback( base::TestLauncher* test_launcher, const std::vector<std::string>& test_names, @@ -526,11 +580,6 @@ return nullptr; } -void TestLauncherDelegate::OnDoneRunningTests() {} - -TestLauncherDelegate::~TestLauncherDelegate() { -} - int LaunchTests(TestLauncherDelegate* launcher_delegate, size_t parallel_jobs, int argc,
diff --git a/content/public/test/test_launcher.h b/content/public/test/test_launcher.h index 7a0adc4..9445b64 100644 --- a/content/public/test/test_launcher.h +++ b/content/public/test/test_launcher.h
@@ -62,13 +62,17 @@ // jobs. virtual void PreSharding() {} + // Invoked when a child process times out immediately before it is terminated. + // |command_line| is the command line of the child process. + virtual void OnTestTimedOut(const base::CommandLine& command_line) {} + // Called prior to returning from LaunchTests(). Gives the delegate a chance // to do cleanup before state created by TestLauncher has been destroyed (such // as the AtExitManager). - virtual void OnDoneRunningTests(); + virtual void OnDoneRunningTests() {} protected: - virtual ~TestLauncherDelegate(); + virtual ~TestLauncherDelegate() = default; }; // Launches tests using |launcher_delegate|. |parallel_jobs| is the number
diff --git a/third_party/WebKit/LayoutTests/LeakExpectations b/third_party/WebKit/LayoutTests/LeakExpectations index 55f65c99..a310ee1b 100644 --- a/third_party/WebKit/LayoutTests/LeakExpectations +++ b/third_party/WebKit/LayoutTests/LeakExpectations
@@ -44,6 +44,7 @@ # ----------------------------------------------------------------- crbug.com/762353 external/wpt/beacon/headers/header-referrer-no-referrer.html [ Leak Pass ] crbug.com/651742 external/wpt/content-security-policy/connect-src/connect-src-beacon-allowed.sub.html [ Leak Pass ] +crbug.com/780386 external/wpt/html/dom/reflection-grouping.html [ Leak Pass ] # ----------------------------------------------------------------- # Not revert suspected CL as jam@ request, expected to be fixed soon.
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index 688c022..a33d7d0 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -371,6 +371,15 @@ crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/block-in-inline-remove-004.xht [ Failure Pass ] crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/block-in-inline-remove-005.xht [ Failure Pass ] +# Inline: Abspos should start new block layout to compute static position +crbug.com/740993 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/block-non-replaced-width-001.xht [ Failure ] +crbug.com/740993 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-block-replaced-height-004.xht [ Failure ] +crbug.com/740993 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-block-replaced-height-005.xht [ Failure ] +crbug.com/740993 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-block-replaced-height-007.xht [ Failure ] +crbug.com/740993 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-height-004.xht [ Failure ] +crbug.com/740993 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-height-005.xht [ Failure ] +crbug.com/740993 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-height-007.xht [ Failure ] + # Block: margin collapsing. crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/root-box-001.xht [ Failure ] @@ -551,9 +560,7 @@ crbug.com/635619 virtual/layout_ng/fast/block/margin-collapse/webkit-margin-collapse-siblings.html [ Failure ] ### virtual/layout_ng/overflow -crbug.com/728378 virtual/layout_ng/overflow/overflow-basic-003.html [ Failure ] crbug.com/724701 virtual/layout_ng/overflow/overflow-basic-004.html [ Failure ] -crbug.com/728378 virtual/layout_ng/overflow/overflow-bug-chrome-ng-001.html [ Failure ] crbug.com/728378 virtual/layout_ng/overflow/overflow-position-003.html [ Failure ] # ====== LayoutNG-only failures until here ======
diff --git a/third_party/WebKit/LayoutTests/shadow-dom/resources/focus-utils.js b/third_party/WebKit/LayoutTests/shadow-dom/resources/focus-utils.js index 8992ed23..5f2c5e63 100644 --- a/third_party/WebKit/LayoutTests/shadow-dom/resources/focus-utils.js +++ b/third_party/WebKit/LayoutTests/shadow-dom/resources/focus-utils.js
@@ -1,84 +1,78 @@ 'use strict'; -function innermostActiveElement(element) -{ - element = element || document.activeElement; - if (isIFrameElement(element)) { - if (element.contentDocument.activeElement) - return innermostActiveElement(element.contentDocument.activeElement); - return element; - } - if (isShadowHost(element)) { - let shadowRoot = window.internals.oldestShadowRoot(element); - while (shadowRoot) { - if (shadowRoot.activeElement) - return innermostActiveElement(shadowRoot.activeElement); - shadowRoot = window.internals.youngerShadowRoot(shadowRoot); - } - } +function innermostActiveElement(element) { + element = element || document.activeElement; + if (isIFrameElement(element)) { + if (element.contentDocument.activeElement) + return innermostActiveElement(element.contentDocument.activeElement); return element; + } + if (isShadowHost(element)) { + let shadowRoot = window.internals.oldestShadowRoot(element); + while (shadowRoot) { + if (shadowRoot.activeElement) + return innermostActiveElement(shadowRoot.activeElement); + shadowRoot = window.internals.youngerShadowRoot(shadowRoot); + } + } + return element; } -function isInnermostActiveElement(path) -{ - const element = getNodeInComposedTree(path); - if (!element) - return false; - return element === innermostActiveElement(); +function isInnermostActiveElement(path) { + const element = getNodeInComposedTree(path); + if (!element) + return false; + return element === innermostActiveElement(); } -function shouldNavigateFocus(from, to, direction) -{ - const fromElement = getNodeInComposedTree(from); - if (!fromElement) - return false; +function shouldNavigateFocus(from, to, direction) { + const fromElement = getNodeInComposedTree(from); + if (!fromElement) + return false; - fromElement.focus(); - if (!isInnermostActiveElement(from)) - return false; + fromElement.focus(); + if (!isInnermostActiveElement(from)) + return false; - if (direction == 'forward') - navigateFocusForward(); - else - navigateFocusBackward(); + if (direction == 'forward') + navigateFocusForward(); + else + navigateFocusBackward(); - return true; + return true; } -function navigateFocusForward() -{ - if (window.eventSender) - eventSender.keyDown('\t'); +function navigateFocusForward() { + if (window.eventSender) + eventSender.keyDown('\t'); } -function navigateFocusBackward() -{ - if (window.eventSender) - eventSender.keyDown('\t', ['shiftKey']); +function navigateFocusBackward() { + if (window.eventSender) + eventSender.keyDown('\t', ['shiftKey']); } -function assert_focus_navigation(from, to, direction) -{ - const result = shouldNavigateFocus(from, to, direction); - assert_true(result, 'Failed to focus ' + from); - const message = 'Focus should move ' + direction + - ' from ' + from + ' to ' + to; - var toElement = getNodeInComposedTree(to); - assert_equals(innermostActiveElement(), toElement, message); +function assert_focus_navigation(from, to, direction) { + const result = shouldNavigateFocus(from, to, direction); + assert_true(result, 'Failed to focus ' + from); + const message = + 'Focus should move ' + direction + ' from ' + from + ' to ' + to; + var toElement = getNodeInComposedTree(to); + assert_equals(innermostActiveElement(), toElement, message); } -function assert_focus_navigation_forward(elements) -{ - assert_true(elements.length >= 2, - 'length of elements should be greater than or equal to 2.'); - for (var i = 0; i + 1 < elements.length; ++i) - assert_focus_navigation(elements[i], elements[i + 1], 'forward'); +function assert_focus_navigation_forward(elements) { + assert_true( + elements.length >= 2, + 'length of elements should be greater than or equal to 2.'); + for (var i = 0; i + 1 < elements.length; ++i) + assert_focus_navigation(elements[i], elements[i + 1], 'forward'); } -function assert_focus_navigation_backward(elements) -{ - assert_true(elements.length >= 2, - 'length of elements should be greater than or equal to 2.'); - for (var i = 0; i + 1 < elements.length; ++i) - assert_focus_navigation(elements[i], elements[i + 1], 'backward'); +function assert_focus_navigation_backward(elements) { + assert_true( + elements.length >= 2, + 'length of elements should be greater than or equal to 2.'); + for (var i = 0; i + 1 < elements.length; ++i) + assert_focus_navigation(elements[i], elements[i + 1], 'backward'); }
diff --git a/third_party/WebKit/LayoutTests/shadow-dom/resources/shadow-dom.js b/third_party/WebKit/LayoutTests/shadow-dom/resources/shadow-dom.js index d64bb7f..51bb0240 100644 --- a/third_party/WebKit/LayoutTests/shadow-dom/resources/shadow-dom.js +++ b/third_party/WebKit/LayoutTests/shadow-dom/resources/shadow-dom.js
@@ -1,11 +1,13 @@ -function removeWhiteSpaceOnlyTextNodes(node) -{ +function removeWhiteSpaceOnlyTextNodes(node) { for (var i = 0; i < node.childNodes.length; i++) { var child = node.childNodes[i]; - if (child.nodeType === Node.TEXT_NODE && child.nodeValue.trim().length == 0) { + if (child.nodeType === Node.TEXT_NODE && + child.nodeValue.trim().length == 0) { node.removeChild(child); i--; - } else if (child.nodeType === Node.ELEMENT_NODE || child.nodeType === Node.DOCUMENT_FRAGMENT_NODE) { + } else if ( + child.nodeType === Node.ELEMENT_NODE || + child.nodeType === Node.DOCUMENT_FRAGMENT_NODE) { removeWhiteSpaceOnlyTextNodes(child); } } @@ -15,60 +17,56 @@ } function convertTemplatesToShadowRootsWithin(node) { - var nodes = node.querySelectorAll("template"); - for (var i = 0; i < nodes.length; ++i) { - var template = nodes[i]; - var mode = template.getAttribute("data-mode"); - var delegatesFocus = template.hasAttribute("data-delegatesFocus"); - var parent = template.parentNode; - parent.removeChild(template); - var shadowRoot; - if (!mode || mode == 'v0'){ - shadowRoot = parent.createShadowRoot(); - } else { - shadowRoot = parent.attachShadow({'mode': mode, - 'delegatesFocus': delegatesFocus}); - } - var expose = template.getAttribute("data-expose-as"); - if (expose) - window[expose] = shadowRoot; - if (template.id) - shadowRoot.id = template.id; - var fragments = document.importNode(template.content, true); - shadowRoot.appendChild(fragments); - - convertTemplatesToShadowRootsWithin(shadowRoot); + var nodes = node.querySelectorAll('template'); + for (var i = 0; i < nodes.length; ++i) { + var template = nodes[i]; + var mode = template.getAttribute('data-mode'); + var delegatesFocus = template.hasAttribute('data-delegatesFocus'); + var parent = template.parentNode; + parent.removeChild(template); + var shadowRoot; + if (!mode || mode == 'v0') { + shadowRoot = parent.createShadowRoot(); + } else { + shadowRoot = + parent.attachShadow({'mode': mode, 'delegatesFocus': delegatesFocus}); } + var expose = template.getAttribute('data-expose-as'); + if (expose) + window[expose] = shadowRoot; + if (template.id) + shadowRoot.id = template.id; + var fragments = document.importNode(template.content, true); + shadowRoot.appendChild(fragments); + + convertTemplatesToShadowRootsWithin(shadowRoot); + } } -function isShadowHost(node) -{ - return node && node.nodeType == Node.ELEMENT_NODE && node.shadowRoot; +function isShadowHost(node) { + return node && node.nodeType == Node.ELEMENT_NODE && node.shadowRoot; } -function isIFrameElement(element) -{ - return element && element.nodeName == 'IFRAME'; +function isIFrameElement(element) { + return element && element.nodeName == 'IFRAME'; } // Returns node from shadow/iframe tree "path". -function getNodeInComposedTree(path) -{ - var ids = path.split('/'); - var node = document.getElementById(ids[0]); - for (var i = 1; node != null && i < ids.length; ++i) { - if (isIFrameElement(node)) - node = node.contentDocument.getElementById(ids[i]); - else if (isShadowHost(node)) - node = node.shadowRoot.getElementById(ids[i]); - else - return null; - } - return node; +function getNodeInComposedTree(path) { + var ids = path.split('/'); + var node = document.getElementById(ids[0]); + for (var i = 1; node != null && i < ids.length; ++i) { + if (isIFrameElement(node)) + node = node.contentDocument.getElementById(ids[i]); + else if (isShadowHost(node)) + node = node.shadowRoot.getElementById(ids[i]); + else + return null; + } + return node; } function createTestTree(node) { - let ids = {}; function attachShadowFromTemplate(template) { @@ -79,7 +77,8 @@ // For legacy Shadow DOM shadowRoot = parent.createShadowRoot(); } else { - shadowRoot = parent.attachShadow({mode: template.getAttribute('data-mode')}); + shadowRoot = + parent.attachShadow({mode: template.getAttribute('data-mode')}); } let id = template.id; if (id) { @@ -107,7 +106,6 @@ } function dispatchEventWithLog(nodes, target, event) { - function labelFor(e) { return e.id || e.tagName; } @@ -125,12 +123,13 @@ attachedNodes.push(node); node.addEventListener(event.type, (e) => { // Record [currentTarget, target, relatedTarget, composedPath()] - log.push([id, - labelFor(e.target), - e.relatedTarget ? labelFor(e.relatedTarget) : null, - e.composedPath().map((n) => { - return labelFor(n); - })]); + log.push([ + id, labelFor(e.target), + e.relatedTarget ? labelFor(e.relatedTarget) : null, + e.composedPath().map((n) => { + return labelFor(n); + }) + ]); }); } } @@ -140,7 +139,9 @@ function debugEventLog(log) { for (let i = 0; i < log.length; i++) { - console.log('[' + i + '] currentTarget: ' + log[i][0] + ' target: ' + log[i][1] + ' relatedTarget: ' + log[i][2] + ' composedPath(): ' + log[i][3]); + console.log( + '[' + i + '] currentTarget: ' + log[i][0] + ' target: ' + log[i][1] + + ' relatedTarget: ' + log[i][2] + ' composedPath(): ' + log[i][3]); } } @@ -154,15 +155,22 @@ function assert_event_path_equals(actual, expected) { assert_equals(actual.length, expected.length); for (let i = 0; i < actual.length; ++i) { - assert_equals(actual[i][0], expected[i][0], 'currentTarget at ' + i + ' should be same'); - assert_equals(actual[i][1], expected[i][1], 'target at ' + i + ' should be same'); - assert_equals(actual[i][2], expected[i][2], 'relatedTarget at ' + i + ' should be same'); - assert_array_equals(actual[i][3], expected[i][3], 'composedPath at ' + i + ' should be same'); + assert_equals( + actual[i][0], expected[i][0], + 'currentTarget at ' + i + ' should be same'); + assert_equals( + actual[i][1], expected[i][1], 'target at ' + i + ' should be same'); + assert_equals( + actual[i][2], expected[i][2], + 'relatedTarget at ' + i + ' should be same'); + assert_array_equals( + actual[i][3], expected[i][3], + 'composedPath at ' + i + ' should be same'); } } -function assert_background_color(path, color) -{ - assert_equals(window.getComputedStyle(getNodeInComposedTree(path)).backgroundColor, color, - 'backgroundColor for ' + path + ' should be ' + color); +function assert_background_color(path, color) { + assert_equals( + window.getComputedStyle(getNodeInComposedTree(path)).backgroundColor, + color, 'backgroundColor for ' + path + ' should be ' + color); }
diff --git a/third_party/WebKit/Source/core/editing/LayoutSelection.cpp b/third_party/WebKit/Source/core/editing/LayoutSelection.cpp index 948bc84..94c6be9 100644 --- a/third_party/WebKit/Source/core/editing/LayoutSelection.cpp +++ b/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
@@ -258,7 +258,7 @@ // above. for (LayoutObject* layout_object : old_invalidation_set) { const SelectionState old_state = layout_object->GetSelectionState(); - layout_object->SetSelectionStateIfNeeded(SelectionState::kNone); + layout_object->LayoutObject::SetSelectionState(SelectionState::kNone); if (layout_object->GetSelectionState() == old_state) continue; SetShouldInvalidateIfNeeds(layout_object); @@ -593,43 +593,15 @@ DCHECK(frame_selection_->GetDocument().GetLayoutView()->GetFrameView()); SetShouldInvalidateSelection(new_range, paint_range_); paint_range_ = new_paint_range; - // TODO(yoichio): Remove this if state. - // This SelectionState reassignment is ad-hoc patch for - // prohibiting use-after-free(crbug.com/752715). - // LayoutText::setSelectionState(state) propergates |state| to ancestor - // LayoutObjects, which can accidentally change start/end LayoutObject state - // then LayoutObject::IsSelectionBorder() returns false although we should - // clear selection at LayoutObject::WillBeRemoved(). - // We should make LayoutObject::setSelectionState() trivial and remove - // such propagation or at least do it in LayoutSelection. - if ((paint_range_.StartLayoutObject()->GetSelectionState() != - SelectionState::kStart && - paint_range_.StartLayoutObject()->GetSelectionState() != - SelectionState::kStartAndEnd) || - (paint_range_.EndLayoutObject()->GetSelectionState() != - SelectionState::kEnd && - paint_range_.EndLayoutObject()->GetSelectionState() != - SelectionState::kStartAndEnd)) { - if (paint_range_.StartLayoutObject() == paint_range_.EndLayoutObject()) { - paint_range_.StartLayoutObject()->SetSelectionStateIfNeeded( - SelectionState::kStartAndEnd); - } else { - paint_range_.StartLayoutObject()->SetSelectionStateIfNeeded( - SelectionState::kStart); - paint_range_.EndLayoutObject()->SetSelectionStateIfNeeded( - SelectionState::kEnd); - } + if (paint_range_.StartLayoutObject() == paint_range_.EndLayoutObject()) { + DCHECK_EQ(paint_range_.StartLayoutObject()->GetSelectionState(), + SelectionState::kStartAndEnd); + return; } - // TODO(yoichio): If start == end, they should be kStartAndEnd. - // If not, start.SelectionState == kStart and vice versa. - DCHECK(paint_range_.StartLayoutObject()->GetSelectionState() == - SelectionState::kStart || - paint_range_.StartLayoutObject()->GetSelectionState() == - SelectionState::kStartAndEnd); - DCHECK(paint_range_.EndLayoutObject()->GetSelectionState() == - SelectionState::kEnd || - paint_range_.EndLayoutObject()->GetSelectionState() == - SelectionState::kStartAndEnd); + DCHECK_EQ(paint_range_.StartLayoutObject()->GetSelectionState(), + SelectionState::kStart); + DCHECK_EQ(paint_range_.EndLayoutObject()->GetSelectionState(), + SelectionState::kEnd); } void LayoutSelection::OnDocumentShutdown() {
diff --git a/third_party/WebKit/Source/core/editing/LayoutSelectionTest.cpp b/third_party/WebKit/Source/core/editing/LayoutSelectionTest.cpp index 0074e14..32db53a 100644 --- a/third_party/WebKit/Source/core/editing/LayoutSelectionTest.cpp +++ b/third_party/WebKit/Source/core/editing/LayoutSelectionTest.cpp
@@ -266,10 +266,10 @@ .Build()); // This commit should not crash. Selection().CommitAppearanceIfNeeded(); - TEST_NEXT(IsLayoutBlock, kNone, NotInvalidate); + TEST_NEXT(IsLayoutBlock, kStartAndEnd, NotInvalidate); TEST_NEXT(IsLayoutBlockFlow, kStartAndEnd, NotInvalidate); TEST_NEXT("div1", kStartAndEnd, ShouldInvalidate); - TEST_NEXT(IsLayoutBlockFlow, kNone, NotInvalidate); + TEST_NEXT(IsLayoutBlockFlow, kStartAndEnd, NotInvalidate); TEST_NEXT("foo", kNone, NotInvalidate); TEST_NEXT(IsLayoutInline, kNone, NotInvalidate); TEST_NEXT("bar", kNone, ShouldInvalidate); @@ -364,8 +364,8 @@ .SetBaseAndExtent({baz, 2}, {baz, 3}) .Build()); Selection().CommitAppearanceIfNeeded(); - TEST_NEXT(IsLayoutBlock, kNone, NotInvalidate); - TEST_NEXT(IsLayoutBlock, kNone, NotInvalidate); + TEST_NEXT(IsLayoutBlock, kStartAndEnd, NotInvalidate); + TEST_NEXT(IsLayoutBlock, kStart, NotInvalidate); TEST_NEXT("foo", kNone, ShouldInvalidate); // TODO(yoichio): Invalidating next LayoutBlock is flaky but it doesn't // matter in wild because we don't paint selection gap. I will update
diff --git a/third_party/WebKit/Source/core/layout/LayoutBlock.cpp b/third_party/WebKit/Source/core/layout/LayoutBlock.cpp index 84f066f..c8888e529 100644 --- a/third_party/WebKit/Source/core/layout/LayoutBlock.cpp +++ b/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
@@ -794,7 +794,8 @@ if (!positioned_object->NormalChildNeedsLayout() && (relayout_children || height_available_to_children_changed_ || - NeedsLayoutDueToStaticPosition(positioned_object))) + (!IsLayoutNGBlockFlow() && + NeedsLayoutDueToStaticPosition(positioned_object)))) layout_scope.SetChildNeedsLayout(positioned_object); LayoutUnit logical_top_estimate;
diff --git a/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc b/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc index f76cdcd..8489fb6 100644 --- a/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc +++ b/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc
@@ -71,6 +71,13 @@ PlaceItems(line_info, *exclusion_space); } +TextDirection NGInlineLayoutAlgorithm::CurrentDirection( + TextDirection default_direction) const { + if (!box_states_) + return default_direction; + return box_states_->LineBoxState().style->Direction(); +} + void NGInlineLayoutAlgorithm::BidiReorder(NGInlineItemResults* line_items) { // TODO(kojii): UAX#9 L1 is not supported yet. Supporting L1 may change // embedding levels of parts of runs, which requires to split items. @@ -209,16 +216,10 @@ PlaceListMarker(item, &item_result, *line_info); DCHECK_GT(line_box_.size(), list_marker_index.value()); } else if (item.Type() == NGInlineItem::kOutOfFlowPositioned) { - // TODO(layout-dev): Report the correct static position for the out of - // flow descendant. We can't do this here yet as it doesn't know the - // size of the line box. - container_builder_.AddOutOfFlowDescendant( - // Absolute positioning blockifies the box's display type. - // https://drafts.csswg.org/css-display/#transformations - {NGBlockNode(ToLayoutBox(item.GetLayoutObject())), - NGStaticPosition::Create(ConstraintSpace().WritingMode(), - ConstraintSpace().Direction(), - NGPhysicalOffset())}); + NGBlockNode node(ToLayoutBox(item.GetLayoutObject())); + container_builder_.AddOutOfFlowChildCandidate( + node, NGLogicalOffset(position, LayoutUnit()), + CurrentDirection(line_info->BaseDirection())); continue; } else { continue; @@ -571,6 +572,11 @@ container_builder_.SwapPositionedFloats(&positioned_floats_); container_builder_.SetExclusionSpace(std::move(exclusion_space)); + Vector<NGOutOfFlowPositionedDescendant> descendant_candidates; + container_builder_.GetAndClearOutOfFlowDescendantCandidates( + &descendant_candidates); + for (auto& descendant : descendant_candidates) + container_builder_.AddOutOfFlowDescendant(descendant); return container_builder_.ToLineBoxFragment(); } @@ -589,14 +595,9 @@ ConstraintSpace().PercentageResolutionSize(), bfc_line_offset, bfc_line_offset, margins, node, /* break_token */ nullptr)); } else if (item.Type() == NGInlineItem::kOutOfFlowPositioned) { - // TODO(layout-dev): Report the correct static position for the out of - // flow descendant. We can't do this here yet as it doesn't know the - // size of the line box. NGBlockNode node(ToLayoutBox(item.GetLayoutObject())); - container_builder_.AddOutOfFlowDescendant( - {node, NGStaticPosition::Create(ConstraintSpace().WritingMode(), - ConstraintSpace().Direction(), - NGPhysicalOffset())}); + container_builder_.AddOutOfFlowChildCandidate( + node, NGLogicalOffset(), CurrentDirection(node.Style().Direction())); } else { DCHECK_NE(item.Type(), NGInlineItem::kAtomicInline); DCHECK_NE(item.Type(), NGInlineItem::kControl); @@ -617,6 +618,11 @@ container_builder_.SetEndMarginStrut(ConstraintSpace().MarginStrut()); container_builder_.SetExclusionSpace(std::move(exclusion_space)); + Vector<NGOutOfFlowPositionedDescendant> descendant_candidates; + container_builder_.GetAndClearOutOfFlowDescendantCandidates( + &descendant_candidates); + for (auto& descendant : descendant_candidates) + container_builder_.AddOutOfFlowDescendant(descendant); return container_builder_.ToLineBoxFragment(); }
diff --git a/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.h b/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.h index feeeb4f..ebb64fb8 100644 --- a/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.h +++ b/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.h
@@ -52,6 +52,8 @@ bool IsHorizontalWritingMode() const { return is_horizontal_writing_mode_; } + TextDirection CurrentDirection(TextDirection default_direction) const; + void BidiReorder(NGInlineItemResults*); void PlaceItems(NGLineInfo*, const NGExclusionSpace&);
diff --git a/third_party/WebKit/Source/core/layout/ng/ng_container_fragment_builder.cc b/third_party/WebKit/Source/core/layout/ng/ng_container_fragment_builder.cc index 4d1d048..b2840880 100644 --- a/third_party/WebKit/Source/core/layout/ng/ng_container_fragment_builder.cc +++ b/third_party/WebKit/Source/core/layout/ng/ng_container_fragment_builder.cc
@@ -22,6 +22,7 @@ NGContainerFragmentBuilder& NGContainerFragmentBuilder::SetInlineSize( LayoutUnit inline_size) { + DCHECK_GE(inline_size, LayoutUnit()); inline_size_ = inline_size; return *this; } @@ -54,10 +55,44 @@ scoped_refptr<NGLayoutResult> child, const NGLogicalOffset& child_offset) { // Collect the child's out of flow descendants. - for (const NGOutOfFlowPositionedDescendant& descendant : - child->OutOfFlowPositionedDescendants()) { - oof_positioned_candidates_.push_back( - NGOutOfFlowPositionedCandidate{descendant, child_offset}); + // child_offset is offset of inline_start/block_start vertex. + // Candidates need offset of top/left vertex. + const auto& ouf_of_flow_descendants = child->OutOfFlowPositionedDescendants(); + if (!ouf_of_flow_descendants.IsEmpty()) { + NGLogicalOffset top_left_offset; + NGPhysicalSize child_size = child->PhysicalFragment()->Size(); + switch (WritingMode()) { + case kHorizontalTopBottom: + top_left_offset = + (IsRtl(Direction())) + ? NGLogicalOffset{child_offset.inline_offset + child_size.width, + child_offset.block_offset} + : child_offset; + break; + case kVerticalRightLeft: + case kSidewaysRightLeft: + top_left_offset = + (IsRtl(Direction())) + ? NGLogicalOffset{child_offset.inline_offset + + child_size.height, + child_offset.block_offset + child_size.width} + : NGLogicalOffset{child_offset.inline_offset, + child_offset.block_offset + child_size.width}; + break; + case kVerticalLeftRight: + case kSidewaysLeftRight: + top_left_offset = (IsRtl(Direction())) + ? NGLogicalOffset{child_offset.inline_offset + + child_size.height, + child_offset.block_offset} + : child_offset; + break; + } + for (const NGOutOfFlowPositionedDescendant& descendant : + ouf_of_flow_descendants) { + oof_positioned_candidates_.push_back( + NGOutOfFlowPositionedCandidate{descendant, top_left_offset}); + } } return AddChild(child->PhysicalFragment(), child_offset); @@ -74,12 +109,14 @@ NGContainerFragmentBuilder& NGContainerFragmentBuilder::AddOutOfFlowChildCandidate( NGBlockNode child, - const NGLogicalOffset& child_offset) { + const NGLogicalOffset& child_offset, + Optional<TextDirection> direction) { DCHECK(child); oof_positioned_candidates_.push_back(NGOutOfFlowPositionedCandidate{ NGOutOfFlowPositionedDescendant{ - child, NGStaticPosition::Create(WritingMode(), Direction(), + child, NGStaticPosition::Create(WritingMode(), + direction ? *direction : Direction(), NGPhysicalOffset())}, child_offset});
diff --git a/third_party/WebKit/Source/core/layout/ng/ng_container_fragment_builder.h b/third_party/WebKit/Source/core/layout/ng/ng_container_fragment_builder.h index 9bb14c11..3592cad48 100644 --- a/third_party/WebKit/Source/core/layout/ng/ng_container_fragment_builder.h +++ b/third_party/WebKit/Source/core/layout/ng/ng_container_fragment_builder.h
@@ -84,9 +84,13 @@ // NGOutOfFlowLayoutPart(container_style, builder).Run(); // // See layout part for builder interaction. + // + // @param direction: default candidate direction is builder's direction. + // Pass in direction if candidates direction does not match. NGContainerFragmentBuilder& AddOutOfFlowChildCandidate( NGBlockNode, - const NGLogicalOffset&); + const NGLogicalOffset& child_offset, + Optional<TextDirection> direction = WTF::nullopt); NGContainerFragmentBuilder& AddOutOfFlowDescendant( NGOutOfFlowPositionedDescendant); @@ -112,7 +116,7 @@ // physical size the fragment builder. struct NGOutOfFlowPositionedCandidate { NGOutOfFlowPositionedDescendant descendant; - NGLogicalOffset child_offset; + NGLogicalOffset child_offset; // Logical offset of child's top left vertex. }; NGContainerFragmentBuilder(scoped_refptr<const ComputedStyle>,