Replace WTF::Optional the base::Optional implementation.

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}

Committed: https://crrev.com/f337675ea340b9d2070e773ec4c14a8e179c78ab
Cr-Commit-Position: refs/heads/master@{#390594}

[danakj, 2016-04-19 20:15:09]

While blink-dev figures out using DEPS in more places, we can make this a type
alias.

[danakj, 2016-04-19 20:15:12]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-19 20:15:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1897863006/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1897863006/1

[jbroman, 2016-04-19 20:19:17]

I have no objection here (other than I'm not sure aliases like this add
long-term value, but here it's presumably a drop-in replacement for the existing
WTF::Optional), but you might want an opinion from someone from
platform-architecture-dev.

https://codereview.chromium.org/1897863006/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/wtf/Optional.h (right):

https://codereview.chromium.org/1897863006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/wtf/Optional.h:15: constexpr base::nullopt_t
nullopt(0);
nit: I'd prefer " = base::nullopt" (and similar below) to avoid exposing
internals here.

[danakj, 2016-04-19 20:28:48]

danakj@chromium.org changed reviewers:
+ esprehn@chromium.org

[danakj, 2016-04-19 20:28:49]

> I have no objection here (other than I'm not sure aliases like this add
> long-term value, but here it's presumably a drop-in replacement for the
existing
> WTF::Optional), but you might want an opinion from someone from
> platform-architecture-dev.

Yeah, I'm not sold on aliases over DEPS rules, but so far we've been wrapping
base stuff somewhere in wtf or platform with typedefs or wrapper classes when we
want to expose them to blink.

+esprehn from platform-architecture-dev

https://codereview.chromium.org/1897863006/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/wtf/Optional.h (right):

https://codereview.chromium.org/1897863006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/wtf/Optional.h:15: constexpr base::nullopt_t
nullopt(0);
On 2016/04/19 20:19:17, jbroman wrote:
> nit: I'd prefer " = base::nullopt" (and similar below) to avoid exposing
> internals here.

Oh, yes good idea, I didn't like this either. Done.

[danakj, 2016-04-19 20:29:15]

(it's also possible this should move to platform, though that's somewhat
orthogonal)

[esprehn, 2016-04-19 21:10:38]

I actually don't think we want to do this, the WTF one can be a subclass I
guess? I want emplace() to fail to compile if you use a GC type.

[danakj, 2016-04-19 21:20:25]

On Tue, Apr 19, 2016 at 2:10 PM, <esprehn@chromium.org> wrote:

> I actually don't think we want to do this, the WTF one can be a subclass I
> guess? I want emplace() to fail to compile if you use a GC type.
>

I don't see what you're referring to in the current implementation, can you
explain? Optional<T> has the exact same behaviour/characteristics as making
a T on the stack, there's no pointer allocation to look at.

The class isn't virtual so we can't override emplace.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[danakj, 2016-04-19 21:20:26]

On Tue, Apr 19, 2016 at 2:10 PM, <esprehn@chromium.org> wrote:

> I actually don't think we want to do this, the WTF one can be a subclass I
> guess? I want emplace() to fail to compile if you use a GC type.
>

I don't see what you're referring to in the current implementation, can you
explain? Optional<T> has the exact same behaviour/characteristics as making
a T on the stack, there's no pointer allocation to look at.

The class isn't virtual so we can't override emplace.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[esprehn, 2016-04-19 22:26:53]

I'm suggesting this:


template<T>
class Optional : public base::Optional<T> {
  static_assert(!IsGarbageCollectedType<T>::value);
  static_assert(!IsRefCountedType<T>::value);
}

That's actually one of the primary reasons for requiring these wrapper headers,
so we can adapt the types to WTF and Oilpan.

[danakj, 2016-04-19 23:04:51]

On Tue, Apr 19, 2016 at 3:26 PM, <esprehn@chromium.org> wrote:

> I'm suggesting this:
>
>
> template<T>
> class Optional : public base::Optional<T> {
> static_assert(!IsGarbageCollectedType<T>::value);
> static_assert(!IsRefCountedType<T>::value);
> }
>

Ah, I see. Optional does not have a virtual destructor though, and we don't
want to diverge from the one that will appear in std. So I don't think this
works.


> That's actually one of the primary reasons for requiring these wrapper
> headers,
> so we can adapt the types to WTF and Oilpan.
>

Also, I guess I don't understand why you want static asserts for

Optional<T> t;

Where you wouldn't have them for

T t;

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[danakj, 2016-04-19 23:04:52]

On Tue, Apr 19, 2016 at 3:26 PM, <esprehn@chromium.org> wrote:

> I'm suggesting this:
>
>
> template<T>
> class Optional : public base::Optional<T> {
> static_assert(!IsGarbageCollectedType<T>::value);
> static_assert(!IsRefCountedType<T>::value);
> }
>

Ah, I see. Optional does not have a virtual destructor though, and we don't
want to diverge from the one that will appear in std. So I don't think this
works.


> That's actually one of the primary reasons for requiring these wrapper
> headers,
> so we can adapt the types to WTF and Oilpan.
>

Also, I guess I don't understand why you want static asserts for

Optional<T> t;

Where you wouldn't have them for

T t;

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2016-04-19 23:25:33]

