Introduce the nested namespace ::blink::cssvalue, start with CSSColorValue.

Introduce the nested namespace ::blink::cssvalue, start with CSSColorValue.

This patch is the first in a series that will move all of the classes in the range CSS.*Value from the ::blink namespace to the ::blink::cssvalue namespace. It will be followed by work to introduce the ::blink::cssom namespace.

The original design doc for this work is linked in the attached bug. Note that this patch does not rename the class or move the file to a new subdirectory. The immediate goal is to prevent name collisions cssvalue and cssom types - there is no need to re-organize or rename the files to achieve this basic goal.

BUG=667961
Review-Url: https://codereview.chromium.org/2796183002
Cr-Commit-Position: refs/heads/master@{#462010}
Committed: https://chromium.googlesource.com/chromium/src/+/80cc48147b27aa364144e4b6d49524fb111274a9

[slangley, 2017-04-05 00:25:18]

Description was changed from

==========
Use the correct namespace 'cssvalue' where CSSColorValue is being used.


Put CSSColorValue into the blink::cssvalue namespace.

BUG=
==========

to

==========
Introduce the nested namespace ::blink::cssvalue, start with CSSColorValue.

This patch is the first in a series that will move all of the classes in the
range CSS.*Value from the ::blink namespace to the ::blink::cssvalue namespace.
It will be followed by work to introduce the ::blink::cssom namespace.

The original design doc for this work is linked in the attached bug. Note that
this patch does not rename the class or move the file to a new subdirectory. The
immediate goal is to prevent name collisions cssvalue and cssom types - there is
no need to re-organize or rename the files to achieve this basic goal.

BUG=667961
==========

[slangley, 2017-04-05 00:29:32]

slangley@chromium.org changed reviewers:
+ dcheng@chromium.org

[slangley, 2017-04-05 00:29:33]

dcheng@ - can you ptal with you blink coding style owner hat on.

Thanks.

https://codereview.chromium.org/2796183002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/CSSColorInterpolationType.cpp
(right):

https://codereview.chromium.org/2796183002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/CSSColorInterpolationType.cpp:17: using
namespace cssvalue;
The Google c++ style guide forbids the "using directive", but I don't see any
similar rule for blink.

[dcheng, 2017-04-05 02:31:34]

One other nit is to wrap the CL description at 72 chars so things wrap a bit
more nicely.

If the "using namespace" things are just to temporarily make things easier
during the transition, I think that's OK to have for now, but I think we don't
want to keep this around forever.

https://codereview.chromium.org/2796183002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/CSSColorInterpolationType.cpp
(right):

https://codereview.chromium.org/2796183002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/CSSColorInterpolationType.cpp:17: using
namespace cssvalue;
On 2017/04/05 00:29:33, slangley wrote:
> The Google c++ style guide forbids the "using directive", but I don't see any
> similar rule for blink.
> 

Doesn't doing this mean that we're likely to end up with collisions between
things in cssvalue and cssom?

https://codereview.chromium.org/2796183002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/CSSValuePool.h (right):

https://codereview.chromium.org/2796183002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/CSSValuePool.h:47: using
::blink::cssvalue::CSSColorValue;
It would be nicer not to have using declarations in headers (this is against the
Google C++ style guide)

[slangley, 2017-04-05 04:40:18]

ptal

https://codereview.chromium.org/2796183002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/CSSColorInterpolationType.cpp
(right):

https://codereview.chromium.org/2796183002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/CSSColorInterpolationType.cpp:17: using
namespace cssvalue;
On 2017/04/05 at 02:31:34, dcheng wrote:
> On 2017/04/05 00:29:33, slangley wrote:
> > The Google c++ style guide forbids the "using directive", but I don't see
any
> > similar rule for blink.
> > 
> 
> Doesn't doing this mean that we're likely to end up with collisions between
things in cssvalue and cssom?

The idea is that by tactically having "using namespace cssvalue" while we're
converting the types to use the nested namespace we can have smaller CL's for
review. Then once we've nested all of the types we can remove these - so for not
it's really just to keep the cl churn low.

https://codereview.chromium.org/2796183002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/CSSValuePool.h (right):

https://codereview.chromium.org/2796183002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/CSSValuePool.h:47: using
::blink::cssvalue::CSSColorValue;
On 2017/04/05 at 02:31:34, dcheng wrote:
> It would be nicer not to have using declarations in headers (this is against
the Google C++ style guide)

done

[dcheng, 2017-04-05 06:21:27]

lgtm with nits

https://codereview.chromium.org/2796183002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/CSSColorInterpolationType.cpp
(right):

https://codereview.chromium.org/2796183002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/CSSColorInterpolationType.cpp:17: using
namespace cssvalue;
On 2017/04/05 04:40:18, slangley wrote:
> On 2017/04/05 at 02:31:34, dcheng wrote:
> > On 2017/04/05 00:29:33, slangley wrote:
> > > The Google c++ style guide forbids the "using directive", but I don't see
> any
> > > similar rule for blink.
> > > 
> > 
> > Doesn't doing this mean that we're likely to end up with collisions between
> things in cssvalue and cssom?
> 
> The idea is that by tactically having "using namespace cssvalue" while we're
> converting the types to use the nested namespace we can have smaller CL's for
> review. Then once we've nested all of the types we can remove these - so for
not
> it's really just to keep the cl churn low.

OK, using these to ease the transition seems fine.

https://codereview.chromium.org/2796183002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSColorValue.h (right):

https://codereview.chromium.org/2796183002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSColorValue.h:39: friend class
::blink::CSSValuePool;
Nit: is this necessary to qualify with ::blink here? I think it /should/ just
work since there's the forward decl on line 14 now.

https://codereview.chromium.org/2796183002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSValuePool.h (right):

https://codereview.chromium.org/2796183002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSValuePool.h:54: using CSSColorValue =
::blink::cssvalue::CSSColorValue;
Nit: I think ::blink:: can be omitted.

(We don't have a lot of examples of using nested namespaces, but I did a quick
search of how blink::protocol is referenced, and minimal qualification seems to
be preferred over full qualification)

[slangley, 2017-04-05 06:45:57]

thanks!

https://codereview.chromium.org/2796183002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSColorValue.h (right):

https://codereview.chromium.org/2796183002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSColorValue.h:39: friend class
::blink::CSSValuePool;
On 2017/04/05 at 06:21:26, dcheng wrote:
> Nit: is this necessary to qualify with ::blink here? I think it /should/ just
work since there's the forward decl on line 14 now.

Unfortunately doesn't compile without the explicit qualification.

https://codereview.chromium.org/2796183002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSValuePool.h (right):

https://codereview.chromium.org/2796183002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSValuePool.h:54: using CSSColorValue =
::blink::cssvalue::CSSColorValue;
On 2017/04/05 at 06:21:26, dcheng wrote:
> Nit: I think ::blink:: can be omitted.
> 
> (We don't have a lot of examples of using nested namespaces, but I did a quick
search of how blink::protocol is referenced, and minimal qualification seems to
be preferred over full qualification)

Done

[slangley, 2017-04-05 06:48:19]

The CQ bit was checked by slangley@chromium.org

[slangley, 2017-04-05 06:48:19]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2796183002/#ps40001
(title: "Use the least possible namespace scoping when aliasing.")

[commit-bot: I haz the power, 2017-04-05 06:48:37]

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

[commit-bot: I haz the power, 2017-04-05 08:44:49]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491374899460650,
"parent_rev": "a93efbaa0146b717e42c78fd320074852822435a", "commit_rev":
"80cc48147b27aa364144e4b6d49524fb111274a9"}

