Skip to content

Commit

Permalink
Use comparison against nullptr when using Member in boolean contexts
Browse files Browse the repository at this point in the history
This allows moving to explicit operator T* for Member and also speeds up
the methods as boolean checks can work on compressed Member without
requiring decompression. See here [1] for the operator== overload.

This was automatically converted using
```
autoninja -C out/Release blink_tests -k 0  \
| grep " error: no viable conversion from returned value of type" \
| sort \
| uniq -c \
| sort -nr
| grep "'bool'" > out-bool.log
```

The build errors lead to a simple replace:
```
import fileinput
import re
import sys

bool_regex = r'(.*return\s)(.*)(;.*)$'
bool_replace = r'\1 \2 != nullptr \3'

def replace(file_name, line_nr):
  i = 1
  for line in fileinput.input(file_name, inplace=1):
    if i == line_nr:
      line = re.sub(bool_regex, bool_replace, line)
    sys.stdout.write(line)
    i += 1

with open (sys.argv[1]) as log_file:
  for line in log_file:
    file_parts = line.lstrip().split(' ')[1].split(':')
    file_name, line_nr = file_parts[0], int(file_parts[1])
    replace(file_name, line_nr)
```

This approach is conservative and may leave behind some more
complicated uses which will be fixed manually later.

[1] https://source.chromium.org/chromium/chromium/src/+/main:v8/include/cppgc/member.h;l=454&ss%3Dchromium

Bug: chromium:1492410
Change-Id: If78d43b6ef99b810844fb78f71d9c3b73a36b34c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4942151
Reviewed-by: Steinar H Gunderson <sesse@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1210150}
  • Loading branch information
mlippautz authored and Chromium LUCI CQ committed Oct 16, 2023
1 parent be0573e commit 076876b
Show file tree
Hide file tree
Showing 22 changed files with 34 additions and 22 deletions.
4 changes: 3 additions & 1 deletion third_party/blink/renderer/bindings/core/v8/async_iterable.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ class CORE_EXPORT AsyncIterationSourceBase
// [1] https://webidl.spec.whatwg.org/#dfn-get-the-next-iteration-result
virtual void GetNextIterationResult() = 0;

bool HasPendingPromise() const { return pending_promise_resolver_; }
bool HasPendingPromise() const {
return pending_promise_resolver_ != nullptr;
}