On 2016/04/19 23:04:52, danakj wrote:
> On Tue, Apr 19, 2016 at 3:26 PM, <mailto:esprehn@chromium.org> wrote:
> 
> > I'm suggesting this:
> >
> >
> > template<T>
> > class Optional : public base::Optional<T> {
> > static_assert(!IsGarbageCollectedType<T>::value);
> > static_assert(!IsRefCountedType<T>::value);
> > }
> >
> 
> Ah, I see. Optional does not have a virtual destructor though, and we don't
> want to diverge from the one that will appear in std. So I don't think this
> works.

Oh, I'm thinking of this backward, I guess it's possible, the WTF destructor
would call base's and the base one doesn't need to know the WTF one.

> 
> > That's actually one of the primary reasons for requiring these wrapper
> > headers,
> > so we can adapt the types to WTF and Oilpan.
> >
> 
> Also, I guess I don't understand why you want static asserts for
> 
> Optional<T> t;
> 
> Where you wouldn't have them for
> 
> T t;

But this, it's only kinda pretending to solve some larger thing in a few cases?

[jbroman, 2016-04-19 23:28:37]

On 2016/04/19 at 23:25:33, danakj wrote:
> On 2016/04/19 23:04:52, danakj wrote:
> > On Tue, Apr 19, 2016 at 3:26 PM, <mailto:esprehn@chromium.org> wrote:
> > 
> > > I'm suggesting this:
> > >
> > >
> > > template<T>
> > > class Optional : public base::Optional<T> {
> > > static_assert(!IsGarbageCollectedType<T>::value);
> > > static_assert(!IsRefCountedType<T>::value);
> > > }
> > >
> > 
> > Ah, I see. Optional does not have a virtual destructor though, and we don't
> > want to diverge from the one that will appear in std. So I don't think this
> > works.
> 
> Oh, I'm thinking of this backward, I guess it's possible, the WTF destructor
would call base's and the base one doesn't need to know the WTF one.
> 
> > 
> > > That's actually one of the primary reasons for requiring these wrapper
> > > headers,
> > > so we can adapt the types to WTF and Oilpan.
> > >
> > 
> > Also, I guess I don't understand why you want static asserts for
> > 
> > Optional<T> t;
> > 
> > Where you wouldn't have them for
> > 
> > T t;
> 
> But this, it's only kinda pretending to solve some larger thing in a few
cases?

Shouldn't the constructor be non-public in these cases anyways, in which case
Optional::emplace won't compile? It seems like you have as much right to
construct an Optional<T> on the stack as a T, so a solution that breaks both of
them seems better.