[commit-bot: I haz the power, 2017-04-05 08:46:27]

Description was changed from

==========
Introduce the nested namespace ::blink::cssvalue, start with CSSColorValue.

This patch is the first in a series that will move all of the classes in the
range CSS.*Value from the ::blink namespace to the ::blink::cssvalue namespace.
It will be followed by work to introduce the ::blink::cssom namespace.

The original design doc for this work is linked in the attached bug. Note that
this patch does not rename the class or move the file to a new subdirectory. The
immediate goal is to prevent name collisions cssvalue and cssom types - there is
no need to re-organize or rename the files to achieve this basic goal.

BUG=667961
==========

to

==========
Introduce the nested namespace ::blink::cssvalue, start with CSSColorValue.

This patch is the first in a series that will move all of the classes in the
range CSS.*Value from the ::blink namespace to the ::blink::cssvalue namespace.
It will be followed by work to introduce the ::blink::cssom namespace.

The original design doc for this work is linked in the attached bug. Note that
this patch does not rename the class or move the file to a new subdirectory. The
immediate goal is to prevent name collisions cssvalue and cssom types - there is
no need to re-organize or rename the files to achieve this basic goal.

BUG=667961

Review-Url: https://codereview.chromium.org/2796183002
Cr-Commit-Position: refs/heads/master@{#462010}
Committed:
https://chromium.googlesource.com/chromium/src/+/80cc48147b27aa364144e4b6d495...
==========

[commit-bot: I haz the power, 2017-04-05 08:46:28]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/80cc48147b27aa364144e4b6d495...