Remove ash_util namespace

Remove ash_util namespace

This CL removes the namespace for ash_util functions, following the
convention "Use namespaces based on the path, and not invent special
namespaces for a single file" [1].

Changes were automated using the following bash command:

    for file in `find -type f -name '*.cc' -o -name '*.h' | \
       xargs grep '/ash_util.h"' | sed 's/:.*//'`; do \
           sed -i 's/ash_util:://' $file; \
    done

[1] https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/namespace$20usage/chromium-dev/RVps606IB_g/ZLes1CVel40J

BUG=697053

[fwang, 2017-03-01 09:43:28]

The CQ bit was checked by fwang@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-01 09:43:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fwang, 2017-03-01 09:44:20]

Description was changed from

==========
Remove ash_util namespace

This CL removes the namespace for ash_util functions, following the
convention "Use namespaces based on the path, and not invent special
namespaces for a single file" [1].

Changes were automated using the following bash command:

    for file in `find -type f -name '*.cc' -o -name '*.h' | \
       xargs grep '/ash_util.h"' | sed 's/:.*//'`; do \
           sed -i 's/ash_util:://' $file; \
    done

[1]
https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/namesp...

BUG=697053
==========

to

==========
Remove ash_util namespace

This CL removes the namespace for ash_util functions, following the
convention "Use namespaces based on the path, and not invent special
namespaces for a single file" [1].

Changes were automated using the following bash command:

    for file in `find -type f -name '*.cc' -o -name '*.h' | \
       xargs grep '/ash_util.h"' | sed 's/:.*//'`; do \
           sed -i 's/ash_util:://' $file; \
    done

[1]
https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/namesp...

BUG=697053
==========

[commit-bot: I haz the power, 2017-03-01 10:25:08]

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

[commit-bot: I haz the power, 2017-03-01 10:25:12]

Dry run: This issue passed the CQ dry run.

[fwang, 2017-03-01 10:27:11]

fwang@igalia.com changed reviewers:
+ sky@chromium.org, stevenjb@chromium.org

[fwang, 2017-03-01 10:27:12]

PTAL

[achuithb, 2017-03-01 10:46:46]

achuith@chromium.org changed reviewers:
+ achuith@chromium.org

[achuithb, 2017-03-01 10:46:46]

Feels weird to have these in the global namespace. sky@ - how do you feel about
putting these static functions in the ash namespace?

LGTM.

https://codereview.chromium.org/2721153004/diff/1/chrome/browser/ui/ash/ash_u...
File chrome/browser/ui/ash/ash_util.h (right):

https://codereview.chromium.org/2721153004/diff/1/chrome/browser/ui/ash/ash_u...
chrome/browser/ui/ash/ash_util.h:19: }  // namespace ui
Can probably drop this comment.

[stevenjb, 2017-03-01 17:42:31]

stevenjb@chromium.org changed reviewers:
+ jamescook@chromium.org

[stevenjb, 2017-03-01 17:42:32]

I'm not in favor of arbitrarily removing this namespace, especially when
"IsAcceleratorDeprecated" would be in the global namespace.

My recommentation would be:
1) Move ash_util to somewhere in src/ash (jamescook@ - Does that sound
reasonable? suggestions as to where exactly?)
2) use the ash:: namespace instead

Alternatively, at minimum we need to rename IsAcceleratorDeprecated.

https://codereview.chromium.org/2721153004/diff/1/chrome/browser/ui/ash/ash_u...
File chrome/browser/ui/ash/ash_util.cc (right):

https://codereview.chromium.org/2721153004/diff/1/chrome/browser/ui/ash/ash_u...
chrome/browser/ui/ash/ash_util.cc:12: #include
"chrome/browser/ui/ash/ash_init.h"
This dependency appears to be unnecessary and it looks like the rest of the code
can move to src/ash.

[James Cook, 2017-03-01 18:11:15]

I don't mean to bikeshed, but I don't like removing this namespace either.

In particular, when I see "IsRunningInMash" in an arbitrary piece of code in
//chrome having either a static AshUtil class or an ash_util namespace makes it
immediately obvious to me that it isn't a member function, isn't an anonymous
function in the same file, and the implementation is in ash_util.cc.

But since you didn't ask me to review I'll defer to sky@.

https://codereview.chromium.org/2721153004/diff/1/chrome/browser/ui/ash/ash_u...
File chrome/browser/ui/ash/ash_util.cc (right):

https://codereview.chromium.org/2721153004/diff/1/chrome/browser/ui/ash/ash_u...
chrome/browser/ui/ash/ash_util.cc:12: #include
"chrome/browser/ui/ash/ash_init.h"
On 2017/03/01 17:42:32, stevenjb wrote:
> This dependency appears to be unnecessary and it looks like the rest of the
code
> can move to src/ash.

I am uncertain if IsRunningInMash() as written below would work if called from
code in src/ash. src/ash has its own WmShell::IsRunningInMash(). sky@ would know
better.

Likewise ash doesn't want to have content/ DEPS.

I think these utilities should be kept in //chrome.

[sky, 2017-03-01 20:50:15]

I think I was hasty in leading you down this path.
https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/namesp...
earlier on recommended not creating namespaces like ash_util, but the thread was
revived a while back with less clear guidance...

https://codereview.chromium.org/2721153004/diff/1/chrome/browser/ui/ash/ash_u...
File chrome/browser/ui/ash/ash_util.cc (right):

https://codereview.chromium.org/2721153004/diff/1/chrome/browser/ui/ash/ash_u...
chrome/browser/ui/ash/ash_util.cc:12: #include
"chrome/browser/ui/ash/ash_init.h"
On 2017/03/01 18:11:15, James Cook wrote:
> On 2017/03/01 17:42:32, stevenjb wrote:
> > This dependency appears to be unnecessary and it looks like the rest of the
> code
> > can move to src/ash.

tl;dr this check should be in the client code, not ash.

Going forward the only dependency we want chrome to have on ash is
ash/public/cpp. Think of that as the client library for ash, by which I mean any
clients that need to do ash specific things use the ash client library. Given
that, does IsRunningInMash makes sense in ash's public library? I view that as
linking with a bunch of code and then having a function in that code,
AmIUsingYou()? That's weird. The code consuming the client library should know
whether it is actually using the client library, not the client library itself.
So, IsRunningInMash should be here.

> I am uncertain if IsRunningInMash() as written below would work if called from
> code in src/ash. src/ash has its own WmShell::IsRunningInMash(). sky@ would
know
> better.
> 
> Likewise ash doesn't want to have content/ DEPS.
> 
> I think these utilities should be kept in //chrome.

Agreed.

[fwang, 2017-03-02 10:52:59]

This change seems controversial and ash is not on my main work goal [*]. I
believe enough clean up of ash_util is done for now. Hence I'm closing this CL
and I'm resolving the corresponding bug. Thank you again for the feedback /
review! 

[*] All started in https://codereview.chromium.org/2710673002/ when I found a
small TODO while analyzing the startup sequence of "chrome --mash"...