// Returns the pending promise resolver by removing it from this instance.
ScriptPromiseResolver* TakePendingPromiseResolver() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class CORE_EXPORT JSEventHandler : public JSBasedEventListener {
void SetCompiledHandler(ScriptState* incumbent_script_state,
v8::Local<v8::Function> listener);

bool HasCompiledHandler() const { return event_handler_; }
bool HasCompiledHandler() const { return event_handler_ != nullptr; }

// For checking special types of EventHandler.
bool IsOnErrorEventHandler() const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ class CORE_EXPORT StringKeyframe : public Keyframe {
// Two shorthands with overlapping longhand properties are sorted based
// on the number of longhand properties in their expansions.
bool IsLogical() { return is_logical_; }
bool IsShorthand() { return css_property_value_set_; }
bool IsShorthand() { return css_property_value_set_ != nullptr; }
unsigned ExpansionCount() {
return css_property_value_set_ ? css_property_value_set_->PropertyCount()
: 1;
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/css/font_face.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class CORE_EXPORT FontFace : public ScriptWrappable,
}
FontMetricsOverride GetFontMetricsOverride() const;

bool HasSizeAdjust() const { return size_adjust_; }
bool HasSizeAdjust() const { return size_adjust_ != nullptr; }
float GetSizeAdjust() const;

Document* GetDocument() const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class CORE_EXPORT CSSParserContext final
void Count(WebFeature) const;
void Count(CSSParserMode, CSSPropertyID) const;
void CountDeprecation(WebFeature) const;
bool IsUseCounterRecordingEnabled() const { return document_; }
bool IsUseCounterRecordingEnabled() const { return document_ != nullptr; }
bool IsDocumentHandleEqual(const Document* other) const;
const Document* GetDocument() const;
const ExecutionContext* GetExecutionContext() const;
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/css/style_scope.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class CORE_EXPORT StyleScope final : public GarbageCollected<StyleScope> {
StyleRule* RuleForNesting() const { return from_; }

// https://drafts.csswg.org/css-cascade-6/#implicit-scope
bool IsImplicit() const { return contents_; }
bool IsImplicit() const { return contents_ != nullptr; }

// True if this StyleScope has an implicit root at the specified element.
// This is used to find the roots for prelude-less @scope rules.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ class CORE_EXPORT DisplayLockContext final
is_details_slot_ = is_details_slot;
}

bool HasElement() const { return element_; }
bool HasElement() const { return element_ != nullptr; }

// Top layer implementation.
void NotifyHasTopLayerElement();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ class LazyLoadFrameObserver final
~LazyLoadFrameObserver();

void DeferLoadUntilNearViewport(const ResourceRequestHead&, WebFrameLoadType);
bool IsLazyLoadPending() const { return lazy_load_intersection_observer_; }
bool IsLazyLoadPending() const {
return lazy_load_intersection_observer_ != nullptr;
}
void CancelPendingLazyLoad();

void LoadImmediately();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class PortalContents : public GarbageCollected<PortalContents>,
bool IsValid() const { return remote_portal_.is_bound(); }

// Returns true if this contents is currently being activated.
bool IsActivating() const { return activation_delegate_; }
bool IsActivating() const { return activation_delegate_ != nullptr; }

// Returns an unguessable token which uniquely identifies the contents, if
// valid.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class CORE_EXPORT LayoutImageResource

void SetImageResource(ImageResourceContent*);
ImageResourceContent* CachedImage() const { return cached_image_.Get(); }
virtual bool HasImage() const { return cached_image_; }
virtual bool HasImage() const { return cached_image_ != nullptr; }
ResourcePriority ComputeResourcePriority() const;

void ResetAnimation();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class CORE_EXPORT UnpositionedListMarker final {
explicit UnpositionedListMarker(LayoutOutsideListMarker*);
explicit UnpositionedListMarker(const NGBlockNode&);

explicit operator bool() const { return marker_layout_object_; }
explicit operator bool() const { return marker_layout_object_ != nullptr; }

// Returns the baseline that the list-marker should place itself along.
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ struct CORE_EXPORT NGLayoutOpportunity final {

// Returns if the opportunity has any shapes which may affect a line layout
// opportunity.
bool HasShapeExclusions() const { return shape_exclusions; }
bool HasShapeExclusions() const { return shape_exclusions != nullptr; }

// Returns if the given delta (relative to the start of the opportunity) will
// be below any shapes.
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/layout/scroll_anchor.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class CORE_EXPORT ScrollAnchor final {
void SetScroller(ScrollableArea*);

// Returns true if the underlying scroller is set.
bool HasScroller() const { return scroller_; }
bool HasScroller() const { return scroller_ != nullptr; }

// The LayoutObject we are currently anchored to. Lazily computed during
// notifyBeforeLayout() and cached until the next call to clear().
Expand Down
4 changes: 3 additions & 1 deletion third_party/blink/renderer/core/loader/document_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,9 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected<DocumentLoader>,
// same number of time than BlockParser().
void BlockParser() override;
void ResumeParser() override;
bool HasBeenLoadedAsWebArchive() const override { return archive_; }
bool HasBeenLoadedAsWebArchive() const override {
return archive_ != nullptr;
}
WebArchiveInfo GetArchiveInfo() const override;
bool LastNavigationHadTransientUserActivation() const override {
return last_navigation_had_transient_user_activation_;
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/script/pending_script.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class CORE_EXPORT PendingScript : public GarbageCollected<PendingScript>,
return false;
}

bool IsWatchingForLoad() const { return client_; }
bool IsWatchingForLoad() const { return client_ != nullptr; }

protected:
PendingScript(ScriptElementBase*,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ class XMLParserScriptRunner final
XMLParserScriptRunner& operator=(const XMLParserScriptRunner&) = delete;
~XMLParserScriptRunner() override;

bool HasParserBlockingScript() const { return parser_blocking_script_; }
bool HasParserBlockingScript() const {
return parser_blocking_script_ != nullptr;
}

void ProcessScriptElement(Document&, Element*, TextPosition);
void Detach();
Expand Down
4 changes: 3 additions & 1 deletion third_party/blink/renderer/core/streams/readable_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,9 @@ class CORE_EXPORT ReadableStream : public ScriptWrappable {
}

// https://streams.spec.whatwg.org/#is-readable-stream-locked
static bool IsLocked(const ReadableStream* stream) { return stream->reader_; }
static bool IsLocked(const ReadableStream* stream) {
return stream->reader_ != nullptr;
}

// https://streams.spec.whatwg.org/#readable-stream-pipe-to
static ScriptPromise PipeTo(ScriptState*,
Expand Down
4 changes: 3 additions & 1 deletion third_party/blink/renderer/core/streams/writable_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ class CORE_EXPORT WritableStream : public ScriptWrappable {

// Inherited methods used internally.

static bool IsLocked(const WritableStream* stream) { return stream->writer_; }
static bool IsLocked(const WritableStream* stream) {
return stream->writer_ != nullptr;
}

void Serialize(ScriptState*, MessagePort*, ExceptionState&);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class MODULES_EXPORT AnimatorDefinition final
V8AnimatorConstructor* ConstructorFunction() const { return constructor_; }
V8AnimateCallback* AnimateFunction() const { return animate_; }
V8StateCallback* StateFunction() const { return state_; }
bool IsStateful() const { return state_; }
bool IsStateful() const { return state_ != nullptr; }

private:
// This object keeps the constructor function, animate, and state function
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/modules/serial/serial_port.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class SerialPort final : public EventTarget,
ScriptPromise ContinueClose(ScriptState*);
void AbortClose();
void StreamsClosed();
bool IsClosing() const { return close_resolver_; }
bool IsClosing() const { return close_resolver_ != nullptr; }

void Flush(device::mojom::blink::SerialPortFlushMode mode,
device::mojom::blink::SerialPort::FlushCallback callback);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class WebGLContextObject : public WebGLObject {
protected:
explicit WebGLContextObject(WebGLRenderingContextBase*);

bool HasGroupOrContext() const final { return context_; }
bool HasGroupOrContext() const final { return context_ != nullptr; }

uint32_t CurrentNumberOfContextLosses() const final;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class WebGLSharedObject : public WebGLObject {
protected:
explicit WebGLSharedObject(WebGLRenderingContextBase*);

bool HasGroupOrContext() const final { return context_group_; }
bool HasGroupOrContext() const final { return context_group_ != nullptr; }

uint32_t CurrentNumberOfContextLosses() const final;

Expand Down

0 comments on commit 076876b

Please sign in to comment.