Clipboard buildfix: USE_AURA without TOOLKIT_VIEWS failed to compile.

Clipboard buildfix: USE_AURA without TOOLKIT_VIEWS failed to compile.

See: https://codereview.chromium.org/685043002/
R=dcheng@chromium.org,andresantoso@chromium.org
BUG=None

Committed: https://crrev.com/726be5f93e1bb7334368b52bd7f5a977391a89b3
Cr-Commit-Position: refs/heads/master@{#301961}

[dcheng, 2014-10-29 20:34:10]

Is this broken on the main waterfall, or somewhere else?

[gunsch, 2014-10-29 20:39:57]

On 2014/10/29 20:34:10, dcheng wrote:
> Is this broken on the main waterfall, or somewhere else?

The Chromecast build is broken. We are still trying to get a trybot. See last
comment on https://codereview.chromium.org/685043002/ for error.

It can be built with:

GYP_GENERATOR_FLAGS="output_dir=out_cast" build/gyp_chromium -Dchromecast=1 &&
ninja -C out_cast/Debug cast_shell -k500

[dcheng, 2014-10-29 20:41:25]

I think the right fix is to guard the operator< definition with the same
#define, no?

[gunsch, 2014-10-29 20:48:20]

On 2014/10/29 20:41:25, dcheng wrote:
> I think the right fix is to guard the operator< definition with the same
> #define, no?

That doesn't fix the problem: other Aura code still uses it. Compile failure
with that change:

In file included from
../../ui/base/dragdrop/os_exchange_data_provider_aura.cc:5:
In file included from ../../ui/base/dragdrop/os_exchange_data_provider_aura.h:8:
In file included from
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/map:60:
In file included from
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_tree.h:63:
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_function.h:235:20:
error: invalid operands to binary expression ('const ui::Clipboard::FormatType'
and 'const ui::Clipboard::FormatType')
      { return __x < __y; }
               ~~~ ^ ~~~
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_map.h:463:22:
note: in instantiation of member function
'std::less<ui::Clipboard::FormatType>::operator()' requested here
        if (__i == end() || key_comp()(__k, (*__i).first))
                            ^
../../ui/base/dragdrop/os_exchange_data_provider_aura.cc:78:15: note: in
instantiation of member function 'std::__cxx1998::map<ui::Clipboard::FormatType,
Pickle, std::less<ui::Clipboard::FormatType>, std::allocator<std::pair<const
ui::Clipboard::FormatType, Pickle> > >::operator[]' requested here
  pickle_data_[format] = data;
              ^
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_pair.h:220:5:
note: candidate template ignored: could not match 'pair<type-parameter-0-0,
type-parameter-0-1>' against 'const ui::Clipboard::FormatType'
    operator<(const pair<_T1, _T2>& __x, const pair<_T1, _T2>& __y)
    ^
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_iterator.h:297:5:
note: candidate template ignored: could not match
'reverse_iterator<type-parameter-0-0>' against 'const ui::Clipboard::FormatType'
    operator<(const reverse_iterator<_Iterator>& __x,
    ^
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_iterator.h:347:5:
note: candidate template ignored: could not match
'reverse_iterator<type-parameter-0-0>' against 'const ui::Clipboard::FormatType'
    operator<(const reverse_iterator<_IteratorL>& __x,
    ^
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_iterator.h:1055:5:
note: candidate template ignored: could not match
'move_iterator<type-parameter-0-0>' against 'const ui::Clipboard::FormatType'
    operator<(const move_iterator<_IteratorL>& __x,
    ^
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_iterator.h:1061:5:
note: candidate template ignored: could not match
'move_iterator<type-parameter-0-0>' against 'const ui::Clipboard::FormatType'
    operator<(const move_iterator<_Iterator>& __x,
    ^
1 error generated.

I think the operator is still a valid thing to use, in this case to store in a
map.

[Andre, 2014-10-29 20:57:40]

On 2014/10/29 20:48:20, gunsch wrote:
> On 2014/10/29 20:41:25, dcheng wrote:
> > I think the right fix is to guard the operator< definition with the same
> > #define, no?
> 
> That doesn't fix the problem: other Aura code still uses it. Compile failure
> with that change:
> 
> In file included from
> ../../ui/base/dragdrop/os_exchange_data_provider_aura.cc:5:
> In file included from
../../ui/base/dragdrop/os_exchange_data_provider_aura.h:8:
> In file included from
> /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/map:60:
> In file included from
>
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_tree.h:63:
>
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_function.h:235:20:
> error: invalid operands to binary expression ('const
ui::Clipboard::FormatType'
> and 'const ui::Clipboard::FormatType')
>       { return __x < __y; }
>                ~~~ ^ ~~~
>
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_map.h:463:22:
> note: in instantiation of member function
> 'std::less<ui::Clipboard::FormatType>::operator()' requested here
>         if (__i == end() || key_comp()(__k, (*__i).first))
>                             ^
> ../../ui/base/dragdrop/os_exchange_data_provider_aura.cc:78:15: note: in
> instantiation of member function
'std::__cxx1998::map<ui::Clipboard::FormatType,
> Pickle, std::less<ui::Clipboard::FormatType>, std::allocator<std::pair<const
> ui::Clipboard::FormatType, Pickle> > >::operator[]' requested here
>   pickle_data_[format] = data;
>               ^
>
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_pair.h:220:5:
> note: candidate template ignored: could not match 'pair<type-parameter-0-0,
> type-parameter-0-1>' against 'const ui::Clipboard::FormatType'
>     operator<(const pair<_T1, _T2>& __x, const pair<_T1, _T2>& __y)
>     ^
>
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_iterator.h:297:5:
> note: candidate template ignored: could not match
> 'reverse_iterator<type-parameter-0-0>' against 'const
ui::Clipboard::FormatType'
>     operator<(const reverse_iterator<_Iterator>& __x,
>     ^
>
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_iterator.h:347:5:
> note: candidate template ignored: could not match
> 'reverse_iterator<type-parameter-0-0>' against 'const
ui::Clipboard::FormatType'
>     operator<(const reverse_iterator<_IteratorL>& __x,
>     ^
>
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_iterator.h:1055:5:
> note: candidate template ignored: could not match
> 'move_iterator<type-parameter-0-0>' against 'const ui::Clipboard::FormatType'
>     operator<(const move_iterator<_IteratorL>& __x,
>     ^
>
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_iterator.h:1061:5:
> note: candidate template ignored: could not match
> 'move_iterator<type-parameter-0-0>' against 'const ui::Clipboard::FormatType'
>     operator<(const move_iterator<_Iterator>& __x,
>     ^
> 1 error generated.
> 
> I think the operator is still a valid thing to use, in this case to store in a
> map.


I think using !defined(OS_ANDROID) in the declaration is simplest and matches
the implementations.
dcheng@, you still don't like that?

[gunsch, 2014-10-29 21:00:11]

I don't know that I like !OS_ANDROID either---it has the implicit assumption
that "!OS_ANDROID == TOOLKIT_VIEWS || USE_AURA", which I don't know is always
true. Why make the assumption?

[dcheng, 2014-10-29 21:00:24]

On 2014/10/29 at 20:57:40, andresantoso wrote:
> On 2014/10/29 20:48:20, gunsch wrote:
> > On 2014/10/29 20:41:25, dcheng wrote:
> > > I think the right fix is to guard the operator< definition with the same
> > > #define, no?
> > 
> > That doesn't fix the problem: other Aura code still uses it. Compile failure
> > with that change:
> > 
> > In file included from
> > ../../ui/base/dragdrop/os_exchange_data_provider_aura.cc:5:
> > In file included from
../../ui/base/dragdrop/os_exchange_data_provider_aura.h:8:
> > In file included from
> > /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/map:60:
> > In file included from
> >
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_tree.h:63:
> >
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_function.h:235:20:
> > error: invalid operands to binary expression ('const
ui::Clipboard::FormatType'
> > and 'const ui::Clipboard::FormatType')
> >       { return __x < __y; }
> >                ~~~ ^ ~~~
> >
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_map.h:463:22:
> > note: in instantiation of member function
> > 'std::less<ui::Clipboard::FormatType>::operator()' requested here
> >         if (__i == end() || key_comp()(__k, (*__i).first))
> >                             ^
> > ../../ui/base/dragdrop/os_exchange_data_provider_aura.cc:78:15: note: in
> > instantiation of member function
'std::__cxx1998::map<ui::Clipboard::FormatType,
> > Pickle, std::less<ui::Clipboard::FormatType>, std::allocator<std::pair<const
> > ui::Clipboard::FormatType, Pickle> > >::operator[]' requested here
> >   pickle_data_[format] = data;
> >               ^
> >
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_pair.h:220:5:
> > note: candidate template ignored: could not match 'pair<type-parameter-0-0,
> > type-parameter-0-1>' against 'const ui::Clipboard::FormatType'
> >     operator<(const pair<_T1, _T2>& __x, const pair<_T1, _T2>& __y)
> >     ^
> >
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_iterator.h:297:5:
> > note: candidate template ignored: could not match
> > 'reverse_iterator<type-parameter-0-0>' against 'const
ui::Clipboard::FormatType'
> >     operator<(const reverse_iterator<_Iterator>& __x,
> >     ^
> >
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_iterator.h:347:5:
> > note: candidate template ignored: could not match
> > 'reverse_iterator<type-parameter-0-0>' against 'const
ui::Clipboard::FormatType'
> >     operator<(const reverse_iterator<_IteratorL>& __x,
> >     ^
> >
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_iterator.h:1055:5:
> > note: candidate template ignored: could not match
> > 'move_iterator<type-parameter-0-0>' against 'const
ui::Clipboard::FormatType'
> >     operator<(const move_iterator<_IteratorL>& __x,
> >     ^
> >
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_iterator.h:1061:5:
> > note: candidate template ignored: could not match
> > 'move_iterator<type-parameter-0-0>' against 'const
ui::Clipboard::FormatType'
> >     operator<(const move_iterator<_Iterator>& __x,
> >     ^
> > 1 error generated.
> > 
> > I think the operator is still a valid thing to use, in this case to store in
a
> > map.
> 
> 
> I think using !defined(OS_ANDROID) in the declaration is simplest and matches
the implementations.
> dcheng@, you still don't like that?

Yeah let's just do that. I was hoping to make it clear what conditions this
operator was actually needed, but that seems more trouble than it's worth.

[Andre, 2014-10-29 21:01:21]

On 2014/10/29 21:00:11, gunsch wrote:
> I don't know that I like !OS_ANDROID either---it has the implicit assumption
> that "!OS_ANDROID == TOOLKIT_VIEWS || USE_AURA", which I don't know is always
> true. Why make the assumption?

Because clipboard_android.cc does not implement it, while all the other
clipboard implementations do.

[dcheng, 2014-10-29 21:01:28]

On 2014/10/29 at 21:00:11, gunsch wrote:
> I don't know that I like !OS_ANDROID either---it has the implicit assumption
that "!OS_ANDROID == TOOLKIT_VIEWS || USE_AURA", which I don't know is always
true. Why make the assumption?

Like Andre pointed out, Android is the only platform that doesn't define this
operator.

[Andre, 2014-10-29 21:11:22]

On 2014/10/29 21:01:28, dcheng wrote:
> On 2014/10/29 at 21:00:11, gunsch wrote:
> > I don't know that I like !OS_ANDROID either---it has the implicit assumption
> that "!OS_ANDROID == TOOLKIT_VIEWS || USE_AURA", which I don't know is always
> true. Why make the assumption?
> 
> Like Andre pointed out, Android is the only platform that doesn't define this
> operator.

Don't forget to remove the #if defined(TOOLKIT_VIEWS) from clipboard_mac if you
go this route (using !ANDROID).

[gunsch, 2014-10-29 21:18:05]

PTAL

[dcheng, 2014-10-29 21:18:36]

lgtm

[gunsch, 2014-10-29 21:20:05]

The CQ bit was checked by gunsch@chromium.org

[commit-bot: I haz the power, 2014-10-29 21:22:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/687293003/20001

[commit-bot: I haz the power, 2014-10-29 22:58:25]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-10-29 22:59:18]

Patchset 2 (id:??) landed as
https://crrev.com/726be5f93e1bb7334368b52bd7f5a977391a89b3
Cr-Commit-Position: refs/heads/master@{#301961}