diff options
Diffstat (limited to 'chromium/third_party/blink/renderer/core')
37 files changed, 554 insertions, 128 deletions
diff --git a/chromium/third_party/blink/renderer/core/BUILD.gn b/chromium/third_party/blink/renderer/core/BUILD.gn index 79a7f1d5010..6b856daa7e2 100644 --- a/chromium/third_party/blink/renderer/core/BUILD.gn +++ b/chromium/third_party/blink/renderer/core/BUILD.gn @@ -1452,6 +1452,7 @@ jumbo_source_set("unit_tests") { "page/slot_scoped_traversal_test.cc", "page/spatial_navigation_test.cc", "page/touch_adjustment_test.cc", + "page/validation_message_overlay_delegate_test.cc", "page/viewport_test.cc", "page/window_features_test.cc", "page/zoom_test.cc", diff --git a/chromium/third_party/blink/renderer/core/animation/animation_clock.cc b/chromium/third_party/blink/renderer/core/animation/animation_clock.cc index 1c8ae77d3e0..19ba57ea1e9 100644 --- a/chromium/third_party/blink/renderer/core/animation/animation_clock.cc +++ b/chromium/third_party/blink/renderer/core/animation/animation_clock.cc @@ -35,7 +35,18 @@ namespace blink { +namespace { +// This is an approximation of time between frames, used when ticking the +// animation clock outside of animation frame callbacks. +constexpr base::TimeDelta kApproximateFrameTime = + base::TimeDelta::FromSecondsD(1 / 60.0); +} // namespace + +unsigned AnimationClock::currently_running_task_ = 0; + void AnimationClock::UpdateTime(base::TimeTicks time) { + task_for_which_time_was_calculated_ = currently_running_task_; + // TODO(crbug.com/985770): Change this to a DCHECK_GE(time, time_) when // VR no longer sends historical timestamps. if (time < time_) @@ -43,7 +54,35 @@ void AnimationClock::UpdateTime(base::TimeTicks time) { time_ = time; } -base::TimeTicks AnimationClock::CurrentTime() const { +base::TimeTicks AnimationClock::CurrentTime() { + // By spec, within a single rendering lifecycle the AnimationClock time should + // not change (as it is set from the frame time). + if (!can_dynamically_update_time_) + return time_; + + // Outside of the rendering lifecycle, we may have to dynamically advance our + // own time (see comments on |SetAllowedToDynamicallyUpdateTime|). However we + // should never dynamically advance time inside a single task, as otherwise a + // single long-running JavaScript function could see multiple different times + // from document.timeline.currentTime. + if (task_for_which_time_was_calculated_ == currently_running_task_) + return time_; + + // Otherwise, we may need to dynamically update our own time. Again see the + // comments on |SetAllowedToDynamicallyUpdateTime|. + const base::TimeTicks current_time = clock_->NowTicks(); + base::TimeTicks new_time = time_; + if (time_ < current_time) { + // Attempt to predict what the most recent timestamp would have been. This + // may not produce a result greater than |time_|, but it greatly reduces the + // chance of conflicting with any future frame timestamp that does come in. + const base::TimeDelta frame_shift = + (current_time - time_) % kApproximateFrameTime; + new_time = current_time - frame_shift; + DCHECK_GE(new_time, time_); + } + UpdateTime(new_time); + return time_; } @@ -51,4 +90,11 @@ void AnimationClock::ResetTimeForTesting() { time_ = base::TimeTicks(); } +void AnimationClock::OverrideDynamicClockForTesting( + const base::TickClock* clock) { + clock_ = clock; + ResetTimeForTesting(); + UpdateTime(clock_->NowTicks()); +} + } // namespace blink diff --git a/chromium/third_party/blink/renderer/core/animation/animation_clock.h b/chromium/third_party/blink/renderer/core/animation/animation_clock.h index b780210abdb..8f10d8b6852 100644 --- a/chromium/third_party/blink/renderer/core/animation/animation_clock.h +++ b/chromium/third_party/blink/renderer/core/animation/animation_clock.h @@ -34,6 +34,7 @@ #include <limits> #include "base/macros.h" +#include "base/time/default_tick_clock.h" #include "third_party/blink/renderer/core/core_export.h" #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h" @@ -46,14 +47,57 @@ class CORE_EXPORT AnimationClock { DISALLOW_NEW(); public: - AnimationClock() : time_() {} + AnimationClock() + : time_(), + can_dynamically_update_time_(false), + clock_(base::DefaultTickClock::GetInstance()), + task_for_which_time_was_calculated_( + std::numeric_limits<unsigned>::max()) {} void UpdateTime(base::TimeTicks time); - base::TimeTicks CurrentTime() const; + base::TimeTicks CurrentTime(); + + // The HTML spec says that the clock for animations is only updated once per + // rendering lifecycle, at the start. However the spec also assumes that the + // user agent runs rendering lifecycles constantly, back-to-back. In Blink we + // attempt to *not* run rendering lifecycles as much as possible, to avoid + // unnecessary CPU usage. + // + // As such, when outside a rendering lifecycle (for example, if a setInterval + // triggers) we allow the AnimationClock to dynamically adjust its time to + // look like it is being updated by the rendering lifecycles that never + // happened. + // + // TODO(crbug.com/995806): Allowing the AnimationClock to update itself is + // error prone. We should instead get the latest impl-frame time from the + // compositor when outside of a Blink rendering lifecycle (whilst still + // not changing within the same task). + void SetAllowedToDynamicallyUpdateTime(bool can_dynamically_update_time) { + can_dynamically_update_time_ = can_dynamically_update_time; + } + + // When using our dynamically update behavior outside rendering lifecycles, we + // still do not want the time to move forward within the same task (e.g. + // within a single setInterval callback). To achieve this we track the task in + // which the time was last updated, and don't update it again until we are in + // a new task. + static void NotifyTaskStart() { ++currently_running_task_; } + void ResetTimeForTesting(); + // The caller owns the passed in clock, which must outlive the AnimationClock. + void OverrideDynamicClockForTesting(const base::TickClock*); private: base::TimeTicks time_; + + // See |SetAllowedToDynamicallyUpdateTime| documentation for these members. + bool can_dynamically_update_time_; + const base::TickClock* clock_; + + // See |NotifyTaskStart| documentation for these members. + unsigned task_for_which_time_was_calculated_; + static unsigned currently_running_task_; + DISALLOW_COPY_AND_ASSIGN(AnimationClock); }; diff --git a/chromium/third_party/blink/renderer/core/animation/compositor_animations.cc b/chromium/third_party/blink/renderer/core/animation/compositor_animations.cc index be4f5e1fa50..a467082f74f 100644 --- a/chromium/third_party/blink/renderer/core/animation/compositor_animations.cc +++ b/chromium/third_party/blink/renderer/core/animation/compositor_animations.cc @@ -43,6 +43,7 @@ #include "third_party/blink/renderer/core/animation/element_animations.h" #include "third_party/blink/renderer/core/animation/keyframe_effect_model.h" #include "third_party/blink/renderer/core/dom/dom_node_ids.h" +#include "third_party/blink/renderer/core/frame/settings.h" #include "third_party/blink/renderer/core/layout/layout_box_model_object.h" #include "third_party/blink/renderer/core/layout/layout_object.h" #include "third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.h" @@ -343,8 +344,15 @@ CompositorAnimations::CheckCanStartElementOnCompositor( const Element& target_element) { FailureReasons reasons = kNoFailure; - if (!Platform::Current()->IsThreadedAnimationEnabled()) + // Both of these checks are required. It is legal to enable the compositor + // thread but disable threaded animations, and there are situations where + // threaded animations are enabled globally but this particular LocalFrame + // does not have a compositor (e.g. for overlays). + const Settings* settings = target_element.GetDocument().GetSettings(); + if ((settings && !settings->GetAcceleratedCompositingEnabled()) || + !Platform::Current()->IsThreadedAnimationEnabled()) { reasons |= kAcceleratedAnimationsDisabled; + } if (const auto* layout_object = target_element.GetLayoutObject()) { // We query paint property tree state below to determine whether the diff --git a/chromium/third_party/blink/renderer/core/animation/compositor_animations_test.cc b/chromium/third_party/blink/renderer/core/animation/compositor_animations_test.cc index 358dfd06f0b..bfaae4f10db 100644 --- a/chromium/third_party/blink/renderer/core/animation/compositor_animations_test.cc +++ b/chromium/third_party/blink/renderer/core/animation/compositor_animations_test.cc @@ -1998,4 +1998,25 @@ TEST_P(AnimationCompositorAnimationsTest, EXPECT_EQ(host->CompositedAnimationsCount(), 0u); } +// Regression test for https://crbug.com/999333. We were relying on the Document +// always having Settings, which will not be the case if it is not attached to a +// Frame. +TEST_P(AnimationCompositorAnimationsTest, + DocumentWithoutSettingShouldNotCauseCrash) { + SetBodyInnerHTML("<div id='target'></div>"); + Element* target = GetElementById("target"); + ASSERT_TRUE(target); + + // Move the target element to another Document, that does not have a frame + // (and thus no Settings). + Document* another_document = MakeGarbageCollected<Document>(); + ASSERT_FALSE(another_document->GetSettings()); + + another_document->adoptNode(target, ASSERT_NO_EXCEPTION); + + // This should not crash. + EXPECT_NE(CheckCanStartElementOnCompositor(*target), + CompositorAnimations::kNoFailure); +} + } // namespace blink diff --git a/chromium/third_party/blink/renderer/core/animation/document_timeline.cc b/chromium/third_party/blink/renderer/core/animation/document_timeline.cc index 7c19e649a3b..b6b0268ecd9 100644 --- a/chromium/third_party/blink/renderer/core/animation/document_timeline.cc +++ b/chromium/third_party/blink/renderer/core/animation/document_timeline.cc @@ -56,7 +56,7 @@ bool CompareAnimations(const Member<Animation>& left, // Returns the current animation time for a given |document|. This is // the animation clock time capped to be at least this document's // ZeroTime() such that the animation time is never negative when converted. -base::TimeTicks CurrentAnimationTime(const Document* document) { +base::TimeTicks CurrentAnimationTime(Document* document) { base::TimeTicks animation_time = document->GetAnimationClock().CurrentTime(); base::TimeTicks document_zero_time = document->Timeline().ZeroTime(); diff --git a/chromium/third_party/blink/renderer/core/animation/document_timeline_test.cc b/chromium/third_party/blink/renderer/core/animation/document_timeline_test.cc index d32736e9008..801ba257b80 100644 --- a/chromium/third_party/blink/renderer/core/animation/document_timeline_test.cc +++ b/chromium/third_party/blink/renderer/core/animation/document_timeline_test.cc @@ -30,6 +30,7 @@ #include "third_party/blink/renderer/core/animation/document_timeline.h" +#include "base/test/simple_test_tick_clock.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/blink/renderer/core/animation/animation_clock.h" @@ -68,6 +69,7 @@ class AnimationDocumentTimelineTest : public PageTestBase { PageTestBase::SetUp(IntSize()); document = &GetDocument(); GetAnimationClock().ResetTimeForTesting(); + GetAnimationClock().SetAllowedToDynamicallyUpdateTime(false); element = Element::Create(QualifiedName::Null(), document.Get()); platform_timing = MakeGarbageCollected<MockPlatformTiming>(); timeline = document->Timeline(); @@ -113,6 +115,7 @@ class AnimationDocumentTimelineRealTimeTest : public PageTestBase { PageTestBase::SetUp(IntSize()); document = &GetDocument(); timeline = document->Timeline(); + GetAnimationClock().SetAllowedToDynamicallyUpdateTime(false); } void TearDown() override { @@ -454,6 +457,39 @@ TEST_F(AnimationDocumentTimelineTest, PlayAfterDocumentDeref) { timeline->Play(keyframe_effect); } +// Regression test for https://crbug.com/995806, ensuring that we do dynamically +// progress the time when outside a rendering loop (so that we can serve e.g. +// setInterval), but also that we *only* dynamically progress the time when +// outside a rendering loop (so that we are mostly spec compliant). +TEST_F(AnimationDocumentTimelineTest, + PredictionBehaviorOnlyAppliesOutsideRenderingLoop) { + base::SimpleTestTickClock test_clock; + GetAnimationClock().OverrideDynamicClockForTesting(&test_clock); + ASSERT_EQ(GetAnimationClock().CurrentTime(), test_clock.NowTicks()); + + // As long as we are inside the rendering loop, we shouldn't update even + // across tasks. + base::TimeTicks before_time = GetAnimationClock().CurrentTime(); + test_clock.Advance(base::TimeDelta::FromSeconds(1)); + EXPECT_EQ(GetAnimationClock().CurrentTime(), before_time); + + AnimationClock::NotifyTaskStart(); + test_clock.Advance(base::TimeDelta::FromSeconds(1)); + EXPECT_EQ(GetAnimationClock().CurrentTime(), before_time); + + // Once we leave the rendering loop, however, it is valid for the time to + // increase *once* per task. + GetAnimationClock().SetAllowedToDynamicallyUpdateTime(true); + EXPECT_GT(GetAnimationClock().CurrentTime(), before_time); + + // The clock shouldn't tick again until we change task, however. + base::TimeTicks current_time = GetAnimationClock().CurrentTime(); + test_clock.Advance(base::TimeDelta::FromSeconds(1)); + EXPECT_EQ(GetAnimationClock().CurrentTime(), current_time); + AnimationClock::NotifyTaskStart(); + EXPECT_GT(GetAnimationClock().CurrentTime(), current_time); +} + // Ensure that origin time is correctly calculated even when the animation // clock has not yet been initialized. TEST_F(AnimationDocumentTimelineRealTimeTest, diff --git a/chromium/third_party/blink/renderer/core/fileapi/file_reader_loader.cc b/chromium/third_party/blink/renderer/core/fileapi/file_reader_loader.cc index c63a32f28bb..27824b365c4 100644 --- a/chromium/third_party/blink/renderer/core/fileapi/file_reader_loader.cc +++ b/chromium/third_party/blink/renderer/core/fileapi/file_reader_loader.cc @@ -148,10 +148,12 @@ DOMArrayBuffer* FileReaderLoader::ArrayBufferResult() { if (!finished_loading_) { return DOMArrayBuffer::Create(ArrayBuffer::Create( - raw_data_->Data(), static_cast<unsigned>(bytes_loaded_))); + raw_data_.Data(), static_cast<unsigned>(bytes_loaded_))); } - array_buffer_result_ = DOMArrayBuffer::Create(std::move(raw_data_)); + WTF::ArrayBufferContents contents(std::move(raw_data_), + WTF::ArrayBufferContents::kNotShared); + array_buffer_result_ = DOMArrayBuffer::Create(contents); AdjustReportedMemoryUsageToV8(-1 * static_cast<int64_t>(bytes_loaded_)); raw_data_.reset(); return array_buffer_result_; @@ -171,7 +173,7 @@ String FileReaderLoader::StringResult() { // No conversion is needed. return string_result_; case kReadAsBinaryString: - SetStringResult(String(static_cast<const char*>(raw_data_->Data()), + SetStringResult(String(static_cast<const char*>(raw_data_.Data()), static_cast<size_t>(bytes_loaded_))); break; case kReadAsText: @@ -194,6 +196,17 @@ String FileReaderLoader::StringResult() { return string_result_; } +WTF::ArrayBufferContents::DataHandle FileReaderLoader::TakeDataHandle() { + if (!raw_data_ || error_code_ != FileErrorCode::kOK) + return WTF::ArrayBufferContents::DataHandle(); + + DCHECK(finished_loading_); + WTF::ArrayBufferContents::DataHandle handle = std::move(raw_data_); + AdjustReportedMemoryUsageToV8(-1 * static_cast<int64_t>(bytes_loaded_)); + raw_data_.reset(); + return handle; +} + void FileReaderLoader::SetEncoding(const String& encoding) { if (!encoding.IsEmpty()) encoding_ = WTF::TextEncoding(encoding); @@ -242,7 +255,9 @@ void FileReaderLoader::OnStartLoading(uint64_t total_bytes) { return; } - raw_data_ = ArrayBuffer::Create(static_cast<unsigned>(total_bytes), 1); + raw_data_ = WTF::ArrayBufferContents::CreateDataHandle( + static_cast<unsigned>(total_bytes), + WTF::ArrayBufferContents::kDontInitialize); if (!raw_data_) { Failed(FileErrorCode::kNotReadableErr, FailureType::kArrayBufferBuilderCreation); @@ -274,14 +289,14 @@ void FileReaderLoader::OnReceivedData(const char* data, unsigned data_length) { // that the BlobPtr is actually backed by a "real" blob, so to defend against // compromised renderer processes we still need to carefully validate anything // received. So return an error if we received too much data. - if (bytes_loaded_ + data_length > raw_data_->ByteLength()) { + if (bytes_loaded_ + data_length > raw_data_.DataLength()) { raw_data_.reset(); bytes_loaded_ = 0; Failed(FileErrorCode::kNotReadableErr, FailureType::kArrayBufferBuilderAppend); return; } - memcpy(static_cast<char*>(raw_data_->Data()) + bytes_loaded_, data, + memcpy(static_cast<char*>(raw_data_.Data()) + bytes_loaded_, data, data_length); bytes_loaded_ += data_length; is_raw_data_converted_ = false; @@ -293,7 +308,7 @@ void FileReaderLoader::OnReceivedData(const char* data, unsigned data_length) { void FileReaderLoader::OnFinishLoading() { if (read_type_ != kReadByClient && raw_data_) { - DCHECK_EQ(bytes_loaded_, raw_data_->ByteLength()); + DCHECK_EQ(bytes_loaded_, raw_data_.DataLength()); is_raw_data_converted_ = false; } @@ -436,7 +451,7 @@ String FileReaderLoader::ConvertToText() { TextResourceDecoderOptions::kPlainTextContent, encoding_.IsValid() ? encoding_ : UTF8Encoding())); } - builder.Append(decoder_->Decode(static_cast<const char*>(raw_data_->Data()), + builder.Append(decoder_->Decode(static_cast<const char*>(raw_data_.Data()), static_cast<size_t>(bytes_loaded_))); if (finished_loading_) @@ -462,7 +477,7 @@ String FileReaderLoader::ConvertToDataURL() { builder.Append(";base64,"); Vector<char> out; - Base64Encode(base::make_span(static_cast<const uint8_t*>(raw_data_->Data()), + Base64Encode(base::make_span(static_cast<const uint8_t*>(raw_data_.Data()), SafeCast<unsigned>(bytes_loaded_)), out); builder.Append(out.data(), out.size()); diff --git a/chromium/third_party/blink/renderer/core/fileapi/file_reader_loader.h b/chromium/third_party/blink/renderer/core/fileapi/file_reader_loader.h index d460a3f50c9..5bf738e4b2b 100644 --- a/chromium/third_party/blink/renderer/core/fileapi/file_reader_loader.h +++ b/chromium/third_party/blink/renderer/core/fileapi/file_reader_loader.h @@ -44,6 +44,7 @@ #include "third_party/blink/renderer/platform/wtf/text/text_encoding.h" #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h" #include "third_party/blink/renderer/platform/wtf/typed_arrays/array_buffer.h" +#include "third_party/blink/renderer/platform/wtf/typed_arrays/array_buffer_contents.h" namespace blink { @@ -85,6 +86,7 @@ class CORE_EXPORT FileReaderLoader : public mojom::blink::BlobReaderClient { DOMArrayBuffer* ArrayBufferResult(); String StringResult(); + WTF::ArrayBufferContents::DataHandle TakeDataHandle(); // Returns the total bytes received. Bytes ignored by m_rawData won't be // counted. @@ -154,7 +156,7 @@ class CORE_EXPORT FileReaderLoader : public mojom::blink::BlobReaderClient { WTF::TextEncoding encoding_; String data_type_; - scoped_refptr<ArrayBuffer> raw_data_; + WTF::ArrayBufferContents::DataHandle raw_data_; bool is_raw_data_converted_ = false; Persistent<DOMArrayBuffer> array_buffer_result_; diff --git a/chromium/third_party/blink/renderer/core/frame/frame_overlay.cc b/chromium/third_party/blink/renderer/core/frame/frame_overlay.cc index 775930a98f0..f39011767f9 100644 --- a/chromium/third_party/blink/renderer/core/frame/frame_overlay.cc +++ b/chromium/third_party/blink/renderer/core/frame/frame_overlay.cc @@ -119,6 +119,11 @@ void FrameOverlay::PaintContents(const GraphicsLayer* graphics_layer, Paint(context); } +void FrameOverlay::ServiceScriptedAnimations( + base::TimeTicks monotonic_frame_begin_time) { + delegate_->ServiceScriptedAnimations(monotonic_frame_begin_time); +} + String FrameOverlay::DebugName(const GraphicsLayer*) const { DCHECK(!RuntimeEnabledFeatures::CompositeAfterPaintEnabled()); return "Frame Overlay Content Layer"; diff --git a/chromium/third_party/blink/renderer/core/frame/frame_overlay.h b/chromium/third_party/blink/renderer/core/frame/frame_overlay.h index 267312c66de..ae0df9d1353 100644 --- a/chromium/third_party/blink/renderer/core/frame/frame_overlay.h +++ b/chromium/third_party/blink/renderer/core/frame/frame_overlay.h @@ -56,6 +56,10 @@ class CORE_EXPORT FrameOverlay : public GraphicsLayerClient, // For CompositeAfterPaint. Invalidates composited layers managed by the // delegate if any. virtual void Invalidate() {} + + // Service any animations managed by the delegate. + virtual void ServiceScriptedAnimations( + base::TimeTicks monotonic_frame_begin_time) {} }; FrameOverlay(LocalFrame*, std::unique_ptr<FrameOverlay::Delegate>); @@ -76,6 +80,9 @@ class CORE_EXPORT FrameOverlay : public GraphicsLayerClient, const Delegate* GetDelegate() const { return delegate_.get(); } const LocalFrame& Frame() const { return *frame_; } + // Services any animations that the overlay may be managing. + void ServiceScriptedAnimations(base::TimeTicks monotonic_frame_begin_time); + // DisplayItemClient methods. String DebugName() const final { return "FrameOverlay"; } IntRect VisualRect() const override; diff --git a/chromium/third_party/blink/renderer/core/frame/local_frame.cc b/chromium/third_party/blink/renderer/core/frame/local_frame.cc index 1f5407c50ed..bcc0869cc2f 100644 --- a/chromium/third_party/blink/renderer/core/frame/local_frame.cc +++ b/chromium/third_party/blink/renderer/core/frame/local_frame.cc @@ -1661,6 +1661,10 @@ void LocalFrame::UnpauseContext() { } void LocalFrame::SetLifecycleState(mojom::FrameLifecycleState state) { + // Don't allow lifecycle state changes for detached frames. + if (!IsAttached()) + return; + // If we have asked to be frozen we will only do this once the // load event has fired. if ((state == mojom::FrameLifecycleState::kFrozen || @@ -1688,13 +1692,21 @@ void LocalFrame::SetLifecycleState(mojom::FrameLifecycleState state) { lifecycle_state_ = state; if (freeze) { - if (lifecycle_state_ != mojom::FrameLifecycleState::kPaused) + if (lifecycle_state_ != mojom::FrameLifecycleState::kPaused) { DidFreeze(); + // DidFreeze can dispatch JS events, causing |this| to be detached. + if (!IsAttached()) + return; + } PauseContext(); } else { UnpauseContext(); - if (old_state != mojom::FrameLifecycleState::kPaused) + if (old_state != mojom::FrameLifecycleState::kPaused) { DidResume(); + // DidResume can dispatch JS events, causing |this| to be detached. + if (!IsAttached()) + return; + } } if (Client()) Client()->LifecycleStateChanged(state); diff --git a/chromium/third_party/blink/renderer/core/html/lazy_load_image_observer_test.cc b/chromium/third_party/blink/renderer/core/html/lazy_load_image_observer_test.cc index 096a501994e..00fa7e0abcb 100644 --- a/chromium/third_party/blink/renderer/core/html/lazy_load_image_observer_test.cc +++ b/chromium/third_party/blink/renderer/core/html/lazy_load_image_observer_test.cc @@ -907,6 +907,7 @@ class LazyLoadAutomaticImagesTest : public SimTest { ScrollOffset(0, kLoadingDistanceThreshold + kViewportHeight), kProgrammaticScroll); Compositor().BeginFrame(); + test::RunPendingTasks(); full_resource.Complete(ReadTestImage()); ExpectResourceIsFullImage(GetDocument().Fetcher()->CachedResource( KURL("https://example.com/image.png"))); @@ -1349,6 +1350,7 @@ TEST_F(LazyLoadAutomaticImagesTest, LazyLoadDisabledOnReload) { LazyLoadAutomaticImagesTest::kViewportHeight), kProgrammaticScroll); Compositor().BeginFrame(); + test::RunPendingTasks(); auto_image.Complete(ReadTestImage()); lazy_image.Complete(ReadTestImage()); test::RunPendingTasks(); @@ -1376,6 +1378,7 @@ TEST_F(LazyLoadAutomaticImagesTest, LazyLoadDisabledOnReload) { SimSubresourceRequest lazy_image("https://example.com/image_lazy.png", "image/png"); Compositor().BeginFrame(); + test::RunPendingTasks(); lazy_image.Complete(ReadTestImage()); test::RunPendingTasks(); histogram_tester.ExpectTotalCount( diff --git a/chromium/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.cc b/chromium/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.cc index 338eabd4ad2..0f73d79e946 100644 --- a/chromium/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.cc +++ b/chromium/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.cc @@ -49,7 +49,6 @@ #include "third_party/blink/renderer/core/svg/svg_image_element.h" #include "third_party/blink/renderer/core/workers/worker_global_scope.h" #include "third_party/blink/renderer/platform/bindings/exception_state.h" -#include "third_party/blink/renderer/platform/image-decoders/image_decoder.h" #include "third_party/blink/renderer/platform/instrumentation/histogram.h" #include "third_party/blink/renderer/platform/instrumentation/use_counter.h" #include "third_party/blink/renderer/platform/scheduler/public/post_cross_thread_task.h" @@ -298,64 +297,67 @@ void ImageBitmapFactories::ImageBitmapLoader::ContextDestroyed( } void ImageBitmapFactories::ImageBitmapLoader::DidFinishLoading() { - DOMArrayBuffer* array_buffer = loader_->ArrayBufferResult(); + auto data_handle = loader_->TakeDataHandle(); loader_.reset(); - if (!array_buffer) { + if (!data_handle) { RejectPromise(kAllocationFailureImageBitmapRejectionReason); return; } - ScheduleAsyncImageBitmapDecoding(array_buffer); + ScheduleAsyncImageBitmapDecoding(std::move(data_handle)); } void ImageBitmapFactories::ImageBitmapLoader::DidFail(FileErrorCode) { RejectPromise(kUndecodableImageBitmapRejectionReason); } -void ImageBitmapFactories::ImageBitmapLoader::ScheduleAsyncImageBitmapDecoding( - DOMArrayBuffer* array_buffer) { - scoped_refptr<base::SingleThreadTaskRunner> task_runner = - Thread::Current()->GetTaskRunner(); - worker_pool::PostTask( - FROM_HERE, - CrossThreadBindOnce( - &ImageBitmapFactories::ImageBitmapLoader::DecodeImageOnDecoderThread, - WrapCrossThreadPersistent(this), std::move(task_runner), - WrapCrossThreadPersistent(array_buffer), options_->premultiplyAlpha(), - options_->colorSpaceConversion())); -} - -void ImageBitmapFactories::ImageBitmapLoader::DecodeImageOnDecoderThread( +namespace { +void DecodeImageOnDecoderThread( scoped_refptr<base::SingleThreadTaskRunner> task_runner, - DOMArrayBuffer* array_buffer, - const String& premultiply_alpha_option, - const String& color_space_conversion_option) { - DCHECK(!IsMainThread()); - - ImageDecoder::AlphaOption alpha_op = ImageDecoder::kAlphaPremultiplied; - if (premultiply_alpha_option == "none") - alpha_op = ImageDecoder::kAlphaNotPremultiplied; - bool ignore_color_space = false; - if (color_space_conversion_option == "none") - ignore_color_space = true; + WTF::ArrayBufferContents::DataHandle data_handle, + ImageDecoder::AlphaOption alpha_option, + ColorBehavior color_behavior, + WTF::CrossThreadOnceFunction<void(sk_sp<SkImage>)> result_callback) { const bool data_complete = true; std::unique_ptr<ImageDecoder> decoder = ImageDecoder::Create( SegmentReader::CreateFromSkData(SkData::MakeWithoutCopy( - array_buffer->Data(), array_buffer->ByteLength())), - data_complete, alpha_op, ImageDecoder::kDefaultBitDepth, - ignore_color_space ? ColorBehavior::Ignore() : ColorBehavior::Tag()); + data_handle.Data(), data_handle.DataLength())), + data_complete, alpha_option, ImageDecoder::kDefaultBitDepth, + color_behavior); sk_sp<SkImage> frame; if (decoder) { frame = ImageBitmap::GetSkImageFromDecoder(std::move(decoder)); } PostCrossThreadTask( *task_runner, FROM_HERE, - CrossThreadBindOnce(&ImageBitmapFactories::ImageBitmapLoader:: - ResolvePromiseOnOriginalThread, - WrapCrossThreadPersistent(this), std::move(frame))); + CrossThreadBindOnce(std::move(result_callback), std::move(frame))); +} +} // namespace + +void ImageBitmapFactories::ImageBitmapLoader::ScheduleAsyncImageBitmapDecoding( + WTF::ArrayBufferContents::DataHandle data_handle) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + scoped_refptr<base::SingleThreadTaskRunner> task_runner = + Thread::Current()->GetTaskRunner(); + ImageDecoder::AlphaOption alpha_option = + options_->premultiplyAlpha() != "none" + ? ImageDecoder::AlphaOption::kAlphaPremultiplied + : ImageDecoder::AlphaOption::kAlphaNotPremultiplied; + ColorBehavior color_behavior = options_->colorSpaceConversion() == "none" + ? ColorBehavior::Ignore() + : ColorBehavior::Tag(); + worker_pool::PostTask( + FROM_HERE, + CrossThreadBindOnce( + DecodeImageOnDecoderThread, std::move(task_runner), + std::move(data_handle), alpha_option, color_behavior, + CrossThreadBindOnce(&ImageBitmapFactories::ImageBitmapLoader:: + ResolvePromiseOnOriginalThread, + WrapCrossThreadWeakPersistent(this)))); } void ImageBitmapFactories::ImageBitmapLoader::ResolvePromiseOnOriginalThread( sk_sp<SkImage> frame) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); if (!frame) { RejectPromise(kUndecodableImageBitmapRejectionReason); return; diff --git a/chromium/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.h b/chromium/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.h index 503364df46b..465ccbfa787 100644 --- a/chromium/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.h +++ b/chromium/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.h @@ -47,6 +47,7 @@ #include "third_party/blink/renderer/platform/bindings/name_client.h" #include "third_party/blink/renderer/platform/bindings/script_state.h" #include "third_party/blink/renderer/platform/geometry/int_rect.h" +#include "third_party/blink/renderer/platform/image-decoders/image_decoder.h" #include "third_party/blink/renderer/platform/supplementable.h" #include "third_party/skia/include/core/SkRefCnt.h" @@ -129,6 +130,8 @@ class ImageBitmapFactories final ~ImageBitmapLoader() override; private: + SEQUENCE_CHECKER(sequence_checker_); + enum ImageBitmapRejectionReason { kUndecodableImageBitmapRejectionReason, kAllocationFailureImageBitmapRejectionReason, @@ -136,12 +139,7 @@ class ImageBitmapFactories final void RejectPromise(ImageBitmapRejectionReason); - void ScheduleAsyncImageBitmapDecoding(DOMArrayBuffer*); - void DecodeImageOnDecoderThread( - scoped_refptr<base::SingleThreadTaskRunner>, - DOMArrayBuffer*, - const String& premultiply_alpha_option, - const String& color_space_conversion_option); + void ScheduleAsyncImageBitmapDecoding(WTF::ArrayBufferContents::DataHandle); void ResolvePromiseOnOriginalThread(sk_sp<SkImage>); // ContextLifecycleObserver diff --git a/chromium/third_party/blink/renderer/core/layout/layout_flexible_box.cc b/chromium/third_party/blink/renderer/core/layout/layout_flexible_box.cc index 4fcefb3de1d..160d6e2a19b 100644 --- a/chromium/third_party/blink/renderer/core/layout/layout_flexible_box.cc +++ b/chromium/third_party/blink/renderer/core/layout/layout_flexible_box.cc @@ -499,12 +499,6 @@ LayoutUnit LayoutFlexibleBox::ChildUnstretchedLogicalHeight( // This should only be called if the logical height is the cross size DCHECK(MainAxisIsInlineAxis(child)); if (NeedToStretchChildLogicalHeight(child)) { - LayoutUnit old_override_height = LayoutUnit(-1); - if (child.HasOverrideLogicalHeight()) { - old_override_height = child.OverrideLogicalHeight(); - const_cast<LayoutBox&>(child).ClearOverrideLogicalHeight(); - } - LayoutUnit child_intrinsic_content_logical_height; if (!child.ShouldApplySizeContainment()) { if (child.DisplayLockInducesSizeContainment()) { @@ -522,10 +516,6 @@ LayoutUnit LayoutFlexibleBox::ChildUnstretchedLogicalHeight( LogicalExtentComputedValues values; child.ComputeLogicalHeight(child_intrinsic_logical_height, LayoutUnit(), values); - if (old_override_height != LayoutUnit(-1)) { - const_cast<LayoutBox&>(child).SetOverrideLogicalHeight( - old_override_height); - } return values.extent_; } return child.LogicalHeight(); @@ -536,23 +526,14 @@ LayoutUnit LayoutFlexibleBox::ChildUnstretchedLogicalWidth( const LayoutBox& child) const { // This should only be called if the logical width is the cross size DCHECK(!MainAxisIsInlineAxis(child)); - + DCHECK(!child.HasOverrideLogicalWidth()); // We compute the width as if we were unstretched. Only the main axis // override size is set at this point. // However, if our cross axis length is definite we don't need to recompute // and can just return the already-set logical width. if (!CrossAxisLengthIsDefinite(child, child.StyleRef().LogicalWidth())) { - LayoutUnit old_override_width = LayoutUnit(-1); - if (child.HasOverrideLogicalWidth()) { - old_override_width = child.OverrideLogicalWidth(); - const_cast<LayoutBox&>(child).ClearOverrideLogicalWidth(); - } - LogicalExtentComputedValues values; child.ComputeLogicalWidth(values); - - if (old_override_width != LayoutUnit(-1)) - const_cast<LayoutBox&>(child).SetOverrideLogicalWidth(old_override_width); return values.extent_; } @@ -1223,6 +1204,8 @@ void LayoutFlexibleBox::ConstructAndAppendFlexItem( FlexLayoutAlgorithm* algorithm, LayoutBox& child, ChildLayoutType layout_type) { + if (layout_type != kNeverLayout) + child.ClearOverrideSize(); if (layout_type != kNeverLayout && ChildHasIntrinsicMainAxisSize(*algorithm, child)) { // If this condition is true, then ComputeMainAxisExtentForChild will call @@ -1237,7 +1220,6 @@ void LayoutFlexibleBox::ConstructAndAppendFlexItem( UpdateBlockChildDirtyBitsBeforeLayout(layout_type == kForceLayout, child); if (child.NeedsLayout() || layout_type == kForceLayout || !intrinsic_size_along_main_axis_.Contains(&child)) { - child.ClearOverrideSize(); child.ForceLayout(); CacheChildMainSize(child); } @@ -1504,19 +1486,8 @@ void LayoutFlexibleBox::LayoutLineItems(FlexLine* current_line, UpdateBlockChildDirtyBitsBeforeLayout(force_child_relayout, *child); if (!child->NeedsLayout()) MarkChildForPaginationRelayoutIfNeeded(*child, layout_scope); - if (child->NeedsLayout()) { + if (child->NeedsLayout()) relaid_out_children_.insert(child); - // It is very important that we only clear the cross axis override size - // if we are in fact going to lay out the child. Otherwise, the cross - // axis size and the actual laid out size get out of sync, which will - // cause problems if we later lay out the child in simplified layout, - // which does not go through regular flex layout and therefore would - // not reset the cross axis size. - if (MainAxisIsInlineAxis(*child)) - child->ClearOverrideLogicalHeight(); - else - child->ClearOverrideLogicalWidth(); - } child->LayoutIfNeeded(); // This shouldn't be necessary, because we set the override size to be diff --git a/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_inline_items_builder.cc b/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_inline_items_builder.cc index a3d84fb4390..e73dbce4bd4 100644 --- a/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_inline_items_builder.cc +++ b/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_inline_items_builder.cc @@ -456,6 +456,8 @@ void NGInlineItemsBuilderTemplate<OffsetMappingBuilder>::AppendText( // If not create a new item as needed. if (UNLIKELY(layout_text->IsWordBreak())) { + typename OffsetMappingBuilder::SourceNodeScope scope(&mapping_builder_, + layout_text); AppendBreakOpportunity(layout_text); return; } diff --git a/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.cc b/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.cc index 39d7c563bf0..e3dfe0194be 100644 --- a/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.cc +++ b/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.cc @@ -1485,16 +1485,19 @@ void NGLineBreaker::HandleCloseTag(const NGInlineItem& item, MoveToNextOf(item); // If the line can break after the previous item, prohibit it and allow break - // after this close tag instead. - if (was_auto_wrap) { - const NGInlineItemResults& item_results = line_info->Results(); - if (item_results.size() >= 2) { - NGInlineItemResult* last = std::prev(item_result); + // after this close tag instead. Even when the close tag has "nowrap", break + // after it is allowed if the line is breakable after the previous item. + const NGInlineItemResults& item_results = line_info->Results(); + if (item_results.size() >= 2) { + NGInlineItemResult* last = std::prev(item_result); + if (was_auto_wrap || last->can_break_after) { item_result->can_break_after = last->can_break_after; last->can_break_after = false; + return; } - return; } + if (was_auto_wrap) + return; DCHECK(!item_result->can_break_after); if (!auto_wrap_) @@ -1513,13 +1516,14 @@ void NGLineBreaker::HandleCloseTag(const NGInlineItem& item, ComputeCanBreakAfter(item_result, auto_wrap_, break_iterator_); } +// Returns whether this item contains only spaces that can hang. bool NGLineBreaker::ShouldHangTraillingSpaces(const NGInlineItem& item) { if (!item.Length()) return false; const ComputedStyle& style = *item.Style(); - if (!auto_wrap_ || (!style.CollapseWhiteSpace() && - style.WhiteSpace() == EWhiteSpace::kBreakSpaces)) + if (!style.AutoWrap() || (!style.CollapseWhiteSpace() && + style.WhiteSpace() == EWhiteSpace::kBreakSpaces)) return false; const String& text = Text(); diff --git a/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_offset_mapping.cc b/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_offset_mapping.cc index f8895564880..9e5ef9dc4aa 100644 --- a/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_offset_mapping.cc +++ b/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_offset_mapping.cc @@ -107,7 +107,8 @@ void NGOffsetMappingUnit::AssertValid() const { SECURITY_DCHECK(dom_start_ <= dom_end_) << dom_start_ << " vs. " << dom_end_; SECURITY_DCHECK(text_content_start_ <= text_content_end_) << text_content_start_ << " vs. " << text_content_end_; - if (layout_object_->IsText()) { + if (layout_object_->IsText() && + !ToLayoutText(*layout_object_).IsWordBreak()) { const LayoutText& layout_text = ToLayoutText(*layout_object_); const unsigned text_start = AssociatedNode() ? layout_text.TextStartOffset() : 0; diff --git a/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_offset_mapping_test.cc b/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_offset_mapping_test.cc index 533fb646428..206226f2b87 100644 --- a/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_offset_mapping_test.cc +++ b/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_offset_mapping_test.cc @@ -1452,6 +1452,26 @@ TEST_F(NGOffsetMappingTest, EndOfLastNonCollapsedContentWithPseudo) { EXPECT_EQ(Position(), GetOffsetMapping().EndOfLastNonCollapsedContent(position)); } + +TEST_F(NGOffsetMappingTest, WordBreak) { + SetupHtml("t", "<div id=t>a<wbr>b</div>"); + + const LayoutObject& text_a = *layout_object_; + const LayoutObject& wbr = *text_a.NextSibling(); + const LayoutObject& text_b = *wbr.NextSibling(); + const NGOffsetMapping& result = GetOffsetMapping(); + + EXPECT_EQ((Vector<NGOffsetMappingUnit>{ + NGOffsetMappingUnit(kIdentity, text_a, 0u, 1u, 0u, 1u), + NGOffsetMappingUnit(kIdentity, wbr, 0u, 1u, 1u, 2u), + NGOffsetMappingUnit(kIdentity, text_b, 0u, 1u, 2u, 3u)}), + result.GetUnits()); + + EXPECT_EQ((Vector<NGOffsetMappingUnit>{ + NGOffsetMappingUnit(kIdentity, wbr, 0u, 1u, 1u, 2u)}), + result.GetMappingUnitsForLayoutObject(wbr)); +} + // Test |GetOffsetMapping| which is available both for LayoutNG and for legacy. class NGOffsetMappingGetterTest : public RenderingTest, public testing::WithParamInterface<bool>, diff --git a/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_physical_line_box_fragment.cc b/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_physical_line_box_fragment.cc index 2c37e28cbdd..aeaa0c8ba9a 100644 --- a/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_physical_line_box_fragment.cc +++ b/chromium/third_party/blink/renderer/core/layout/ng/inline/ng_physical_line_box_fragment.cc @@ -84,7 +84,7 @@ PhysicalRect NGPhysicalLineBoxFragment::ScrollableOverflow( PhysicalSize container_physical_size) const { WritingMode container_writing_mode = container_style->GetWritingMode(); TextDirection container_direction = container_style->Direction(); - PhysicalRect overflow({}, Size()); + PhysicalRect overflow; for (const auto& child : Children()) { PhysicalRect child_scroll_overflow = child->ScrollableOverflowForPropagation(container); @@ -116,6 +116,15 @@ PhysicalRect NGPhysicalLineBoxFragment::ScrollableOverflow( } overflow.Unite(child_scroll_overflow); } + + // Make sure we include the inline-size of the line-box in the overflow. + PhysicalRect rect; + if (IsHorizontalWritingMode(container_writing_mode)) + rect.size.width = Size().width; + else + rect.size.height = Size().height; + overflow.UniteEvenIfEmpty(rect); + return overflow; } diff --git a/chromium/third_party/blink/renderer/core/layout/ng/ng_block_node.cc b/chromium/third_party/blink/renderer/core/layout/ng/ng_block_node.cc index 15980a27bae..b2e4a86c335 100644 --- a/chromium/third_party/blink/renderer/core/layout/ng/ng_block_node.cc +++ b/chromium/third_party/blink/renderer/core/layout/ng/ng_block_node.cc @@ -729,7 +729,12 @@ void NGBlockNode::CopyFragmentDataToLayoutBox( if (LIKELY(IsLastFragment(physical_fragment))) intrinsic_content_logical_height -= border_scrollbar_padding.BlockSum(); - box_->SetIntrinsicContentLogicalHeight(intrinsic_content_logical_height); + if (!constraint_space.IsFixedSizeBlock()) { + // If we had a fixed block size, our children will have sized themselves + // relative to the fixed size, which would make our intrinsic size + // incorrect (too big). + box_->SetIntrinsicContentLogicalHeight(intrinsic_content_logical_height); + } // TODO(mstensho): This should always be done by the parent algorithm, since // we may have auto margins, which only the parent is able to resolve. Remove diff --git a/chromium/third_party/blink/renderer/core/loader/image_loader.cc b/chromium/third_party/blink/renderer/core/loader/image_loader.cc index 2f53751c117..ac5b8fee680 100644 --- a/chromium/third_party/blink/renderer/core/loader/image_loader.cc +++ b/chromium/third_party/blink/renderer/core/loader/image_loader.cc @@ -668,7 +668,18 @@ void ImageLoader::DoUpdateFromElement( params.SetLazyImageAutoReload(); } - new_image_content = ImageResourceContent::Fetch(params, document.Fetcher()); + if (lazy_image_load_state_ == LazyImageLoadState::kDeferred && + was_fully_deferred_) { + // TODO(rajendrant): Remove this temporary workaround of creating a 1x1 + // placeholder to fix an intersection observer issue not firing with + // certain styles (https://crbug.com/992765). Instead + // NoImageResourceToLoad() should be skipped when the image is deferred. + // https://crbug.com/999209 + new_image_content = ImageResourceContent::CreateLazyImagePlaceholder(); + } else { + new_image_content = + ImageResourceContent::Fetch(params, document.Fetcher()); + } // If this load is starting while navigating away, treat it as an auditing // keepalive request, and don't report its results back to the element. diff --git a/chromium/third_party/blink/renderer/core/loader/resource/image_resource_content.cc b/chromium/third_party/blink/renderer/core/loader/resource/image_resource_content.cc index 04b8d7dfc2f..b277d667a12 100644 --- a/chromium/third_party/blink/renderer/core/loader/resource/image_resource_content.cc +++ b/chromium/third_party/blink/renderer/core/loader/resource/image_resource_content.cc @@ -119,6 +119,14 @@ ImageResourceContent* ImageResourceContent::CreateLoaded( return content; } +ImageResourceContent* ImageResourceContent::CreateLazyImagePlaceholder() { + ImageResourceContent* content = MakeGarbageCollected<ImageResourceContent>(); + content->content_status_ = ResourceStatus::kCached; + content->image_ = + PlaceholderImage::CreateForLazyImages(content, IntSize(1, 1)); + return content; +} + ImageResourceContent* ImageResourceContent::Fetch(FetchParameters& params, ResourceFetcher* fetcher) { // TODO(hiroshige): Remove direct references to ImageResource by making diff --git a/chromium/third_party/blink/renderer/core/loader/resource/image_resource_content.h b/chromium/third_party/blink/renderer/core/loader/resource/image_resource_content.h index 7c91be4f723..c022783bd1a 100644 --- a/chromium/third_party/blink/renderer/core/loader/resource/image_resource_content.h +++ b/chromium/third_party/blink/renderer/core/loader/resource/image_resource_content.h @@ -56,6 +56,8 @@ class CORE_EXPORT ImageResourceContent final // Creates ImageResourceContent from an already loaded image. static ImageResourceContent* CreateLoaded(scoped_refptr<blink::Image>); + static ImageResourceContent* CreateLazyImagePlaceholder(); + static ImageResourceContent* Fetch(FetchParameters&, ResourceFetcher*); explicit ImageResourceContent(scoped_refptr<blink::Image> = nullptr); diff --git a/chromium/third_party/blink/renderer/core/page/DEPS b/chromium/third_party/blink/renderer/core/page/DEPS new file mode 100644 index 00000000000..a45c5f7cb81 --- /dev/null +++ b/chromium/third_party/blink/renderer/core/page/DEPS @@ -0,0 +1,4 @@ +include_rules = [ + # For validation_message_overlay_delegate_test.cc + "+base/strings/utf_string_conversions.h", +] diff --git a/chromium/third_party/blink/renderer/core/page/page_animator.cc b/chromium/third_party/blink/renderer/core/page/page_animator.cc index 42ea986e571..4a16ce2d429 100644 --- a/chromium/third_party/blink/renderer/core/page/page_animator.cc +++ b/chromium/third_party/blink/renderer/core/page/page_animator.cc @@ -30,6 +30,10 @@ void PageAnimator::Trace(blink::Visitor* visitor) { void PageAnimator::ServiceScriptedAnimations( base::TimeTicks monotonic_animation_start_time) { base::AutoReset<bool> servicing(&servicing_animations_, true); + + // Once we are inside a frame's lifecycle, the AnimationClock should hold its + // time value until the end of the frame. + Clock().SetAllowedToDynamicallyUpdateTime(false); Clock().UpdateTime(monotonic_animation_start_time); HeapVector<Member<Document>, 32> documents; @@ -84,7 +88,7 @@ void PageAnimator::ServiceScriptedAnimations( page_->GetValidationMessageClient().LayoutOverlay(); } -void PageAnimator::RunPostAnimationFrameCallbacks() { +void PageAnimator::PostAnimate() { HeapVector<Member<Document>, 32> documents; for (Frame* frame = page_->MainFrame(); frame; frame = frame->Tree().TraverseNext()) { @@ -92,8 +96,21 @@ void PageAnimator::RunPostAnimationFrameCallbacks() { documents.push_back(To<LocalFrame>(frame)->GetDocument()); } + // Run the post-animation frame callbacks. See + // https://github.com/WICG/requestPostAnimationFrame for (auto& document : documents) document->RunPostAnimationFrameCallbacks(); + + // If we don't have an imminently incoming frame, we need to let the + // AnimationClock update its own time to properly service out-of-lifecycle + // events such as setInterval (see https://crbug.com/995806). This isn't a + // perfect heuristic, but at the very least we know that if there is a pending + // RAF we will be getting a new frame and thus don't need to unlock the clock. + bool next_frame_has_raf = false; + for (auto& document : documents) + next_frame_has_raf |= document->NextFrameHasPendingRAF(); + if (!next_frame_has_raf) + Clock().SetAllowedToDynamicallyUpdateTime(true); } void PageAnimator::SetSuppressFrameRequestsWorkaroundFor704763Only( diff --git a/chromium/third_party/blink/renderer/core/page/page_animator.h b/chromium/third_party/blink/renderer/core/page/page_animator.h index 0e2616f8385..84701414048 100644 --- a/chromium/third_party/blink/renderer/core/page/page_animator.h +++ b/chromium/third_party/blink/renderer/core/page/page_animator.h @@ -23,7 +23,7 @@ class CORE_EXPORT PageAnimator final : public GarbageCollected<PageAnimator> { void ScheduleVisualUpdate(LocalFrame*); void ServiceScriptedAnimations( base::TimeTicks monotonic_animation_start_time); - void RunPostAnimationFrameCallbacks(); + void PostAnimate(); bool IsServicingAnimations() const { return servicing_animations_; } diff --git a/chromium/third_party/blink/renderer/core/page/page_widget_delegate.cc b/chromium/third_party/blink/renderer/core/page/page_widget_delegate.cc index d0777576a07..37f1a9d0cab 100644 --- a/chromium/third_party/blink/renderer/core/page/page_widget_delegate.cc +++ b/chromium/third_party/blink/renderer/core/page/page_widget_delegate.cc @@ -40,6 +40,7 @@ #include "third_party/blink/renderer/core/layout/layout_view.h" #include "third_party/blink/renderer/core/page/autoscroll_controller.h" #include "third_party/blink/renderer/core/page/page.h" +#include "third_party/blink/renderer/core/page/validation_message_client.h" #include "third_party/blink/renderer/platform/runtime_enabled_features.h" #include "third_party/blink/renderer/platform/wtf/time.h" @@ -49,10 +50,15 @@ void PageWidgetDelegate::Animate(Page& page, base::TimeTicks monotonic_frame_begin_time) { page.GetAutoscrollController().Animate(); page.Animator().ServiceScriptedAnimations(monotonic_frame_begin_time); + // The ValidationMessage overlay manages its own internal Page that isn't + // hooked up the normal BeginMainFrame flow, so we manually tick its + // animations here. + page.GetValidationMessageClient().ServiceScriptedAnimations( + monotonic_frame_begin_time); } void PageWidgetDelegate::PostAnimate(Page& page) { - page.Animator().RunPostAnimationFrameCallbacks(); + page.Animator().PostAnimate(); } void PageWidgetDelegate::UpdateLifecycle( diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/fragment_anchor.cc b/chromium/third_party/blink/renderer/core/page/scrolling/fragment_anchor.cc index caf3e53f6a5..a0489a1906b 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/fragment_anchor.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/fragment_anchor.cc @@ -32,14 +32,15 @@ FragmentAnchor* FragmentAnchor::TryCreate(const KURL& url, } // Count how often we see a # in the fragment (i.e. after the # delimiting - // the hash). We do this after trying to find a ##targetText so that we don't - // pollute this metric with our own feature since we're trying to determine - // how prevalent ## is. If a ##targetText is found, it'll strip off the ## - // and any text following it. + // the hash). We avoid counting cases with ##targetText since we're trying to + // determine how often this happens outside our feature so we don't want to + // pollute it with our own usage. if (url.HasFragmentIdentifier()) { size_t hash_pos = url.FragmentIdentifier().Find("#"); - if (hash_pos != kNotFound) - UseCounter::Count(frame.GetDocument(), WebFeature::kFragmentDoubleHash); + if (hash_pos != kNotFound) { + if (url.FragmentIdentifier().Find("#targetText=") == kNotFound) + UseCounter::Count(frame.GetDocument(), WebFeature::kFragmentDoubleHash); + } } bool element_id_anchor_found = false; diff --git a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics_test.cc b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics_test.cc index 03cfaf8333a..a378d56b09f 100644 --- a/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics_test.cc +++ b/chromium/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics_test.cc @@ -356,11 +356,6 @@ TEST_P(TextFragmentRelatedMetricTest, DoubleHashUseCounter) { const int kUncounted = 0; const int kCounted = 1; - // When the TextFragmentAnchors feature is on, we should avoid counting a - // ##targetText directive as a use of double-hash in this case. When it's - // off, we expect to count it. - const int kCountedOnlyIfDisabled = !GetParam() ? kCounted : kUncounted; - Vector<std::pair<String, int>> test_cases = { {"", kUncounted}, {"#element", kUncounted}, @@ -371,10 +366,14 @@ TEST_P(TextFragmentRelatedMetricTest, DoubleHashUseCounter) { {"#element#", kCounted}, {"#foo#bar#", kCounted}, {"#foo%23", kUncounted}, - {"#element##targetText=doesntexist", kCountedOnlyIfDisabled}, - {"#element##targetText=doesntexist#", kCountedOnlyIfDisabled}, - {"#element##targetText=page", kCountedOnlyIfDisabled}, - {"#element##targetText=page#", kCountedOnlyIfDisabled}, + {"#element##targetText=doesntexist", kUncounted}, + {"#element##targetText=doesntexist#", kUncounted}, + {"#element##targetText=page", kUncounted}, + {"#element##targetText=page#", kUncounted}, + {"##targetText=doesntexist", kUncounted}, + {"##targetText=doesntexist#", kUncounted}, + {"##targetText=page", kUncounted}, + {"##targetText=page#", kUncounted}, {"#targetText=doesntexist", kUncounted}, {"#targetText=page", kUncounted}}; diff --git a/chromium/third_party/blink/renderer/core/page/validation_message_client.h b/chromium/third_party/blink/renderer/core/page/validation_message_client.h index 09425262da3..8e9e1a248b4 100644 --- a/chromium/third_party/blink/renderer/core/page/validation_message_client.h +++ b/chromium/third_party/blink/renderer/core/page/validation_message_client.h @@ -62,6 +62,7 @@ class ValidationMessageClient : public GarbageCollectedMixin { virtual void WillBeDestroyed() = 0; + virtual void ServiceScriptedAnimations(base::TimeTicks) {} virtual void LayoutOverlay() {} virtual void UpdatePrePaint() {} // For CompositeAfterPaint. diff --git a/chromium/third_party/blink/renderer/core/page/validation_message_client_impl.cc b/chromium/third_party/blink/renderer/core/page/validation_message_client_impl.cc index 7ab6b3b84f1..b9142462dd6 100644 --- a/chromium/third_party/blink/renderer/core/page/validation_message_client_impl.cc +++ b/chromium/third_party/blink/renderer/core/page/validation_message_client_impl.cc @@ -195,6 +195,12 @@ void ValidationMessageClientImpl::WillOpenPopup() { HideValidationMessage(*current_anchor_); } +void ValidationMessageClientImpl::ServiceScriptedAnimations( + base::TimeTicks monotonic_frame_begin_time) { + if (overlay_) + overlay_->ServiceScriptedAnimations(monotonic_frame_begin_time); +} + void ValidationMessageClientImpl::LayoutOverlay() { if (overlay_) CheckAnchorStatus(nullptr); diff --git a/chromium/third_party/blink/renderer/core/page/validation_message_client_impl.h b/chromium/third_party/blink/renderer/core/page/validation_message_client_impl.h index ead9a0db738..4e9cac17e36 100644 --- a/chromium/third_party/blink/renderer/core/page/validation_message_client_impl.h +++ b/chromium/third_party/blink/renderer/core/page/validation_message_client_impl.h @@ -26,6 +26,7 @@ #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_PAGE_VALIDATION_MESSAGE_CLIENT_IMPL_H_ #define THIRD_PARTY_BLINK_RENDERER_CORE_PAGE_VALIDATION_MESSAGE_CLIENT_IMPL_H_ +#include "third_party/blink/renderer/core/core_export.h" #include "third_party/blink/renderer/core/page/page.h" #include "third_party/blink/renderer/core/page/popup_opening_observer.h" #include "third_party/blink/renderer/core/page/validation_message_client.h" @@ -40,7 +41,7 @@ class LocalFrameView; class FrameOverlay; class ValidationMessageOverlayDelegate; -class ValidationMessageClientImpl final +class CORE_EXPORT ValidationMessageClientImpl final : public GarbageCollectedFinalized<ValidationMessageClientImpl>, public ValidationMessageClient, private PopupOpeningObserver { @@ -50,8 +51,18 @@ class ValidationMessageClientImpl final explicit ValidationMessageClientImpl(Page&); ~ValidationMessageClientImpl() override; + void ShowValidationMessage(const Element& anchor, + const String& message, + TextDirection message_dir, + const String& sub_message, + TextDirection sub_message_dir) override; + void Trace(blink::Visitor*) override; + ValidationMessageOverlayDelegate* GetDelegateForTesting() const { + return overlay_delegate_; + } + private: void CheckAnchorStatus(TimerBase*); LocalFrameView* CurrentView(); @@ -59,16 +70,12 @@ class ValidationMessageClientImpl final void Reset(TimerBase*); void ValidationMessageVisibilityChanged(const Element& anchor); - void ShowValidationMessage(const Element& anchor, - const String& message, - TextDirection message_dir, - const String& sub_message, - TextDirection sub_message_dir) override; void HideValidationMessage(const Element& anchor) override; bool IsValidationMessageVisible(const Element& anchor) override; void DocumentDetached(const Document&) override; void DidChangeFocusTo(const Element* new_element) override; void WillBeDestroyed() override; + void ServiceScriptedAnimations(base::TimeTicks) override; void LayoutOverlay() override; void UpdatePrePaint() override; void PaintOverlay(GraphicsContext&) override; diff --git a/chromium/third_party/blink/renderer/core/page/validation_message_overlay_delegate.cc b/chromium/third_party/blink/renderer/core/page/validation_message_overlay_delegate.cc index 8f738806995..604c73873d7 100644 --- a/chromium/third_party/blink/renderer/core/page/validation_message_overlay_delegate.cc +++ b/chromium/third_party/blink/renderer/core/page/validation_message_overlay_delegate.cc @@ -112,6 +112,11 @@ void ValidationMessageOverlayDelegate::PaintFrameOverlay( } } +void ValidationMessageOverlayDelegate::ServiceScriptedAnimations( + base::TimeTicks monotonic_frame_begin_time) { + page_->Animator().ServiceScriptedAnimations(monotonic_frame_begin_time); +} + void ValidationMessageOverlayDelegate::UpdateFrameViewState( const FrameOverlay& overlay, const IntSize& view_size) { diff --git a/chromium/third_party/blink/renderer/core/page/validation_message_overlay_delegate.h b/chromium/third_party/blink/renderer/core/page/validation_message_overlay_delegate.h index c48cfc16dd6..e3dfbb49c89 100644 --- a/chromium/third_party/blink/renderer/core/page/validation_message_overlay_delegate.h +++ b/chromium/third_party/blink/renderer/core/page/validation_message_overlay_delegate.h @@ -5,6 +5,7 @@ #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_PAGE_VALIDATION_MESSAGE_OVERLAY_DELEGATE_H_ #define THIRD_PARTY_BLINK_RENDERER_CORE_PAGE_VALIDATION_MESSAGE_OVERLAY_DELEGATE_H_ +#include "third_party/blink/renderer/core/core_export.h" #include "third_party/blink/renderer/core/frame/frame_overlay.h" #include "third_party/blink/renderer/platform/text/text_direction.h" #include "third_party/blink/renderer/platform/wtf/forward.h" @@ -23,7 +24,8 @@ class Page; // bubble is shown, and deleted when the bubble is closed. // // Ownership: A FrameOverlay instance owns a ValidationMessageOverlayDelegate. -class ValidationMessageOverlayDelegate : public FrameOverlay::Delegate { +class CORE_EXPORT ValidationMessageOverlayDelegate + : public FrameOverlay::Delegate { public: ValidationMessageOverlayDelegate(Page& main_page, const Element& anchor, @@ -34,12 +36,18 @@ class ValidationMessageOverlayDelegate : public FrameOverlay::Delegate { ~ValidationMessageOverlayDelegate() override; void CreatePage(const FrameOverlay&); + + // FrameOverlay::Delegate implementation. void PaintFrameOverlay(const FrameOverlay&, GraphicsContext&, const IntSize& view_size) const override; + void ServiceScriptedAnimations(base::TimeTicks) override; + void StartToHide(); bool IsHiding() const; + Page* GetPageForTesting() const { return page_; } + private: LocalFrameView& FrameView() const; void UpdateFrameViewState(const FrameOverlay&, const IntSize& view_size); diff --git a/chromium/third_party/blink/renderer/core/page/validation_message_overlay_delegate_test.cc b/chromium/third_party/blink/renderer/core/page/validation_message_overlay_delegate_test.cc new file mode 100644 index 00000000000..0a4a5a3b699 --- /dev/null +++ b/chromium/third_party/blink/renderer/core/page/validation_message_overlay_delegate_test.cc @@ -0,0 +1,139 @@ +// Copyright 2019 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 "third_party/blink/renderer/core/page/validation_message_overlay_delegate.h" + +#include "build/build_config.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/blink/renderer/core/animation/animation.h" +#include "third_party/blink/renderer/core/animation/document_timeline.h" +#include "third_party/blink/renderer/core/page/page_widget_delegate.h" +#include "third_party/blink/renderer/core/page/validation_message_client.h" +#include "third_party/blink/renderer/core/page/validation_message_client_impl.h" +#include "third_party/blink/renderer/core/testing/core_unit_test_helper.h" +#include "third_party/blink/renderer/platform/testing/paint_test_configurations.h" +#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h" +#include "third_party/blink/renderer/platform/web_test_support.h" + +#if defined(OS_WIN) +#include "base/strings/utf_string_conversions.h" +#include "third_party/blink/public/web/win/web_font_rendering.h" +#endif + +namespace blink { + +class ValidationMessageOverlayDelegateTest : public PaintTestConfigurations, + public RenderingTest { +#if defined(OS_WIN) + public: + void SetUp() override { + RenderingTest::SetUp(); + + // These tests appear to trigger a requirement for system fonts. On windows, + // an extra step is required to ensure that the system font is configured. + // See https://crbug.com/969622 + blink::WebFontRendering::SetMenuFontMetrics( + base::ASCIIToUTF16("Arial").c_str(), 12); + } +#endif +}; + +INSTANTIATE_PAINT_TEST_SUITE_P(ValidationMessageOverlayDelegateTest); + +// Regression test for https://crbug.com/990680, where we accidentally +// composited the animations created by ValidationMessageOverlayDelegate. Since +// overlays operate in a Page that has no compositor, the animations broke. +TEST_P(ValidationMessageOverlayDelegateTest, + OverlayAnimationsShouldNotBeComposited) { + // When WebTestSupport::IsRunningWebTest is set, the animations in + // ValidationMessageOverlayDelegate are disabled. We are specifically testing + // animations, so make sure that doesn't happen. + bool was_running_web_test = WebTestSupport::IsRunningWebTest(); + WebTestSupport::SetIsRunningWebTest(false); + + SetBodyInnerHTML("<div id='anchor'></div>"); + Element* anchor = GetElementById("anchor"); + ASSERT_TRUE(anchor); + + auto delegate = std::make_unique<ValidationMessageOverlayDelegate>( + GetPage(), *anchor, "Test message", TextDirection::kLtr, "Sub-message", + TextDirection::kLtr); + ValidationMessageOverlayDelegate* delegate_ptr = delegate.get(); + + auto overlay = + std::make_unique<FrameOverlay>(&GetFrame(), std::move(delegate)); + delegate_ptr->CreatePage(*overlay); + ASSERT_TRUE( + GetFrame().View()->UpdateLifecycleToCompositingCleanPlusScrolling()); + + // Trigger the overlay animations. + auto paint_controller = + std::make_unique<PaintController>(PaintController::kTransient); + GraphicsContext context(*paint_controller); + overlay->Paint(context); + + // Now find the related animations, and make sure they weren't composited. + Document* internal_document = + To<LocalFrame>(delegate_ptr->GetPageForTesting()->MainFrame()) + ->GetDocument(); + HeapVector<Member<Animation>> animations = + internal_document->Timeline().getAnimations(); + ASSERT_FALSE(animations.IsEmpty()); + + for (const auto& animation : animations) { + EXPECT_FALSE(animation->HasActiveAnimationsOnCompositor()); + } + + WebTestSupport::SetIsRunningWebTest(was_running_web_test); +} + +// Regression test for https://crbug.com/990680, where we found we were not +// properly advancing the AnimationClock in the internal Page created by +// ValidationMessageOverlayDelegate. When combined with the fix for +// https://crbug.com/785940, this caused Animations to never be updated. +TEST_P(ValidationMessageOverlayDelegateTest, + DelegatesInternalPageShouldHaveAnimationTimesUpdated) { + // We use a ValidationMessageClientImpl here to create our delegate since we + // need the official path from PageWidgetDelegate::Animate to work. + auto* client = MakeGarbageCollected<ValidationMessageClientImpl>(GetPage()); + ValidationMessageClient* original_client = + &GetPage().GetValidationMessageClient(); + GetPage().SetValidationMessageClientForTesting(client); + + SetBodyInnerHTML(R"HTML( + <style>#anchor { width: 100px; height: 100px; }</style> + <div id='anchor'></div> + )HTML"); + Element* anchor = GetElementById("anchor"); + ASSERT_TRUE(anchor); + + client->ShowValidationMessage(*anchor, "Test message", TextDirection::kLtr, + "Sub-message", TextDirection::kLtr); + ValidationMessageOverlayDelegate* delegate = client->GetDelegateForTesting(); + ASSERT_TRUE(delegate); + + // Initially the AnimationClock will be at 0. + // TODO(crbug.com/785940): Re-enable this EXPECT_EQ once the AnimationClock no + // longer jumps ahead on its own accord. + AnimationClock& internal_clock = + delegate->GetPageForTesting()->Animator().Clock(); + // EXPECT_EQ(internal_clock.CurrentTime(), 0); + + // Now update the main Page's clock. This should trickle down and update the + // inner Page's clock too. + AnimationClock& external_clock = GetPage().Animator().Clock(); + base::TimeTicks current_time = external_clock.CurrentTime(); + + base::TimeTicks new_time = current_time + base::TimeDelta::FromSeconds(1); + PageWidgetDelegate::Animate(GetPage(), new_time); + + // TODO(crbug.com/785940): Until this bug is fixed, this comparison could pass + // even if the underlying behavior regresses (because calling CurrentTime + // could advance the clocks anyway). + EXPECT_EQ(external_clock.CurrentTime(), internal_clock.CurrentTime()); + + GetPage().SetValidationMessageClientForTesting(original_client); +} + +} // namespace blink |