[danakj, 2016-04-19 23:36:15]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-19 23:36:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1897863006/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1897863006/80001

[esprehn, 2016-04-19 23:45:43]

The clang plugin enforces you can't allocate it on the stack, it doesn't
understand every random container and pattern though. RefCounted is handled by
the adoptRef pattern which also fails if you do something bad.

[danakj, 2016-04-20 00:05:08]

On 2016/04/19 22:26:53, esprehn wrote:
> I'm suggesting this:
> 
> 
> template<T>
> class Optional : public base::Optional<T> {
>   static_assert(!IsGarbageCollectedType<T>::value);

IsGarbageCollectedType is in platform/

>   static_assert(!IsRefCountedType<T>::value);
> }
> 
> That's actually one of the primary reasons for requiring these wrapper
headers,
> so we can adapt the types to WTF and Oilpan.

[danakj, 2016-04-20 00:05:33]

On 2016/04/20 00:05:08, danakj wrote:
> On 2016/04/19 22:26:53, esprehn wrote:
> > I'm suggesting this:
> > 
> > 
> > template<T>
> > class Optional : public base::Optional<T> {
> >   static_assert(!IsGarbageCollectedType<T>::value);
> 
> IsGarbageCollectedType is in platform/

oh.. there is one in wtf too? ok.

> 
> >   static_assert(!IsRefCountedType<T>::value);
> > }
> > 
> > That's actually one of the primary reasons for requiring these wrapper
> headers,
> > so we can adapt the types to WTF and Oilpan.

[danakj, 2016-04-20 00:12:06]

PTAL to match clang-plugin behaviours. We should probably just enforce that
constructors are not public, rather than enforce every allocation of the
objects.

I didn't check refcounted, as that's not something we enforce with a plugin and
is pretty weird to enforce here in general.

[danakj, 2016-04-20 00:12:23]

Description was changed from

==========
Replace WTF::Optional with a type alias to base::Optional.

R=jbroman, thakis@chromium.org
BUG=604860
==========

to

==========
Replace WTF::Optional the base::Optional implementation.

R=jbroman, thakis@chromium.org
BUG=604860
==========

[esprehn, 2016-04-20 00:28:03]

https://codereview.chromium.org/1897863006/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.h
(right):

https://codereview.chromium.org/1897863006/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.h:131:
WTF_MAKE_NONCOPYABLE(ImageExtractor);
why this patch?

[danakj, 2016-04-20 00:37:21]

https://codereview.chromium.org/1897863006/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.h
(right):

https://codereview.chromium.org/1897863006/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.h:131:
WTF_MAKE_NONCOPYABLE(ImageExtractor);
On 2016/04/20 00:28:03, esprehn wrote:
> why this patch?

Windows debug generates a copy constructor for ImageExtractor even though it is
not used. Optional's copy constructor will copy ImagePixelLocker, who's copy
constructor is deleted, causing a compile failure.

You couldn't use the copy constructor anyways, for the same reasons, but this
prevents Windows debug from trying to generate it.

[danakj, 2016-04-20 20:34:50]

ping

[jbroman, 2016-04-20 21:13:24]

https://codereview.chromium.org/1897863006/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/wtf/Optional.h (right):

