Create controller/ component in blink. The first non-boilerplate files in here are WebDevToolsAgentImpl. [chromium/src : master]
Nicholas Verne (Gerrit)
Nicholas Verne uploaded patch set #2 to this change.
Create controller/ component in blink.
The first non-boilerplate files in here are WebDevToolsAgentImpl.
The controller/ is depended on by web (temporarily) and is part
Bug:
Change-Id: I7dbe02690ca3a90c08fead13468d160773fe7753
---
A third_party/WebKit/Source/controller/BUILD.gn
A third_party/WebKit/Source/controller/ControllerExport.h
A third_party/WebKit/Source/controller/DEPS
A third_party/WebKit/Source/controller/OWNERS
R third_party/WebKit/Source/controller/WebDevToolsAgentImpl.cpp
R third_party/WebKit/Source/controller/WebDevToolsAgentImpl.h
M third_party/WebKit/Source/core/exported/BUILD.gn
A third_party/WebKit/Source/core/exported/WebViewBase.cpp
M third_party/WebKit/Source/core/exported/WebViewBase.h
M third_party/WebKit/Source/web/BUILD.gn
M third_party/WebKit/Source/web/DEPS
M third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
M third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp
M third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
M third_party/WebKit/Source/web/WebViewImpl.cpp
M third_party/WebKit/public/BUILD.gn
16 files changed, 191 insertions(+), 17 deletions(-)
To view, visit change 547379. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 2:
It's not my preferred option, but here's a CL creating controller/ and moving WebDevToolsAgentImpl there. We can do this if we get too much pushback on https://chromium-review.googlesource.com/c/544726/
Nicholas Verne (Gerrit)
Nicholas Verne would like Sasha Morrissey, Joel Hockey, Stuart Langley and Kentaro Hara to review this change.
Create controller/ component in blink.
The first non-boilerplate files in here are WebDevToolsAgentImpl.
The controller/ is depended on by web (temporarily) and is part
Bug:
Change-Id: I7dbe02690ca3a90c08fead13468d160773fe7753
---
A third_party/WebKit/Source/controller/BUILD.gn
A third_party/WebKit/Source/controller/ControllerExport.h
A third_party/WebKit/Source/controller/DEPS
A third_party/WebKit/Source/controller/OWNERS
R third_party/WebKit/Source/controller/WebDevToolsAgentImpl.cpp
R third_party/WebKit/Source/controller/WebDevToolsAgentImpl.h
M third_party/WebKit/Source/core/exported/BUILD.gn
A third_party/WebKit/Source/core/exported/WebViewBase.cpp
M third_party/WebKit/Source/core/exported/WebViewBase.h
M third_party/WebKit/Source/web/BUILD.gn
M third_party/WebKit/Source/web/DEPS
M third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
M third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp
M third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
M third_party/WebKit/Source/web/WebViewImpl.cpp
M third_party/WebKit/public/BUILD.gn
16 files changed, 191 insertions(+), 17 deletions(-)
To view, visit change 547379. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 2:Commit-Queue +1
Kentaro Hara (Gerrit)
Kentaro Hara posted comments on this change.
Patch set 2:
This CL is just for experiment, right? If so, you can write something like "WIP" in the CL description. (Otherwise, people who don't have the background will be confused :-)
Nicholas Verne (Gerrit)
Nicholas Verne uploaded patch set #3 to this change.
WIP. Create controller/ component in blink.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 4:Commit-Queue +1
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 5:Commit-Queue +1
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 6:Commit-Queue +1
Sasha Morrissey (Gerrit)
Sasha Morrissey posted comments on this change.
Patch set 6:
Oops! Didn't realize this patch was a WIP :) Left some comments anyway, feel free to ignore :)
(6 comments)
File third_party/WebKit/Source/controller/BUILD.gn:
Patch Set #6, Line 6:
import("//testing/test.gni")Don't think this is needed, but if you like, I would be interested in moving at least one unit test so we can create the unit_tests target in this file in the same patch.
Patch Set #6, Line 25:
"//v8",Is this a minimal list? If not, for now we can just start with core, modules, v8 and platform, I think.
Patch Set #6, Line 47:
"Foundation.framework",What's this for? If it's just in wherever you copied the file from I'd try remove it and see if it still passes on the Mac bots. Although it looks pretty essential... Maybe
File third_party/WebKit/Source/controller/OWNERS:
Patch Set #6, Line 20:
joelh...@chromium.orgWhere'd you get this list from? Is this core/ owners + us?
Sgtm but I think maybe we want to prune a few, not all core/ OWNERs might want to be controller/ OWNERsPatch Set #6, Line 24:
per-file WebPerformance.cpp=cshar...@chromium.orgCan remove these
Patch Set #6, Line 26:
# TEAM: platform-arc...@chromium.orgMaybe we should make a controller-specific list for this? Not sure :)
To view, visit change 547379. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 7:Commit-Queue +1
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 8:Commit-Queue +1
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 9:Commit-Queue +1
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 10:Commit-Queue +1
Nicholas Verne (Gerrit)
Kentaro, can you help us with OWNERS selection? I've taken the web/ OWNERS and added our team.
(6 comments)
Patch Set #6, Line 6:
import("//testing/test.gni")Don't think this is needed, but if you like, I would be interested in movin
No need. I'll leave it in, then move a test in a follow-up CL.
Patch Set #6, Line 25:
configs += [
Is this a minimal list? If not, for now we can just start with core, module
Done
What's this for? If it's just in wherever you copied the file from I'd try
Ack
File third_party/WebKit/Source/controller/OWNERS:
Patch Set #6, Line 20:
joelh...@chromium.orgWhere'd you get this list from? Is this core/ owners + us?
It's web/ owners plus us. I think it's the right list for now. We can prune it later.
Patch Set #6, Line 24:
per-file WebPerformance.cpp=cshar...@chromium.orgCan remove these
Done
Patch Set #6, Line 26:
# TEAM: platform-arc...@chromium.orgMaybe we should make a controller-specific list for this? Not sure :)
Ack
To view, visit change 547379. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne uploaded patch set #11 to this change.
Create controller/ component in blink.
The first non-boilerplate files in here are WebDevToolsAgentImpl.
The controller/ is depended on by web (temporarily) and is part
Bug:
Change-Id: I7dbe02690ca3a90c08fead13468d160773fe7753
---
A third_party/WebKit/Source/controller/BUILD.gn
A third_party/WebKit/Source/controller/ControllerExport.h
A third_party/WebKit/Source/controller/DEPS
A third_party/WebKit/Source/controller/OWNERS
R third_party/WebKit/Source/controller/WebDevToolsAgentImpl.cpp
R third_party/WebKit/Source/controller/WebDevToolsAgentImpl.h
M third_party/WebKit/Source/core/exported/BUILD.gn
A third_party/WebKit/Source/core/exported/WebViewBase.cpp
M third_party/WebKit/Source/core/exported/WebViewBase.h
M third_party/WebKit/Source/web/BUILD.gn
M third_party/WebKit/Source/web/DEPS
M third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
M third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp
M third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
M third_party/WebKit/Source/web/WebViewImpl.cpp
M third_party/WebKit/Source/web/WebViewImpl.h
M third_party/WebKit/public/BUILD.gn
17 files changed, 212 insertions(+), 71 deletions(-)
To view, visit change 547379. To unsubscribe, visit settings.
Kentaro Hara (Gerrit)
Kentaro Hara posted comments on this change.
Patch set 11:
Patch Set 10:
(6 comments)
Kentaro, can you help us with OWNERS selection? I've taken the web/ OWNERS and added our team.
In Blink we're conservative about adding OWNERS. We don't add a person to OWNERS until the person gets fully familiar with the basic architecture and convention of the Blink code base. In particular, we don't add the person to OWNERS when we create the directory.
So I'd suggest:
- Not copying the current web/OWNERS to controller/OWNERS. web/ and controller/ are different things.
- Not adding persons who are not yet a committer.
- Probably adding slangley@, haraken@, dcheng@ and tkent@?
We can add more OWNERS as people land more patches.
Kentaro Hara (Gerrit)
It would be better to send a PSA to blink-dev before creating a new top-level directory. I'll send it soon.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 14:
Patch Set 11:
It would be better to send a PSA to blink-dev before creating a new top-level directory. I'll send it soon.
The PSA was 2 days ago. How about we continue reviewing this now?
Kentaro Hara (Gerrit)
Kentaro Hara posted comments on this change.
Patch set 14:
The CL description should have the following format.
CL title
<blank line>
Detailed explanation of the CL. What the CL is doing. What the background is.
<blank line>
Bug: ...
Also I'm not yet convinced that the current WebDevToolsAgentImpl is a good target to be moved to the controller/ layer. Shall we move the WebKit.cpp first?
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 14:
Patch Set 14:
The CL description should have the following format.
CL title
<blank line>
Detailed explanation of the CL. What the CL is doing. What the background is.
<blank line>
Bug: ...Also I'm not yet convinced that the current WebDevToolsAgentImpl is a good target to be moved to the controller/ layer. Shall we move the WebKit.cpp first?
I'll improve the CL description.
The reason for this CL arising now is specifically because we need a home for WebDevToolsAgentImpl outside of core/exported, and no longer in web/. I'm happy to move WebKit first (in this CL), but I intend to move WebDevToolsAgentImpl, or some refactoring of that class, soon after.
Nicholas Verne (Gerrit)
Nicholas Verne uploaded patch set #15 to this change.
Create controller/ component in blink.
This commences the new blink architecture outlined in Onion Soup 2.0
The first non-boilerplate files in controller/ are WebDevToolsAgentImpl.
Most of the remaining files in web/ will migrate to controller/ either as the
existing Web* classes, or as controller-specific refactorings. The controller/
directory is not intended as a dumping ground.
Bug:738768
Change-Id: I7dbe02690ca3a90c08fead13468d160773fe7753
---
A third_party/WebKit/Source/controller/BUILD.gn
A third_party/WebKit/Source/controller/ControllerExport.h
A third_party/WebKit/Source/controller/DEPS
A third_party/WebKit/Source/controller/OWNERS
R third_party/WebKit/Source/controller/WebDevToolsAgentImpl.cpp
R third_party/WebKit/Source/controller/WebDevToolsAgentImpl.h
M third_party/WebKit/Source/core/exported/BUILD.gn
A third_party/WebKit/Source/core/exported/WebViewBase.cpp
M third_party/WebKit/Source/core/exported/WebViewBase.h
M third_party/WebKit/Source/web/BUILD.gn
M third_party/WebKit/Source/web/DEPS
M third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
M third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp
M third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
M third_party/WebKit/Source/web/WebLocalFrameImpl.h
M third_party/WebKit/Source/web/WebViewImpl.cpp
M third_party/WebKit/Source/web/WebViewImpl.h
M third_party/WebKit/public/BUILD.gn
18 files changed, 194 insertions(+), 72 deletions(-)
To view, visit change 547379. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne uploaded patch set #17 to this change.
Create controller/ component in blink.
This commences the new blink architecture outlined in Onion Soup 2.0
Most of the remaining files in web/ will migrate to controller/ either as the
existing Web* classes, or as controller-specific refactorings. The controller/
directory is not intended as a dumping ground.
Bug:738768
Change-Id: I7dbe02690ca3a90c08fead13468d160773fe7753
---
A third_party/WebKit/Source/controller/BUILD.gn
A third_party/WebKit/Source/controller/ControllerExport.h
A third_party/WebKit/Source/controller/DEPS
A third_party/WebKit/Source/controller/OWNERS
A third_party/WebKit/Source/controller/README.md
M third_party/WebKit/Source/web/BUILD.gn
M third_party/WebKit/Source/web/DEPS
M third_party/WebKit/public/BUILD.gn
8 files changed, 112 insertions(+), 0 deletions(-)
To view, visit change 547379. To unsubscribe, visit settings.
Kentaro Hara (Gerrit)
Kentaro Hara posted comments on this change.
Patch set 17:
(3 comments)
File third_party/WebKit/Source/controller/DEPS:
Patch Set #17, Line 1:
include_rules = [For now would you add only:
- core
- modules
- controller
- bindings
?
We can add more dependencies as needed.
File third_party/WebKit/Source/controller/OWNERS:
Patch Set #17, Line 3:
nve...@chromium.orgDid you get a committer?
File third_party/WebKit/Source/controller/README.md:
Patch Set #17, Line 3:
controller/ contains the system infrastructure of the renderer process that uses or drives the web platform. controller/ can directly use core/ and modules/ without using Web types (but with some DEPS rules). Examples are RenderProcess, RenderThread, Android View, Extensions, Native Client etc.Add:
We should avoid making controller/ a "catch-all" directory. Things that are part of core/ should go to core/. Things that are part of modules/ should go to modules/. Only things that lives outside the web platform should go to controller/.
Slangley: Do you want to add more sentences?
To view, visit change 547379. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch Set #17, Line 1:
include_rules = [For now would you add only:
Patch Set #17, Line 3:
nve...@chromium.orgDid you get a committer?
Still waiting. I was hopeful that it would happen this week before I submit!
File third_party/WebKit/Source/controller/README.md:
Patch Set #17, Line 3:
controller/ contains the system infrastructure of the renderer process that uses or drives the web platform. controller/ can directly use core/ and modules/ without using Web types (but with some DEPS rules). Examples are RenderProcess, RenderThread, Android View, Extensions, Native Client etc.Add:
Done
To view, visit change 547379. To unsubscribe, visit settings.
Kentaro Hara (Gerrit)
Kentaro Hara posted comments on this change.
Patch set 17:Code-Review +1
LGTM
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 18:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot posted comments on this change.
Patch set 18:
CQ is trying da patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"need platform" https://chromium-review.googlesource.com/c/547379/18
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/547379/18
Bot data: {"action": "start", "triggered_at": "2017-07-03T07:32:56.0Z", "cq_cfg_revision": "1aa54ba037e9058c2752e0a0d871aeabfcba5974", "revision": "7631846918203a9bfde9dfbd251c4a152e45a7bf"}
Commit Bot (Gerrit)
Try jobs failed on following builders:
chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/479414)
Bot data: {"action": "cancel", "triggered_at": "2017-07-03T07:32:56.0Z", "cq_cfg_revision": "1aa54ba037e9058c2752e0a0d871aeabfcba5974", "revision": "7631846918203a9bfde9dfbd251c4a152e45a7bf"}
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 18:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot posted comments on this change.
Patch set 18:
CQ is trying da patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"need platform" https://chromium-review.googlesource.com/c/547379/18
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/547379/18
Bot data: {"action": "start", "triggered_at": "2017-07-03T08:20:13.0Z", "cq_cfg_revision": "1aa54ba037e9058c2752e0a0d871aeabfcba5974", "revision": "7631846918203a9bfde9dfbd251c4a152e45a7bf"}
Commit Bot (Gerrit)
Try jobs failed on following builders:
chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/479452)
Bot data: {"action": "cancel", "triggered_at": "2017-07-03T08:20:13.0Z", "cq_cfg_revision": "1aa54ba037e9058c2752e0a0d871aeabfcba5974", "revision": "7631846918203a9bfde9dfbd251c4a152e45a7bf"}
Kent Tamura (Gerrit)
Kent Tamura posted comments on this change.
Patch set 18:Code-Review +1
(3 comments)
nit: Remove trailing whitespace
File third_party/WebKit/Source/controller/ControllerExport.h:
Patch Set #18, Line 8:
#include "platform/wtf/Compiler.h"This looks unnecessary.
Patch Set #18, Line 21:
#endif // BLINK_WEB_IMPLEMENTATIONWrong comment.
To view, visit change 547379. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne uploaded patch set #19 to this change.
Create controller/ component in blink.
This commences the new blink architecture outlined in Onion Soup 2.0
Most of the remaining files in web/ will migrate to controller/ either as the
existing Web* classes, or as controller-specific refactorings. The controller/
directory is not intended as a dumping ground.
Bug:738768
Change-Id: I7dbe02690ca3a90c08fead13468d160773fe7753
---
A third_party/WebKit/Source/controller/BUILD.gn
A third_party/WebKit/Source/controller/ControllerExport.h
A third_party/WebKit/Source/controller/DEPS
A third_party/WebKit/Source/controller/OWNERS
A third_party/WebKit/Source/controller/README.md
M third_party/WebKit/Source/web/BUILD.gn
M third_party/WebKit/Source/web/DEPS
M third_party/WebKit/public/BUILD.gn
8 files changed, 105 insertions(+), 0 deletions(-)
To view, visit change 547379. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 20:
(2 comments)
Patch Set #18, Line 8:
#include "platform/wtf/Compiler.h"This looks unnecessary.
Ack
Patch Set #18, Line 21:
#endif // BLINK_CONTROLLER_IMPLEMENTATIONWrong comment.
Done
To view, visit change 547379. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 21:
I have split BlinkInitializer.cpp out of WebKit.cpp so that there is a non-empty
library to build from controller/. (mac and win both require this).
Kentaro, can you PTAL? Thanks in advance.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 23:Commit-Queue +1
Kent Tamura (Gerrit)
Kent Tamura posted comments on this change.
Patch set 23:
(4 comments)
Patch Set #23, Line 11:
Most of the remaining files in web/ will migrate to controller/ either as thetrailing whitespace
File third_party/WebKit/Source/controller/BlinkInitializer.cpp:
Patch Set #23, Line 1:
// Copyright 2017 The Chromium Authors. All rights reserved.Copyright header should be inherited from the original file.
Patch Set #23, Line 4:
#include "controller/BlinkInitializer.h"nit: Add a blank line before this.
File third_party/WebKit/Source/controller/ControllerExport.h:
Patch Set #23, Line 8:
#include "platform/wtf/Compiler.h"This #include looks unnecessary.
To view, visit change 547379. To unsubscribe, visit settings.
Kentaro Hara (Gerrit)
Kentaro Hara posted comments on this change.
Patch set 23:
LGTM
(1 comment)
File third_party/WebKit/Source/controller/BlinkInitializer.h:
Patch Set #23, Line 14:
CONTROLLER_EXPORT void InitializeBlink(Platform*);Add a comment like:
The embedder must call this function before starting using Blink.
To view, visit change 547379. To unsubscribe, visit settings.
Kentaro Hara (Gerrit)
If you decided to go with a totally different approach, you can create a new CL and abandon the old CL.
Otherwise, previous comments / LGs make reviewing confusing.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 24:
Patch Set 23:
If you decided to go with a totally different approach, you can create a new CL and abandon the old CL.
Otherwise, previous comments / LGs make reviewing confusing.
The approach is mostly the same.
(3 comments)
Patch Set #23, Line 1:
// Copyright 2017 The Chromium Authors. All rights reserved.Copyright header should be inherited from the original file.
Ack
nit: Add a blank line before this.
Done
File third_party/WebKit/Source/controller/ControllerExport.h:
Patch Set #23, Line 8:
#include "platform/wtf/Compiler.h"This #include looks unnecessary.
It is needed for the macros
To view, visit change 547379. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 24:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot posted comments on this change.
Patch set 24:
CQ is trying da patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"format and comments" https://chromium-review.googlesource.com/c/547379/24
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/547379/24
Bot data: {"action": "start", "triggered_at": "2017-07-04T06:36:00.0Z", "cq_cfg_revision": "1aa54ba037e9058c2752e0a0d871aeabfcba5974", "revision": "5a4f8d492feb051f8805ea6f26e7c8fbc47b087c"}
Jochen Eisinger (Gerrit)
Jochen Eisinger posted comments on this change.
Patch set 24:
Isn't this a phase 2 project? Why do we start this before phase 1 is done?
Kentaro Hara (Gerrit)
Kentaro Hara posted comments on this change.
Patch set 24:
Patch Set 24:
Isn't this a phase 2 project? Why do we start this before phase 1 is done?
The DevTools team strongly disagreed with merging WebDevToolsAgent to core/ because it's a thing that should be moved to controller/. So we decided to create controller/ and move WebDevToolsAgent to it.
We're not planning to actively move things to controller/ until the phase 1 is done.
Kent Tamura (Gerrit)
Kent Tamura posted comments on this change.
Patch set 24:
(1 comment)
Patch Set #23, Line 8:
#include "platform/wtf/Compiler.h"
It is needed for the macros
What macro needs Compiler.h?
To view, visit change 547379. To unsubscribe, visit settings.
Kent Tamura (Gerrit)
Kent Tamura posted comments on this change.
Patch set 24:Code-Review -1
-1 for the copyright header issue
Commit Bot (Gerrit)
Commit Bot posted comments on this change.
Patch set 24:
CL must be approved before triggering CQ
Bot data: {"action": "cancel", "triggered_at": "2017-07-04T06:36:00.0Z", "cq_cfg_revision": "1aa54ba037e9058c2752e0a0d871aeabfcba5974", "revision": "5a4f8d492feb051f8805ea6f26e7c8fbc47b087c"}
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 24:
Patch Set 24: Code-Review-1
-1 for the copyright header issue
Could you please clarify: do you want the whole old-style copyright header from WebKit.cpp with 2009 as the year, or do you want the new style with 2009 as the year?
Kent Tamura (Gerrit)
Kent Tamura posted comments on this change.
Patch set 24:
Could you please clarify: do you want the whole old-style copyright header from WebKit.cpp with 2009 as the year, or do you want the new style with 2009 as the year?
Please copy the copyright header of WebKit.cpp to BlinkInitializer.cpp.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 24:
Patch Set 24:
(1 comment)
You're right. It was incorrectly included in WebExport.h AFAICT, and that was my model. Sorry for the resistance.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 25:Commit-Queue +1
Nicholas Verne (Gerrit)
Kent, could you please revoke your -1? I think Gerrit will not allow the CL to be submitted if there's a -1
Kent Tamura (Gerrit)
Kent Tamura posted comments on this change.
Patch set 25:Code-Review +1
(2 comments)
Patch Set #25, Line 11:
Most of the remaining files in web/ will migrate to controller/ either as theRemove a trailing whitespace.
File third_party/WebKit/Source/web/WebFactoryImpl.h:
this blank line is unnecessary.
To view, visit change 547379. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne uploaded patch set #26 to this change.
Create controller/ component in blink.
This commences the new blink architecture outlined in Onion Soup 2.0
Most of the remaining files in web/ will migrate to controller/ either as the
existing Web* classes, or as controller-specific refactorings. The controller/
directory is not intended as a dumping ground.
Bug:738768
Change-Id: I7dbe02690ca3a90c08fead13468d160773fe7753
---
A third_party/WebKit/Source/controller/BUILD.gn
A third_party/WebKit/Source/controller/BlinkInitializer.cpp
A third_party/WebKit/Source/controller/BlinkInitializer.h
A third_party/WebKit/Source/controller/ControllerExport.h
A third_party/WebKit/Source/controller/DEPS
A third_party/WebKit/Source/controller/OWNERS
A third_party/WebKit/Source/controller/README.md
M third_party/WebKit/Source/web/BUILD.gn
M third_party/WebKit/Source/web/DEPS
M third_party/WebKit/Source/web/WebFactoryImpl.h
M third_party/WebKit/Source/web/WebKit.cpp
M third_party/WebKit/public/BUILD.gn
12 files changed, 218 insertions(+), 41 deletions(-)
To view, visit change 547379. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 27:
(2 comments)
Patch Set #25, Line 11:
Most of the remaining files in web/ will migrate to controller/ either as theRemove a trailing whitespace.
Done
File third_party/WebKit/Source/web/WebFactoryImpl.h:
Patch Set #25, Line 9:
#include "web/WebExport.h"
this blank line is unnecessary.
Done
To view, visit change 547379. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 27:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot posted comments on this change.
Patch set 27:
CQ is trying da patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"formatting" https://chromium-review.googlesource.com/c/547379/27
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/547379/27
Bot data: {"action": "start", "triggered_at": "2017-07-04T22:49:15.0Z", "cq_cfg_revision": "1aa54ba037e9058c2752e0a0d871aeabfcba5974", "revision": "285b78fe2c885a52ea755decf6fef77f223829c5"}
Commit Bot (Gerrit)
Commit Bot merged this change.
Create controller/ component in blink.
This commences the new blink architecture outlined in Onion Soup 2.0
Most of the remaining files in web/ will migrate to controller/ either as the
existing Web* classes, or as controller-specific refactorings. The controller/
directory is not intended as a dumping ground.
Bug: 738768
Change-Id: I7dbe02690ca3a90c08fead13468d160773fe7753
Reviewed-on: https://chromium-review.googlesource.com/547379
Commit-Queue: Nicholas Verne <nve...@chromium.org>
Reviewed-by: Kent Tamura <tk...@chromium.org>
Reviewed-by: Kentaro Hara <har...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484172}
---
A third_party/WebKit/Source/controller/BUILD.gn
A third_party/WebKit/Source/controller/BlinkInitializer.cpp
A third_party/WebKit/Source/controller/BlinkInitializer.h
A third_party/WebKit/Source/controller/ControllerExport.h
A third_party/WebKit/Source/controller/DEPS
A third_party/WebKit/Source/controller/OWNERS
A third_party/WebKit/Source/controller/README.md
M third_party/WebKit/Source/web/BUILD.gn
M third_party/WebKit/Source/web/DEPS
M third_party/WebKit/Source/web/WebFactoryImpl.h
M third_party/WebKit/Source/web/WebKit.cpp
M third_party/WebKit/public/BUILD.gn
12 files changed, 217 insertions(+), 41 deletions(-)