Commit 001f6234 authored by jsbell's avatar jsbell Committed by Commit bot

Bindings: Require [CachedAttribute] callbacks to be const

Since most "isAttribDirty()" methods were already const, require it
in the docs and via casting in the generated code to ensure that the
check is idempotent. Mostly for consistency, but this prevents us
from triggering unexpected side effects in future optimizations.

Update the one place (navigator.languages) where a non-const method
was being used.

Review-Url: https://codereview.chromium.org/2884493002
Cr-Commit-Position: refs/heads/master@{#472247}
parent 87dc11bd
...@@ -1225,9 +1225,9 @@ These extended attributes are rarely used, generally only in one or two places. ...@@ -1225,9 +1225,9 @@ These extended attributes are rarely used, generally only in one or two places.
### [CachedAttribute] _(a)_ ### [CachedAttribute] _(a)_
Summary: For performance optimization, `[CachedAttribute]` indicates that a wrapped object should be cached on a DOM object. Rarely used (only by IndexDB). Summary: For performance optimization, `[CachedAttribute]` indicates that a wrapped object should be cached on a DOM object. Rarely used.
Usage: `[CachedAttribute]` can be specified on attributes, and takes a required value, generally called is*Dirty (esp. isValueDirty): Usage: `[CachedAttribute]` can be specified on attributes, and takes a required value, generally called is*Dirty (e.g. isValueDirty):
```webidl ```webidl
interface HTMLFoo { interface HTMLFoo {
...@@ -1270,7 +1270,7 @@ In case where `HTMLFoo::serializedValue()`, the deserialization or the operation ...@@ -1270,7 +1270,7 @@ In case where `HTMLFoo::serializedValue()`, the deserialization or the operation
You should cache attributes if and only if it is really important for performance. Not only does caching increase the DOM object size, but also it increases the overhead of "cache-miss"ed getters. In addition, setters always need to invalidate the cache. You should cache attributes if and only if it is really important for performance. Not only does caching increase the DOM object size, but also it increases the overhead of "cache-miss"ed getters. In addition, setters always need to invalidate the cache.
*** ***
`[CachedAttribute]` takes a required parameter which the name of a method to call on the implementation object. The method should take void and return bool. Before the cached attribute is used, the method will be called. If the method returns true the cached value is not used, which will result in the accessor being called again. This allows the implementation to both gain the performance benefit of caching (when the conversion to a script value can be done lazily) while allowing the value to be updated. The typical use pattern is: `[CachedAttribute]` takes a required parameter which the name of a method to call on the implementation object. The method should be const, take void and return bool. Before the cached attribute is used, the method will be called. If the method returns true the cached value is not used, which will result in the accessor being called again. This allows the implementation to both gain the performance benefit of caching (when the conversion to a script value can be done lazily) while allowing the value to be updated. The typical use pattern is:
```c++ ```c++
// Called internally to update value // Called internally to update value
...@@ -1281,7 +1281,7 @@ void Object::setValue(Type data) ...@@ -1281,7 +1281,7 @@ void Object::setValue(Type data)
} }
// Called by generated binding code // Called by generated binding code
bool Object::isAttributeDirty() bool Object::isAttributeDirty() const
{ {
return m_attributeDirty; return m_attributeDirty;
} }
......
...@@ -63,7 +63,7 @@ const v8::FunctionCallbackInfo<v8::Value>& info ...@@ -63,7 +63,7 @@ const v8::FunctionCallbackInfo<v8::Value>& info
V8PrivateProperty::Symbol propertySymbol = V8PrivateProperty::Symbol propertySymbol =
V8PrivateProperty::GetSymbol(info.GetIsolate(), V8PrivateProperty::GetSymbol(info.GetIsolate(),
"{{cpp_class}}#{{attribute.name.capitalize()}}"); "{{cpp_class}}#{{attribute.name.capitalize()}}");
if (!impl->{{attribute.cached_attribute_validation_method}}()) { if (!static_cast<const {{cpp_class}}*>(impl)->{{attribute.cached_attribute_validation_method}}()) {
v8::Local<v8::Value> v8Value = propertySymbol.GetOrUndefined(holder); v8::Local<v8::Value> v8Value = propertySymbol.GetOrUndefined(holder);
if (!v8Value->IsUndefined()) { if (!v8Value->IsUndefined()) {
V8SetReturnValue(info, v8Value); V8SetReturnValue(info, v8Value);
......
...@@ -15,7 +15,6 @@ ...@@ -15,7 +15,6 @@
#include "bindings/core/v8/V8DOMConfiguration.h" #include "bindings/core/v8/V8DOMConfiguration.h"
#include "bindings/core/v8/V8Location.h" #include "bindings/core/v8/V8Location.h"
#include "core/animation/DocumentAnimation.h" #include "core/animation/DocumentAnimation.h"
#include "core/css/DocumentFontFaceSet.h"
#include "core/dom/DocumentFullscreen.h" #include "core/dom/DocumentFullscreen.h"
#include "core/dom/ExecutionContext.h" #include "core/dom/ExecutionContext.h"
#include "core/svg/SVGDocumentExtensions.h" #include "core/svg/SVGDocumentExtensions.h"
......
...@@ -2002,7 +2002,7 @@ static void cachedAttributeAnyAttributeAttributeGetter(const v8::FunctionCallbac ...@@ -2002,7 +2002,7 @@ static void cachedAttributeAnyAttributeAttributeGetter(const v8::FunctionCallbac
V8PrivateProperty::Symbol propertySymbol = V8PrivateProperty::Symbol propertySymbol =
V8PrivateProperty::GetSymbol(info.GetIsolate(), V8PrivateProperty::GetSymbol(info.GetIsolate(),
"TestObject#Cachedattributeanyattribute"); "TestObject#Cachedattributeanyattribute");
if (!impl->isValueDirty()) { if (!static_cast<const TestObject*>(impl)->isValueDirty()) {
v8::Local<v8::Value> v8Value = propertySymbol.GetOrUndefined(holder); v8::Local<v8::Value> v8Value = propertySymbol.GetOrUndefined(holder);
if (!v8Value->IsUndefined()) { if (!v8Value->IsUndefined()) {
V8SetReturnValue(info, v8Value); V8SetReturnValue(info, v8Value);
...@@ -2049,7 +2049,7 @@ static void cachedArrayAttributeAttributeGetter(const v8::FunctionCallbackInfo<v ...@@ -2049,7 +2049,7 @@ static void cachedArrayAttributeAttributeGetter(const v8::FunctionCallbackInfo<v
V8PrivateProperty::Symbol propertySymbol = V8PrivateProperty::Symbol propertySymbol =
V8PrivateProperty::GetSymbol(info.GetIsolate(), V8PrivateProperty::GetSymbol(info.GetIsolate(),
"TestObject#Cachedarrayattribute"); "TestObject#Cachedarrayattribute");
if (!impl->isArrayDirty()) { if (!static_cast<const TestObject*>(impl)->isArrayDirty()) {
v8::Local<v8::Value> v8Value = propertySymbol.GetOrUndefined(holder); v8::Local<v8::Value> v8Value = propertySymbol.GetOrUndefined(holder);
if (!v8Value->IsUndefined()) { if (!v8Value->IsUndefined()) {
V8SetReturnValue(info, v8Value); V8SetReturnValue(info, v8Value);
...@@ -2100,7 +2100,7 @@ static void cachedStringOrNoneAttributeAttributeGetter(const v8::FunctionCallbac ...@@ -2100,7 +2100,7 @@ static void cachedStringOrNoneAttributeAttributeGetter(const v8::FunctionCallbac
V8PrivateProperty::Symbol propertySymbol = V8PrivateProperty::Symbol propertySymbol =
V8PrivateProperty::GetSymbol(info.GetIsolate(), V8PrivateProperty::GetSymbol(info.GetIsolate(),
"TestObject#Cachedstringornoneattribute"); "TestObject#Cachedstringornoneattribute");
if (!impl->isStringDirty()) { if (!static_cast<const TestObject*>(impl)->isStringDirty()) {
v8::Local<v8::Value> v8Value = propertySymbol.GetOrUndefined(holder); v8::Local<v8::Value> v8Value = propertySymbol.GetOrUndefined(holder);
if (!v8Value->IsUndefined()) { if (!v8Value->IsUndefined()) {
V8SetReturnValue(info, v8Value); V8SetReturnValue(info, v8Value);
...@@ -3110,7 +3110,7 @@ static void cachedAttributeRaisesExceptionGetterAnyAttributeAttributeGetter(cons ...@@ -3110,7 +3110,7 @@ static void cachedAttributeRaisesExceptionGetterAnyAttributeAttributeGetter(cons
V8PrivateProperty::Symbol propertySymbol = V8PrivateProperty::Symbol propertySymbol =
V8PrivateProperty::GetSymbol(info.GetIsolate(), V8PrivateProperty::GetSymbol(info.GetIsolate(),
"TestObject#Cachedattributeraisesexceptiongetteranyattribute"); "TestObject#Cachedattributeraisesexceptiongetteranyattribute");
if (!impl->isValueDirty()) { if (!static_cast<const TestObject*>(impl)->isValueDirty()) {
v8::Local<v8::Value> v8Value = propertySymbol.GetOrUndefined(holder); v8::Local<v8::Value> v8Value = propertySymbol.GetOrUndefined(holder);
if (!v8Value->IsUndefined()) { if (!v8Value->IsUndefined()) {
V8SetReturnValue(info, v8Value); V8SetReturnValue(info, v8Value);
......
...@@ -75,6 +75,7 @@ bool Navigator::cookieEnabled() const { ...@@ -75,6 +75,7 @@ bool Navigator::cookieEnabled() const {
Vector<String> Navigator::languages() { Vector<String> Navigator::languages() {
Vector<String> languages; Vector<String> languages;
languages_changed_ = false;
if (!GetFrame() || !GetFrame()->GetPage()) { if (!GetFrame() || !GetFrame()->GetPage()) {
languages.push_back(DefaultLanguage()); languages.push_back(DefaultLanguage());
......
...@@ -8,18 +8,14 @@ ...@@ -8,18 +8,14 @@
namespace blink { namespace blink {
NavigatorLanguage::NavigatorLanguage() : languages_changed_(true) {} NavigatorLanguage::NavigatorLanguage() {}
AtomicString NavigatorLanguage::language() { AtomicString NavigatorLanguage::language() {
return DefaultLanguage(); return DefaultLanguage();
} }
bool NavigatorLanguage::hasLanguagesChanged() { bool NavigatorLanguage::hasLanguagesChanged() const {
if (!languages_changed_) return languages_changed_;
return false;
languages_changed_ = false;
return true;
} }
void NavigatorLanguage::SetLanguagesChanged() { void NavigatorLanguage::SetLanguagesChanged() {
......
...@@ -16,11 +16,11 @@ class CORE_EXPORT NavigatorLanguage { ...@@ -16,11 +16,11 @@ class CORE_EXPORT NavigatorLanguage {
AtomicString language(); AtomicString language();
virtual Vector<String> languages() = 0; virtual Vector<String> languages() = 0;
bool hasLanguagesChanged(); bool hasLanguagesChanged() const;
void SetLanguagesChanged(); void SetLanguagesChanged();
private: protected:
bool languages_changed_; bool languages_changed_ = true;
}; };
} // namespace blink } // namespace blink
......
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