https://codereview.chromium.org/1897863006/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/wtf/Optional.h:16:
static_assert(!IsGarbageCollectedType<T>::value, "Garbage collected types should
not be stack-allocated.");
FYI, doing things this way will cause most of the constructors to go away. I
think "using base::Optional::Optional;" fixes it, but I'm not 100% sure this is
complete. There are a few other awkward hazards, like operator= returning the
base type instead of the derived type.

Another approach might be something like this (warning: untested code):

template <typename T, typename =
std::enable_if<!IsGarbageCollectedType<T>::value>::type>
using Optional = base::Optional<T>;

This should have a similar effect, but should make the type alias, washing a
number of these issues away.

[danakj, 2016-04-20 21:33:22]

PTAL

https://codereview.chromium.org/1897863006/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/wtf/Optional.h (right):

https://codereview.chromium.org/1897863006/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/wtf/Optional.h:16:
static_assert(!IsGarbageCollectedType<T>::value, "Garbage collected types should
not be stack-allocated.");
On 2016/04/20 21:13:24, jbroman wrote:
> FYI, doing things this way will cause most of the constructors to go away. I
> think "using base::Optional::Optional;" fixes it, but I'm not 100% sure this
is
> complete. There are a few other awkward hazards, like operator= returning the
> base type instead of the derived type.
> 
> Another approach might be something like this (warning: untested code):
> 
> template <typename T, typename =
> std::enable_if<!IsGarbageCollectedType<T>::value>::type>
> using Optional = base::Optional<T>;
> 
> This should have a similar effect, but should make the type alias, washing a
> number of these issues away.

Yes! That's what I really want, thank you for the great idea. Confirmed that it
works.

[esprehn, 2016-04-20 22:04:37]

lgtm if it works.

[danakj, 2016-04-20 22:05:48]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2016-04-20 22:06:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1897863006/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1897863006/160001

[jbroman, 2016-04-20 22:17:51]

lgtm as well

[commit-bot: I haz the power, 2016-04-20 22:43:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-20 22:43:42]

Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[danakj, 2016-04-20 22:44:02]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2016-04-20 22:44:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1897863006/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1897863006/160001

[commit-bot: I haz the power, 2016-04-21 00:20:16]

Description was changed from

==========
Replace WTF::Optional the base::Optional implementation.

R=jbroman, thakis@chromium.org
BUG=604860
==========

to

==========
Replace WTF::Optional the base::Optional implementation.

R=jbroman, thakis@chromium.org
BUG=604860
==========

[commit-bot: I haz the power, 2016-04-21 00:20:18]

Committed patchset #9 (id:160001)

[jbudorick, 2016-04-21 01:41:40]

On 2016/04/21 00:20:18, commit-bot: I haz the power wrote:
> Committed patchset #9 (id:160001)

This seems to be responsible for a compile failure on chromium.perf:
https://build.chromium.org/p/chromium.android/builders/Android%20arm64%20Buil...

not reverting yet as I'm not seeing it elsewhere, but will continue to
investigate

[jbudorick, 2016-04-21 01:42:40]

On 2016/04/21 01:41:40, jbudorick wrote:
> On 2016/04/21 00:20:18, commit-bot: I haz the power wrote:
> > Committed patchset #9 (id:160001)
> 
> This seems to be responsible for a compile failure on chromium.perf:
>
https://build.chromium.org/p/chromium.android/builders/Android%20arm64%20Buil...

er, correct link:
https://build.chromium.org/p/chromium.perf/builders/Android%20arm64%20Builder

> 
> not reverting yet as I'm not seeing it elsewhere, but will continue to
> investigate

[jbroman, 2016-04-21 02:17:22]

On 2016/04/21 at 01:42:40, jbudorick wrote:
> On 2016/04/21 01:41:40, jbudorick wrote:
> > On 2016/04/21 00:20:18, commit-bot: I haz the power wrote:
> > > Committed patchset #9 (id:160001)
> > 
> > This seems to be responsible for a compile failure on chromium.perf:
> >
https://build.chromium.org/p/chromium.android/builders/Android%20arm64%20Buil...
> 
> er, correct link:
https://build.chromium.org/p/chromium.perf/builders/Android%20arm64%20Builder
> 
> > 
> > not reverting yet as I'm not seeing it elsewhere, but will continue to
> > investigate

https://build.chromium.org/p/chromium.android/builders/Android%20arm64%20Buil...
is presumably the error?

Looks like -Wmaybe-uninitialized (implied by -Wall in g++, but not in clang for
presumably this reason) is imprecise and can generate spurious warning when it
doesn't realize that stuff is indeed initialized (searching reveals it does this
more often with -Os). :'(

../../third_party/WebKit/Source/wtf/TemporaryChange.h: In member function 'void
blink::FrameView::updateLifecyclePhasesInternal(blink::FrameView::LifeCycleUpdateOption)':
../../third_party/WebKit/Source/wtf/TemporaryChange.h:55:9: error: '*((void*)&
isUpdatingAllLifecyclePhasesScope +16)' may be used uninitialized in this
function [-Werror=maybe-uninitialized]
         m_scopedVariable = m_originalValue;
         ^
