Create a about handler component.

Create a about handler component.

Create a about handler component.
The following files ::
  chrome/browser/net/about_protocol_handler.*
  net/url_request/url_request_about_job.*
moved to new component: components/about_handler.
Also added GYP and GN file for same.

BUG=516695
Committed: https://crrev.com/4f2e9111a6457a4010bb3ca11047dc5a1840f17b
Cr-Commit-Position: refs/heads/master@{#343366}

[Jitu( very slow this week), 2015-08-12 11:45:45]

jitendra.ks@samsung.com changed reviewers:
+ droger@chromium.org, vivekg@chromium.org

[Jitu( very slow this week), 2015-08-12 11:45:46]

PTAL

[droger, 2015-08-12 13:03:25]

Great thanks. Only minor comments.

https://codereview.chromium.org/1284173003/diff/1/components/BUILD.gn
File components/BUILD.gn (right):

https://codereview.chromium.org/1284173003/diff/1/components/BUILD.gn#newcode339
components/BUILD.gn:339: "//components/about_handler",
I think this is not needed, can you remove it?

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/BU...
File components/about_handler/BUILD.gn (right):

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/BU...
components/about_handler/BUILD.gn:14: "//net:net",
"//base",
"//net",

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/DEPS
File components/about_handler/DEPS (right):

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/DE...
components/about_handler/DEPS:2: "+net/url_request",
I think
"+net",
is enough.

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ab...
File components/about_handler/about_protocol_handler.cc (right):

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ab...
components/about_handler/about_protocol_handler.cc:9: namespace
components_about_handler {
namespace about_handler {

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ab...
components/about_handler/about_protocol_handler.cc:23: }  // namespace
components_about_handler
}  // namespace about_handler

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ab...
File components/about_handler/about_protocol_handler.h (right):

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ab...
components/about_handler/about_protocol_handler.h:12: namespace
components_about_handler {
Should be
namespace about_handler {

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ab...
components/about_handler/about_protocol_handler.h:27: }  // namespace
components_about_handler
}  // namespace about_handler

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ur...
File components/about_handler/url_request_about_job.cc (right):

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ur...
components/about_handler/url_request_about_job.cc:17: namespace net {
namespace about_handler {

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ur...
components/about_handler/url_request_about_job.cc:45: }  // namespace net
}  // namespace about_handler

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ur...
File components/about_handler/url_request_about_job.h (right):

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ur...
components/about_handler/url_request_about_job.h:14: namespace net {
namespace about_handler {

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ur...
components/about_handler/url_request_about_job.h:32: }  // namespace net
}  // namespace about_handler

[Jitu( very slow this week), 2015-08-12 14:34:18]

@David Roger,

Fixed the comments.
PTAL...

https://codereview.chromium.org/1284173003/diff/1/components/BUILD.gn
File components/BUILD.gn (right):

https://codereview.chromium.org/1284173003/diff/1/components/BUILD.gn#newcode339
components/BUILD.gn:339: "//components/about_handler",
On 2015/08/12 13:03:25, droger wrote:
> I think this is not needed, can you remove it?

Done.

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/BU...
File components/about_handler/BUILD.gn (right):

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/BU...
components/about_handler/BUILD.gn:14: "//net:net",
On 2015/08/12 13:03:25, droger wrote:
> "//base",
> "//net",

Done.

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/DEPS
File components/about_handler/DEPS (right):

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/DE...
components/about_handler/DEPS:2: "+net/url_request",
On 2015/08/12 13:03:25, droger wrote:
> I think
> "+net",
> is enough.

Done.

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ab...
File components/about_handler/about_protocol_handler.cc (right):

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ab...
components/about_handler/about_protocol_handler.cc:9: namespace
components_about_handler {
On 2015/08/12 13:03:25, droger wrote:
> namespace about_handler {

Done.

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ab...
components/about_handler/about_protocol_handler.cc:23: }  // namespace
components_about_handler
On 2015/08/12 13:03:25, droger wrote:
> }  // namespace about_handler

Done.

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ab...
File components/about_handler/about_protocol_handler.h (right):

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ab...
components/about_handler/about_protocol_handler.h:12: namespace
components_about_handler {
On 2015/08/12 13:03:25, droger wrote:
> Should be
> namespace about_handler {

Done.

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ab...
components/about_handler/about_protocol_handler.h:27: }  // namespace
components_about_handler
On 2015/08/12 13:03:25, droger wrote:
> }  // namespace about_handler

Done.

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ur...
File components/about_handler/url_request_about_job.cc (right):

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ur...
components/about_handler/url_request_about_job.cc:17: namespace net {
On 2015/08/12 13:03:25, droger wrote:
> namespace about_handler {

Done.

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ur...
components/about_handler/url_request_about_job.cc:45: }  // namespace net
On 2015/08/12 13:03:25, droger wrote:
> }  // namespace about_handler

Done.

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ur...
File components/about_handler/url_request_about_job.h (right):

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ur...
components/about_handler/url_request_about_job.h:14: namespace net {
On 2015/08/12 13:03:25, droger wrote:
> namespace about_handler {

Done.

https://codereview.chromium.org/1284173003/diff/1/components/about_handler/ur...
components/about_handler/url_request_about_job.h:32: }  // namespace net
On 2015/08/12 13:03:25, droger wrote:
> }  // namespace about_handler

Done.

[droger, 2015-08-12 14:38:49]

droger@chromium.org changed reviewers:
+ mmenke@chromium.org, sdefresne@chromium.org

[droger, 2015-08-12 14:38:49]

LGTM.
Let me launch a try job for you.

adding owners:
mmenke for //net
sdefresne for //components

The remaining changes are probably simple enough to be TBR'ed .

[droger, 2015-08-12 14:38:57]

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

[commit-bot: I haz the power, 2015-08-12 14:39:08]

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

[droger, 2015-08-12 14:41:02]

https://codereview.chromium.org/1284173003/diff/20001/components/about_handle...
File components/about_handler.gypi (right):

https://codereview.chromium.org/1284173003/diff/20001/components/about_handle...
components/about_handler.gypi:1: # Copyright (c) 2015 The Chromium Authors. All
rights reserved.
Nit: remove (c)

https://codereview.chromium.org/1284173003/diff/20001/components/about_handle...
File components/about_handler/BUILD.gn (right):

https://codereview.chromium.org/1284173003/diff/20001/components/about_handle...
components/about_handler/BUILD.gn:1: # Copyright 2014 The Chromium Authors. All
rights reserved.
Nit: 2015

[commit-bot: I haz the power, 2015-08-12 14:47:00]

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

[commit-bot: I haz the power, 2015-08-12 14:47:00]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[mmenke, 2015-08-12 15:04:10]

I thought we were switching to the new iOS API, where we can't mock out the
network stack?  How does this fit into that?  I'm fine with moving the
about:handler out of net - it probably shouldn't be there in the first place,
just wondering if we actually need it for iOS.

[droger, 2015-08-12 15:09:26]

On 2015/08/12 15:04:10, mmenke wrote:
> I thought we were switching to the new iOS API, where we can't mock out the
> network stack?  How does this fit into that?  I'm fine with moving the
> about:handler out of net - it probably shouldn't be there in the first place,
> just wondering if we actually need it for iOS.

The long-term plan is not clear, but this will be needed at least until we
switch to WKWebView, which is at best several months away.
We're trying to complete upstreaming before that.

[droger, 2015-08-12 15:10:12]

On 2015/08/12 15:09:26, droger wrote:
> On 2015/08/12 15:04:10, mmenke wrote:
> > I thought we were switching to the new iOS API, where we can't mock out the
> > network stack?  How does this fit into that?  I'm fine with moving the
> > about:handler out of net - it probably shouldn't be there in the first
place,
> > just wondering if we actually need it for iOS.
> 
> The long-term plan is not clear, but this will be needed at least until we
> switch to WKWebView, which is at best several months away.
> We're trying to complete upstreaming before that.

I meant:
We're trying to complete iOS-related componentization before that.

[mmenke, 2015-08-12 15:21:26]

On 2015/08/12 15:10:12, droger wrote:
> On 2015/08/12 15:09:26, droger wrote:
> > On 2015/08/12 15:04:10, mmenke wrote:
> > > I thought we were switching to the new iOS API, where we can't mock out
the
> > > network stack?  How does this fit into that?  I'm fine with moving the
> > > about:handler out of net - it probably shouldn't be there in the first
> place,
> > > just wondering if we actually need it for iOS.
> > 
> > The long-term plan is not clear, but this will be needed at least until we
> > switch to WKWebView, which is at best several months away.
> > We're trying to complete upstreaming before that.
> 
> I meant:
> We're trying to complete iOS-related componentization before that.

SGTM.  Could you tell me when you get this compiling on the debug bots, and I'll
look at it then?

[Jitu( very slow this week), 2015-08-13 05:34:52]

Fixed the comments 

PTAL ...

@David,
In trybots i am seeing some linking errors

error: undefined reference to
'about_handler::AboutProtocolHandler::AboutProtocolHandler()'

But same target i am able to build in my machine without any error.

Could you help me in this.

https://codereview.chromium.org/1284173003/diff/20001/components/about_handle...
File components/about_handler.gypi (right):

https://codereview.chromium.org/1284173003/diff/20001/components/about_handle...
components/about_handler.gypi:1: # Copyright (c) 2015 The Chromium Authors. All
rights reserved.
On 2015/08/12 14:41:01, droger wrote:
> Nit: remove (c)

Done.

https://codereview.chromium.org/1284173003/diff/20001/components/about_handle...
File components/about_handler/BUILD.gn (right):

https://codereview.chromium.org/1284173003/diff/20001/components/about_handle...
components/about_handler/BUILD.gn:1: # Copyright 2014 The Chromium Authors. All
rights reserved.
On 2015/08/12 14:41:01, droger wrote:
> Nit: 2015

Done.

[sdefresne, 2015-08-13 08:00:18]

On 2015/08/13 at 05:34:52, jitendra.ks wrote:
> Fixed the comments 
> 
> PTAL ...
> 
> @David,
> In trybots i am seeing some linking errors
> 
> error: undefined reference to
'about_handler::AboutProtocolHandler::AboutProtocolHandler()'
> 
> But same target i am able to build in my machine without any error.
> 
> Could you help me in this.

It look like the target "performance_browser_tests" does not have a dependency
on the new component. Are you building using gyp or gn? Could you try with the
other one?

> 
>
https://codereview.chromium.org/1284173003/diff/20001/components/about_handle...
> File components/about_handler.gypi (right):
> 
>
https://codereview.chromium.org/1284173003/diff/20001/components/about_handle...
> components/about_handler.gypi:1: # Copyright (c) 2015 The Chromium Authors.
All rights reserved.
> On 2015/08/12 14:41:01, droger wrote:
> > Nit: remove (c)
> 
> Done.
> 
>
https://codereview.chromium.org/1284173003/diff/20001/components/about_handle...
> File components/about_handler/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1284173003/diff/20001/components/about_handle...
> components/about_handler/BUILD.gn:1: # Copyright 2014 The Chromium Authors.
All rights reserved.
> On 2015/08/12 14:41:01, droger wrote:
> > Nit: 2015
> 
> Done.

[droger, 2015-08-13 08:23:11]

On 2015/08/13 05:34:52, jitu wrote:
> Fixed the comments 
> 
> PTAL ...
> 
> @David,
> In trybots i am seeing some linking errors
> 
> error: undefined reference to
> 'about_handler::AboutProtocolHandler::AboutProtocolHandler()'
> 
> But same target i am able to build in my machine without any error.
> 
> Could you help me in this.
> 

I would say you need to add the dependency on the new component in
chrome/browser/BUILD.gn

[Jitu( very slow this week), 2015-08-13 08:26:33]

On 2015/08/13 08:23:11, droger wrote:
> On 2015/08/13 05:34:52, jitu wrote:
> > Fixed the comments 
> > 
> > PTAL ...
> > 
> > @David,
> > In trybots i am seeing some linking errors
> > 
> > error: undefined reference to
> > 'about_handler::AboutProtocolHandler::AboutProtocolHandler()'
> > 
> > But same target i am able to build in my machine without any error.
> > 
> > Could you help me in this.
> > 
> 
> I would say you need to add the dependency on the new component in
> chrome/browser/BUILD.gn

In latest code, while making GN build getting below errors

LINK ./nacl_helper
FAILED: ../../third_party/llvm-build/Release+Asserts/bin/clang++ -pie
-Wl,--fatal-warnings -m64 -fPIC -Wl,-z,noexecstack -Wl,-z,now -Wl,-z,relro
-Wl,-z,defs -pthread -B../../third_party/binutils/Linux_x64/Release/bin
-fuse-ld=gold -Wl,--icf=all -Wl,-rpath=\$ORIGIN/ -Wl,-rpath-link=
-Wl,--disable-new-dtags -o ./nacl_helper -Wl,--start-group @./nacl_helper.rsp 
-Wl,--end-group  -ldl -lgmodule-2.0 -lgobject-2.0 -lgthread-2.0 -lglib-2.0 -lrt
-lpangocairo-1.0 -lpango-1.0 -lcairo -lXcomposite -lnss3 -lnssutil3 -lsmime3
-lplds4 -lplc4 -lnspr4 -lgconf-2 -lresolv -lgio-2.0 -lfontconfig -lfreetype
-latk-1.0 -lX11 -lXcursor -lXdamage -lXext -lXfixes -lXi -lXrender -lXtst -lXss
-lXrandr -lm -lz -lasound -lexpat 
../../third_party/mojo/src/mojo/public/cpp/bindings/string.h:40: error:
undefined reference to 'mojo::TypeConverter<mojo::String,
base::BasicStringPiece<std::string>
>::Convert(base::BasicStringPiece<std::string> const&)'
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Any idea ??

[droger, 2015-08-13 08:32:46]

On 2015/08/13 08:23:11, droger wrote:
> On 2015/08/13 05:34:52, jitu wrote:
> I would say you need to add the dependency on the new component in
> chrome/browser/BUILD.gn

Ah, sorry this is already done in your CL. I missed it somehow.

[droger, 2015-08-13 08:53:34]

I did another pass on the CL. THis should probably fix the link error.

https://codereview.chromium.org/1284173003/diff/40001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/1284173003/diff/40001/chrome/browser/BUILD.gn...
chrome/browser/BUILD.gn:93: "//components/about_handler",
This one should not be needed.

https://codereview.chromium.org/1284173003/diff/40001/chrome/browser/BUILD.gn...
chrome/browser/BUILD.gn:1060: "//components/about_handler",
This one should not be needed.

https://codereview.chromium.org/1284173003/diff/40001/chrome/browser/profiles...
File chrome/browser/profiles/off_the_record_profile_io_data.cc (right):

https://codereview.chromium.org/1284173003/diff/40001/chrome/browser/profiles...
chrome/browser/profiles/off_the_record_profile_io_data.cc:25: #include
"components/about_handler/about_protocol_handler.h"
Delete this line.

https://codereview.chromium.org/1284173003/diff/40001/components/about_handle...
File components/about_handler.gypi (right):

https://codereview.chromium.org/1284173003/diff/40001/components/about_handle...
components/about_handler.gypi:9: 'type': '<(component)',
Change this to
'type': 'static_library',

component should work too, but we certainly don't need this component to be a
dynamic library ever.

https://codereview.chromium.org/1284173003/diff/40001/components/about_handle...
File components/about_handler/BUILD.gn (right):

https://codereview.chromium.org/1284173003/diff/40001/components/about_handle...
components/about_handler/BUILD.gn:5: static_library("about_handler") {
source_set("about_handler") {

source sets are slightly preferred over static libraries in general.

https://codereview.chromium.org/1284173003/diff/40001/components/about_handle...
File components/about_handler/about_protocol_handler.h (right):

https://codereview.chromium.org/1284173003/diff/40001/components/about_handle...
components/about_handler/about_protocol_handler.h:17: AboutProtocolHandler();
Add a destructor for this class, and do not inline it.

If there is no destructor, a default, inlined destructor is provided.
However inlined destructor don't work when used in a dynamic library, which is
presumably why you had the link error. The failing bots are probably configured
to build chrome in "components build" which uses dynamic libraries.

[Jitu( very slow this week), 2015-08-13 09:04:46]

Patchset #4 (id:60001) has been deleted

[droger, 2015-08-13 12:06:12]

https://codereview.chromium.org/1284173003/diff/80001/components/about_handle...
File components/about_handler/url_request_about_job.h (right):

https://codereview.chromium.org/1284173003/diff/80001/components/about_handle...
components/about_handler/url_request_about_job.h:16: class NET_EXPORT
URLRequestAboutJob : public net::URLRequestJob {
Remove NET_EXPORT

[Jitu( very slow this week), 2015-08-13 12:14:38]

On 2015/08/13 12:06:12, droger wrote:
>
https://codereview.chromium.org/1284173003/diff/80001/components/about_handle...
> File components/about_handler/url_request_about_job.h (right):
> 
>
https://codereview.chromium.org/1284173003/diff/80001/components/about_handle...
> components/about_handler/url_request_about_job.h:16: class NET_EXPORT
> URLRequestAboutJob : public net::URLRequestJob {
> Remove NET_EXPORT

Build failed in win port 
build\slave\win\build\src\components\about_handler\url_request_about_job.cc(21)
: error C2220: warning treated as error - no 'object' file generated

with warning C4273.

Do i need to add 
'msvs_disabled_warnings': [4273, ], in gyp file ??

[droger, 2015-08-13 13:03:12]

On 2015/08/13 12:14:38, jitu wrote:
> On 2015/08/13 12:06:12, droger wrote:
> >
>
https://codereview.chromium.org/1284173003/diff/80001/components/about_handle...
> > File components/about_handler/url_request_about_job.h (right):
> > 
> >
>
https://codereview.chromium.org/1284173003/diff/80001/components/about_handle...
> > components/about_handler/url_request_about_job.h:16: class NET_EXPORT
> > URLRequestAboutJob : public net::URLRequestJob {
> > Remove NET_EXPORT
> 
> Build failed in win port 
>
build\slave\win\build\src\components\about_handler\url_request_about_job.cc(21)
> : error C2220: warning treated as error - no 'object' file generated
> 
> with warning C4273.
> 
> Do i need to add 
> 'msvs_disabled_warnings': [4273, ], in gyp file ??

Is it solved by removing NET_EXPORT?

[Jitu( very slow this week), 2015-08-13 13:30:10]

Patchset #5 (id:100001) has been deleted

[Jitu( very slow this week), 2015-08-13 14:14:42]

On 2015/08/13 13:03:12, droger wrote:
> On 2015/08/13 12:14:38, jitu wrote:
> > On 2015/08/13 12:06:12, droger wrote:
> > >
> >
>
https://codereview.chromium.org/1284173003/diff/80001/components/about_handle...
> > > File components/about_handler/url_request_about_job.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1284173003/diff/80001/components/about_handle...
> > > components/about_handler/url_request_about_job.h:16: class NET_EXPORT
> > > URLRequestAboutJob : public net::URLRequestJob {
> > > Remove NET_EXPORT
> > 
> > Build failed in win port 
> >
>
build\slave\win\build\src\components\about_handler\url_request_about_job.cc(21)
> > : error C2220: warning treated as error - no 'object' file generated
> > 
> > with warning C4273.
> > 
> > Do i need to add 
> > 'msvs_disabled_warnings': [4273, ], in gyp file ??
> 
> Is it solved by removing NET_EXPORT?

Thanks it solved.

[Jitu( very slow this week), 2015-08-13 14:15:04]

Fixed all comments 
PTAL

https://codereview.chromium.org/1284173003/diff/40001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/1284173003/diff/40001/chrome/browser/BUILD.gn...
chrome/browser/BUILD.gn:93: "//components/about_handler",
On 2015/08/13 08:53:34, droger wrote:
> This one should not be needed.

Done.

https://codereview.chromium.org/1284173003/diff/40001/chrome/browser/BUILD.gn...
chrome/browser/BUILD.gn:1060: "//components/about_handler",
On 2015/08/13 08:53:34, droger wrote:
> This one should not be needed.

Done.

https://codereview.chromium.org/1284173003/diff/40001/chrome/browser/profiles...
File chrome/browser/profiles/off_the_record_profile_io_data.cc (right):

https://codereview.chromium.org/1284173003/diff/40001/chrome/browser/profiles...
chrome/browser/profiles/off_the_record_profile_io_data.cc:25: #include
"components/about_handler/about_protocol_handler.h"
On 2015/08/13 08:53:34, droger wrote:
> Delete this line.

Done.

https://codereview.chromium.org/1284173003/diff/40001/components/about_handle...
File components/about_handler.gypi (right):

https://codereview.chromium.org/1284173003/diff/40001/components/about_handle...
components/about_handler.gypi:9: 'type': '<(component)',
On 2015/08/13 08:53:34, droger wrote:
> Change this to
> 'type': 'static_library',
> 
> component should work too, but we certainly don't need this component to be a
> dynamic library ever.

Done.

https://codereview.chromium.org/1284173003/diff/40001/components/about_handle...
File components/about_handler/BUILD.gn (right):

https://codereview.chromium.org/1284173003/diff/40001/components/about_handle...
components/about_handler/BUILD.gn:5: static_library("about_handler") {
On 2015/08/13 08:53:34, droger wrote:
> source_set("about_handler") {
> 
> source sets are slightly preferred over static libraries in general.
just curious why ?

Done.

https://codereview.chromium.org/1284173003/diff/80001/components/about_handle...
File components/about_handler/url_request_about_job.h (right):

https://codereview.chromium.org/1284173003/diff/80001/components/about_handle...
components/about_handler/url_request_about_job.h:16: class NET_EXPORT
URLRequestAboutJob : public net::URLRequestJob {
On 2015/08/13 12:06:12, droger wrote:
> Remove NET_EXPORT

Done.

[Jitu( very slow this week), 2015-08-13 14:25:27]

jitendra.ks@samsung.com changed reviewers:
+ rockot@chromium.org, sky@chromium.org

[Jitu( very slow this week), 2015-08-13 14:25:28]

Dear Rockot:: Can you please do the owner review of
              ---chrome/browser/extensions/*

Dear sky:: Can you please do the owner review of
           --- chrome/test/BUILD.gn
           --- chrome/browser/BUILD.gn
           --- chrome/chrome_browser.gypi

[mmenke, 2015-08-13 14:49:26]

net/, profiles/, and chrome/browser/net LGTM

Also took a look at the component, and didn't see any issues there.

[mmenke, 2015-08-13 14:49:41]

Oops..one nit I forgot to publish

https://codereview.chromium.org/1284173003/diff/120001/components/about_handl...
File components/about_handler.gypi (right):

https://codereview.chromium.org/1284173003/diff/120001/components/about_handl...
components/about_handler.gypi:23: 
nit:  Remove blank line.

[sky, 2015-08-13 17:22:28]

LGTM

[Jitu( very slow this week), 2015-08-14 04:26:21]

Dear sdefresne,

Could you please do the owner review for
    --- src/components/*

[Ken Rockot(use gerrit already), 2015-08-14 06:47:37]

On 2015/08/13 14:25:28, jitu wrote:
> Dear Rockot:: Can you please do the owner review of
>               ---chrome/browser/extensions/*
> 
> Dear sky:: Can you please do the owner review of
>            --- chrome/test/BUILD.gn
>            --- chrome/browser/BUILD.gn
>            --- chrome/chrome_browser.gypi

chrome/browser/extensions LGTM

[sdefresne, 2015-08-14 08:32:23]

components/ lgtm

https://codereview.chromium.org/1284173003/diff/140001/components/about_handl...
File components/about_handler.gypi (left):

https://codereview.chromium.org/1284173003/diff/140001/components/about_handl...
components/about_handler.gypi:8: 'target_name': 'browsing_data',
nit: add a comment

# GN version: //components/about_handler

https://codereview.chromium.org/1284173003/diff/140001/components/about_handl...
components/about_handler.gypi:19: # Note: sources list duplicated in GN build.
nit: please leave this comment

[Jitu( very slow this week), 2015-08-14 08:44:33]

Fixed

PTAL ...

https://codereview.chromium.org/1284173003/diff/140001/components/about_handl...
File components/about_handler.gypi (left):

https://codereview.chromium.org/1284173003/diff/140001/components/about_handl...
components/about_handler.gypi:8: 'target_name': 'browsing_data',
On 2015/08/14 08:32:23, sdefresne wrote:
> nit: add a comment
> 
> # GN version: //components/about_handler

Done.

https://codereview.chromium.org/1284173003/diff/140001/components/about_handl...
components/about_handler.gypi:19: # Note: sources list duplicated in GN build.
On 2015/08/14 08:32:23, sdefresne wrote:
> nit: please leave this comment

Done.

[sdefresne, 2015-08-14 08:50:48]

still lgtm

[Jitu( very slow this week), 2015-08-14 08:51:31]

The CQ bit was checked by jitendra.ks@samsung.com

[Jitu( very slow this week), 2015-08-14 08:51:31]

The patchset sent to the CQ was uploaded after l-g-t-m from droger@chromium.org,
sky@chromium.org, mmenke@chromium.org, rockot@chromium.org
Link to the patchset: https://codereview.chromium.org/1284173003/#ps160001
(title: "nit")

[commit-bot: I haz the power, 2015-08-14 08:51:41]

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

[commit-bot: I haz the power, 2015-08-14 10:18:48]

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

[commit-bot: I haz the power, 2015-08-14 10:18:49]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Jitu( very slow this week), 2015-08-14 11:36:15]

The CQ bit was checked by jitendra.ks@samsung.com

[commit-bot: I haz the power, 2015-08-14 11:36:24]

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

[commit-bot: I haz the power, 2015-08-14 11:40:35]

Committed patchset #7 (id:160001)

[commit-bot: I haz the power, 2015-08-14 11:41:19]

Patchset 7 (id:??) landed as
https://crrev.com/4f2e9111a6457a4010bb3ca11047dc5a1840f17b
Cr-Commit-Position: refs/heads/master@{#343366}