New issue
Advanced search Search tips

Issue 604860 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner: ----
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 521269



Sign in to add a comment

Replace all the Maybe/Nullable/Optional things with the one true base::Optional

Project Member Reported by danakj@chromium.org, Apr 19 2016

Issue description

No need to have multiple types that do this anymore.
 

Comment 1 by danakj@chromium.org, Apr 19 2016

Blockedon: 521269

Comment 2 by danakj@chromium.org, Apr 19 2016

Things that can be merged include:

WTF/Optional.h
headless/public/util/maybe.h
base/strings/nullable_string16.h

maybe the v8 Nullable class also, though Oilpan considerations are required.

Comment 3 by a...@chromium.org, Apr 19 2016

headless/public/util/maybe.h refers to the "V8 Maybe", so that's perhaps another one on the list.

Comment 4 by a...@chromium.org, Apr 19 2016

And more...

third_party/WebKit/Source/platform/inspector_protocol/Maybe.h <-- shouldn't that be using WTF::Optional?

Comment 5 by danakj@chromium.org, Apr 19 2016

Thanks for more pointers, the latter definitely looks like it should.
Cc: skyos...@chromium.org mlamouri@chromium.org
The generated extensions API types use std::unique_ptr for optional fields.

example: https://code.google.com/p/chromium/codesearch#chromium/src/out/Debug/gen/extensions/common/api/audio.h&l=177
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a17ea4bbbeb80ed2091af19d5e90aa40e5e6f9c

commit 7a17ea4bbbeb80ed2091af19d5e90aa40e5e6f9c
Author: petrcermak <petrcermak@chromium.org>
Date: Thu Apr 21 09:58:35 2016

Revert of Replace WTF::Optional the base::Optional implementation. (patchset #9 id:160001 of https://codereview.chromium.org/1897863006/ )

Reason for revert:
This patch broke Android arm64 Builder on chromium.perf: https://bugs.chromium.org/p/chromium/issues/detail?id=605478

Original issue's description:
> Replace WTF::Optional the base::Optional implementation.
>
> R=jbroman, thakis@chromium.org
> BUG= 604860 

TBR=jbroman@chromium.org,esprehn@chromium.org,thakis@chromium.org,danakj@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 604860 

Review URL: https://codereview.chromium.org/1908903002

Cr-Commit-Position: refs/heads/master@{#388730}

[modify] https://crrev.com/7a17ea4bbbeb80ed2091af19d5e90aa40e5e6f9c/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.h
[modify] https://crrev.com/7a17ea4bbbeb80ed2091af19d5e90aa40e5e6f9c/third_party/WebKit/Source/wtf/DEPS
[modify] https://crrev.com/7a17ea4bbbeb80ed2091af19d5e90aa40e5e6f9c/third_party/WebKit/Source/wtf/Optional.h

Project Member

Comment 13 by sheriffbot@chromium.org, May 1 2017

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been available for more than 365 days, and should be re-evaluated. Please re-triage this issue.
The Hotlist-Recharge-Cold label is applied for tracking purposes, and should not be removed after re-triaging the issue.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 14 by bugdroid1@chromium.org, May 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/86419ce069bfc9718a8b374f192cdb28ecd6f414

commit 86419ce069bfc9718a8b374f192cdb28ecd6f414
Author: Sam McNally <sammc@chromium.org>
Date: Tue May 23 03:24:34 2017

Change NullableString16 to wrap a base::Optional<base::string16>.

This allows the existing mojo traits for string16 to be reused for
NullableString16.

Bug: 577685, 604860 
Change-Id: I69a7be9e14c033ca2c7ab92e7cc0078eaa8a391a
Reviewed-on: https://chromium-review.googlesource.com/509669
Commit-Queue: Sam McNally <sammc@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#473800}
[modify] https://crrev.com/86419ce069bfc9718a8b374f192cdb28ecd6f414/base/strings/nullable_string16.cc
[modify] https://crrev.com/86419ce069bfc9718a8b374f192cdb28ecd6f414/base/strings/nullable_string16.h

Status: Fixed (was: Untriaged)
Marking this as it looks like some of the types have been removed (like in headless/ or are based on base::Optional now). Some code use some mechanism that could be replaced in favour of base::Optional but I don't think we want a bug to go hunt for all these places.

Sign in to add a comment