../../third_party/WebKit/Source/core/frame/FrameView.cpp:2404:37: note:
'*((void*)& isUpdatingAllLifecyclePhasesScope +16)' was declared here
     Optional<TemporaryChange<bool>> isUpdatingAllLifecyclePhasesScope;

My bet is that the implementation of base/optional.h is just unlucky in
triggering this; none of the code seems wrong. The old WTF::Optional indirected
through a pointer that happened to avoid this.

[jbroman, 2016-04-21 02:37:15]

On 2016/04/21 at 02:17:22, jbroman wrote:
> On 2016/04/21 at 01:42:40, jbudorick wrote:
> > On 2016/04/21 01:41:40, jbudorick wrote:
> > > On 2016/04/21 00:20:18, commit-bot: I haz the power wrote:
> > > > Committed patchset #9 (id:160001)
> > > 
> > > This seems to be responsible for a compile failure on chromium.perf:
> > >
https://build.chromium.org/p/chromium.android/builders/Android%20arm64%20Buil...
> > 
> > er, correct link:
https://build.chromium.org/p/chromium.perf/builders/Android%20arm64%20Builder
> > 
> > > 
> > > not reverting yet as I'm not seeing it elsewhere, but will continue to
> > > investigate
> 
>
https://build.chromium.org/p/chromium.android/builders/Android%20arm64%20Buil...
is presumably the error?
> 
> Looks like -Wmaybe-uninitialized (implied by -Wall in g++, but not in clang
for presumably this reason) is imprecise and can generate spurious warning when
it doesn't realize that stuff is indeed initialized (searching reveals it does
this more often with -Os). :'(
> 
> ../../third_party/WebKit/Source/wtf/TemporaryChange.h: In member function
'void
blink::FrameView::updateLifecyclePhasesInternal(blink::FrameView::LifeCycleUpdateOption)':
> ../../third_party/WebKit/Source/wtf/TemporaryChange.h:55:9: error: '*((void*)&
isUpdatingAllLifecyclePhasesScope +16)' may be used uninitialized in this
function [-Werror=maybe-uninitialized]
>          m_scopedVariable = m_originalValue;
>          ^
> ../../third_party/WebKit/Source/core/frame/FrameView.cpp:2404:37: note:
'*((void*)& isUpdatingAllLifecyclePhasesScope +16)' was declared here
>      Optional<TemporaryChange<bool>> isUpdatingAllLifecyclePhasesScope;
> 
> My bet is that the implementation of base/optional.h is just unlucky in
triggering this; none of the code seems wrong. The old WTF::Optional indirected
through a pointer that happened to avoid this.

...interestingly, LLVM seems to globally suppress this warning when building
with GCC:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/llvm/c...

[petrcermak, 2016-04-21 09:58:08]

A revert of this CL (patchset #9 id:160001) has been created in
https://codereview.chromium.org/1908903002/ by petrcermak@chromium.org.

The reason for reverting is: This patch broke Android arm64 Builder on
chromium.perf: https://bugs.chromium.org/p/chromium/issues/detail?id=605478.

[danakj, 2016-04-21 23:44:13]

Description was changed from

==========
Replace WTF::Optional the base::Optional implementation.

R=jbroman, thakis@chromium.org
BUG=604860
==========

to

==========
Replace WTF::Optional the base::Optional implementation.

R=jbroman, thakis@chromium.org
BUG=604860
==========

[danakj, 2016-04-22 00:14:20]

I can't reproduce the warning building blink with gcc locally D:

[danakj, 2016-04-22 00:22:54]

I've updated this CL to disable maybe-initialized warning for GCC, which would
match our clang build config.

I would also explore GCC pragmas or something but this is all pretty meh, and it
looks like GCC is going to have problems with us using libc++ just the same due
to false positives with this warning sooner or later.

thakis@ as a build/ owners what do you think?

[danakj, 2016-04-22 00:30:16]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-22 00:30:40]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1897863006/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1897863006/180001

