Replace all the Maybe/Nullable/Optional things with the one true base::Optional |
||||
Issue descriptionNo need to have multiple types that do this anymore.
,
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.
,
Apr 19 2016
headless/public/util/maybe.h refers to the "V8 Maybe", so that's perhaps another one on the list.
,
Apr 19 2016
And more... third_party/WebKit/Source/platform/inspector_protocol/Maybe.h <-- shouldn't that be using WTF::Optional?
,
Apr 19 2016
Thanks for more pointers, the latter definitely looks like it should.
,
Apr 19 2016
,
Apr 20 2016
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
,
Apr 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b2232e6103ff4c902452d9468f543637e091f95b commit b2232e6103ff4c902452d9468f543637e091f95b Author: skyostil <skyostil@chromium.org> Date: Wed Apr 20 14:39:58 2016 headless: Replace headless::Maybe with base::Optional Covered by existing tests. BUG= 595353 , 604860 Review URL: https://codereview.chromium.org/1906493002 Cr-Commit-Position: refs/heads/master@{#388495} [modify] https://crrev.com/b2232e6103ff4c902452d9468f543637e091f95b/headless/BUILD.gn [modify] https://crrev.com/b2232e6103ff4c902452d9468f543637e091f95b/headless/lib/browser/types_cc.template [modify] https://crrev.com/b2232e6103ff4c902452d9468f543637e091f95b/headless/lib/browser/types_h.template [delete] https://crrev.com/f62311f2c8335dd85071864672d30a149c76c08b/headless/public/util/maybe.h [delete] https://crrev.com/f62311f2c8335dd85071864672d30a149c76c08b/headless/public/util/maybe_unittest.cc
,
Apr 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0a079b3fb27d30609aed6eaaaf2b72423741b5e5 commit 0a079b3fb27d30609aed6eaaaf2b72423741b5e5 Author: danakj <danakj@chromium.org> Date: Thu Apr 21 00:20:05 2016 Replace WTF::Optional the base::Optional implementation. R=jbroman, thakis@chromium.org BUG= 604860 Review URL: https://codereview.chromium.org/1897863006 Cr-Commit-Position: refs/heads/master@{#388624} [modify] https://crrev.com/0a079b3fb27d30609aed6eaaaf2b72423741b5e5/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.h [modify] https://crrev.com/0a079b3fb27d30609aed6eaaaf2b72423741b5e5/third_party/WebKit/Source/wtf/DEPS [modify] https://crrev.com/0a079b3fb27d30609aed6eaaaf2b72423741b5e5/third_party/WebKit/Source/wtf/Optional.h
,
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
,
Apr 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ab29ff6dc90b7b652261f14617e31382f70c31b5 commit ab29ff6dc90b7b652261f14617e31382f70c31b5 Author: liyuqian <liyuqian@google.com> Date: Thu Apr 28 13:46:15 2016 Replace SkTLazy with base::Optional BUG= 604860 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review-Url: https://codereview.chromium.org/1925433002 Cr-Commit-Position: refs/heads/master@{#390364} [modify] https://crrev.com/ab29ff6dc90b7b652261f14617e31382f70c31b5/cc/playback/image_hijack_canvas.cc [modify] https://crrev.com/ab29ff6dc90b7b652261f14617e31382f70c31b5/cc/playback/raster_source.cc [modify] https://crrev.com/ab29ff6dc90b7b652261f14617e31382f70c31b5/skia/ext/benchmarking_canvas.cc
,
Apr 29 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f337675ea340b9d2070e773ec4c14a8e179c78ab commit f337675ea340b9d2070e773ec4c14a8e179c78ab Author: danakj <danakj@chromium.org> Date: Fri Apr 29 05:34:46 2016 Replace WTF::Optional the base::Optional implementation. R=jbroman, thakis@chromium.org BUG= 604860 Committed: https://crrev.com/0a079b3fb27d30609aed6eaaaf2b72423741b5e5 Cr-Commit-Position: refs/heads/master@{#388624} Review-Url: https://codereview.chromium.org/1897863006 Cr-Commit-Position: refs/heads/master@{#390594} [modify] https://crrev.com/f337675ea340b9d2070e773ec4c14a8e179c78ab/build/common.gypi [modify] https://crrev.com/f337675ea340b9d2070e773ec4c14a8e179c78ab/build/config/compiler/BUILD.gn [modify] https://crrev.com/f337675ea340b9d2070e773ec4c14a8e179c78ab/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.h [modify] https://crrev.com/f337675ea340b9d2070e773ec4c14a8e179c78ab/third_party/WebKit/Source/wtf/DEPS [modify] https://crrev.com/f337675ea340b9d2070e773ec4c14a8e179c78ab/third_party/WebKit/Source/wtf/Optional.h
,
May 1 2017
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
,
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
,
May 29 2017
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 |
||||
Comment 1 by danakj@chromium.org
, Apr 19 2016