Commit 1b444796 authored by Keishi Hattori's avatar Keishi Hattori Committed by Commit Bot

Revert "Start calling v8 API SetDetachedWindowReason"

This reverts commit a4168673.

Reason for revert: Instrumentation no longer needed. It was meant only for 1 milestone.

Original change's description:
> Start calling v8 API SetDetachedWindowReason
>
> This enables v8-side tracking of JS function calls made within a context
> of a detached window.
>
> Such calls should be rare, but if our predictions are incorrect, the
> tracking code may impact performance of function calls. Therefore,
> this is implemented behind finch experiments SetDetachedWindowReasonBy*
> (enabled by default, but it'll give as an opportunity to disable
> tracking or limit % of participating users, if needed).
>
> Bug: 1018156
> Change-Id: I878e6f1fabdeb229531af688fea0a5bc7412a145
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1895017
> Commit-Queue: Bartek Nowierski <bartekn@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
> Reviewed-by: Keishi Hattori <keishi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#712517}

TBR=yukishiino@chromium.org,rkaplow@chromium.org,tasak@google.com,haraken@chromium.org,keishi@chromium.org,bartekn@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1018156
Change-Id: I35c58011a6ff0984c339393b083a48d13b6b3f12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2000404Reviewed-by: default avatarKeishi Hattori <keishi@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#731873}
parent 1f69cbbc
...@@ -411,16 +411,6 @@ const base::Feature kVizHitTestOcclusionCheck{ ...@@ -411,16 +411,6 @@ const base::Feature kVizHitTestOcclusionCheck{
const base::Feature kSetLowPriorityForBeacon{"SetLowPriorityForBeacon", const base::Feature kSetLowPriorityForBeacon{"SetLowPriorityForBeacon",
base::FEATURE_DISABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
// When enabled, JS function calls in a detached window will be reported.
// Reporting has a non-zero probability of a performance impact, hence an easy
// way to disable it may come in handy.
const base::Feature kSetDetachedWindowReasonByNavigation{
"SetDetachedWindowReasonByNavigation", base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kSetDetachedWindowReasonByClosing{
"SetDetachedWindowReasonByClosing", base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kSetDetachedWindowReasonByOtherReason{
"SetDetachedWindowReasonByOtherReason", base::FEATURE_ENABLED_BY_DEFAULT};
// When enabled allows the header name used in the blink // When enabled allows the header name used in the blink
// CacheStorageCodeCacheHint runtime feature to be modified. This runtime // CacheStorageCodeCacheHint runtime feature to be modified. This runtime
// feature disables generating full code cache for responses stored in // feature disables generating full code cache for responses stored in
......
...@@ -133,13 +133,6 @@ BLINK_COMMON_EXPORT extern const base::Feature kSubresourceRedirect; ...@@ -133,13 +133,6 @@ BLINK_COMMON_EXPORT extern const base::Feature kSubresourceRedirect;
BLINK_COMMON_EXPORT extern const base::Feature kSetLowPriorityForBeacon; BLINK_COMMON_EXPORT extern const base::Feature kSetLowPriorityForBeacon;
BLINK_COMMON_EXPORT extern const base::Feature
kSetDetachedWindowReasonByNavigation;
BLINK_COMMON_EXPORT extern const base::Feature
kSetDetachedWindowReasonByClosing;
BLINK_COMMON_EXPORT extern const base::Feature
kSetDetachedWindowReasonByOtherReason;
BLINK_COMMON_EXPORT extern const base::Feature kCacheStorageCodeCacheHintHeader; BLINK_COMMON_EXPORT extern const base::Feature kCacheStorageCodeCacheHintHeader;
BLINK_COMMON_EXPORT extern const base::FeatureParam<std::string> BLINK_COMMON_EXPORT extern const base::FeatureParam<std::string>
kCacheStorageCodeCacheHintHeaderName; kCacheStorageCodeCacheHintHeaderName;
......
...@@ -31,7 +31,6 @@ ...@@ -31,7 +31,6 @@
#include "third_party/blink/renderer/bindings/core/v8/local_window_proxy.h" #include "third_party/blink/renderer/bindings/core/v8/local_window_proxy.h"
#include "base/debug/dump_without_crashing.h" #include "base/debug/dump_without_crashing.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/renderer/bindings/core/v8/isolated_world_csp.h" #include "third_party/blink/renderer/bindings/core/v8/isolated_world_csp.h"
#include "third_party/blink/renderer/bindings/core/v8/script_controller.h" #include "third_party/blink/renderer/bindings/core/v8/script_controller.h"
#include "third_party/blink/renderer/bindings/core/v8/to_v8_for_core.h" #include "third_party/blink/renderer/bindings/core/v8/to_v8_for_core.h"
...@@ -80,29 +79,8 @@ void LocalWindowProxy::Trace(blink::Visitor* visitor) { ...@@ -80,29 +79,8 @@ void LocalWindowProxy::Trace(blink::Visitor* visitor) {
WindowProxy::Trace(visitor); WindowProxy::Trace(visitor);
} }
bool LocalWindowProxy::IsSetDetachedWindowReasonEnabled( void LocalWindowProxy::DisposeContext(Lifecycle next_status,
v8::Context::DetachedWindowReason reason) { FrameReuseStatus frame_reuse_status) {
switch (reason) {
case v8::Context::DetachedWindowReason::kWindowNotDetached:
// This shouldn't happen, but if it does, it's always safe to clear the
// reason.
return true;
case v8::Context::DetachedWindowReason::kDetachedWindowByNavigation:
return base::FeatureList::IsEnabled(
features::kSetDetachedWindowReasonByNavigation);
case v8::Context::DetachedWindowReason::kDetachedWindowByClosing:
return base::FeatureList::IsEnabled(
features::kSetDetachedWindowReasonByClosing);
case v8::Context::DetachedWindowReason::kDetachedWindowByOtherReason:
return base::FeatureList::IsEnabled(
features::kSetDetachedWindowReasonByOtherReason);
}
}
void LocalWindowProxy::DisposeContext(
Lifecycle next_status,
FrameReuseStatus frame_reuse_status,
v8::Context::DetachedWindowReason reason) {
DCHECK(next_status == Lifecycle::kV8MemoryIsForciblyPurged || DCHECK(next_status == Lifecycle::kV8MemoryIsForciblyPurged ||
next_status == Lifecycle::kGlobalObjectIsDetached || next_status == Lifecycle::kGlobalObjectIsDetached ||
next_status == Lifecycle::kFrameIsDetached || next_status == Lifecycle::kFrameIsDetached ||
...@@ -149,10 +127,6 @@ void LocalWindowProxy::DisposeContext( ...@@ -149,10 +127,6 @@ void LocalWindowProxy::DisposeContext(
#endif #endif
} }
if (IsSetDetachedWindowReasonEnabled(reason)) {
context->SetDetachedWindowReason(reason);
}
script_state_->DisposePerContextData(); script_state_->DisposePerContextData();
// It's likely that disposing the context has created a lot of // It's likely that disposing the context has created a lot of
// garbage. Notify V8 about this so it'll have a chance of cleaning // garbage. Notify V8 about this so it'll have a chance of cleaning
......
...@@ -70,11 +70,7 @@ class LocalWindowProxy final : public WindowProxy { ...@@ -70,11 +70,7 @@ class LocalWindowProxy final : public WindowProxy {
private: private:
bool IsLocal() const override { return true; } bool IsLocal() const override { return true; }
void Initialize() override; void Initialize() override;
void DisposeContext(Lifecycle next_status, void DisposeContext(Lifecycle next_status, FrameReuseStatus) override;
FrameReuseStatus,
v8::Context::DetachedWindowReason) override;
static bool IsSetDetachedWindowReasonEnabled(
v8::Context::DetachedWindowReason reason);
// Creates a new v8::Context with the window wrapper object as the global // Creates a new v8::Context with the window wrapper object as the global
// object (aka the inner global). Note that the window wrapper and its // object (aka the inner global). Note that the window wrapper and its
......
...@@ -49,8 +49,7 @@ RemoteWindowProxy::RemoteWindowProxy(v8::Isolate* isolate, ...@@ -49,8 +49,7 @@ RemoteWindowProxy::RemoteWindowProxy(v8::Isolate* isolate,
: WindowProxy(isolate, frame, std::move(world)) {} : WindowProxy(isolate, frame, std::move(world)) {}
void RemoteWindowProxy::DisposeContext(Lifecycle next_status, void RemoteWindowProxy::DisposeContext(Lifecycle next_status,
FrameReuseStatus, FrameReuseStatus) {
v8::Context::DetachedWindowReason) {
DCHECK(next_status == Lifecycle::kV8MemoryIsForciblyPurged || DCHECK(next_status == Lifecycle::kV8MemoryIsForciblyPurged ||
next_status == Lifecycle::kGlobalObjectIsDetached || next_status == Lifecycle::kGlobalObjectIsDetached ||
next_status == Lifecycle::kFrameIsDetached || next_status == Lifecycle::kFrameIsDetached ||
......
...@@ -49,9 +49,7 @@ class RemoteWindowProxy final : public WindowProxy { ...@@ -49,9 +49,7 @@ class RemoteWindowProxy final : public WindowProxy {
private: private:
void Initialize() override; void Initialize() override;
void DisposeContext(Lifecycle next_status, void DisposeContext(Lifecycle next_status, FrameReuseStatus) override;
FrameReuseStatus,
v8::Context::DetachedWindowReason) override;
// Creates a new v8::Context with the window wrapper object as the global // Creates a new v8::Context with the window wrapper object as the global
// object (aka the inner global). Note that the window wrapper and its // object (aka the inner global). Note that the window wrapper and its
......
...@@ -65,24 +65,19 @@ void WindowProxy::ClearForClose() { ...@@ -65,24 +65,19 @@ void WindowProxy::ClearForClose() {
DisposeContext(lifecycle_ == Lifecycle::kV8MemoryIsForciblyPurged DisposeContext(lifecycle_ == Lifecycle::kV8MemoryIsForciblyPurged
? Lifecycle::kFrameIsDetachedAndV8MemoryIsPurged ? Lifecycle::kFrameIsDetachedAndV8MemoryIsPurged
: Lifecycle::kFrameIsDetached, : Lifecycle::kFrameIsDetached,
kFrameWillNotBeReused, kFrameWillNotBeReused);
v8::Context::DetachedWindowReason::kDetachedWindowByClosing);
} }
void WindowProxy::ClearForNavigation() { void WindowProxy::ClearForNavigation() {
DisposeContext(Lifecycle::kGlobalObjectIsDetached, kFrameWillBeReused, DisposeContext(Lifecycle::kGlobalObjectIsDetached, kFrameWillBeReused);
v8::Context::kDetachedWindowByNavigation);
} }
void WindowProxy::ClearForSwap() { void WindowProxy::ClearForSwap() {
// This happens on a navigation between local/remote source. DisposeContext(Lifecycle::kGlobalObjectIsDetached, kFrameWillNotBeReused);
DisposeContext(Lifecycle::kGlobalObjectIsDetached, kFrameWillNotBeReused,
v8::Context::kDetachedWindowByNavigation);
} }
void WindowProxy::ClearForV8MemoryPurge() { void WindowProxy::ClearForV8MemoryPurge() {
DisposeContext(Lifecycle::kV8MemoryIsForciblyPurged, kFrameWillNotBeReused, DisposeContext(Lifecycle::kV8MemoryIsForciblyPurged, kFrameWillNotBeReused);
v8::Context::kDetachedWindowByOtherReason);
} }
v8::Local<v8::Object> WindowProxy::GlobalProxyIfNotDetached() { v8::Local<v8::Object> WindowProxy::GlobalProxyIfNotDetached() {
......
...@@ -256,9 +256,7 @@ class WindowProxy : public GarbageCollected<WindowProxy> { ...@@ -256,9 +256,7 @@ class WindowProxy : public GarbageCollected<WindowProxy> {
virtual void Initialize() = 0; virtual void Initialize() = 0;
virtual void DisposeContext(Lifecycle next_status, virtual void DisposeContext(Lifecycle next_status, FrameReuseStatus) = 0;
FrameReuseStatus,
v8::Context::DetachedWindowReason) = 0;
WARN_UNUSED_RESULT v8::Local<v8::Object> AssociateWithWrapper( WARN_UNUSED_RESULT v8::Local<v8::Object> AssociateWithWrapper(
DOMWindow*, DOMWindow*,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment