diff --git a/chrome/VERSION b/chrome/VERSION index f5db663ae..67ffb9d 100644 --- a/chrome/VERSION +++ b/chrome/VERSION
@@ -1,4 +1,4 @@ MAJOR=59 MINOR=0 -BUILD=3032 +BUILD=3033 PATCH=0
diff --git a/content/browser/memory/memory_coordinator_impl.cc b/content/browser/memory/memory_coordinator_impl.cc index 75727c01..0262491 100644 --- a/content/browser/memory/memory_coordinator_impl.cc +++ b/content/browser/memory/memory_coordinator_impl.cc
@@ -57,6 +57,25 @@ return "N/A"; } +MemoryState CalculateMemoryStateForProcess(MemoryCondition condition, + bool is_visible) { + // The current heuristics for state calculation: + // - Foregrounded(visible) processes: THROTTLED when condition is CRITICAL, + // otherwise NORMAL. + // - Backgrounded(invisible) processes: THROTTLED when condition is + // WARNING/CRITICAL, otherwise NORMAL. + switch (condition) { + case MemoryCondition::NORMAL: + return MemoryState::NORMAL; + case MemoryCondition::WARNING: + return is_visible ? MemoryState::NORMAL : MemoryState::THROTTLED; + case MemoryCondition::CRITICAL: + return MemoryState::THROTTLED; + } + NOTREACHED(); + return MemoryState::NORMAL; +} + } // namespace // The implementation of MemoryCoordinatorHandle. See memory_coordinator.mojom @@ -237,25 +256,8 @@ RenderProcessHost* process = render_widget_host->GetProcess(); if (!process) return; - auto iter = children().find(process->GetID()); - if (iter == children().end()) - return; - - iter->second.is_visible = *Details<bool>(details).ptr(); - // The current heuristics for state calculation: - // - Foregrounded renderers: THROTTLED when condition is CRITICAL, otherwise - // NORMAL. - // - Backgrounded renderers: THROTTLED when condition is WARNING/CRITICAL, - // otherwise NORMAL. - MemoryState new_state = MemoryState::NORMAL; - MemoryCondition condition = GetMemoryCondition(); - if (condition == MemoryCondition::WARNING) { - new_state = - iter->second.is_visible ? MemoryState::NORMAL : MemoryState::THROTTLED; - } else if (condition == MemoryCondition::CRITICAL) { - new_state = MemoryState::THROTTLED; - } - SetChildMemoryState(iter->first, new_state); + bool is_visible = *Details<bool>(details).ptr(); + OnChildVisibilityChanged(process->GetID(), is_visible); } MemoryState MemoryCoordinatorImpl::GetStateForProcess( @@ -357,13 +359,28 @@ } void MemoryCoordinatorImpl::OnChildAdded(int render_process_id) { - // Populate an initial state of a newly created process, assuming it's - // foregrounded. - // TODO(bashi): Don't assume the process is foregrounded. It can be added - // as a background process. - auto new_state = GetMemoryCondition() == MemoryCondition::CRITICAL - ? MemoryState::THROTTLED - : MemoryState::NORMAL; + RenderProcessHost* render_process_host = + GetRenderProcessHost(render_process_id); + if (!render_process_host) + return; + + // Populate an initial state of a newly created process. + // TODO(bashi): IsProcessBackgrounded() may return true even when tabs in the + // renderer process are invisible (e.g. restoring tabs all at once). + // Figure out a better way to set visibility. + OnChildVisibilityChanged(render_process_id, + !render_process_host->IsProcessBackgrounded()); +} + +void MemoryCoordinatorImpl::OnChildVisibilityChanged(int render_process_id, + bool is_visible) { + auto iter = children().find(render_process_id); + if (iter == children().end()) + return; + + iter->second.is_visible = is_visible; + MemoryState new_state = + CalculateMemoryStateForProcess(GetMemoryCondition(), is_visible); SetChildMemoryState(render_process_id, new_state); }
diff --git a/content/browser/memory/memory_coordinator_impl.h b/content/browser/memory/memory_coordinator_impl.h index 7bf66b1..4b63fb4c 100644 --- a/content/browser/memory/memory_coordinator_impl.h +++ b/content/browser/memory/memory_coordinator_impl.h
@@ -160,6 +160,7 @@ FRIEND_TEST_ALL_PREFIXES(MemoryCoordinatorWithServiceWorkerTest, CannotSuspendRendererWithServiceWorker); #endif + FRIEND_TEST_ALL_PREFIXES(MemoryCoordinatorImplTest, OnChildVisibilityChanged); FRIEND_TEST_ALL_PREFIXES(MemoryCoordinatorImplTest, CalculateNextCondition); FRIEND_TEST_ALL_PREFIXES(MemoryCoordinatorImplTest, UpdateCondition); FRIEND_TEST_ALL_PREFIXES(MemoryCoordinatorImplTest, SetMemoryStateForTesting); @@ -171,6 +172,9 @@ // Called when ChildMemoryCoordinator calls AddChild(). void OnChildAdded(int render_process_id); + // Called when visibility of a child process is changed. + void OnChildVisibilityChanged(int render_process_id, bool is_visible); + // Called by SetChildMemoryState() to determine a child memory state based on // the current status of the child process. MemoryState OverrideState(MemoryState memroy_state, const ChildInfo& child);
diff --git a/content/browser/memory/memory_coordinator_impl_unittest.cc b/content/browser/memory/memory_coordinator_impl_unittest.cc index d0c7413..7104ef9 100644 --- a/content/browser/memory/memory_coordinator_impl_unittest.cc +++ b/content/browser/memory/memory_coordinator_impl_unittest.cc
@@ -303,6 +303,38 @@ render_process_host->DecrementSharedWorkerRefCount(); } +TEST_F(MemoryCoordinatorImplTest, OnChildVisibilityChanged) { + auto* child = coordinator_->CreateChildMemoryCoordinator(1); + + coordinator_->memory_condition_ = MemoryCondition::NORMAL; + coordinator_->OnChildVisibilityChanged(1, true); + RunUntilIdle(); + EXPECT_EQ(mojom::MemoryState::NORMAL, child->state()); + coordinator_->OnChildVisibilityChanged(1, false); + RunUntilIdle(); +#if defined(OS_ANDROID) + EXPECT_EQ(mojom::MemoryState::THROTTLED, child->state()); +#else + EXPECT_EQ(mojom::MemoryState::NORMAL, child->state()); +#endif + + coordinator_->memory_condition_ = MemoryCondition::WARNING; + coordinator_->OnChildVisibilityChanged(1, true); + RunUntilIdle(); + EXPECT_EQ(mojom::MemoryState::NORMAL, child->state()); + coordinator_->OnChildVisibilityChanged(1, false); + RunUntilIdle(); + EXPECT_EQ(mojom::MemoryState::THROTTLED, child->state()); + + coordinator_->memory_condition_ = MemoryCondition::CRITICAL; + coordinator_->OnChildVisibilityChanged(1, true); + RunUntilIdle(); + EXPECT_EQ(mojom::MemoryState::THROTTLED, child->state()); + coordinator_->OnChildVisibilityChanged(1, false); + RunUntilIdle(); + EXPECT_EQ(mojom::MemoryState::THROTTLED, child->state()); +} + TEST_F(MemoryCoordinatorImplTest, CalculateNextCondition) { auto* condition_observer = coordinator_->condition_observer_.get(); condition_observer->expected_renderer_size_ = 10;
diff --git a/content/shell/test_runner/mock_web_speech_recognizer.cc b/content/shell/test_runner/mock_web_speech_recognizer.cc index 56c35ae..29ca528b 100644 --- a/content/shell/test_runner/mock_web_speech_recognizer.cc +++ b/content/shell/test_runner/mock_web_speech_recognizer.cc
@@ -160,7 +160,7 @@ weak_factory_(this) {} MockWebSpeechRecognizer::~MockWebSpeechRecognizer() { - ClearTaskQueue(); + SetDelegate(nullptr); } bool MockWebSpeechRecognizer::Task::isNewContextTask() const { @@ -169,6 +169,9 @@ void MockWebSpeechRecognizer::SetDelegate(WebTestDelegate* delegate) { delegate_ = delegate; + // No delegate to forward to, clear out pending tasks. + if (!delegate_) + ClearTaskQueue(); } void MockWebSpeechRecognizer::SetClientContext(
diff --git a/content/shell/test_runner/web_view_test_proxy.cc b/content/shell/test_runner/web_view_test_proxy.cc index 1404003..3fdc4561 100644 --- a/content/shell/test_runner/web_view_test_proxy.cc +++ b/content/shell/test_runner/web_view_test_proxy.cc
@@ -32,6 +32,8 @@ WebViewTestProxyBase::~WebViewTestProxyBase() { test_interfaces_->WindowClosed(this); + if (test_interfaces_->GetDelegate() == delegate_) + test_interfaces_->SetDelegate(nullptr); } void WebViewTestProxyBase::SetInterfaces(WebTestInterfaces* interfaces) {
diff --git a/content/test/gpu/generate_buildbot_json.py b/content/test/gpu/generate_buildbot_json.py index 79604c45..81a77fe 100755 --- a/content/test/gpu/generate_buildbot_json.py +++ b/content/test/gpu/generate_buildbot_json.py
@@ -690,6 +690,21 @@ 'swarming': False, 'os_type': 'android', }, + 'Android Release (NVIDIA Shield)': { + 'swarming_dimensions': [ + { + # There are no PCI IDs on Android. + # This is a hack to get the script working. + 'gpu': '0000:0000', + 'os': 'Android' + }, + ], + 'build_config': 'android-chromium', + # This bot is a one-off and doesn't have similar slaves in the + # swarming pool. + 'swarming': False, + 'os_type': 'android', + }, # The following "optional" testers don't actually exist on the # waterfall. They are present here merely to specify additional
diff --git a/testing/buildbot/chromium.gpu.fyi.json b/testing/buildbot/chromium.gpu.fyi.json index 5a69cad..4a1b34e 100644 --- a/testing/buildbot/chromium.gpu.fyi.json +++ b/testing/buildbot/chromium.gpu.fyi.json
@@ -1,6 +1,363 @@ { "AAAAA1 AUTOGENERATED FILE DO NOT EDIT": {}, "AAAAA2 See generate_buildbot_json.py to make changes": {}, + "Android Release (NVIDIA Shield)": { + "gtest_tests": [ + { + "override_isolate_target": "angle_end2end_tests", + "swarming": { + "can_use_on_swarming_builders": false, + "cipd_packages": [ + { + "cipd_package": "infra/tools/luci/logdog/butler/${platform}", + "location": "bin", + "revision": "git_revision:25755a2c316937ee44a6432163dc5e2f9c85cf58" + } + ], + "dimension_sets": [ + { + "gpu": "0000:0000", + "os": "Android" + } + ], + "output_links": [ + { + "link": [ + "https://luci-logdog.appspot.com/v/?s", + "=android%2Fswarming%2Flogcats%2F", + "${TASK_ID}%2F%2B%2Funified_logcats" + ], + "name": "shard #${SHARD_INDEX} logcats" + } + ] + }, + "test": "angle_end2end_tests", + "use_xvfb": false + }, + { + "override_isolate_target": "angle_unittests", + "swarming": { + "can_use_on_swarming_builders": false, + "cipd_packages": [ + { + "cipd_package": "infra/tools/luci/logdog/butler/${platform}", + "location": "bin", + "revision": "git_revision:25755a2c316937ee44a6432163dc5e2f9c85cf58" + } + ], + "dimension_sets": [ + { + "gpu": "0000:0000", + "os": "Android" + } + ], + "output_links": [ + { + "link": [ + "https://luci-logdog.appspot.com/v/?s", + "=android%2Fswarming%2Flogcats%2F", + "${TASK_ID}%2F%2B%2Funified_logcats" + ], + "name": "shard #${SHARD_INDEX} logcats" + } + ] + }, + "test": "angle_unittests", + "use_xvfb": false + }, + { + "override_isolate_target": "gl_tests", + "swarming": { + "can_use_on_swarming_builders": false, + "cipd_packages": [ + { + "cipd_package": "infra/tools/luci/logdog/butler/${platform}", + "location": "bin", + "revision": "git_revision:25755a2c316937ee44a6432163dc5e2f9c85cf58" + } + ], + "dimension_sets": [ + { + "gpu": "0000:0000", + "os": "Android" + } + ], + "output_links": [ + { + "link": [ + "https://luci-logdog.appspot.com/v/?s", + "=android%2Fswarming%2Flogcats%2F", + "${TASK_ID}%2F%2B%2Funified_logcats" + ], + "name": "shard #${SHARD_INDEX} logcats" + } + ] + }, + "test": "gl_tests", + "use_xvfb": false + }, + { + "override_isolate_target": "gl_unittests", + "swarming": { + "can_use_on_swarming_builders": false, + "cipd_packages": [ + { + "cipd_package": "infra/tools/luci/logdog/butler/${platform}", + "location": "bin", + "revision": "git_revision:25755a2c316937ee44a6432163dc5e2f9c85cf58" + } + ], + "dimension_sets": [ + { + "gpu": "0000:0000", + "os": "Android" + } + ], + "output_links": [ + { + "link": [ + "https://luci-logdog.appspot.com/v/?s", + "=android%2Fswarming%2Flogcats%2F", + "${TASK_ID}%2F%2B%2Funified_logcats" + ], + "name": "shard #${SHARD_INDEX} logcats" + } + ] + }, + "test": "gl_unittests", + "use_xvfb": false + } + ], + "isolated_scripts": [ + { + "args": [ + "context_lost", + "--show-stdout", + "--browser=android-chromium", + "-v", + "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc" + ], + "isolate_name": "telemetry_gpu_integration_test", + "name": "context_lost_tests", + "override_compile_targets": [ + "telemetry_gpu_integration_test_run" + ], + "swarming": { + "can_use_on_swarming_builders": false, + "dimension_sets": [ + { + "gpu": "0000:0000", + "os": "Android" + } + ] + } + }, + { + "args": [ + "depth_capture", + "--show-stdout", + "--browser=android-chromium", + "-v", + "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc" + ], + "isolate_name": "telemetry_gpu_integration_test", + "name": "depth_capture_tests", + "override_compile_targets": [ + "telemetry_gpu_integration_test_run" + ], + "swarming": { + "can_use_on_swarming_builders": false, + "dimension_sets": [ + { + "gpu": "0000:0000", + "os": "Android" + } + ] + } + }, + { + "args": [ + "gpu_process", + "--show-stdout", + "--browser=android-chromium", + "-v", + "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc" + ], + "isolate_name": "telemetry_gpu_integration_test", + "name": "gpu_process_launch_tests", + "override_compile_targets": [ + "telemetry_gpu_integration_test_run" + ], + "swarming": { + "can_use_on_swarming_builders": false, + "dimension_sets": [ + { + "gpu": "0000:0000", + "os": "Android" + } + ] + } + }, + { + "args": [ + "hardware_accelerated_feature", + "--show-stdout", + "--browser=android-chromium", + "-v", + "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc" + ], + "isolate_name": "telemetry_gpu_integration_test", + "name": "hardware_accelerated_feature_tests", + "override_compile_targets": [ + "telemetry_gpu_integration_test_run" + ], + "swarming": { + "can_use_on_swarming_builders": false, + "dimension_sets": [ + { + "gpu": "0000:0000", + "os": "Android" + } + ] + } + }, + { + "args": [ + "maps", + "--show-stdout", + "--browser=android-chromium", + "-v", + "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc", + "--os-type", + "android", + "--build-revision", + "${got_revision}", + "--test-machine-name", + "${buildername}" + ], + "isolate_name": "telemetry_gpu_integration_test", + "name": "maps_pixel_test", + "override_compile_targets": [ + "telemetry_gpu_integration_test_run" + ], + "swarming": { + "can_use_on_swarming_builders": false, + "dimension_sets": [ + { + "gpu": "0000:0000", + "os": "Android" + } + ] + } + }, + { + "args": [ + "pixel", + "--show-stdout", + "--browser=android-chromium", + "-v", + "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc", + "--refimg-cloud-storage-bucket", + "chromium-gpu-archive/reference-images", + "--os-type", + "android", + "--build-revision", + "${got_revision}", + "--test-machine-name", + "${buildername}" + ], + "isolate_name": "telemetry_gpu_integration_test", + "name": "pixel_test", + "non_precommit_args": [ + "--upload-refimg-to-cloud-storage" + ], + "override_compile_targets": [ + "telemetry_gpu_integration_test_run" + ], + "precommit_args": [ + "--download-refimg-from-cloud-storage" + ], + "swarming": { + "can_use_on_swarming_builders": false, + "dimension_sets": [ + { + "gpu": "0000:0000", + "os": "Android" + } + ] + } + }, + { + "args": [ + "screenshot_sync", + "--show-stdout", + "--browser=android-chromium", + "-v", + "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc" + ], + "isolate_name": "telemetry_gpu_integration_test", + "name": "screenshot_sync_tests", + "override_compile_targets": [ + "telemetry_gpu_integration_test_run" + ], + "swarming": { + "can_use_on_swarming_builders": false, + "dimension_sets": [ + { + "gpu": "0000:0000", + "os": "Android" + } + ] + } + }, + { + "args": [ + "trace_test", + "--show-stdout", + "--browser=android-chromium", + "-v", + "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc" + ], + "isolate_name": "telemetry_gpu_integration_test", + "name": "trace_test", + "override_compile_targets": [ + "telemetry_gpu_integration_test_run" + ], + "swarming": { + "can_use_on_swarming_builders": false, + "dimension_sets": [ + { + "gpu": "0000:0000", + "os": "Android" + } + ] + } + }, + { + "args": [ + "webgl_conformance", + "--show-stdout", + "--browser=android-chromium", + "-v", + "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc" + ], + "isolate_name": "telemetry_gpu_integration_test", + "name": "webgl_conformance_tests", + "override_compile_targets": [ + "telemetry_gpu_integration_test_run" + ], + "swarming": { + "can_use_on_swarming_builders": false, + "dimension_sets": [ + { + "gpu": "0000:0000", + "os": "Android" + } + ] + } + } + ] + }, "Android Release (Nexus 5)": { "gtest_tests": [ {
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index 38df11c..58a4665 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -1779,14 +1779,6 @@ # 2017-02-17: These directories were just imported but expectations and baselines haven't been set yet. crbug.com/692105 external/wpt/service-workers/service-worker/fetch-event-within-sw-manual.https.html [ Skip ] crbug.com/692105 external/wpt/service-workers/service-worker/fetch-event-within-sw.https.html [ Skip ] -crbug.com/692105 external/wpt/streams/readable-streams/floating-point-total-queue-size.dedicatedworker.html [ Skip ] -crbug.com/692105 external/wpt/streams/readable-streams/floating-point-total-queue-size.html [ Skip ] -crbug.com/692105 external/wpt/streams/readable-streams/floating-point-total-queue-size.serviceworker.https.html [ Skip ] -crbug.com/692105 external/wpt/streams/readable-streams/floating-point-total-queue-size.sharedworker.html [ Skip ] -crbug.com/692105 external/wpt/streams/writable-streams/floating-point-total-queue-size.dedicatedworker.html [ Skip ] -crbug.com/692105 external/wpt/streams/writable-streams/floating-point-total-queue-size.html [ Skip ] -crbug.com/692105 external/wpt/streams/writable-streams/floating-point-total-queue-size.serviceworker.https.html [ Skip ] -crbug.com/692105 external/wpt/streams/writable-streams/floating-point-total-queue-size.sharedworker.html [ Skip ] # These policies are not implemented yet. crbug.com/627968 external/wpt/referrer-policy/origin-when-cross-origin/http-rp/same-origin/ [ Skip ] @@ -1841,6 +1833,10 @@ crbug.com/695270 [ Linux ] external/csswg-test/css-writing-modes-3/overconstrained-rel-pos-rtl-top-bottom-vrl-006.xht [ Failure ] # ====== New tests from w3c-test-autoroller added here ====== +crbug.com/626703 external/wpt/streams/readable-streams/floating-point-total-queue-size.dedicatedworker.html [ Timeout ] +crbug.com/626703 external/wpt/streams/readable-streams/floating-point-total-queue-size.sharedworker.html [ Timeout ] +crbug.com/626703 external/wpt/streams/writable-streams/floating-point-total-queue-size.dedicatedworker.html [ Timeout ] +crbug.com/626703 external/wpt/streams/writable-streams/floating-point-total-queue-size.sharedworker.html [ Timeout ] crbug.com/626703 external/wpt/mediacapture-streams/MediaStreamTrack-end-manual.https.html [ Timeout ] crbug.com/626703 external/wpt/webvtt/rendering/cues-with-video/processing-model/selectors/cue_function/underline_object/underline_with_class_object_specific_selector.html [ Failure ] crbug.com/626703 external/wpt/webvtt/rendering/cues-with-video/processing-model/selectors/default_styles/inherit_as_default_value_inherits_values_from_media_element.html [ Failure ] @@ -2148,7 +2144,6 @@ crbug.com/626703 external/wpt/streams/readable-streams/count-queuing-strategy-integration.html [ Failure Timeout ] crbug.com/626703 external/wpt/streams/readable-streams/garbage-collection.html [ Failure Timeout ] crbug.com/626703 external/wpt/streams/readable-streams/general.html [ Failure Timeout ] -crbug.com/626703 external/wpt/streams/readable-streams/pipe-through.html [ Failure Timeout ] crbug.com/626703 external/wpt/streams/readable-streams/readable-stream-reader.html [ Failure Timeout ] crbug.com/626703 external/wpt/streams/readable-streams/tee.html [ Failure Timeout ] crbug.com/626703 external/wpt/streams/readable-streams/templated.html [ Failure Timeout ] @@ -2447,8 +2442,6 @@ crbug.com/681468 fast/forms/suggestion-picker/date-suggestion-picker-appearance-zoom125.html [ Failure Pass ] crbug.com/681468 fast/forms/suggestion-picker/date-suggestion-picker-appearance-zoom200.html [ Failure Pass ] -crbug.com/v8/6005 inspector/console/console-format.html [ NeedsManualRebaseline ] - crbug.com/683800 [ Win7 Debug ] external/wpt/selection/ [ Failure Pass ] # CQ and Rebaseline-cl for crrev.com/2626973005 does not include this
diff --git a/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/floating-point-total-queue-size-expected.txt b/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/floating-point-total-queue-size-expected.txt new file mode 100644 index 0000000..41e78c1b --- /dev/null +++ b/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/floating-point-total-queue-size-expected.txt
@@ -0,0 +1,7 @@ +This is a testharness.js-based test. +FAIL Floating point arithmetic must manifest near NUMBER.MAX_SAFE_INTEGER (total ends up positive) assert_equals: [[queueTotalSize]] must clamp to 0 if it becomes negative expected 0 but got 1 +FAIL Floating point arithmetic must manifest near 0 (total ends up positive, but clamped) assert_equals: [[queueTotalSize]] must clamp to 0 if it becomes negative expected 0 but got 1.1102230246251565e-16 +PASS Floating point arithmetic must manifest near 0 (total ends up positive, and not clamped) +PASS Floating point arithmetic must manifest near 0 (total ends up zero) +Harness: the test ran to completion. +
diff --git a/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/floating-point-total-queue-size.serviceworker.https-expected.txt b/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/floating-point-total-queue-size.serviceworker.https-expected.txt new file mode 100644 index 0000000..a2c0250b --- /dev/null +++ b/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/floating-point-total-queue-size.serviceworker.https-expected.txt
@@ -0,0 +1,8 @@ +This is a testharness.js-based test. +PASS Service worker test setup +FAIL Floating point arithmetic must manifest near NUMBER.MAX_SAFE_INTEGER (total ends up positive) assert_equals: [[queueTotalSize]] must clamp to 0 if it becomes negative expected 0 but got 1 +FAIL Floating point arithmetic must manifest near 0 (total ends up positive, but clamped) assert_equals: [[queueTotalSize]] must clamp to 0 if it becomes negative expected 0 but got 1.1102230246251565e-16 +PASS Floating point arithmetic must manifest near 0 (total ends up positive, and not clamped) +PASS Floating point arithmetic must manifest near 0 (total ends up zero) +Harness: the test ran to completion. +
diff --git a/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/floating-point-total-queue-size-expected.txt b/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/floating-point-total-queue-size-expected.txt new file mode 100644 index 0000000..41e78c1b --- /dev/null +++ b/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/floating-point-total-queue-size-expected.txt
@@ -0,0 +1,7 @@ +This is a testharness.js-based test. +FAIL Floating point arithmetic must manifest near NUMBER.MAX_SAFE_INTEGER (total ends up positive) assert_equals: [[queueTotalSize]] must clamp to 0 if it becomes negative expected 0 but got 1 +FAIL Floating point arithmetic must manifest near 0 (total ends up positive, but clamped) assert_equals: [[queueTotalSize]] must clamp to 0 if it becomes negative expected 0 but got 1.1102230246251565e-16 +PASS Floating point arithmetic must manifest near 0 (total ends up positive, and not clamped) +PASS Floating point arithmetic must manifest near 0 (total ends up zero) +Harness: the test ran to completion. +
diff --git a/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/floating-point-total-queue-size.serviceworker.https-expected.txt b/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/floating-point-total-queue-size.serviceworker.https-expected.txt new file mode 100644 index 0000000..a2c0250b --- /dev/null +++ b/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/floating-point-total-queue-size.serviceworker.https-expected.txt
@@ -0,0 +1,8 @@ +This is a testharness.js-based test. +PASS Service worker test setup +FAIL Floating point arithmetic must manifest near NUMBER.MAX_SAFE_INTEGER (total ends up positive) assert_equals: [[queueTotalSize]] must clamp to 0 if it becomes negative expected 0 but got 1 +FAIL Floating point arithmetic must manifest near 0 (total ends up positive, but clamped) assert_equals: [[queueTotalSize]] must clamp to 0 if it becomes negative expected 0 but got 1.1102230246251565e-16 +PASS Floating point arithmetic must manifest near 0 (total ends up positive, and not clamped) +PASS Floating point arithmetic must manifest near 0 (total ends up zero) +Harness: the test ran to completion. +
diff --git a/third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement-crash-style-recalc-after-dialog-close-expected.html b/third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement-crash-style-recalc-after-dialog-close-expected.html new file mode 100644 index 0000000..27e4ca1 --- /dev/null +++ b/third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement-crash-style-recalc-after-dialog-close-expected.html
@@ -0,0 +1,2 @@ +<!DOCTYPE html> +<div>The test passes if it doesn't crash</div>
diff --git a/third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement-crash-style-recalc-after-dialog-close.html b/third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement-crash-style-recalc-after-dialog-close.html new file mode 100644 index 0000000..d76af48 --- /dev/null +++ b/third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement-crash-style-recalc-after-dialog-close.html
@@ -0,0 +1,10 @@ +<!DOCTYPE html> +<div>The test passes if it doesn't crash</div> +<br> +<dialog id="dialog" style="position: relative"> +<input> +<script> + dialog.show(); + document.body.offsetTop; // force layout + dialog.close(); +</script> \ No newline at end of file
diff --git a/third_party/WebKit/PerformanceTests/Editing/move-down-with-hidden-elements.html b/third_party/WebKit/PerformanceTests/Editing/move-down-with-hidden-elements.html index 17b86fd..69de6a9 100644 --- a/third_party/WebKit/PerformanceTests/Editing/move-down-with-hidden-elements.html +++ b/third_party/WebKit/PerformanceTests/Editing/move-down-with-hidden-elements.html
@@ -2,7 +2,6 @@ <script src="../resources/runner.js"></script> <div id="sample"></div> <script> -const kCount = 10; const kElements = 10000; const metaElements = (() => { @@ -13,21 +12,25 @@ })(); const sample = document.getElementById('sample'); sample.innerHTML = [ - '<div hiddent>', ...metaElements, '</div>', - '<h1 id="target">first line of renderered text</h1>', - '<div hiddent>', ...metaElements, '</div>', + '<h1 id="before">first line of renderered text</h1>', + '<div hidden>', ...metaElements, '</div>', + '<h1 id="target">second line of renderered text</h1>', + '<div hidden>', ...metaElements, '</div>', + '<h1 id="after">third line of renderered text</h1>', ].join(''); const selection = window.getSelection(); -const target = document.getElementById('target'); -PerfTestRunner.measureRunsPerSecond({ +PerfTestRunner.measureTime({ description: 'Measures performance of move-down through non-renderered elements', + setup: () => { + selection.removeAllRanges(); + const target = document.getElementById('target'); + selection.collapse(target.firstChild, 5); + selection.extend(target.firstChild, 10); + }, run: () => { - selection.collapse(target, 0); - selection.extend(target, target.childNodes.length); - for (let counter = 0; counter < kCount; ++counter) - selection.modify('move', 'forward', 'line'); - }, + selection.modify('extend', 'forward', 'line'); + }, }); </script>
diff --git a/third_party/WebKit/PerformanceTests/Editing/move-up-with-hidden-elements.html b/third_party/WebKit/PerformanceTests/Editing/move-up-with-hidden-elements.html index b2def76..17891155 100644 --- a/third_party/WebKit/PerformanceTests/Editing/move-up-with-hidden-elements.html +++ b/third_party/WebKit/PerformanceTests/Editing/move-up-with-hidden-elements.html
@@ -2,7 +2,6 @@ <script src="../resources/runner.js"></script> <div id="sample"></div> <script> -const kCount = 10; const kElements = 10000; const metaElements = (() => { @@ -13,21 +12,25 @@ })(); const sample = document.getElementById('sample'); sample.innerHTML = [ - '<div hiddent>', ...metaElements, '</div>', - '<h1 id="target">first line of renderered text</h1>', - '<div hiddent>', ...metaElements, '</div>', + '<h1 id="before">first line of renderered text</h1>', + '<div hidden>', ...metaElements, '</div>', + '<h1 id="target">second line of renderered text</h1>', + '<div hidden>', ...metaElements, '</div>', + '<h1 id="after">third line of renderered text</h1>', ].join(''); const selection = window.getSelection(); -const target = document.getElementById('target'); -PerfTestRunner.measureRunsPerSecond({ +PerfTestRunner.measureTime({ description: 'Measures performance of move-up through non-renderered elements', + setup: () => { + selection.removeAllRanges(); + const target = document.getElementById('target'); + selection.collapse(target.firstChild, 5); + selection.extend(target.firstChild, 10); + }, run: () => { - selection.collapse(target, 0); - selection.extend(target, target.childNodes.length); - for (let counter = 0; counter < kCount; ++counter) - selection.modify('move', 'backward', 'line'); - }, + selection.modify('extend', 'backward', 'line'); + }, }); </script>
diff --git a/third_party/WebKit/Source/core/animation/CSSImageInterpolationType.cpp b/third_party/WebKit/Source/core/animation/CSSImageInterpolationType.cpp index 08a2428..ae4340c 100644 --- a/third_party/WebKit/Source/core/animation/CSSImageInterpolationType.cpp +++ b/third_party/WebKit/Source/core/animation/CSSImageInterpolationType.cpp
@@ -25,7 +25,8 @@ bool isSingle() const { return m_isSingle; } bool equals(const CSSImageNonInterpolableValue& other) const { - return m_start->equals(*other.m_start) && m_end->equals(*other.m_end); + return dataEquivalent(m_start, other.m_start) && + dataEquivalent(m_end, other.m_end); } static PassRefPtr<CSSImageNonInterpolableValue> merge(
diff --git a/third_party/WebKit/Source/core/animation/CSSInterpolationType.cpp b/third_party/WebKit/Source/core/animation/CSSInterpolationType.cpp index 861d27a5..11a2c54 100644 --- a/third_party/WebKit/Source/core/animation/CSSInterpolationType.cpp +++ b/third_party/WebKit/Source/core/animation/CSSInterpolationType.cpp
@@ -49,7 +49,7 @@ CSSVariableResolver::resolveVariableReferences( environment.state(), m_property, *m_variableReference, omitAnimationTainted); - return m_resolvedValue->equals(*resolvedValue); + return dataEquivalent(m_resolvedValue.get(), resolvedValue); } CSSPropertyID m_property; @@ -87,13 +87,7 @@ if (!inheritedValue) { inheritedValue = m_initialValue.get(); } - if (inheritedValue == m_inheritedValue.get()) { - return true; - } - if (!inheritedValue || !m_inheritedValue) { - return false; - } - return m_inheritedValue->equals(*inheritedValue); + return dataEquivalent(m_inheritedValue.get(), inheritedValue); } const AtomicString& m_name;
diff --git a/third_party/WebKit/Source/core/animation/animatable/AnimatableImage.cpp b/third_party/WebKit/Source/core/animation/animatable/AnimatableImage.cpp index eb2a5139..5949017 100644 --- a/third_party/WebKit/Source/core/animation/animatable/AnimatableImage.cpp +++ b/third_party/WebKit/Source/core/animation/animatable/AnimatableImage.cpp
@@ -60,7 +60,7 @@ } bool AnimatableImage::equalTo(const AnimatableValue* value) const { - return m_value->equals(*toAnimatableImage(value)->m_value.get()); + return dataEquivalent(m_value, toAnimatableImage(value)->m_value); } } // namespace blink
diff --git a/third_party/WebKit/Source/core/animation/animatable/AnimatableUnknown.h b/third_party/WebKit/Source/core/animation/animatable/AnimatableUnknown.h index 8d5c0b8..af25e09a 100644 --- a/third_party/WebKit/Source/core/animation/animatable/AnimatableUnknown.h +++ b/third_party/WebKit/Source/core/animation/animatable/AnimatableUnknown.h
@@ -75,13 +75,12 @@ inline bool AnimatableUnknown::equalTo(const AnimatableValue* value) const { const AnimatableUnknown* unknown = toAnimatableUnknown(value); - return m_value == unknown->m_value || m_value->equals(*unknown->m_value); + return dataEquivalent(m_value, unknown->m_value); } inline bool AnimatableUnknown::usesDefaultInterpolationWith( const AnimatableValue* value) const { - const AnimatableUnknown& unknown = toAnimatableUnknown(*value); - return !m_value->equals(*unknown.m_value); + return !equalTo(value); } } // namespace blink
diff --git a/third_party/WebKit/Source/core/animation/css/CSSAnimationUpdate.h b/third_party/WebKit/Source/core/animation/css/CSSAnimationUpdate.h index 001b805..14e4299 100644 --- a/third_party/WebKit/Source/core/animation/css/CSSAnimationUpdate.h +++ b/third_party/WebKit/Source/core/animation/css/CSSAnimationUpdate.h
@@ -106,8 +106,10 @@ m_newTransitions = update.newTransitions(); m_activeInterpolationsForAnimations = update.activeInterpolationsForAnimations(); - m_activeInterpolationsForTransitions = - update.activeInterpolationsForTransitions(); + m_activeInterpolationsForCustomTransitions = + update.activeInterpolationsForCustomTransitions(); + m_activeInterpolationsForStandardTransitions = + update.activeInterpolationsForStandardTransitions(); m_cancelledAnimationIndices = update.cancelledAnimationIndices(); m_animationIndicesWithPauseToggled = update.animationIndicesWithPauseToggled(); @@ -121,7 +123,8 @@ m_animationsWithUpdates.clear(); m_newTransitions.clear(); m_activeInterpolationsForAnimations.clear(); - m_activeInterpolationsForTransitions.clear(); + m_activeInterpolationsForCustomTransitions.clear(); + m_activeInterpolationsForStandardTransitions.clear(); m_cancelledAnimationIndices.clear(); m_animationIndicesWithPauseToggled.clear(); m_cancelledTransitions.clear(); @@ -234,15 +237,24 @@ void adoptActiveInterpolationsForAnimations(ActiveInterpolationsMap& newMap) { newMap.swap(m_activeInterpolationsForAnimations); } - void adoptActiveInterpolationsForTransitions( + void adoptActiveInterpolationsForCustomTransitions( ActiveInterpolationsMap& newMap) { - newMap.swap(m_activeInterpolationsForTransitions); + newMap.swap(m_activeInterpolationsForCustomTransitions); + } + void adoptActiveInterpolationsForStandardTransitions( + ActiveInterpolationsMap& newMap) { + newMap.swap(m_activeInterpolationsForStandardTransitions); } const ActiveInterpolationsMap& activeInterpolationsForAnimations() const { return m_activeInterpolationsForAnimations; } - const ActiveInterpolationsMap& activeInterpolationsForTransitions() const { - return m_activeInterpolationsForTransitions; + const ActiveInterpolationsMap& activeInterpolationsForCustomTransitions() + const { + return m_activeInterpolationsForCustomTransitions; + } + const ActiveInterpolationsMap& activeInterpolationsForStandardTransitions() + const { + return m_activeInterpolationsForStandardTransitions; } ActiveInterpolationsMap& activeInterpolationsForAnimations() { return m_activeInterpolationsForAnimations; @@ -256,7 +268,8 @@ m_cancelledTransitions.isEmpty() && m_finishedTransitions.isEmpty() && m_activeInterpolationsForAnimations.isEmpty() && - m_activeInterpolationsForTransitions.isEmpty() && + m_activeInterpolationsForCustomTransitions.isEmpty() && + m_activeInterpolationsForStandardTransitions.isEmpty() && m_updatedCompositorKeyframes.isEmpty(); } @@ -285,7 +298,8 @@ HashSet<PropertyHandle> m_finishedTransitions; ActiveInterpolationsMap m_activeInterpolationsForAnimations; - ActiveInterpolationsMap m_activeInterpolationsForTransitions; + ActiveInterpolationsMap m_activeInterpolationsForCustomTransitions; + ActiveInterpolationsMap m_activeInterpolationsForStandardTransitions; friend class PendingAnimationUpdate; };
diff --git a/third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp b/third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp index 3f8a3c3..67ca8e7a 100644 --- a/third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp +++ b/third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp
@@ -955,7 +955,7 @@ for (const auto& entry : update.activeInterpolationsForAnimations()) activeInterpolationsForTransitions.erase(entry.key); } - update.adoptActiveInterpolationsForTransitions( + update.adoptActiveInterpolationsForStandardTransitions( activeInterpolationsForTransitions); }
diff --git a/third_party/WebKit/Source/core/css/CSSBasicShapeValues.cpp b/third_party/WebKit/Source/core/css/CSSBasicShapeValues.cpp index 3e78cad..93fa4124 100644 --- a/third_party/WebKit/Source/core/css/CSSBasicShapeValues.cpp +++ b/third_party/WebKit/Source/core/css/CSSBasicShapeValues.cpp
@@ -130,9 +130,9 @@ bool CSSBasicShapeCircleValue::equals( const CSSBasicShapeCircleValue& other) const { - return compareCSSValuePtr(m_centerX, other.m_centerX) && - compareCSSValuePtr(m_centerY, other.m_centerY) && - compareCSSValuePtr(m_radius, other.m_radius); + return dataEquivalent(m_centerX, other.m_centerX) && + dataEquivalent(m_centerY, other.m_centerY) && + dataEquivalent(m_radius, other.m_radius); } DEFINE_TRACE_AFTER_DISPATCH(CSSBasicShapeCircleValue) { @@ -208,10 +208,10 @@ bool CSSBasicShapeEllipseValue::equals( const CSSBasicShapeEllipseValue& other) const { - return compareCSSValuePtr(m_centerX, other.m_centerX) && - compareCSSValuePtr(m_centerY, other.m_centerY) && - compareCSSValuePtr(m_radiusX, other.m_radiusX) && - compareCSSValuePtr(m_radiusY, other.m_radiusY); + return dataEquivalent(m_centerX, other.m_centerX) && + dataEquivalent(m_centerY, other.m_centerY) && + dataEquivalent(m_radiusX, other.m_radiusX) && + dataEquivalent(m_radiusY, other.m_radiusY); } DEFINE_TRACE_AFTER_DISPATCH(CSSBasicShapeEllipseValue) { @@ -411,14 +411,14 @@ bool CSSBasicShapeInsetValue::equals( const CSSBasicShapeInsetValue& other) const { - return compareCSSValuePtr(m_top, other.m_top) && - compareCSSValuePtr(m_right, other.m_right) && - compareCSSValuePtr(m_bottom, other.m_bottom) && - compareCSSValuePtr(m_left, other.m_left) && - compareCSSValuePtr(m_topLeftRadius, other.m_topLeftRadius) && - compareCSSValuePtr(m_topRightRadius, other.m_topRightRadius) && - compareCSSValuePtr(m_bottomRightRadius, other.m_bottomRightRadius) && - compareCSSValuePtr(m_bottomLeftRadius, other.m_bottomLeftRadius); + return dataEquivalent(m_top, other.m_top) && + dataEquivalent(m_right, other.m_right) && + dataEquivalent(m_bottom, other.m_bottom) && + dataEquivalent(m_left, other.m_left) && + dataEquivalent(m_topLeftRadius, other.m_topLeftRadius) && + dataEquivalent(m_topRightRadius, other.m_topRightRadius) && + dataEquivalent(m_bottomRightRadius, other.m_bottomRightRadius) && + dataEquivalent(m_bottomLeftRadius, other.m_bottomLeftRadius); } DEFINE_TRACE_AFTER_DISPATCH(CSSBasicShapeInsetValue) {
diff --git a/third_party/WebKit/Source/core/css/CSSBorderImageSliceValue.cpp b/third_party/WebKit/Source/core/css/CSSBorderImageSliceValue.cpp index 9d2e959..66cf3120 100644 --- a/third_party/WebKit/Source/core/css/CSSBorderImageSliceValue.cpp +++ b/third_party/WebKit/Source/core/css/CSSBorderImageSliceValue.cpp
@@ -47,7 +47,7 @@ bool CSSBorderImageSliceValue::equals( const CSSBorderImageSliceValue& other) const { - return m_fill == other.m_fill && compareCSSValuePtr(m_slices, other.m_slices); + return m_fill == other.m_fill && dataEquivalent(m_slices, other.m_slices); } DEFINE_TRACE_AFTER_DISPATCH(CSSBorderImageSliceValue) {
diff --git a/third_party/WebKit/Source/core/css/CSSCalculationValue.cpp b/third_party/WebKit/Source/core/css/CSSCalculationValue.cpp index b1be505..1a9a382 100644 --- a/third_party/WebKit/Source/core/css/CSSCalculationValue.cpp +++ b/third_party/WebKit/Source/core/css/CSSCalculationValue.cpp
@@ -143,7 +143,7 @@ } bool CSSCalcValue::equals(const CSSCalcValue& other) const { - return compareCSSValuePtr(m_expression, other.m_expression); + return dataEquivalent(m_expression, other.m_expression); } double CSSCalcValue::clampToPermittedRange(double value) const { @@ -243,11 +243,11 @@ m_value->accumulateLengthArray(lengthArray, multiplier); } - bool equals(const CSSCalcExpressionNode& other) const override { + bool operator==(const CSSCalcExpressionNode& other) const override { if (getType() != other.getType()) return false; - return compareCSSValuePtr( + return dataEquivalent( m_value, static_cast<const CSSCalcPrimitiveValue&>(other).m_value); } @@ -556,14 +556,14 @@ m_rightSide->customCSSText(), m_operator); } - bool equals(const CSSCalcExpressionNode& exp) const override { + bool operator==(const CSSCalcExpressionNode& exp) const override { if (getType() != exp.getType()) return false; const CSSCalcBinaryOperation& other = static_cast<const CSSCalcBinaryOperation&>(exp); - return compareCSSValuePtr(m_leftSide, other.m_leftSide) && - compareCSSValuePtr(m_rightSide, other.m_rightSide) && + return dataEquivalent(m_leftSide, other.m_leftSide) && + dataEquivalent(m_rightSide, other.m_rightSide) && m_operator == other.m_operator; }
diff --git a/third_party/WebKit/Source/core/css/CSSCalculationValue.h b/third_party/WebKit/Source/core/css/CSSCalculationValue.h index 086aba2..46ce3d7 100644 --- a/third_party/WebKit/Source/core/css/CSSCalculationValue.h +++ b/third_party/WebKit/Source/core/css/CSSCalculationValue.h
@@ -78,7 +78,7 @@ PixelsAndPercent&, float multiplier = 1) const = 0; virtual String customCSSText() const = 0; - virtual bool equals(const CSSCalcExpressionNode& other) const { + virtual bool operator==(const CSSCalcExpressionNode& other) const { return m_category == other.m_category && m_isInteger == other.m_isInteger; } virtual Type getType() const = 0;
diff --git a/third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.cpp b/third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.cpp index 93b0508..f140e9e88 100644 --- a/third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.cpp +++ b/third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.cpp
@@ -464,7 +464,7 @@ } } const CSSValue* value = getPropertyCSSValue(propertyID); - return value && propertyValue && value->equals(*propertyValue); + return dataEquivalent(value, propertyValue); } MutableStylePropertySet* CSSComputedStyleDeclaration::copyProperties() const {
diff --git a/third_party/WebKit/Source/core/css/CSSCrossfadeValue.cpp b/third_party/WebKit/Source/core/css/CSSCrossfadeValue.cpp index 2829755..4aabbb8e 100644 --- a/third_party/WebKit/Source/core/css/CSSCrossfadeValue.cpp +++ b/third_party/WebKit/Source/core/css/CSSCrossfadeValue.cpp
@@ -286,9 +286,9 @@ } bool CSSCrossfadeValue::equals(const CSSCrossfadeValue& other) const { - return compareCSSValuePtr(m_fromValue, other.m_fromValue) && - compareCSSValuePtr(m_toValue, other.m_toValue) && - compareCSSValuePtr(m_percentageValue, other.m_percentageValue); + return dataEquivalent(m_fromValue, other.m_fromValue) && + dataEquivalent(m_toValue, other.m_toValue) && + dataEquivalent(m_percentageValue, other.m_percentageValue); } DEFINE_TRACE_AFTER_DISPATCH(CSSCrossfadeValue) {
diff --git a/third_party/WebKit/Source/core/css/CSSCursorImageValue.cpp b/third_party/WebKit/Source/core/css/CSSCursorImageValue.cpp index 9309d823..a3af03c5 100644 --- a/third_party/WebKit/Source/core/css/CSSCursorImageValue.cpp +++ b/third_party/WebKit/Source/core/css/CSSCursorImageValue.cpp
@@ -54,7 +54,7 @@ return (m_hotSpotSpecified ? other.m_hotSpotSpecified && m_hotSpot == other.m_hotSpot : !other.m_hotSpotSpecified) && - compareCSSValuePtr(m_imageValue, other.m_imageValue); + dataEquivalent(m_imageValue, other.m_imageValue); } DEFINE_TRACE_AFTER_DISPATCH(CSSCursorImageValue) {
diff --git a/third_party/WebKit/Source/core/css/CSSGradientValue.cpp b/third_party/WebKit/Source/core/css/CSSGradientValue.cpp index c07244c..7887781 100644 --- a/third_party/WebKit/Source/core/css/CSSGradientValue.cpp +++ b/third_party/WebKit/Source/core/css/CSSGradientValue.cpp
@@ -892,34 +892,32 @@ bool CSSLinearGradientValue::equals(const CSSLinearGradientValue& other) const { if (m_gradientType == CSSDeprecatedLinearGradient) return other.m_gradientType == m_gradientType && - compareCSSValuePtr(m_firstX, other.m_firstX) && - compareCSSValuePtr(m_firstY, other.m_firstY) && - compareCSSValuePtr(m_secondX, other.m_secondX) && - compareCSSValuePtr(m_secondY, other.m_secondY) && + dataEquivalent(m_firstX, other.m_firstX) && + dataEquivalent(m_firstY, other.m_firstY) && + dataEquivalent(m_secondX, other.m_secondX) && + dataEquivalent(m_secondY, other.m_secondY) && m_stops == other.m_stops; if (m_repeating != other.m_repeating) return false; if (m_angle) - return compareCSSValuePtr(m_angle, other.m_angle) && - m_stops == other.m_stops; + return dataEquivalent(m_angle, other.m_angle) && m_stops == other.m_stops; if (other.m_angle) return false; bool equalXandY = false; - if (m_firstX && m_firstY) - equalXandY = compareCSSValuePtr(m_firstX, other.m_firstX) && - compareCSSValuePtr(m_firstY, other.m_firstY); - else if (m_firstX) - equalXandY = - compareCSSValuePtr(m_firstX, other.m_firstX) && !other.m_firstY; - else if (m_firstY) - equalXandY = - compareCSSValuePtr(m_firstY, other.m_firstY) && !other.m_firstX; - else + if (m_firstX && m_firstY) { + equalXandY = dataEquivalent(m_firstX, other.m_firstX) && + dataEquivalent(m_firstY, other.m_firstY); + } else if (m_firstX) { + equalXandY = dataEquivalent(m_firstX, other.m_firstX) && !other.m_firstY; + } else if (m_firstY) { + equalXandY = dataEquivalent(m_firstY, other.m_firstY) && !other.m_firstX; + } else { equalXandY = !other.m_firstX && !other.m_firstY; + } return equalXandY && m_stops == other.m_stops; } @@ -1263,29 +1261,28 @@ bool CSSRadialGradientValue::equals(const CSSRadialGradientValue& other) const { if (m_gradientType == CSSDeprecatedRadialGradient) return other.m_gradientType == m_gradientType && - compareCSSValuePtr(m_firstX, other.m_firstX) && - compareCSSValuePtr(m_firstY, other.m_firstY) && - compareCSSValuePtr(m_secondX, other.m_secondX) && - compareCSSValuePtr(m_secondY, other.m_secondY) && - compareCSSValuePtr(m_firstRadius, other.m_firstRadius) && - compareCSSValuePtr(m_secondRadius, other.m_secondRadius) && + dataEquivalent(m_firstX, other.m_firstX) && + dataEquivalent(m_firstY, other.m_firstY) && + dataEquivalent(m_secondX, other.m_secondX) && + dataEquivalent(m_secondY, other.m_secondY) && + dataEquivalent(m_firstRadius, other.m_firstRadius) && + dataEquivalent(m_secondRadius, other.m_secondRadius) && m_stops == other.m_stops; if (m_repeating != other.m_repeating) return false; bool equalXandY = false; - if (m_firstX && m_firstY) - equalXandY = compareCSSValuePtr(m_firstX, other.m_firstX) && - compareCSSValuePtr(m_firstY, other.m_firstY); - else if (m_firstX) - equalXandY = - compareCSSValuePtr(m_firstX, other.m_firstX) && !other.m_firstY; - else if (m_firstY) - equalXandY = - compareCSSValuePtr(m_firstY, other.m_firstY) && !other.m_firstX; - else + if (m_firstX && m_firstY) { + equalXandY = dataEquivalent(m_firstX, other.m_firstX) && + dataEquivalent(m_firstY, other.m_firstY); + } else if (m_firstX) { + equalXandY = dataEquivalent(m_firstX, other.m_firstX) && !other.m_firstY; + } else if (m_firstY) { + equalXandY = dataEquivalent(m_firstY, other.m_firstY) && !other.m_firstX; + } else { equalXandY = !other.m_firstX && !other.m_firstY; + } if (!equalXandY) return false; @@ -1294,16 +1291,16 @@ bool equalSizingBehavior = true; bool equalHorizontalAndVerticalSize = true; - if (m_shape) - equalShape = compareCSSValuePtr(m_shape, other.m_shape); - else if (m_sizingBehavior) + if (m_shape) { + equalShape = dataEquivalent(m_shape, other.m_shape); + } else if (m_sizingBehavior) { equalSizingBehavior = - compareCSSValuePtr(m_sizingBehavior, other.m_sizingBehavior); - else if (m_endHorizontalSize && m_endVerticalSize) + dataEquivalent(m_sizingBehavior, other.m_sizingBehavior); + } else if (m_endHorizontalSize && m_endVerticalSize) { equalHorizontalAndVerticalSize = - compareCSSValuePtr(m_endHorizontalSize, other.m_endHorizontalSize) && - compareCSSValuePtr(m_endVerticalSize, other.m_endVerticalSize); - else { + dataEquivalent(m_endHorizontalSize, other.m_endHorizontalSize) && + dataEquivalent(m_endVerticalSize, other.m_endVerticalSize); + } else { equalShape = !other.m_shape; equalSizingBehavior = !other.m_sizingBehavior; equalHorizontalAndVerticalSize =
diff --git a/third_party/WebKit/Source/core/css/CSSGradientValue.h b/third_party/WebKit/Source/core/css/CSSGradientValue.h index 8aa8553..0fcead9 100644 --- a/third_party/WebKit/Source/core/css/CSSGradientValue.h +++ b/third_party/WebKit/Source/core/css/CSSGradientValue.h
@@ -64,8 +64,8 @@ Member<CSSValue> m_color; bool m_colorIsDerivedFromElement; bool operator==(const CSSGradientColorStop& other) const { - return compareCSSValuePtr(m_color, other.m_color) && - compareCSSValuePtr(m_position, other.m_position); + return dataEquivalent(m_color, other.m_color) && + dataEquivalent(m_position, other.m_position); } bool isHint() const { ASSERT(m_color || m_position);
diff --git a/third_party/WebKit/Source/core/css/CSSProperty.cpp b/third_party/WebKit/Source/core/css/CSSProperty.cpp index b6740a8..b3adce7 100644 --- a/third_party/WebKit/Source/core/css/CSSProperty.cpp +++ b/third_party/WebKit/Source/core/css/CSSProperty.cpp
@@ -289,7 +289,7 @@ } bool CSSProperty::operator==(const CSSProperty& other) const { - return m_value->equals(*other.m_value) && + return dataEquivalent(m_value, other.m_value) && isImportant() == other.isImportant(); }
diff --git a/third_party/WebKit/Source/core/css/CSSQuadValue.h b/third_party/WebKit/Source/core/css/CSSQuadValue.h index 3cfbb66..28ec9d3 100644 --- a/third_party/WebKit/Source/core/css/CSSQuadValue.h +++ b/third_party/WebKit/Source/core/css/CSSQuadValue.h
@@ -49,10 +49,10 @@ String customCSSText() const; bool equals(const CSSQuadValue& other) const { - return compareCSSValuePtr(m_top, other.m_top) && - compareCSSValuePtr(m_right, other.m_right) && - compareCSSValuePtr(m_left, other.m_left) && - compareCSSValuePtr(m_bottom, other.m_bottom); + return dataEquivalent(m_top, other.m_top) && + dataEquivalent(m_right, other.m_right) && + dataEquivalent(m_left, other.m_left) && + dataEquivalent(m_bottom, other.m_bottom); } DECLARE_TRACE_AFTER_DISPATCH();
diff --git a/third_party/WebKit/Source/core/css/CSSReflectValue.cpp b/third_party/WebKit/Source/core/css/CSSReflectValue.cpp index 918f99c..7bed6aa 100644 --- a/third_party/WebKit/Source/core/css/CSSReflectValue.cpp +++ b/third_party/WebKit/Source/core/css/CSSReflectValue.cpp
@@ -39,8 +39,8 @@ bool CSSReflectValue::equals(const CSSReflectValue& other) const { return m_direction == other.m_direction && - compareCSSValuePtr(m_offset, other.m_offset) && - compareCSSValuePtr(m_mask, other.m_mask); + dataEquivalent(m_offset, other.m_offset) && + dataEquivalent(m_mask, other.m_mask); } DEFINE_TRACE_AFTER_DISPATCH(CSSReflectValue) {
diff --git a/third_party/WebKit/Source/core/css/CSSShadowValue.cpp b/third_party/WebKit/Source/core/css/CSSShadowValue.cpp index 6ae71871..8bfd1f2 100644 --- a/third_party/WebKit/Source/core/css/CSSShadowValue.cpp +++ b/third_party/WebKit/Source/core/css/CSSShadowValue.cpp
@@ -76,11 +76,10 @@ } bool CSSShadowValue::equals(const CSSShadowValue& other) const { - return compareCSSValuePtr(color, other.color) && - compareCSSValuePtr(x, other.x) && compareCSSValuePtr(y, other.y) && - compareCSSValuePtr(blur, other.blur) && - compareCSSValuePtr(spread, other.spread) && - compareCSSValuePtr(style, other.style); + return dataEquivalent(color, other.color) && dataEquivalent(x, other.x) && + dataEquivalent(y, other.y) && dataEquivalent(blur, other.blur) && + dataEquivalent(spread, other.spread) && + dataEquivalent(style, other.style); } DEFINE_TRACE_AFTER_DISPATCH(CSSShadowValue) {
diff --git a/third_party/WebKit/Source/core/css/CSSValue.cpp b/third_party/WebKit/Source/core/css/CSSValue.cpp index 257b780b..3098e4ef 100644 --- a/third_party/WebKit/Source/core/css/CSSValue.cpp +++ b/third_party/WebKit/Source/core/css/CSSValue.cpp
@@ -141,7 +141,7 @@ static_cast<const ChildClassType&>(second)); } -bool CSSValue::equals(const CSSValue& other) const { +bool CSSValue::operator==(const CSSValue& other) const { if (m_classType == other.m_classType) { switch (getClassType()) { case BasicShapeCircleClass:
diff --git a/third_party/WebKit/Source/core/css/CSSValue.h b/third_party/WebKit/Source/core/css/CSSValue.h index 23ad709f..8ac3077e 100644 --- a/third_party/WebKit/Source/core/css/CSSValue.h +++ b/third_party/WebKit/Source/core/css/CSSValue.h
@@ -22,6 +22,7 @@ #define CSSValue_h #include "core/CoreExport.h" +#include "core/style/DataEquivalency.h" #include "platform/heap/Handle.h" #include "wtf/RefPtr.h" @@ -151,7 +152,7 @@ bool mayContainUrl() const; void reResolveUrl(const Document&) const; - bool equals(const CSSValue&) const; + bool operator==(const CSSValue&) const; void finalizeGarbageCollectedObject(); DEFINE_INLINE_TRACE_AFTER_DISPATCH() {} @@ -261,50 +262,18 @@ const HeapVector<Member<CSSValueType>, inlineCapacity>& firstVector, const HeapVector<Member<CSSValueType>, inlineCapacity>& secondVector) { size_t size = firstVector.size(); - if (size != secondVector.size()) - return false; - - for (size_t i = 0; i < size; i++) { - const Member<CSSValueType>& firstPtr = firstVector[i]; - const Member<CSSValueType>& secondPtr = secondVector[i]; - if (firstPtr == secondPtr || - (firstPtr && secondPtr && firstPtr->equals(*secondPtr))) - continue; + if (size != secondVector.size()) { return false; } + + for (size_t i = 0; i < size; i++) { + if (!dataEquivalent(firstVector[i], secondVector[i])) { + return false; + } + } return true; } -template <typename CSSValueType> -inline bool compareCSSValuePtr(const RefPtr<CSSValueType>& first, - const RefPtr<CSSValueType>& second) { - if (first == second) - return true; - if (!first || !second) - return false; - return first->equals(*second); -} - -template <typename CSSValueType> -inline bool compareCSSValuePtr(const CSSValueType* first, - const CSSValueType* second) { - if (first == second) - return true; - if (!first || !second) - return false; - return first->equals(*second); -} - -template <typename CSSValueType> -inline bool compareCSSValuePtr(const Member<CSSValueType>& first, - const Member<CSSValueType>& second) { - if (first == second) - return true; - if (!first || !second) - return false; - return first->equals(*second); -} - #define DEFINE_CSS_VALUE_TYPE_CASTS(thisType, predicate) \ DEFINE_TYPE_CASTS(thisType, CSSValue, value, value->predicate, \ value.predicate)
diff --git a/third_party/WebKit/Source/core/css/CSSValueList.cpp b/third_party/WebKit/Source/core/css/CSSValueList.cpp index 2e768e0..fac73e7 100644 --- a/third_party/WebKit/Source/core/css/CSSValueList.cpp +++ b/third_party/WebKit/Source/core/css/CSSValueList.cpp
@@ -47,7 +47,7 @@ bool found = false; for (int index = m_values.size() - 1; index >= 0; --index) { Member<const CSSValue>& value = m_values.at(index); - if (value && value->equals(val)) { + if (value && *value == val) { m_values.remove(index); found = true; } @@ -59,8 +59,9 @@ bool CSSValueList::hasValue(const CSSValue& val) const { for (size_t index = 0; index < m_values.size(); index++) { const Member<const CSSValue>& value = m_values.at(index); - if (value && value->equals(val)) + if (value && *value == val) { return true; + } } return false; }
diff --git a/third_party/WebKit/Source/core/css/CSSValuePair.h b/third_party/WebKit/Source/core/css/CSSValuePair.h index 6e7b9bb..06213db 100644 --- a/third_party/WebKit/Source/core/css/CSSValuePair.h +++ b/third_party/WebKit/Source/core/css/CSSValuePair.h
@@ -50,8 +50,8 @@ bool equals(const CSSValuePair& other) const { ASSERT(m_identicalValuesPolicy == other.m_identicalValuesPolicy); - return compareCSSValuePtr(m_first, other.m_first) && - compareCSSValuePtr(m_second, other.m_second); + return dataEquivalent(m_first, other.m_first) && + dataEquivalent(m_second, other.m_second); } DECLARE_TRACE_AFTER_DISPATCH();
diff --git a/third_party/WebKit/Source/core/css/CSSValueTestHelper.h b/third_party/WebKit/Source/core/css/CSSValueTestHelper.h index f418285..70f52715 100644 --- a/third_party/WebKit/Source/core/css/CSSValueTestHelper.h +++ b/third_party/WebKit/Source/core/css/CSSValueTestHelper.h
@@ -55,10 +55,6 @@ namespace blink { -inline bool operator==(const CSSValue& a, const CSSValue& b) { - return a.equals(b); -} - inline void PrintTo(const CSSValue& cssValue, ::std::ostream* os, const char* typeName = "CSSValue") {
diff --git a/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp b/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp index f93e329..7cf3f8ef 100644 --- a/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp +++ b/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
@@ -1341,7 +1341,7 @@ static const CSSValue& valueForBorderRadiusCorner(LengthSize radius, const ComputedStyle& style) { CSSValueList& list = *valuesForBorderRadiusCorner(radius, style); - if (list.item(0).equals(list.item(1))) + if (list.item(0) == list.item(1)) return list.item(0); return list; } @@ -1582,9 +1582,9 @@ if (!topValue || !rightValue || !bottomValue || !leftValue) return nullptr; - bool showLeft = !compareCSSValuePtr(rightValue, leftValue); - bool showBottom = !compareCSSValuePtr(topValue, bottomValue) || showLeft; - bool showRight = !compareCSSValuePtr(topValue, rightValue) || showBottom; + bool showLeft = !dataEquivalent(rightValue, leftValue); + bool showBottom = !dataEquivalent(topValue, bottomValue) || showLeft; + bool showRight = !dataEquivalent(topValue, rightValue) || showBottom; list->append(*topValue); if (showRight) @@ -1850,8 +1850,10 @@ // this serialization. CSSValue* ligaturesValue = valueForFontVariantLigatures(style); CSSValue* numericValue = valueForFontVariantNumeric(style); - if (!ligaturesValue->equals(*CSSIdentifierValue::create(CSSValueNormal)) || - !numericValue->equals(*CSSIdentifierValue::create(CSSValueNormal))) + if (!dataEquivalent<CSSValue>(ligaturesValue, + CSSIdentifierValue::create(CSSValueNormal)) || + !dataEquivalent<CSSValue>(numericValue, + CSSIdentifierValue::create(CSSValueNormal))) return nullptr; CSSIdentifierValue* capsValue = valueForFontVariantCaps(style); @@ -3258,9 +3260,8 @@ CSSPropertyBorderBottom, CSSPropertyBorderLeft}; for (size_t i = 0; i < WTF_ARRAY_LENGTH(properties); ++i) { - if (!compareCSSValuePtr<CSSValue>( - value, get(properties[i], style, layoutObject, styledNode, - allowVisitedStyle))) + if (!dataEquivalent(value, get(properties[i], style, layoutObject, + styledNode, allowVisitedStyle))) return nullptr; } return value;
diff --git a/third_party/WebKit/Source/core/css/StylePropertySerializer.cpp b/third_party/WebKit/Source/core/css/StylePropertySerializer.cpp index 9314b06a..70798f7 100644 --- a/third_party/WebKit/Source/core/css/StylePropertySerializer.cpp +++ b/third_party/WebKit/Source/core/css/StylePropertySerializer.cpp
@@ -53,7 +53,7 @@ continue; if (static_cast<unsigned>(m_allIndex) >= i) continue; - if (property.value().equals(allProperty.value()) && + if (property.value() == allProperty.value() && property.isImportant() == allProperty.isImportant()) continue; m_needToExpandAll = true; @@ -366,7 +366,7 @@ longhands[0]->isPendingSubstitutionValue()) { bool success = true; for (int i = 1; i < longhandCount; i++) { - if (!longhands[i]->equals(*longhands[0])) { + if (!dataEquivalent(longhands[i], longhands[0])) { // This should just return emptyString but some shorthands currently // allow 'initial' for their longhands. success = false; @@ -663,9 +663,9 @@ m_propertySet.propertyAt(bottomValueIndex); PropertyValueForSerializer left = m_propertySet.propertyAt(leftValueIndex); - bool showLeft = !right.value()->equals(*left.value()); - bool showBottom = !top.value()->equals(*bottom.value()) || showLeft; - bool showRight = !top.value()->equals(*right.value()) || showBottom; + bool showLeft = !dataEquivalent(right.value(), left.value()); + bool showBottom = !dataEquivalent(top.value(), bottom.value()) || showLeft; + bool showRight = !dataEquivalent(top.value(), right.value()) || showBottom; StringBuilder result; result.append(top.value()->cssText());
diff --git a/third_party/WebKit/Source/core/css/StylePropertySet.cpp b/third_party/WebKit/Source/core/css/StylePropertySet.cpp index 3049d035..78a70e9 100644 --- a/third_party/WebKit/Source/core/css/StylePropertySet.cpp +++ b/third_party/WebKit/Source/core/css/StylePropertySet.cpp
@@ -499,7 +499,7 @@ int foundPropertyIndex = findPropertyIndex(propertyID); if (foundPropertyIndex == -1) return false; - return propertyAt(foundPropertyIndex).value().equals(propertyValue); + return propertyAt(foundPropertyIndex).value() == propertyValue; } void MutableStylePropertySet::removeEquivalentProperties(
diff --git a/third_party/WebKit/Source/core/css/resolver/SharedStyleFinder.cpp b/third_party/WebKit/Source/core/css/resolver/SharedStyleFinder.cpp index 6c0529d..02f4f52 100644 --- a/third_party/WebKit/Source/core/css/resolver/SharedStyleFinder.cpp +++ b/third_party/WebKit/Source/core/css/resolver/SharedStyleFinder.cpp
@@ -59,6 +59,16 @@ using namespace HTMLNames; +inline ComputedStyle* getElementStyle(Element& element) { + if (element.needsReattachLayoutTree()) { + StyleReattachData styleReattachData = + element.document().getStyleReattachData(element); + if (styleReattachData.computedStyle) + return styleReattachData.computedStyle.get(); + } + return element.mutableComputedStyle(); +} + bool SharedStyleFinder::canShareStyleWithControl(Element& candidate) const { if (!isHTMLInputElement(candidate) || !isHTMLInputElement(element())) return false; @@ -228,21 +238,21 @@ if (element() == candidate) return false; Element* parent = candidate.parentOrShadowHostElement(); - const ComputedStyle* style = candidate.computedStyle(); + const ComputedStyle* style = getElementStyle(candidate); if (!style) return false; if (!style->isSharable()) return false; if (!parent) return false; - if (element().parentOrShadowHostElement()->computedStyle() != - parent->computedStyle()) + if (getElementStyle(*element().parentOrShadowHostElement()) != + getElementStyle(*parent)) return false; if (candidate.tagQName() != element().tagQName()) return false; if (candidate.inlineStyle()) return false; - if (candidate.needsStyleRecalc()) + if (candidate.needsStyleRecalc() && !candidate.needsReattachLayoutTree()) return false; if (candidate.isSVGElement() && toSVGElement(candidate).animatedSMILStyleProperties()) @@ -407,7 +417,7 @@ return nullptr; } - return shareElement->mutableComputedStyle(); + return getElementStyle(*shareElement); } } // namespace blink
diff --git a/third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp b/third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp index b8f65d26..1dd8cab 100644 --- a/third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp +++ b/third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp
@@ -1130,20 +1130,20 @@ const ActiveInterpolationsMap& activeInterpolationsMapForAnimations = state.animationUpdate().activeInterpolationsForAnimations(); - const ActiveInterpolationsMap& activeInterpolationsMapForTransitions = - state.animationUpdate().activeInterpolationsForTransitions(); + const ActiveInterpolationsMap& activeInterpolationsMapForStandardTransitions = + state.animationUpdate().activeInterpolationsForStandardTransitions(); // TODO(crbug.com/644148): Apply animations on custom properties. applyAnimatedProperties<HighPropertyPriority>( state, activeInterpolationsMapForAnimations); applyAnimatedProperties<HighPropertyPriority>( - state, activeInterpolationsMapForTransitions); + state, activeInterpolationsMapForStandardTransitions); updateFont(state); applyAnimatedProperties<LowPropertyPriority>( state, activeInterpolationsMapForAnimations); applyAnimatedProperties<LowPropertyPriority>( - state, activeInterpolationsMapForTransitions); + state, activeInterpolationsMapForStandardTransitions); // Start loading resources used by animations. loadPendingResources(state);
diff --git a/third_party/WebKit/Source/core/dom/ContainerNode.cpp b/third_party/WebKit/Source/core/dom/ContainerNode.cpp index cfe0428..1be048ee 100644 --- a/third_party/WebKit/Source/core/dom/ContainerNode.cpp +++ b/third_party/WebKit/Source/core/dom/ContainerNode.cpp
@@ -788,6 +788,7 @@ child->detachLayoutTree(childrenContext); setChildNeedsStyleRecalc(); + setChildNeedsReattachLayoutTree(); Node::detachLayoutTree(context); } @@ -795,9 +796,15 @@ document().incDOMTreeVersion(); document().notifyChangeChildren(*this); invalidateNodeListCachesInAncestors(); - if (change.isChildInsertion() && !childNeedsStyleRecalc()) { - setChildNeedsStyleRecalc(); - markAncestorsWithChildNeedsStyleRecalc(); + if (change.isChildInsertion()) { + if (!childNeedsStyleRecalc()) { + setChildNeedsStyleRecalc(); + markAncestorsWithChildNeedsStyleRecalc(); + } + if (!childNeedsReattachLayoutTree()) { + setChildNeedsReattachLayoutTree(); + markAncestorsWithChildNeedsReattachLayoutTree(); + } } } @@ -1305,6 +1312,24 @@ } } +void ContainerNode::rebuildChildrenLayoutTrees() { + DCHECK(!needsReattachLayoutTree()); + + for (Node* child = lastChild(); child; child = child->previousSibling()) { + if (child->needsReattachLayoutTree() || + child->childNeedsReattachLayoutTree()) { + if (child->isTextNode()) + toText(child)->rebuildTextLayoutTree(); + else if (child->isElementNode()) + toElement(child)->rebuildLayoutTree(); + } + } + // This is done in ContainerNode::attachLayoutTree but will never be cleared + // if we don't enter ContainerNode::attachLayoutTree so we do it here. + clearChildNeedsStyleRecalc(); + clearChildNeedsReattachLayoutTree(); +} + void ContainerNode::checkForSiblingStyleChanges(SiblingCheckType changeType, Element* changedElement, Node* nodeBeforeChange,
diff --git a/third_party/WebKit/Source/core/dom/ContainerNode.h b/third_party/WebKit/Source/core/dom/ContainerNode.h index 1855eed9..26f5e92 100644 --- a/third_party/WebKit/Source/core/dom/ContainerNode.h +++ b/third_party/WebKit/Source/core/dom/ContainerNode.h
@@ -240,6 +240,7 @@ Node* nodeBeforeChange, Node* nodeAfterChange); void recalcDescendantStyles(StyleRecalcChange); + void rebuildChildrenLayoutTrees(); bool childrenSupportStyleSharing() const { return !hasRestyleFlags(); }
diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp index 58de534..5ad6519 100644 --- a/third_party/WebKit/Source/core/dom/Document.cpp +++ b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -2098,6 +2098,9 @@ inheritHtmlAndBodyElementStyles(change); if (documentElement->shouldCallRecalcStyle(change)) documentElement->recalcStyle(change); + if (documentElement->needsReattachLayoutTree() || + documentElement->childNeedsReattachLayoutTree()) + documentElement->rebuildLayoutTree(); } view()->recalcOverflowAfterStyleChange(); @@ -5642,6 +5645,11 @@ return true; } +bool Document::isRenderingReady() const { + return m_styleEngine->ignoringPendingStylesheets() || + (haveImportsLoaded() && haveRenderBlockingStylesheetsLoaded()); +} + bool Document::allowInlineEventHandler(Node* node, EventListener* listener, const String& contextURL,
diff --git a/third_party/WebKit/Source/core/dom/Document.h b/third_party/WebKit/Source/core/dom/Document.h index 31b930a..88b124a 100644 --- a/third_party/WebKit/Source/core/dom/Document.h +++ b/third_party/WebKit/Source/core/dom/Document.h
@@ -445,9 +445,7 @@ } bool canExecuteScripts(ReasonForCallingCanExecuteScripts) override; - bool isRenderingReady() const { - return haveImportsLoaded() && haveRenderBlockingStylesheetsLoaded(); - } + bool isRenderingReady() const; bool isScriptExecutionReady() const { return haveImportsLoaded() && haveScriptBlockingStylesheetsLoaded(); }
diff --git a/third_party/WebKit/Source/core/dom/Element.cpp b/third_party/WebKit/Source/core/dom/Element.cpp index 945e0dc..cc1d397a 100644 --- a/third_party/WebKit/Source/core/dom/Element.cpp +++ b/third_party/WebKit/Source/core/dom/Element.cpp
@@ -1930,14 +1930,17 @@ } if (parentComputedStyle()) change = recalcOwnStyle(change, nextTextSibling); - clearNeedsStyleRecalc(); - clearNeedsReattachLayoutTree(); + // Needed because the rebuildLayoutTree code needs to see what the + // styleChangeType() was on reattach roots. See Node::reattachLayoutTree() + // for an example. + if (change != Reattach) + clearNeedsStyleRecalc(); } - // If we reattached we don't need to recalc the style of our descendants - // anymore. - if ((change >= UpdatePseudoElements && change < Reattach) || - childNeedsStyleRecalc()) { + // If we are going to reattach we don't need to recalc the style of + // our descendants anymore. + if (change < Reattach && + (change >= UpdatePseudoElements || childNeedsStyleRecalc())) { SelectorFilterParentScope filterScope(*this); StyleSharingDepthScope sharingScope(*this); @@ -1963,7 +1966,6 @@ childNeedsStyleRecalc() ? Force : change); clearChildNeedsStyleRecalc(); - clearChildNeedsReattachLayoutTree(); } if (hasCustomStyleCallbacks()) @@ -2023,7 +2025,7 @@ styleReattachData.nextTextSibling = nextTextSibling; document().addStyleReattachData(*this, styleReattachData); setNeedsReattachLayoutTree(); - return rebuildLayoutTree(); + return Reattach; } DCHECK(oldStyle); @@ -2067,34 +2069,57 @@ return localChange; } -StyleRecalcChange Element::rebuildLayoutTree() { +void Element::rebuildLayoutTree() { DCHECK(inActiveDocument()); - StyleReattachData styleReattachData = document().getStyleReattachData(*this); - AttachContext reattachContext; - reattachContext.resolvedStyle = styleReattachData.computedStyle.get(); - bool layoutObjectWillChange = needsAttach() || layoutObject(); - - // We are calling Element::rebuildLayoutTree() from inside - // Element::recalcOwnStyle where we set the NeedsReattachLayoutTree - // flag - so needsReattachLayoutTree() should always be true. DCHECK(parentNode()); - DCHECK(parentNode()->childNeedsReattachLayoutTree()); - DCHECK(needsReattachLayoutTree()); - reattachLayoutTree(reattachContext); - // Since needsReattachLayoutTree() is always true we go into - // reattachLayoutTree() which reattaches all the descendant - // sub-trees. At this point no child should need reattaching. - DCHECK(!childNeedsReattachLayoutTree()); - if (layoutObjectWillChange || layoutObject()) { - // nextTextSibling is passed on to recalcStyle from recalcDescendantStyles - // we can either traverse the current subtree from this node onwards - // or store it. - // The choice is between increased time and increased memory complexity. - reattachWhitespaceSiblingsIfNeeded(styleReattachData.nextTextSibling); - return Reattach; + if (needsReattachLayoutTree()) { + StyleReattachData styleReattachData = + document().getStyleReattachData(*this); + AttachContext reattachContext; + reattachContext.resolvedStyle = styleReattachData.computedStyle.get(); + bool layoutObjectWillChange = needsAttach() || layoutObject(); + reattachLayoutTree(reattachContext); + if (layoutObjectWillChange || layoutObject()) { + // nextTextSibling is passed on to recalcStyle from recalcDescendantStyles + // we can either traverse the current subtree from this node onwards + // or store it. + // The choice is between increased time and increased memory complexity. + reattachWhitespaceSiblingsIfNeeded(styleReattachData.nextTextSibling); + } + } else if (childNeedsReattachLayoutTree()) { + DCHECK(!needsReattachLayoutTree()); + SelectorFilterParentScope filterScope(*this); + StyleSharingDepthScope sharingScope(*this); + reattachPseudoElementLayoutTree(PseudoIdBefore); + rebuildShadowRootLayoutTree(); + rebuildChildrenLayoutTrees(); + reattachPseudoElementLayoutTree(PseudoIdAfter); + reattachPseudoElementLayoutTree(PseudoIdBackdrop); + reattachPseudoElementLayoutTree(PseudoIdFirstLetter); } - return ReattachNoLayoutObject; + DCHECK(!needsStyleRecalc()); + DCHECK(!childNeedsStyleRecalc()); + DCHECK(!needsReattachLayoutTree()); + DCHECK(!childNeedsReattachLayoutTree()); +} + +void Element::rebuildShadowRootLayoutTree() { + for (ShadowRoot* root = youngestShadowRoot(); root; + root = root->olderShadowRoot()) { + if (root->needsReattachLayoutTree() || root->childNeedsReattachLayoutTree()) + root->rebuildLayoutTree(); + } +} + +void Element::reattachPseudoElementLayoutTree(PseudoId pseudoId) { + if (PseudoElement* element = pseudoElement(pseudoId)) { + if (element->needsReattachLayoutTree() || + element->childNeedsReattachLayoutTree()) + element->rebuildLayoutTree(); + } else { + createPseudoElementIfNeeded(pseudoId); + } } void Element::updateCallbackSelectors(const ComputedStyle* oldStyle,
diff --git a/third_party/WebKit/Source/core/dom/Element.h b/third_party/WebKit/Source/core/dom/Element.h index 4df08b26..4e02a99 100644 --- a/third_party/WebKit/Source/core/dom/Element.h +++ b/third_party/WebKit/Source/core/dom/Element.h
@@ -417,7 +417,7 @@ virtual LayoutObject* createLayoutObject(const ComputedStyle&); virtual bool layoutObjectIsNeeded(const ComputedStyle&); void recalcStyle(StyleRecalcChange, Text* nextTextSibling = nullptr); - StyleRecalcChange rebuildLayoutTree(); + void rebuildLayoutTree(); void pseudoStateChanged(CSSSelector::PseudoType); void setAnimationStyleChange(bool); void clearAnimationStyleChange(); @@ -847,6 +847,8 @@ PassRefPtr<ComputedStyle> propagateInheritedProperties(StyleRecalcChange); StyleRecalcChange recalcOwnStyle(StyleRecalcChange, Text*); + void reattachPseudoElementLayoutTree(PseudoId); + void rebuildShadowRootLayoutTree(); inline void checkForEmptyStyleChange(); void updatePseudoElement(PseudoId, StyleRecalcChange);
diff --git a/third_party/WebKit/Source/core/dom/Node.cpp b/third_party/WebKit/Source/core/dom/Node.cpp index 9136af0..94a3878 100644 --- a/third_party/WebKit/Source/core/dom/Node.cpp +++ b/third_party/WebKit/Source/core/dom/Node.cpp
@@ -931,6 +931,7 @@ layoutObject()->destroyAndCleanupAnonymousWrappers(); setLayoutObject(nullptr); setStyleChange(NeedsReattachStyleChange); + setFlag(NeedsReattachLayoutTree); clearChildNeedsStyleInvalidation(); }
diff --git a/third_party/WebKit/Source/core/dom/Node.h b/third_party/WebKit/Source/core/dom/Node.h index 63b03b0..198c0ae 100644 --- a/third_party/WebKit/Source/core/dom/Node.h +++ b/third_party/WebKit/Source/core/dom/Node.h
@@ -421,8 +421,10 @@ void setNeedsStyleRecalc(StyleChangeType, const StyleChangeReasonForTracing&); void clearNeedsStyleRecalc(); - bool needsReattachLayoutTree() { return getFlag(NeedsReattachLayoutTree); } - bool childNeedsReattachLayoutTree() { + bool needsReattachLayoutTree() const { + return getFlag(NeedsReattachLayoutTree); + } + bool childNeedsReattachLayoutTree() const { return getFlag(ChildNeedsReattachLayoutTree); } @@ -830,7 +832,9 @@ NeedsReattachLayoutTree = 1 << 26, ChildNeedsReattachLayoutTree = 1 << 27, - DefaultNodeFlags = IsFinishedParsingChildrenFlag | NeedsReattachStyleChange + DefaultNodeFlags = IsFinishedParsingChildrenFlag | + NeedsReattachStyleChange | + NeedsReattachLayoutTree }; // 4 bits remaining. @@ -853,8 +857,9 @@ enum ConstructionType { CreateOther = DefaultNodeFlags, CreateText = DefaultNodeFlags | IsTextFlag, - CreateContainer = - DefaultNodeFlags | ChildNeedsStyleRecalcFlag | IsContainerFlag, + CreateContainer = DefaultNodeFlags | ChildNeedsStyleRecalcFlag | + ChildNeedsReattachLayoutTree | + IsContainerFlag, CreateElement = CreateContainer | IsElementFlag, CreateShadowRoot = CreateContainer | IsDocumentFragmentFlag | IsInShadowTreeFlag, @@ -974,6 +979,7 @@ detachLayoutTree(context); markAncestorsWithChildNeedsStyleRecalc(); + markAncestorsWithChildNeedsReattachLayoutTree(); } inline bool Node::shouldCallRecalcStyle(StyleRecalcChange change) {
diff --git a/third_party/WebKit/Source/core/dom/Text.cpp b/third_party/WebKit/Source/core/dom/Text.cpp index c77bf86..f69b855 100644 --- a/third_party/WebKit/Source/core/dom/Text.cpp +++ b/third_party/WebKit/Source/core/dom/Text.cpp
@@ -400,14 +400,23 @@ layoutItem.setText(dataImpl()); clearNeedsStyleRecalc(); } else if (needsStyleRecalc() || needsWhitespaceLayoutObject()) { - rebuildTextLayoutTree(nextTextSibling); + StyleReattachData styleReattachData; + styleReattachData.nextTextSibling = nextTextSibling; + document().addStyleReattachData(*this, styleReattachData); + setNeedsReattachLayoutTree(); } } -void Text::rebuildTextLayoutTree(Text* nextTextSibling) { +void Text::rebuildTextLayoutTree() { + DCHECK(!childNeedsStyleRecalc()); + DCHECK(needsReattachLayoutTree()); + DCHECK(parentNode()); + reattachLayoutTree(); - if (layoutObject()) - reattachWhitespaceSiblingsIfNeeded(nextTextSibling); + if (layoutObject()) { + reattachWhitespaceSiblingsIfNeeded( + document().getStyleReattachData(*this).nextTextSibling); + } clearNeedsReattachLayoutTree(); }
diff --git a/third_party/WebKit/Source/core/dom/Text.h b/third_party/WebKit/Source/core/dom/Text.h index 7c865df3..96111466 100644 --- a/third_party/WebKit/Source/core/dom/Text.h +++ b/third_party/WebKit/Source/core/dom/Text.h
@@ -54,7 +54,7 @@ Text* replaceWholeText(const String&); void recalcTextStyle(StyleRecalcChange, Text* nextTextSibling); - void rebuildTextLayoutTree(Text* nextTextSibling); + void rebuildTextLayoutTree(); bool textLayoutObjectIsNeeded(const ComputedStyle&, const LayoutObject& parent) const; LayoutText* createTextLayoutObject(const ComputedStyle&);
diff --git a/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp b/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp index 8e5eec44..c0c743f 100644 --- a/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp +++ b/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp
@@ -86,6 +86,7 @@ shadowHost.setNeedsStyleRecalc( SubtreeStyleChange, StyleChangeReasonForTracing::create(StyleChangeReason::Shadow)); + shadowRoot->setNeedsReattachLayoutTree(); probe::didPushShadowRoot(&shadowHost, shadowRoot);
diff --git a/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp b/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp index c72f2a0..3d99595 100644 --- a/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp +++ b/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp
@@ -147,10 +147,19 @@ // There's no style to update so just calling recalcStyle means we're updated. clearNeedsStyleRecalc(); - clearNeedsReattachLayoutTree(); recalcDescendantStyles(change); clearChildNeedsStyleRecalc(); +} + +void ShadowRoot::rebuildLayoutTree() { + // ShadowRoot doesn't support custom callbacks. + DCHECK(!hasCustomStyleCallbacks()); + + StyleSharingDepthScope sharingScope(*this); + + clearNeedsReattachLayoutTree(); + rebuildChildrenLayoutTrees(); clearChildNeedsReattachLayoutTree(); }
diff --git a/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.h b/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.h index c8bcbda..a009760 100644 --- a/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.h +++ b/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.h
@@ -119,6 +119,7 @@ unsigned childShadowRootCount() const { return m_childShadowRootCount; } void recalcStyle(StyleRecalcChange); + void rebuildLayoutTree(); void registerScopedHTMLStyleChild(); void unregisterScopedHTMLStyleChild();
diff --git a/third_party/WebKit/Source/core/dom/stylerecalc.md b/third_party/WebKit/Source/core/dom/stylerecalc.md index aacc22f..468713f 100644 --- a/third_party/WebKit/Source/core/dom/stylerecalc.md +++ b/third_party/WebKit/Source/core/dom/stylerecalc.md
@@ -12,5 +12,4 @@ 4. IndependentInherit -> Same as Inherit except style recalc stops early if only independent properties were changed. We still visit every descendant, but we apply the styles directly instead of doing selector matching to compute a new style. Independent properties are those which do not depend on and do not affect any other properties on ComputedStyle (e.g. visibility and Pointer Events). 5. Inherit -> Do a full style recalc of children. 6. Force -> Fallback that causes us to do a full style recalc. This is as we don't know what changes. The primary reason for it is SubtreeStyleChange. -7. Reattach -> reattachLayoutTree() has been completed. -8. ReattachNoLayoutObject -> reattachLayoutTree() has been completed and now dont have a layoutObject. \ No newline at end of file +7. Reattach -> reattachLayoutTree() needs to be performed.
diff --git a/third_party/WebKit/Source/core/editing/EditingStyle.cpp b/third_party/WebKit/Source/core/editing/EditingStyle.cpp index 68759778..b9f9a18 100644 --- a/third_party/WebKit/Source/core/editing/EditingStyle.cpp +++ b/third_party/WebKit/Source/core/editing/EditingStyle.cpp
@@ -311,7 +311,7 @@ const CSSValue* value = attributeValueAsCSSValue(element); const CSSValue* styleValue = style->getPropertyCSSValue(m_propertyID); - return compareCSSValuePtr(value, styleValue); + return dataEquivalent(value, styleValue); } void HTMLAttributeEquivalent::addToStyle(Element* element,
diff --git a/third_party/WebKit/Source/core/editing/FrameSelection.cpp b/third_party/WebKit/Source/core/editing/FrameSelection.cpp index 9247096..04b7419d 100644 --- a/third_party/WebKit/Source/core/editing/FrameSelection.cpp +++ b/third_party/WebKit/Source/core/editing/FrameSelection.cpp
@@ -325,19 +325,6 @@ setSelection(newSelection.asSelection(), options); } -void FrameSelection::setSelection( - const VisibleSelectionInFlatTree& newSelection, - HandleVisibility handleVisibility, - SetSelectionOptions options, - CursorAlignOnScroll align, - TextGranularity granularity) { - setSelection( - SelectionInFlatTree::Builder(newSelection.asSelection()) - .setIsHandleVisible(handleVisibility == HandleVisibility::Visible) - .build(), - options, align, granularity); -} - void FrameSelection::nodeChildrenWillBeRemoved(ContainerNode& container) { if (!container.inActiveDocument()) return; @@ -687,11 +674,6 @@ SetSelectionOptions options) { if (range.isNull()) return false; - m_selectionEditor->resetLogicalRange(); - // Since |FrameSeleciton::setSelection()| dispatches events and DOM tree - // can be modified by event handlers, we should create |Range| object before - // calling it. - Range* logicalRange = createRange(range); setSelection(SelectionInDOMTree::Builder() .setBaseAndExtent(range) .setAffinity(affinity) @@ -699,7 +681,6 @@ SelectionDirectionalMode::Directional) .build(), options); - m_selectionEditor->setLogicalRange(logicalRange); return true; }
diff --git a/third_party/WebKit/Source/core/editing/FrameSelection.h b/third_party/WebKit/Source/core/editing/FrameSelection.h index 7be6e9d5..1cee348a 100644 --- a/third_party/WebKit/Source/core/editing/FrameSelection.h +++ b/third_party/WebKit/Source/core/editing/FrameSelection.h
@@ -132,13 +132,6 @@ CursorAlignOnScroll = CursorAlignOnScroll::IfNeeded, TextGranularity = CharacterGranularity); void setSelection(const VisibleSelection&, SetSelectionOptions); - // TODO(yosin): We should use |SelectionInFlatTree| version instead of - // |VisibleSelectionInFlatTree| version. - void setSelection(const VisibleSelectionInFlatTree&, - HandleVisibility = HandleVisibility::NotVisible, - SetSelectionOptions = CloseTyping | ClearTypingStyle, - CursorAlignOnScroll = CursorAlignOnScroll::IfNeeded, - TextGranularity = CharacterGranularity); bool setSelectedRange( const EphemeralRange&, TextAffinity,
diff --git a/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp b/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp index 403316ed..d124ac4 100644 --- a/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp +++ b/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp
@@ -56,6 +56,22 @@ return text; } +TEST_F(FrameSelectionTest, FirstRange) { + setBodyContent("<div id=sample>0123456789</div>abc"); + Element* const sample = document().getElementById("sample"); + Node* const text = sample->firstChild(); + selection().setSelectedRange( + EphemeralRange(Position(text, 3), Position(text, 6)), VP_DEFAULT_AFFINITY, + SelectionDirectionalMode::NonDirectional, 0); + sample->setAttribute(HTMLNames::styleAttr, "display:none"); + // Move |VisibleSelection| before "abc". + updateAllLifecyclePhases(); + Range* const range = selection().firstRange(); + EXPECT_EQ(Position(sample->nextSibling(), 0), range->startPosition()) + << "firstRagne() should return current selection value"; + EXPECT_EQ(Position(sample->nextSibling(), 0), range->endPosition()); +} + TEST_F(FrameSelectionTest, SetValidSelection) { Text* text = appendTextNode("Hello, World!"); document().view()->updateAllLifecyclePhases();
diff --git a/third_party/WebKit/Source/core/editing/SelectionEditor.cpp b/third_party/WebKit/Source/core/editing/SelectionEditor.cpp index a06bc771..14f7509 100644 --- a/third_party/WebKit/Source/core/editing/SelectionEditor.cpp +++ b/third_party/WebKit/Source/core/editing/SelectionEditor.cpp
@@ -61,7 +61,6 @@ } void SelectionEditor::dispose() { - resetLogicalRange(); clearDocumentCachedRange(); clearVisibleSelection(); } @@ -122,7 +121,6 @@ newSelection.assertValidFor(document()); if (m_selection == newSelection) return; - resetLogicalRange(); clearDocumentCachedRange(); markCacheDirty(); m_selection = newSelection; @@ -350,24 +348,7 @@ didFinishTextChange(newBase, newExtent); } -void SelectionEditor::resetLogicalRange() { - // Non-collapsed ranges are not allowed to start at the end of a line that - // is wrapped, they start at the beginning of the next line instead - if (!m_logicalRange) - return; - m_logicalRange->dispose(); - m_logicalRange = nullptr; -} - -void SelectionEditor::setLogicalRange(Range* range) { - DCHECK_EQ(range->ownerDocument(), document()); - DCHECK(!m_logicalRange) << "A logical range should be one."; - m_logicalRange = range; -} - Range* SelectionEditor::firstRange() const { - if (m_logicalRange) - return m_logicalRange->cloneRange(); return createRange(firstEphemeralRangeOf(computeVisibleSelectionInDOMTree())); } @@ -420,7 +401,6 @@ visitor->trace(m_selection); visitor->trace(m_cachedVisibleSelectionInDOMTree); visitor->trace(m_cachedVisibleSelectionInFlatTree); - visitor->trace(m_logicalRange); visitor->trace(m_cachedRange); SynchronousMutationObserver::trace(visitor); }
diff --git a/third_party/WebKit/Source/core/editing/SelectionEditor.h b/third_party/WebKit/Source/core/editing/SelectionEditor.h index e853cfe..322b3b4 100644 --- a/third_party/WebKit/Source/core/editing/SelectionEditor.h +++ b/third_party/WebKit/Source/core/editing/SelectionEditor.h
@@ -67,9 +67,6 @@ Range* firstRange() const; // There functions are exposed for |FrameSelection|. - void resetLogicalRange(); - void setLogicalRange(Range*); - void cacheRangeOfDocument(Range*); Range* documentCachedRange() const; void clearDocumentCachedRange(); @@ -112,13 +109,6 @@ SelectionInDOMTree m_selection; - // TODO(editing-dev): Removing |m_logicalRange| - // The range specified by the user, which may not be visually canonicalized - // (hence "logical"). This will be invalidated if the underlying - // |VisibleSelection| changes. If that happens, this variable will - // become |nullptr|, in which case logical positions == visible positions. - Member<Range> m_logicalRange; - // If document is root, document.getSelection().addRange(range) is cached on // this. Member<Range> m_cachedRange;
diff --git a/third_party/WebKit/Source/core/editing/VisibleUnits.cpp b/third_party/WebKit/Source/core/editing/VisibleUnits.cpp index 32c5d1c..6a27a1a 100644 --- a/third_party/WebKit/Source/core/editing/VisibleUnits.cpp +++ b/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
@@ -810,14 +810,14 @@ bool& needMoreContext); template <typename Strategy> -static VisiblePositionTemplate<Strategy> previousBoundary( +static PositionTemplate<Strategy> previousBoundary( const VisiblePositionTemplate<Strategy>& c, BoundarySearchFunction searchFunction) { DCHECK(c.isValid()) << c; const PositionTemplate<Strategy> pos = c.deepEquivalent(); Node* boundary = parentEditingBoundary(pos); if (!boundary) - return VisiblePositionTemplate<Strategy>(); + return PositionTemplate<Strategy>(); const PositionTemplate<Strategy> start = PositionTemplate<Strategy>::editingPositionOf(boundary, 0) @@ -888,14 +888,13 @@ } if (!next) - return createVisiblePosition(it.atEnd() ? it.startPosition() : pos); + return it.atEnd() ? it.startPosition() : pos; Node* node = it.startContainer(); int boundaryOffset = remainingLength + next; if (node->isTextNode() && boundaryOffset <= node->maxCharacterOffset()) { // The next variable contains a usable index into a text node - return createVisiblePosition( - PositionTemplate<Strategy>(node, boundaryOffset)); + return PositionTemplate<Strategy>(node, boundaryOffset); } // Use the character iterator to translate the next value into a DOM @@ -903,7 +902,7 @@ BackwardsCharacterIteratorAlgorithm<Strategy> charIt(start, end); charIt.advance(string.size() - suffixLength - next); // TODO(yosin) charIt can get out of shadow host. - return createVisiblePosition(charIt.endPosition()); + return charIt.endPosition(); } template <typename Strategy> @@ -1060,7 +1059,7 @@ if (p.isNull()) return c; } - return previousBoundary(p, startWordBoundary); + return createVisiblePosition(previousBoundary(p, startWordBoundary)); } VisiblePosition startOfWord(const VisiblePosition& c, EWordSide side) { @@ -1090,33 +1089,43 @@ } template <typename Strategy> -static VisiblePositionTemplate<Strategy> endOfWordAlgorithm( +static PositionTemplate<Strategy> endOfWordAlgorithm( const VisiblePositionTemplate<Strategy>& c, EWordSide side) { DCHECK(c.isValid()) << c; VisiblePositionTemplate<Strategy> p = c; if (side == LeftWordIfOnBoundary) { if (isStartOfParagraph(c)) - return c; + return c.deepEquivalent(); p = previousPositionOf(c); if (p.isNull()) - return c; + return c.deepEquivalent(); } else if (isEndOfParagraph(c)) { - return c; + return c.deepEquivalent(); } - return createVisiblePosition(nextBoundary(p, endWordBoundary), + return nextBoundary(p, endWordBoundary); +} + +Position endOfWordPosition(const VisiblePosition& position, EWordSide side) { + return endOfWordAlgorithm<EditingStrategy>(position, side); +} + +VisiblePosition endOfWord(const VisiblePosition& position, EWordSide side) { + return createVisiblePosition(endOfWordPosition(position, side), VP_UPSTREAM_IF_POSSIBLE); } -VisiblePosition endOfWord(const VisiblePosition& c, EWordSide side) { - return endOfWordAlgorithm<EditingStrategy>(c, side); +PositionInFlatTree endOfWordPosition(const VisiblePositionInFlatTree& position, + EWordSide side) { + return endOfWordAlgorithm<EditingInFlatTreeStrategy>(position, side); } -VisiblePositionInFlatTree endOfWord(const VisiblePositionInFlatTree& c, +VisiblePositionInFlatTree endOfWord(const VisiblePositionInFlatTree& position, EWordSide side) { - return endOfWordAlgorithm<EditingInFlatTreeStrategy>(c, side); + return createVisiblePosition(endOfWordPosition(position, side), + VP_UPSTREAM_IF_POSSIBLE); } static unsigned previousWordPositionBoundary( @@ -1136,7 +1145,8 @@ VisiblePosition previousWordPosition(const VisiblePosition& c) { DCHECK(c.isValid()) << c; - VisiblePosition prev = previousBoundary(c, previousWordPositionBoundary); + VisiblePosition prev = + createVisiblePosition(previousBoundary(c, previousWordPositionBoundary)); return honorEditingBoundaryAtOrBefore(prev, c.deepEquivalent()); } @@ -1675,7 +1685,7 @@ static VisiblePositionTemplate<Strategy> startOfSentenceAlgorithm( const VisiblePositionTemplate<Strategy>& c) { DCHECK(c.isValid()) << c; - return previousBoundary(c, startSentenceBoundary); + return createVisiblePosition(previousBoundary(c, startSentenceBoundary)); } VisiblePosition startOfSentence(const VisiblePosition& c) { @@ -1728,7 +1738,8 @@ VisiblePosition previousSentencePosition(const VisiblePosition& c) { DCHECK(c.isValid()) << c; - VisiblePosition prev = previousBoundary(c, previousSentencePositionBoundary); + VisiblePosition prev = createVisiblePosition( + previousBoundary(c, previousSentencePositionBoundary)); return honorEditingBoundaryAtOrBefore(prev, c.deepEquivalent()); }
diff --git a/third_party/WebKit/Source/core/editing/VisibleUnits.h b/third_party/WebKit/Source/core/editing/VisibleUnits.h index d53d38b..154a9845 100644 --- a/third_party/WebKit/Source/core/editing/VisibleUnits.h +++ b/third_party/WebKit/Source/core/editing/VisibleUnits.h
@@ -161,8 +161,15 @@ CORE_EXPORT VisiblePositionInFlatTree startOfWord(const VisiblePositionInFlatTree&, EWordSide = RightWordIfOnBoundary); +// TODO(yoichio): Replace |endOfWord| to |endOfWordPosition| because returned +// Position should be canonicalized with |nextBoundary()| by TextItetator. +CORE_EXPORT Position endOfWordPosition(const VisiblePosition&, + EWordSide = RightWordIfOnBoundary); CORE_EXPORT VisiblePosition endOfWord(const VisiblePosition&, EWordSide = RightWordIfOnBoundary); +CORE_EXPORT PositionInFlatTree +endOfWordPosition(const VisiblePositionInFlatTree&, + EWordSide = RightWordIfOnBoundary); CORE_EXPORT VisiblePositionInFlatTree endOfWord(const VisiblePositionInFlatTree&, EWordSide = RightWordIfOnBoundary); VisiblePosition previousWordPosition(const VisiblePosition&);
diff --git a/third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp b/third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp index b9fa99b6..b07bdaa 100644 --- a/third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp +++ b/third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp
@@ -190,9 +190,9 @@ if (!isSpellCheckingEnabled()) return; - IdleDeadline* deadline = - IdleDeadline::create(kForcedInvocationDeadlineSeconds, - IdleDeadline::CallbackType::CalledWhenIdle); + IdleDeadline* deadline = IdleDeadline::create( + kForcedInvocationDeadlineSeconds + monotonicallyIncreasingTime(), + IdleDeadline::CallbackType::CalledWhenIdle); switch (m_state) { case State::kColdModeTimerStarted:
diff --git a/third_party/WebKit/Source/core/html/HTMLFrameSetElement.cpp b/third_party/WebKit/Source/core/html/HTMLFrameSetElement.cpp index f3f5b17..581d619 100644 --- a/third_party/WebKit/Source/core/html/HTMLFrameSetElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLFrameSetElement.cpp
@@ -275,7 +275,6 @@ layoutObject()->setNeedsLayoutAndFullPaintInvalidation( LayoutInvalidationReason::StyleChange); clearNeedsStyleRecalc(); - clearNeedsReattachLayoutTree(); } }
diff --git a/third_party/WebKit/Source/core/style/ComputedStyleConstants.h b/third_party/WebKit/Source/core/style/ComputedStyleConstants.h index 7cfc428..1840c76 100644 --- a/third_party/WebKit/Source/core/style/ComputedStyleConstants.h +++ b/third_party/WebKit/Source/core/style/ComputedStyleConstants.h
@@ -52,8 +52,7 @@ IndependentInherit, Inherit, Force, - Reattach, - ReattachNoLayoutObject + Reattach }; // Static pseudo styles. Dynamic ones are produced on the fly.
diff --git a/third_party/WebKit/Source/web/tests/DocumentLoadingRenderingTest.cpp b/third_party/WebKit/Source/web/tests/DocumentLoadingRenderingTest.cpp index 9fdbe5e..3a7abbe 100644 --- a/third_party/WebKit/Source/web/tests/DocumentLoadingRenderingTest.cpp +++ b/third_party/WebKit/Source/web/tests/DocumentLoadingRenderingTest.cpp
@@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "core/dom/ClientRect.h" #include "core/dom/Document.h" #include "core/dom/FrameRequestCallback.h" #include "core/html/HTMLIFrameElement.h" @@ -380,4 +381,43 @@ EXPECT_TRUE(document().isRenderingReady()); } +TEST_F(DocumentLoadingRenderingTest, + returnBoundingClientRectCorrectlyWhileLoadingImport) { + SimRequest mainResource("https://example.com/test.html", "text/html"); + SimRequest importResource("https://example.com/import.css", "text/css"); + + loadURL("https://example.com/test.html"); + + webView().resize(WebSize(800, 600)); + + mainResource.start(); + + mainResource.write( + "<html><body>" + " <div id='test' style='font-size: 16px'>test</div>" + " <script>" + " var link = document.createElement('link');" + " link.rel = 'import';" + " link.href = 'import.css';" + " document.head.appendChild(link);" + " </script>"); + importResource.start(); + + // Import loader isn't finish, shoudn't paint. + EXPECT_FALSE(document().isRenderingReady()); + + // If ignoringPendingStylesheets==true, element should get non-empty rect. + Element* element = document().getElementById("test"); + ClientRect* rect = element->getBoundingClientRect(); + EXPECT_TRUE(rect->width() > 0.f); + EXPECT_TRUE(rect->height() > 0.f); + + // After reset ignoringPendingStylesheets, we should block rendering again. + EXPECT_FALSE(document().isRenderingReady()); + + importResource.write("div { color: red; }"); + importResource.finish(); + mainResource.finish(); +} + } // namespace blink
diff --git a/tools/mb/mb_config.pyl b/tools/mb/mb_config.pyl index 9d128cf..4ac66145 100644 --- a/tools/mb/mb_config.pyl +++ b/tools/mb/mb_config.pyl
@@ -241,6 +241,7 @@ 'Android Release (Nexus 6P)': 'android_release_trybot_arm64', 'Android Release (Nexus 9)': 'android_release_trybot_arm64', 'Android Release (Pixel C)': 'android_release_trybot_arm64', + 'Android Release (NVIDIA Shield)': 'android_release_trybot', 'GPU Linux Builder (dbg)': 'gpu_fyi_tests_debug_trybot', 'GPU Linux Builder': 'gpu_fyi_tests_release_trybot', 'GPU Mac Builder': 'gpu_fyi_tests_release_trybot',
diff --git a/ui/views/examples/dialog_example.cc b/ui/views/examples/dialog_example.cc index 637c6da..37e8770 100644 --- a/ui/views/examples/dialog_example.cc +++ b/ui/views/examples/dialog_example.cc
@@ -245,6 +245,10 @@ // Q: Do we need NonClientFrameView::GetWindowBoundsForClientBounds() here? // A: When DialogCientView properly feeds back sizes, we do not. widget->SetBoundsConstrained(preferred_bounds); + + // For user-resizable dialogs, ensure the window manager enforces any new + // minimum size. + widget->OnSizeConstraintsChanged(); } void DialogExample::ButtonPressed(Button* sender, const ui::Event& event) {
diff --git a/ui/views/window/dialog_client_view.cc b/ui/views/window/dialog_client_view.cc index f646639..5c10986 100644 --- a/ui/views/window/dialog_client_view.cc +++ b/ui/views/window/dialog_client_view.cc
@@ -15,6 +15,7 @@ #include "ui/views/controls/button/custom_button.h" #include "ui/views/controls/button/label_button.h" #include "ui/views/controls/button/md_text_button.h" +#include "ui/views/layout/grid_layout.h" #include "ui/views/style/platform_style.h" #include "ui/views/views_delegate.h" #include "ui/views/widget/widget.h" @@ -40,27 +41,35 @@ return view && view->visible(); } -// Do the layout for a button. -void LayoutButton(LabelButton* button, - gfx::Rect* row_bounds, - int button_height) { - if (!button) - return; - - const gfx::Size size = button->GetPreferredSize(); - row_bounds->set_width(row_bounds->width() - size.width()); - DCHECK_LE(button_height, row_bounds->height()); - button->SetBounds( - row_bounds->right(), - row_bounds->y() + (row_bounds->height() - button_height) / 2, - size.width(), button_height); - const int spacing = - ViewsDelegate::GetInstance()->GetDialogRelatedButtonHorizontalSpacing(); - row_bounds->set_width(row_bounds->width() - spacing); +// Returns the bounding box required to contain |size1| and |size2|, placed one +// atop the other. +gfx::Size GetBoundingSizeForVerticalStack(const gfx::Size& size1, + const gfx::Size& size2) { + return gfx::Size(std::max(size1.width(), size2.width()), + size1.height() + size2.height()); } } // namespace +// Simple container to bubble child view changes up the view hierarchy. +class DialogClientView::ButtonRowContainer : public View { + public: + explicit ButtonRowContainer(DialogClientView* owner) : owner_(owner) {} + + // View: + void ChildPreferredSizeChanged(View* child) override { + owner_->ChildPreferredSizeChanged(child); + } + void ChildVisibilityChanged(View* child) override { + owner_->ChildVisibilityChanged(child); + } + + private: + DialogClientView* const owner_; + + DISALLOW_COPY_AND_ASSIGN(ButtonRowContainer); +}; + /////////////////////////////////////////////////////////////////////////////// // DialogClientView, public: @@ -71,10 +80,11 @@ // Doing this now ensures this accelerator will have lower priority than // one set by the contents view. AddAccelerator(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE)); + button_row_container_ = new ButtonRowContainer(this); + AddChildView(button_row_container_); } -DialogClientView::~DialogClientView() { -} +DialogClientView::~DialogClientView() {} void DialogClientView::AcceptWindow() { // Only notify the delegate once. See |delegate_allowed_close_|'s comment. @@ -93,8 +103,8 @@ } void DialogClientView::UpdateDialogButtons() { - SetupViews(); - SetupFocusChain(); + SetupLayout(); + InvalidateLayout(); } /////////////////////////////////////////////////////////////////////////////// @@ -120,90 +130,41 @@ // DialogClientView, View overrides: gfx::Size DialogClientView::GetPreferredSize() const { - // Initialize the size to fit the buttons and extra view row. - gfx::Size size( - (ok_button_ ? ok_button_->GetPreferredSize().width() : 0) + - (cancel_button_ ? cancel_button_->GetPreferredSize().width() : 0) + - (cancel_button_ && ok_button_ - ? ViewsDelegate::GetInstance() - ->GetDialogRelatedButtonHorizontalSpacing() - : 0) + - (ShouldShow(extra_view_) ? extra_view_->GetPreferredSize().width() - : 0) + - GetExtraViewSpacing(), - 0); + return GetBoundingSizeForVerticalStack( + ClientView::GetPreferredSize(), + button_row_container_->GetPreferredSize()); +} - int buttons_height = GetButtonsAndExtraViewRowHeight(); - if (buttons_height != 0) { - size.Enlarge(0, buttons_height); - // Inset the buttons and extra view. - const gfx::Insets insets = GetButtonRowInsets(); - size.Enlarge(insets.width(), insets.height()); - } +gfx::Size DialogClientView::GetMinimumSize() const { + return GetBoundingSizeForVerticalStack( + ClientView::GetMinimumSize(), button_row_container_->GetMinimumSize()); +} - // Increase the size as needed to fit the contents view. - // NOTE: The contents view is not inset on the top or side client view edges. - gfx::Size contents_size = contents_view()->GetPreferredSize(); - size.Enlarge(0, contents_size.height()); - size.set_width(std::max(size.width(), contents_size.width())); +gfx::Size DialogClientView::GetMaximumSize() const { + constexpr int kUnconstrained = 0; + DCHECK(gfx::Size(kUnconstrained, kUnconstrained) == + button_row_container_->GetMaximumSize()); + gfx::Size max_size = ClientView::GetMaximumSize(); - size.SetToMax(minimum_size_); + // If the height is constrained, add the button row height. Leave the width as + // it is (be it constrained or unconstrained). + if (max_size.height() != kUnconstrained) + max_size.Enlarge(0, button_row_container_->GetPreferredSize().height()); - return size; + // Note not all constraints can be met. E.g. it's possible here for + // GetMinimumSize().width() to be larger than max_size.width() since the + // former includes the button row width, but the latter does not. It is up to + // the DialogDelegate to ensure its maximum size is reasonable for the buttons + // it wants, or leave it unconstrained. + return max_size; } void DialogClientView::Layout() { - gfx::Rect bounds = GetContentsBounds(); - - // Layout the row containing the buttons and the extra view. - if (has_dialog_buttons() || ShouldShow(extra_view_)) { - gfx::Insets button_row_insets = GetButtonRowInsets(); - // Don't apply the top inset here because it's supposed to go above the - // buttons, not at the top of the dialog. - bounds.Inset(button_row_insets.left(), 0, button_row_insets.right(), - button_row_insets.bottom()); - const int height = GetButtonsAndExtraViewRowHeight(); - gfx::Rect row_bounds(bounds.x(), bounds.bottom() - height, - bounds.width(), height); - // If the |extra_view_| is a also button, then the |button_height| is the - // maximum height of the three buttons, otherwise it is the maximum height - // of the ok and cancel buttons. - const int button_height = - CustomButton::AsCustomButton(extra_view_) ? height : GetButtonHeight(); - if (kIsOkButtonOnLeftSide) { - LayoutButton(cancel_button_, &row_bounds, button_height); - LayoutButton(ok_button_, &row_bounds, button_height); - } else { - LayoutButton(ok_button_, &row_bounds, button_height); - LayoutButton(cancel_button_, &row_bounds, button_height); - } - if (extra_view_) { - int custom_padding = 0; - if (has_dialog_buttons() && - GetDialogDelegate()->GetExtraViewPadding(&custom_padding)) { - // The padding between buttons applied in LayoutButton() will already - // have accounted for some of the distance here. - custom_padding -= ViewsDelegate::GetInstance() - ->GetDialogRelatedButtonHorizontalSpacing(); - row_bounds.set_width(row_bounds.width() - custom_padding); - } - row_bounds.set_width(std::min(row_bounds.width(), - extra_view_->GetPreferredSize().width())); - extra_view_->SetBoundsRect(row_bounds); - } - - if (height > 0) { - // Inset to the top of the buttons, plus their top padding, in order to - // exclude that area from the content view's bounds. - bounds.Inset(0, 0, 0, height + button_row_insets.top()); - } - } - - // Layout the contents view to the top and side edges of the contents bounds. - // NOTE: The local insets do not apply to the contents view sides or top. - const gfx::Rect contents_bounds = GetContentsBounds(); - contents_view()->SetBounds(contents_bounds.x(), contents_bounds.y(), - contents_bounds.width(), bounds.bottom() - contents_bounds.y()); + button_row_container_->SetSize( + gfx::Size(width(), button_row_container_->GetHeightForWidth(width()))); + button_row_container_->SetY(height() - button_row_container_->height()); + if (contents_view()) + contents_view()->SetSize(gfx::Size(width(), button_row_container_->y())); } bool DialogClientView::AcceleratorPressed(const ui::Accelerator& accelerator) { @@ -217,23 +178,31 @@ const ViewHierarchyChangedDetails& details) { View* const child = details.child; - // Dialogs must add children to contents_view(), not client_view(). - if (details.is_add && details.parent == this) { - DCHECK(child == contents_view() || child == ok_button_ || - child == cancel_button_ || child == extra_view_); + ClientView::ViewHierarchyChanged(details); + + if (details.is_add) { + if (child == this) + UpdateDialogButtons(); + return; } - ClientView::ViewHierarchyChanged(details); - if (details.is_add && child == this) { - UpdateDialogButtons(); - } else if (!details.is_add) { - if (child == ok_button_) - ok_button_ = nullptr; - else if (child == cancel_button_) - cancel_button_ = nullptr; - else if (child == extra_view_) - extra_view_ = nullptr; - } + if (details.parent != button_row_container_) + return; + + // SetupViews() removes all children, managing data members itself. + if (preserve_button_row_data_members_) + return; + + // Otherwise, this should only happen during teardown. Ensure there are no + // references to deleted Views. + button_row_container_->SetLayoutManager(nullptr); + + if (child == ok_button_) + ok_button_ = nullptr; + else if (child == cancel_button_) + cancel_button_ = nullptr; + else if (child == extra_view_) + extra_view_ = nullptr; } void DialogClientView::OnNativeThemeChanged(const ui::NativeTheme* theme) { @@ -276,91 +245,56 @@ } void DialogClientView::ChildVisibilityChanged(View* child) { + // Showing or hiding |extra_view_| can alter which columns have linked sizes. + if (child == extra_view_) + UpdateDialogButtons(); ChildPreferredSizeChanged(child); } -LabelButton* DialogClientView::CreateDialogButton(ui::DialogButton type) { - const base::string16 title = GetDialogDelegate()->GetDialogButtonLabel(type); - LabelButton* button = nullptr; - - const bool is_default = - GetDialogDelegate()->GetDefaultDialogButton() == type && - (type != ui::DIALOG_BUTTON_CANCEL || - PlatformStyle::kDialogDefaultButtonCanBeCancel); - - // The default button is always blue in Harmony. - if (is_default && (ui::MaterialDesignController::IsSecondaryUiMaterial() || - GetDialogDelegate()->ShouldDefaultButtonBeBlue())) { - button = MdTextButton::CreateSecondaryUiBlueButton(this, title); - } else { - button = MdTextButton::CreateSecondaryUiButton(this, title); +void DialogClientView::UpdateDialogButton(LabelButton** member, + ui::DialogButton type) { + DialogDelegate* const delegate = GetDialogDelegate(); + if (!(delegate->GetDialogButtons() & type)) { + delete *member; + *member = nullptr; + return; } - const int minimum_width = - ViewsDelegate::GetInstance()->GetDialogButtonMinimumWidth(); - button->SetMinSize(gfx::Size(minimum_width, 0)); + if (!*member) { + // In theory, this should only need to assign a newly constructed Button to + // |*member|. DialogDelegate::UpdateButton(), and any overrides of that, + // should be responsible for the rest. TODO(tapted): When there is only + // MdTextButton, make it so. Note that some overrides may not always update + // the title (they should). See http://crbug.com/697303 . + const base::string16 title = delegate->GetDialogButtonLabel(type); + LabelButton* button = nullptr; - button->SetGroup(kButtonGroup); - return button; -} + const bool is_default = delegate->GetDefaultDialogButton() == type && + (type != ui::DIALOG_BUTTON_CANCEL || + PlatformStyle::kDialogDefaultButtonCanBeCancel); -int DialogClientView::GetButtonHeight() const { - return std::max( - ok_button_ ? ok_button_->GetPreferredSize().height() : 0, - cancel_button_ ? cancel_button_->GetPreferredSize().height() : 0); -} + // The default button is always blue in Harmony. + if (is_default && (ui::MaterialDesignController::IsSecondaryUiMaterial() || + delegate->ShouldDefaultButtonBeBlue())) { + button = MdTextButton::CreateSecondaryUiBlueButton(this, title); + } else { + button = MdTextButton::CreateSecondaryUiButton(this, title); + } -int DialogClientView::GetExtraViewHeight() const { - return ShouldShow(extra_view_) ? extra_view_->GetPreferredSize().height() : 0; -} + const int minimum_width = + ViewsDelegate::GetInstance()->GetDialogButtonMinimumWidth(); + button->SetMinSize(gfx::Size(minimum_width, 0)); -int DialogClientView::GetButtonsAndExtraViewRowHeight() const { - return std::max(GetExtraViewHeight(), GetButtonHeight()); -} + button->SetGroup(kButtonGroup); -gfx::Insets DialogClientView::GetButtonRowInsets() const { - if (GetButtonsAndExtraViewRowHeight() == 0) - return gfx::Insets(); - - // Some subclasses of DialogClientView, in order to do their own layout, set - // button_row_insets_ to gfx::Insets(). To avoid breaking behavior of those - // dialogs, supplying 0 for the top inset of the row falls back to - // ViewsDelegate::GetRelatedControlVerticalSpacing. - // TODO(bsep): The top inset should never be 0 when harmony is enabled. - const int top = button_row_insets_.top() == 0 - ? ViewsDelegate::GetInstance() - ->GetDialogRelatedControlVerticalSpacing() - : button_row_insets_.top(); - return gfx::Insets(top, button_row_insets_.left(), - button_row_insets_.bottom(), button_row_insets_.right()); -} - -void DialogClientView::SetupFocusChain() { - // Create a vector of child views in the order of intended focus. - std::vector<View*> child_views; - child_views.push_back(contents_view()); - child_views.push_back(extra_view_); - if (kIsOkButtonOnLeftSide) { - child_views.push_back(ok_button_); - child_views.push_back(cancel_button_); - } else { - child_views.push_back(cancel_button_); - child_views.push_back(ok_button_); + *member = button; } - // Remove all null views from the vector. - child_views.erase( - std::remove(child_views.begin(), child_views.end(), nullptr), - child_views.end()); - - // Setup focus by reordering views. It is not safe to use SetNextFocusableView - // since child views may be added externally to this view. - for (size_t i = 0; i < child_views.size(); i++) - ReorderChildView(child_views[i], i); + delegate->UpdateButton(*member, type); } int DialogClientView::GetExtraViewSpacing() const { - if (!ShouldShow(extra_view_) || !has_dialog_buttons()) + if (!ShouldShow(extra_view_) || !(ok_button_ || cancel_button_)) return 0; int extra_view_padding = 0; @@ -371,41 +305,120 @@ ->GetDialogRelatedButtonHorizontalSpacing(); } +std::array<View*, DialogClientView::kNumButtons> +DialogClientView::GetButtonRowViews() { + View* first = ShouldShow(extra_view_) ? extra_view_ : nullptr; + View* second = cancel_button_; + View* third = ok_button_; + if (kIsOkButtonOnLeftSide) + std::swap(second, third); + return {{first, second, third}}; +} + +void DialogClientView::SetupLayout() { + GridLayout* layout = new GridLayout(button_row_container_); + layout->set_minimum_size(minimum_size_); + + // Clobber any existing LayoutManager since it has weak references to child + // Views which may be removed by SetupViews(). + button_row_container_->SetLayoutManager(layout); + SetupViews(); + const std::array<View*, kNumButtons> views = GetButtonRowViews(); + + // Visibility changes on |extra_view_| must be observed to re-Layout. However, + // when hidden it's not included in the button row (it can't influence layout) + // and it can't be added to |button_row_container_| (GridLayout complains). + // So add it, hidden, to |this| so it can be observed. + if (extra_view_ && !views[0]) + AddChildView(extra_view_); + + if (std::count(views.begin(), views.end(), nullptr) == kNumButtons) + return; + + gfx::Insets insets = button_row_insets_; + // Support dialogs that clear |button_row_insets_| to do their own layout. + // They expect GetDialogRelatedControlVerticalSpacing() in this case. + // TODO(tapted): Remove this under Harmony. + if (insets.top() == 0) { + const int top = + ViewsDelegate::GetInstance()->GetDialogRelatedControlVerticalSpacing(); + insets.Set(top, insets.left(), insets.bottom(), insets.right()); + } + + // The |resize_percent| constants. There's only one stretchy column (padding + // to the left of ok/cancel buttons). + constexpr float kFixed = 0.f; + constexpr float kStretchy = 1.f; + + // Button row is [ extra <pad+stretchy> second <pad> third ]. Ensure the <pad> + // column is zero width if there isn't a button on either side. + // GetExtraViewSpacing() handles <pad+stretchy>. + const int button_spacing = + (ok_button_ && cancel_button_) + ? ViewsDelegate::GetInstance() + ->GetDialogRelatedButtonHorizontalSpacing() + : 0; + + constexpr int kButtonRowId = 0; + ColumnSet* column_set = layout->AddColumnSet(kButtonRowId); + + // Rather than giving |button_row_container_| a Border, incorporate the insets + // into the layout. This simplifies min/max size calculations. + column_set->AddPaddingColumn(kFixed, insets.left()); + column_set->AddColumn(GridLayout::FILL, GridLayout::FILL, kFixed, + GridLayout::USE_PREF, 0, 0); + column_set->AddPaddingColumn(kStretchy, GetExtraViewSpacing()); + column_set->AddColumn(GridLayout::FILL, GridLayout::FILL, kFixed, + GridLayout::USE_PREF, 0, 0); + column_set->AddPaddingColumn(kFixed, button_spacing); + column_set->AddColumn(GridLayout::FILL, GridLayout::FILL, kFixed, + GridLayout::USE_PREF, 0, 0); + column_set->AddPaddingColumn(kFixed, insets.right()); + + // Track which columns to link sizes under MD. + constexpr int kViewToColumnIndex[] = {1, 3, 5}; + int link[] = {-1, -1, -1}; + size_t link_index = 0; + + layout->StartRowWithPadding(kFixed, kButtonRowId, kFixed, insets.top()); + for (size_t view_index = 0; view_index < kNumButtons; ++view_index) { + if (views[view_index]) { + layout->AddView(views[view_index]); + link[link_index++] = kViewToColumnIndex[view_index]; + } else { + layout->SkipColumns(1); + } + } + + if (ui::MaterialDesignController::IsSecondaryUiMaterial()) { + // Only link the extra view column if it is a button. + if (views[0] && !CustomButton::AsCustomButton(views[0])) + column_set->LinkColumnSizes(link[1], link[2], -1); + else + column_set->LinkColumnSizes(link[0], link[1], link[2], -1); + } + layout->AddPaddingRow(kFixed, insets.bottom()); +} + void DialogClientView::SetupViews() { - const int buttons = GetDialogDelegate()->GetDialogButtons(); - - if (buttons & ui::DIALOG_BUTTON_OK) { - if (!ok_button_) { - ok_button_ = CreateDialogButton(ui::DIALOG_BUTTON_OK); - AddChildView(ok_button_); - } - - GetDialogDelegate()->UpdateButton(ok_button_, ui::DIALOG_BUTTON_OK); - } else if (ok_button_) { - delete ok_button_; - ok_button_ = nullptr; + { + base::AutoReset<bool> auto_reset(&preserve_button_row_data_members_, true); + button_row_container_->RemoveAllChildViews(false /* delete children */); + // If SetupLayout() "stored" a hidden |extra_view_| in |this|, ensure it can + // be re-added to the layout when becoming visible. + if (extra_view_) + RemoveChildView(extra_view_); } - if (buttons & ui::DIALOG_BUTTON_CANCEL) { - if (!cancel_button_) { - cancel_button_ = CreateDialogButton(ui::DIALOG_BUTTON_CANCEL); - AddChildView(cancel_button_); - } - - GetDialogDelegate()->UpdateButton(cancel_button_, ui::DIALOG_BUTTON_CANCEL); - } else if (cancel_button_) { - delete cancel_button_; - cancel_button_ = nullptr; - } + UpdateDialogButton(&ok_button_, ui::DIALOG_BUTTON_OK); + UpdateDialogButton(&cancel_button_, ui::DIALOG_BUTTON_CANCEL); if (extra_view_) return; extra_view_ = GetDialogDelegate()->CreateExtraView(); - if (extra_view_) { + if (extra_view_) extra_view_->SetGroup(kButtonGroup); - AddChildView(extra_view_); - } } } // namespace views
diff --git a/ui/views/window/dialog_client_view.h b/ui/views/window/dialog_client_view.h index 36028128..ff184b87 100644 --- a/ui/views/window/dialog_client_view.h +++ b/ui/views/window/dialog_client_view.h
@@ -51,6 +51,9 @@ // View implementation: gfx::Size GetPreferredSize() const override; + gfx::Size GetMinimumSize() const override; + gfx::Size GetMaximumSize() const override; + void Layout() override; bool AcceleratorPressed(const ui::Accelerator& accelerator) override; void ViewHierarchyChanged( @@ -67,7 +70,11 @@ void set_minimum_size(const gfx::Size& size) { minimum_size_ = size; } private: - bool has_dialog_buttons() const { return ok_button_ || cancel_button_; } + enum { + // The number of buttons that DialogClientView can support. + kNumButtons = 3 + }; + class ButtonRowContainer; // Returns the DialogDelegate for the window. DialogDelegate* GetDialogDelegate() const; @@ -76,34 +83,25 @@ void ChildPreferredSizeChanged(View* child) override; void ChildVisibilityChanged(View* child) override; - // Create a dialog button of the appropriate type. - LabelButton* CreateDialogButton(ui::DialogButton type); - - // Update |button|'s text and enabled state according to the delegate's state. - void UpdateButton(LabelButton* button, ui::DialogButton type); - - // Returns the height of the buttons. - int GetButtonHeight() const; - - // Returns the height of the extra view. - int GetExtraViewHeight() const; - - // Returns the height of the row containing the buttons and the extra view. - int GetButtonsAndExtraViewRowHeight() const; - - // Returns the insets for the buttons and extra view, including the vertical - // padding between them and the contents view. - gfx::Insets GetButtonRowInsets() const; - - // Sets up the focus chain for the child views. This is required since the - // delegate may choose to add/remove views at any time. - void SetupFocusChain(); + // Creates, deletes, or updates the appearance of the button of type |type| + // (which must be pointed to by |member|). Which action is chosen is based on + // whether DialogDelegate::GetDialogButtons() includes |type|, and whether + // |member| points to a button that already exists. + void UpdateDialogButton(LabelButton** member, ui::DialogButton type); // Returns the spacing between the extra view and the ok/cancel buttons. 0 if // no extra view. Otherwise uses GetExtraViewPadding() or the default padding. int GetExtraViewSpacing() const; + // Returns Views in the button row, as they should appear in the layout. If + // a View should not appear, it will be null. + std::array<View*, kNumButtons> GetButtonRowViews(); + + // Installs and configures the LayoutManager for |button_row_container_|. + void SetupLayout(); + // Creates or deletes any buttons that are required. Updates data members. + // After calling this, no button row Views will be in the view hierarchy. void SetupViews(); // How much to inset the button row. @@ -120,12 +118,18 @@ // The extra view shown in the row of buttons; may be NULL. View* extra_view_ = nullptr; + // Container view for the button row. + ButtonRowContainer* button_row_container_ = nullptr; + // True if we've notified the delegate the window is closing and the delegate // allowed the close. In some situations it's possible to get two closes (see // http://crbug.com/71940). This is used to avoid notifying the delegate // twice, which can have bad consequences. bool delegate_allowed_close_ = false; + // When true, prevents ViewHierarchyChanged() from clearing out data members. + bool preserve_button_row_data_members_ = false; + DISALLOW_COPY_AND_ASSIGN(DialogClientView); };
diff --git a/ui/views/window/dialog_client_view_unittest.cc b/ui/views/window/dialog_client_view_unittest.cc index e4d7eeb..a8d3d8d 100644 --- a/ui/views/window/dialog_client_view_unittest.cc +++ b/ui/views/window/dialog_client_view_unittest.cc
@@ -7,6 +7,7 @@ #include "base/macros.h" #include "base/strings/utf_string_conversions.h" #include "build/build_config.h" +#include "ui/base/test/material_design_controller_test_api.h" #include "ui/base/ui_base_types.h" #include "ui/views/controls/button/label_button.h" #include "ui/views/style/platform_style.h" @@ -43,6 +44,9 @@ } // DialogDelegateView: + gfx::Size GetPreferredSize() const override { return preferred_size_; } + gfx::Size GetMinimumSize() const override { return min_size_; } + gfx::Size GetMaximumSize() const override { return max_size_; } ClientView* CreateClientView(Widget* widget) override { client_view_ = new DialogClientView(widget, this); return client_view_; @@ -63,6 +67,11 @@ } int GetDialogButtons() const override { return dialog_buttons_; } + base::string16 GetDialogButtonLabel(ui::DialogButton button) const override { + return button == ui::DIALOG_BUTTON_CANCEL && !cancel_label_.empty() + ? cancel_label_ + : DialogDelegate::GetDialogButtonLabel(button); + } protected: gfx::Rect GetUpdatedClientBounds() { @@ -102,7 +111,15 @@ void SetExtraViewPadding(int padding) { DCHECK(!extra_view_padding_); extra_view_padding_.reset(new int(padding)); - client_view_->Layout(); + client_view_->UpdateDialogButtons(); + } + + void SetSizeConstraints(const gfx::Size& min_size, + const gfx::Size& preferred_size, + const gfx::Size& max_size) { + min_size_ = min_size; + preferred_size_ = preferred_size; + max_size_ = max_size; } View* FocusableViewAfter(View* view) { @@ -112,6 +129,12 @@ dont_loop); } + // Set a longer than normal Cancel label so that the minimum button width is + // exceeded. + void SetLongCancelLabel() { + cancel_label_ = base::ASCIIToUTF16("Cancel Cancel Cancel"); + } + DialogClientView* client_view() { return client_view_; } private: @@ -129,6 +152,12 @@ std::unique_ptr<int> extra_view_padding_; + gfx::Size preferred_size_; + gfx::Size min_size_; + gfx::Size max_size_; + + base::string16 cancel_label_; // If set, the label for the Cancel button. + DISALLOW_COPY_AND_ASSIGN(DialogClientViewTest); }; @@ -188,24 +217,27 @@ const bool kIsOkButtonOnLeftSide = false; #endif + GetContentsView()->SetFocusBehavior(View::FocusBehavior::ALWAYS); // Initially the dialog client view only contains the content view. - EXPECT_EQ(nullptr, GetContentsView()->GetNextFocusableView()); + EXPECT_EQ(GetContentsView(), FocusableViewAfter(GetContentsView())); // Add OK and cancel buttons. SetDialogButtons(ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL); if (kIsOkButtonOnLeftSide) { EXPECT_EQ(client_view()->ok_button(), - GetContentsView()->GetNextFocusableView()); + FocusableViewAfter(GetContentsView())); EXPECT_EQ(client_view()->cancel_button(), - client_view()->ok_button()->GetNextFocusableView()); - EXPECT_EQ(nullptr, client_view()->cancel_button()->GetNextFocusableView()); + FocusableViewAfter(client_view()->ok_button())); + EXPECT_EQ(GetContentsView(), + FocusableViewAfter(client_view()->cancel_button())); } else { EXPECT_EQ(client_view()->cancel_button(), - GetContentsView()->GetNextFocusableView()); + FocusableViewAfter(GetContentsView())); EXPECT_EQ(client_view()->ok_button(), - client_view()->cancel_button()->GetNextFocusableView()); - EXPECT_EQ(nullptr, client_view()->ok_button()->GetNextFocusableView()); + FocusableViewAfter(client_view()->cancel_button())); + EXPECT_EQ(GetContentsView(), + FocusableViewAfter(client_view()->ok_button())); } // Add extra view and remove OK button. @@ -214,14 +246,15 @@ SetExtraView(extra_view); SetDialogButtons(ui::DIALOG_BUTTON_CANCEL); - EXPECT_EQ(extra_view, GetContentsView()->GetNextFocusableView()); - EXPECT_EQ(client_view()->cancel_button(), extra_view->GetNextFocusableView()); - EXPECT_EQ(nullptr, client_view()->cancel_button()->GetNextFocusableView()); + EXPECT_EQ(extra_view, FocusableViewAfter(GetContentsView())); + EXPECT_EQ(client_view()->cancel_button(), FocusableViewAfter(extra_view)); + EXPECT_EQ(GetContentsView(), FocusableViewAfter(client_view())); // Add a dummy view to the contents view. Consult the FocusManager for the // traversal order since it now spans different levels of the view hierarchy. View* dummy_view = new StaticSizedView(gfx::Size(200, 200)); dummy_view->SetFocusBehavior(View::FocusBehavior::ALWAYS); + GetContentsView()->SetFocusBehavior(View::FocusBehavior::NEVER); GetContentsView()->AddChildView(dummy_view); EXPECT_EQ(dummy_view, FocusableViewAfter(client_view()->cancel_button())); EXPECT_EQ(extra_view, FocusableViewAfter(dummy_view)); @@ -248,31 +281,128 @@ EXPECT_LT(GetContentsView()->bounds().bottom(), client_view()->bounds().bottom()); - gfx::Size no_extra_view_size = client_view()->bounds().size(); + const gfx::Size no_extra_view_size = client_view()->bounds().size(); View* extra_view = new StaticSizedView(gfx::Size(200, 200)); SetExtraView(extra_view); CheckContentsIsSetToPreferredSize(); EXPECT_GT(client_view()->bounds().height(), no_extra_view_size.height()); - int width_of_dialog = client_view()->bounds().width(); - int width_of_extra_view = extra_view->bounds().width(); + const int width_of_dialog_small_padding = client_view()->width(); // Try with an adjusted padding for the extra view. SetExtraViewPadding(250); CheckContentsIsSetToPreferredSize(); - EXPECT_GT(client_view()->bounds().width(), width_of_dialog); + EXPECT_GT(client_view()->bounds().width(), width_of_dialog_small_padding); - // Visibility of extra view is respected. + const gfx::Size with_extra_view_size = client_view()->size(); + EXPECT_NE(no_extra_view_size, with_extra_view_size); + + // Hiding the extra view removes it as well as the extra padding. extra_view->SetVisible(false); CheckContentsIsSetToPreferredSize(); - EXPECT_EQ(no_extra_view_size.height(), client_view()->bounds().height()); - EXPECT_EQ(no_extra_view_size.width(), client_view()->bounds().width()); + EXPECT_EQ(no_extra_view_size, client_view()->size()); - // Try with a reduced-size dialog. + // Making it visible again adds it all back. extra_view->SetVisible(true); - client_view()->SetBoundsRect(gfx::Rect(gfx::Point(0, 0), no_extra_view_size)); - client_view()->Layout(); - EXPECT_GT(width_of_extra_view, extra_view->bounds().width()); + CheckContentsIsSetToPreferredSize(); + EXPECT_EQ(with_extra_view_size, client_view()->size()); + + // Leave |extra_view| hidden. It should still have a parent, to ensure it is + // owned by a View hierarchy and gets deleted. + extra_view->SetVisible(false); + EXPECT_TRUE(extra_view->parent()); +} + +// Ensure the minimum, maximum and preferred sizes of the contents view are +// respected by the client view, and that the client view includes the button +// row in its minimum and preferred size calculations. +TEST_F(DialogClientViewTest, MinMaxPreferredSize) { + SetDialogButtons(ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL); + const gfx::Size buttons_size = client_view()->GetPreferredSize(); + EXPECT_FALSE(buttons_size.IsEmpty()); + + // When the contents view has no preference, just fit the buttons. The + // maximum size should be unconstrained in both directions. + EXPECT_EQ(buttons_size, client_view()->GetMinimumSize()); + EXPECT_EQ(gfx::Size(), client_view()->GetMaximumSize()); + + // Ensure buttons are between these widths, for the constants below. + EXPECT_LT(20, buttons_size.width()); + EXPECT_GT(300, buttons_size.width()); + + // With no buttons, client view should match the contents view. + SetDialogButtons(ui::DIALOG_BUTTON_NONE); + SetSizeConstraints(gfx::Size(10, 15), gfx::Size(20, 25), gfx::Size(300, 350)); + EXPECT_EQ(gfx::Size(10, 15), client_view()->GetMinimumSize()); + EXPECT_EQ(gfx::Size(20, 25), client_view()->GetPreferredSize()); + EXPECT_EQ(gfx::Size(300, 350), client_view()->GetMaximumSize()); + + // With buttons, size should increase vertically only. + SetDialogButtons(ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL); + EXPECT_EQ(gfx::Size(buttons_size.width(), 15 + buttons_size.height()), + client_view()->GetMinimumSize()); + EXPECT_EQ(gfx::Size(buttons_size.width(), 25 + buttons_size.height()), + client_view()->GetPreferredSize()); + EXPECT_EQ(gfx::Size(300, 350 + buttons_size.height()), + client_view()->GetMaximumSize()); + + // If the contents view gets bigger, it should take over the width. + SetSizeConstraints(gfx::Size(400, 450), gfx::Size(500, 550), + gfx::Size(600, 650)); + EXPECT_EQ(gfx::Size(400, 450 + buttons_size.height()), + client_view()->GetMinimumSize()); + EXPECT_EQ(gfx::Size(500, 550 + buttons_size.height()), + client_view()->GetPreferredSize()); + EXPECT_EQ(gfx::Size(600, 650 + buttons_size.height()), + client_view()->GetMaximumSize()); +} + +// Ensure button widths are linked under MD. +TEST_F(DialogClientViewTest, LinkedWidths) { + ui::test::MaterialDesignControllerTestAPI md_test_api( + ui::MaterialDesignController::MATERIAL_NORMAL); + md_test_api.SetSecondaryUiMaterial(true); + SetLongCancelLabel(); + + SetDialogButtons(ui::DIALOG_BUTTON_OK); + CheckContentsIsSetToPreferredSize(); + const int ok_button_only_width = client_view()->ok_button()->width(); + + SetDialogButtons(ui::DIALOG_BUTTON_CANCEL); + CheckContentsIsSetToPreferredSize(); + const int cancel_button_width = client_view()->cancel_button()->width(); + + // Ensure the single buttons have different preferred widths when alone, and + // that the Cancel button is bigger (so that it dominates the size). + EXPECT_GT(cancel_button_width, ok_button_only_width); + + SetDialogButtons(ui::DIALOG_BUTTON_CANCEL | ui::DIALOG_BUTTON_OK); + CheckContentsIsSetToPreferredSize(); + + // OK button should now match the bigger, cancel button. + EXPECT_EQ(cancel_button_width, client_view()->ok_button()->width()); + + // But not under non-MD. + md_test_api.SetSecondaryUiMaterial(false); + client_view()->UpdateDialogButtons(); + CheckContentsIsSetToPreferredSize(); + EXPECT_EQ(ok_button_only_width, client_view()->ok_button()->width()); + md_test_api.SetSecondaryUiMaterial(true); + + // The extra view should also match, if it's a button. + LabelButton* extra_button = new LabelButton(nullptr, base::string16()); + SetExtraView(extra_button); + CheckContentsIsSetToPreferredSize(); + EXPECT_EQ(cancel_button_width, extra_button->width()); + + // Remove |extra_button| from the View hierarchy so that it can be replaced. + delete extra_button; + + // Non-buttons should always be sized to their preferred size. + View* boring_view = new StaticSizedView(gfx::Size(20, 20)); + SetExtraView(boring_view); + CheckContentsIsSetToPreferredSize(); + EXPECT_EQ(20, boring_view->width()); } } // namespace views