[danakj, 2016-04-22 00:31:25]

The CQ bit was unchecked by danakj@chromium.org

[danakj, 2016-04-22 01:04:53]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-22 01:05:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1897863006/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1897863006/200001

[commit-bot: I haz the power, 2016-04-22 02:54:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-22 02:54:28]

Dry run: Try jobs failed on following builders:
  win_clang on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Nico, 2016-04-22 18:41:18]

build change lgtm, that makes a lot of sense to me.

[danakj, 2016-04-22 18:41:36]

The CQ bit was checked by danakj@chromium.org

[danakj, 2016-04-22 18:41:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
esprehn@chromium.org, jbroman@chromium.org
Link to the patchset: https://codereview.chromium.org/1897863006/#ps200001
(title: "wtfoptional: notonclang")

[commit-bot: I haz the power, 2016-04-22 18:42:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1897863006/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1897863006/200001

[commit-bot: I haz the power, 2016-04-22 19:28:29]

Description was changed from

==========
Replace WTF::Optional the base::Optional implementation.

R=jbroman, thakis@chromium.org
BUG=604860
==========

to

==========
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}
==========

[commit-bot: I haz the power, 2016-04-22 19:28:31]

Patchset 11 (id:??) landed as
https://crrev.com/0a079b3fb27d30609aed6eaaaf2b72423741b5e5
Cr-Commit-Position: refs/heads/master@{#388624}

[danakj, 2016-04-22 19:56:32]

On 2016/04/22 19:28:31, commit-bot: I haz the power wrote:
> Patchset 11 (id:??) landed as
> https://crrev.com/0a079b3fb27d30609aed6eaaaf2b72423741b5e5
> Cr-Commit-Position: refs/heads/master@{#388624}

I think
https://build.chromium.org/p/chromium.perf/builders/Android%20arm64%20Builder...
is happy with the change. I don't see it there but the 200th newest build is
from a newer CL # than this.

[danakj, 2016-04-29 02:26:02]

On 2016/04/22 19:28:31, commit-bot: I haz the power wrote:
> Patchset 11 (id:??) landed as
> https://crrev.com/0a079b3fb27d30609aed6eaaaf2b72423741b5e5
> Cr-Commit-Position: refs/heads/master@{#388624}

Uh, this was the original land. CQ failed to reland it.

[danakj, 2016-04-29 02:26:07]

Description was changed from

==========
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}
==========

to

==========
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}
==========

[danakj, 2016-04-29 02:26:13]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2016-04-29 02:26:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1897863006/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1897863006/200001

[danakj, 2016-04-29 02:27:49]

Filed https://bugs.chromium.org/p/chromium/issues/detail?id=607783 about the CQ.

[commit-bot: I haz the power, 2016-04-29 05:34:56]

Description was changed from

==========
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}
==========

to

==========
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}
==========

[commit-bot: I haz the power, 2016-04-29 05:34:57]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-04-30 17:24:24]

Patchset 11 (id:??) landed as
https://crrev.com/f337675ea340b9d2070e773ec4c14a8e179c78ab
Cr-Commit-Position: refs/heads/master@{#390594}

[commit-bot: I haz the power, 2016-04-30 17:59:43]

Description was changed from

==========
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}
==========

to

==========
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}

Committed: https://crrev.com/f337675ea340b9d2070e773ec4c14a8e179c78ab
Cr-Commit-Position: refs/heads/master@{#390594}
==========