Moves WebDevToolsAgentImpl to core. This required removing direct calls to modules/ agents. Instead, these agents register a callback from ModulesInitializer::Initialize [chromium/src : master]
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 1:
There are problems I need help with in this CL:
SessionInitCallbackArgs are using raw pointers to GC'd types. But to use Member<> means that the complete type must be known where this struct is declared. What do we usually have to do here?
Also, we intend to move (eventually) WebDevToolsAgentImpl into controller/, which (IIUC) should be able to see modules/. Is the callback mechanism worth it? It was cheap to insert and should be cheap to remove later.
There are still visibility issues with WebLocalFrameBase.h being used from core/. Stuart?
To view, visit change 544726. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne would like Stuart Langley, Kentaro Hara and Daniel Cheng to review this change.
Moves WebDevToolsAgentImpl to core.
This required removing direct calls to modules/ agents.
Instead, these agents register a callback from ModulesInitializer::Initialize
Bug:712963
Change-Id: I9159327618118b2a2f652ed15a76271040b5bc30
---
M third_party/WebKit/Source/core/exported/BUILD.gn
R third_party/WebKit/Source/core/exported/WebDevToolsAgentImpl.cpp
R third_party/WebKit/Source/core/exported/WebDevToolsAgentImpl.h
A third_party/WebKit/Source/core/exported/WebViewBase.cpp
M third_party/WebKit/Source/core/exported/WebViewBase.h
M third_party/WebKit/Source/core/inspector/BUILD.gn
A third_party/WebKit/Source/core/inspector/InspectorBaseAgent.cpp
M third_party/WebKit/Source/core/inspector/InspectorBaseAgent.h
M third_party/WebKit/Source/modules/ModulesInitializer.cpp
M third_party/WebKit/Source/web/BUILD.gn
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
14 files changed, 138 insertions(+), 53 deletions(-)
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 1:Commit-Queue +1
Kentaro Hara (Gerrit)
Kentaro Hara posted comments on this change.
Patch set 1:
Patch Set 1:
There are problems I need help with in this CL:
SessionInitCallbackArgs are using raw pointers to GC'd types. But to use Member<> means that the complete type must be known where this struct is declared. What do we usually have to do here?
Member should not need a complete type. Either way it's fine to use a raw pointer in STACK_ALLOCATED objects.
Also, we intend to move (eventually) WebDevToolsAgentImpl into controller/, which (IIUC) should be able to see modules/. Is the callback mechanism worth it? It was cheap to insert and should be cheap to remove later.There are still visibility issues with WebLocalFrameBase.h being used from core/. Stuart?
To view, visit change 544726. To unsubscribe, visit settings.
Stuart Langley (Gerrit)
Stuart Langley posted comments on this change.
Patch set 1:
> There are still visibility issues with WebLocalFrameBase.h being used from core/. Stuart?
Should not warn you for code in core/exported - if it did we might have the DEPS rules wrong (I think sasha had a patch to fix that might not have landed yet)
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 1:
Patch Set 1:
Patch Set 1:
There are problems I need help with in this CL:
SessionInitCallbackArgs are using raw pointers to GC'd types. But to use Member<> means that the complete type must be known where this struct is declared. What do we usually have to do here?
Member should not need a complete type. Either way it's fine to use a raw pointer in STACK_ALLOCATED objects.
Also, we intend to move (eventually) WebDevToolsAgentImpl into controller/, which (IIUC) should be able to see modules/. Is the callback mechanism worth it? It was cheap to insert and should be cheap to remove later.
There are still visibility issues with WebLocalFrameBase.h being used from core/. Stuart?
You are right. I confused myself by trying (during development) to make SessionInitArgs garbage collected.
Sasha Morrissey (Gerrit)
Sasha Morrissey posted comments on this change.
Patch set 2:Code-Review +1
LGTM
Yeah the DEPS fix is here - https://chromium-review.googlesource.com/c/542556/3/third_party/WebKit/Source/core/exported/DEPS
Been trying to land this for 3 days :'( It'll get there eventually, I have other stuff blocked on it as well
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 2:Commit-Queue +1
Nicholas Verne (Gerrit)
Patch Set 1:
Patch Set 1:
There are problems I need help with in this CL:
SessionInitCallbackArgs are using raw pointers to GC'd types. But to use Member<> means that the complete type must be known where this struct is declared. What do we usually have to do here?
Member should not need a complete type. Either way it's fine to use a raw pointer in STACK_ALLOCATED objects.
Also, we intend to move (eventually) WebDevToolsAgentImpl into controller/, which (IIUC) should be able to see modules/. Is the callback mechanism worth it? It was cheap to insert and should be cheap to remove later.There are still visibility issues with WebLocalFrameBase.h being used from core/. Stuart?
PTAL
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 3:Commit-Queue +1
Dmitry Gozman (Gerrit)
Dmitry Gozman posted comments on this change.
Patch set 3:
(1 comment)
Patch Set #3, Line 8:
This required removing direct calls to modules/ agents.Didn't we agree that abstracting this class from modules is against the strategy, since it should be in controller which depends on core+modules? Looks like something we are going to undo this work, so why do it now?
To view, visit change 544726. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 3:
Patch Set 3:
(1 comment)
It would have been nice to go to that meeting.
Nicholas Verne (Gerrit)
I'll work up a CL that introduces src/controller/ and plumbs it into the various builds. This is quite a high risk CL (to a newbie like me) :-)
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 3:Commit-Queue +1
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 4:Commit-Queue +1
Nicholas Verne (Gerrit)
After clarifying with slangley@ and sashab@ who were at the meeting.
- we will proceed with them move to core/ before moving back some files to controller/
- this is because the moves to controller/ won't just be file moves. In some cases, the classes will split into core and controller portions through inheritance or composition
- my attempt to create a controller/ directory first then move WebDevToolsAgent* there is far too heavyweight for a single CL because it requires refactoring of several classes as mentioned above, as well as setting up BUILD, DEPS, etc.
I still want to submit this CL.
Stuart Langley (Gerrit)
Stuart Langley posted comments on this change.
Patch set 4:
Patch Set 4:
After clarifying with slangley@ and sashab@ who were at the meeting.
- we will proceed with them move to core/ before moving back some files to controller/
- this is because the moves to controller/ won't just be file moves. In some cases, the classes will split into core and controller portions through inheritance or composition
- my attempt to create a controller/ directory first then move WebDevToolsAgent* there is far too heavyweight for a single CL because it requires refactoring of several classes as mentioned above, as well as setting up BUILD, DEPS, etc.
I still want to submit this CL.
+1
The SYD team will have Q3 OKRs for creating controller and start factoring controllers out of core/exported/ into the controller layer. There's also design work being undertaken to see if we can rationalize the modules constructor pattern, so the work here is really temporary and required to unblock the work that will end up creating and populating the control layer.
Kentaro Hara (Gerrit)
Kentaro Hara posted comments on this change.
Patch set 4:
Patch Set 4:
Patch Set 4:
After clarifying with slangley@ and sashab@ who were at the meeting.
- we will proceed with them move to core/ before moving back some files to controller/
- this is because the moves to controller/ won't just be file moves. In some cases, the classes will split into core and controller portions through inheritance or composition
- my attempt to create a controller/ directory first then move WebDevToolsAgent* there is far too heavyweight for a single CL because it requires refactoring of several classes as mentioned above, as well as setting up BUILD, DEPS, etc.
I still want to submit this CL.
+1
The SYD team will have Q3 OKRs for creating controller and start factoring controllers out of core/exported/ into the controller layer. There's also design work being undertaken to see if we can rationalize the modules constructor pattern, so the work here is really temporary and required to unblock the work that will end up creating and populating the control layer.
Yeah, the plan sounds reasonable.
I don't want to make controller/ a "catch-all" directory, so I'd prefer not moving things in controller/ until the things are well split between the part for core/ and the part for controller/.
Also note that core/ cannot depend on core/exported/. Even if we move WebDevToosAgent* to core/exported/, that doesn't mean that core/ may start using it.
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
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 7:Commit-Queue +1
Sasha Morrissey (Gerrit)
Sasha Morrissey posted comments on this change.
Patch set 7:
Patch Set 4:
Patch Set 4:
Patch Set 4:
After clarifying with slangley@ and sashab@ who were at the meeting.
- we will proceed with them move to core/ before moving back some files to controller/
- this is because the moves to controller/ won't just be file moves. In some cases, the classes will split into core and controller portions through inheritance or composition
- my attempt to create a controller/ directory first then move WebDevToolsAgent* there is far too heavyweight for a single CL because it requires refactoring of several classes as mentioned above, as well as setting up BUILD, DEPS, etc.
I still want to submit this CL.
+1
The SYD team will have Q3 OKRs for creating controller and start factoring controllers out of core/exported/ into the controller layer. There's also design work being undertaken to see if we can rationalize the modules constructor pattern, so the work here is really temporary and required to unblock the work that will end up creating and populating the control layer.
Yeah, the plan sounds reasonable.
I don't want to make controller/ a "catch-all" directory, so I'd prefer not moving things in controller/ until the things are well split between the part for core/ and the part for controller/.
Also note that core/ cannot depend on core/exported/. Even if we move WebDevToosAgent* to core/exported/, that doesn't mean that core/ may start using it.
+1, this matches my understanding of the plan forward too.
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)
Nicholas Verne posted comments on this change.
Patch set 11:Commit-Queue +1
Pavel Feldman (Gerrit)
Pavel Feldman posted comments on this change.
Patch set 11:Code-Review -1
As a core/ owner and DevTools code owner, I don't think this move is appropriate course of action. This class needs to stay in web/ being re-targeted to become controller/ as agreed on the meeting. What differs it from the other classes being moved from web/ to core/exported is the /modules dependency: in this case InspectorAgent gets explicit callback hook for modules for reversing the dependency. This contradicts the spirit of Daniel's summary statement from the discussion.
My position stays the same: moving web/ to core/exported is artificial and does not add value. The team admits that the meaningful follow up is scheduled for Q3, which I think we could started with. Steps that have no value should not be taken or should be taken with minimal damage. Simply moving web/ to core/exported could be considered minimal damage, but mixing modules deps into the picture should not be.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 12:
Patch Set 11: Code-Review-1
As a core/ owner and DevTools code owner, I don't think this move is appropriate course of action. This class needs to stay in web/ being re-targeted to become controller/ as agreed on the meeting. What differs it from the other classes being moved from web/ to core/exported is the /modules dependency: in this case InspectorAgent gets explicit callback hook for modules for reversing the dependency. This contradicts the spirit of Daniel's summary statement from the discussion.
My position stays the same: moving web/ to core/exported is artificial and does not add value. The team admits that the meaningful follow up is scheduled for Q3, which I think we could started with. Steps that have no value should not be taken or should be taken with minimal damage. Simply moving web/ to core/exported could be considered minimal damage, but mixing modules deps into the picture should not be.
"This contradicts the spirit of Daniel's summary statement from the discussion."
Care to share it with me?
"Steps that have no value should not be taken or should be taken with minimal damage. Simply moving web/ to core/exported could be considered minimal damage, but mixing modules deps into the picture should not be."
That's a matter of opinion, and I don't agree FWIW. However, since your position is one of uncompromising obstruction ( ;-) </joke>), would you care to take a look at https://chromium-review.googlesource.com/c/547379/6
Kentaro Hara (Gerrit)
Kentaro Hara posted comments on this change.
Patch set 12:
Pavel: Do you think all the code in WebDevToolsAgentImpl should live in controller/ in the end? If yes, I agree that we should move it to controller/ now. If no, I'd prefer moving it to core/exported/ and then move the controller part to controller/. I'd like to avoid making controller/ a "catch-all" directory.
Stuart Langley (Gerrit)
Stuart Langley posted comments on this change.
Patch set 12:
Patch Set 12:
Pavel: Do you think all the code in WebDevToolsAgentImpl should live in controller/ in the end? If yes, I agree that we should move it to controller/ now. If no, I'd prefer moving it to core/exported/ and then move the controller part to controller/. I'd like to avoid making controller/ a "catch-all" directory.
Note: WDTAI relies on LocalFrameClientImpl to break layering encapsulation to allow core/ to call WDTAI - so presumably this needs some kind of fix.
Nicholas Verne (Gerrit)
Nicholas Verne abandoned this change.
To view, visit change 544726. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Pavel Feldman (Gerrit)
Pavel Feldman posted comments on this change.
Patch set 13:-Code-Review
(1 comment)
File third_party/WebKit/Source/modules/ModulesInitializer.cpp:
Patch Set #13, Line 183:
InspectorAgent::RegisterSessionInitCallback(Please add a FIXME with the link to the bug that explains how this is going to be removed and when.
To view, visit change 544726. To unsubscribe, visit settings.
Kentaro Hara (Gerrit)
Kentaro Hara posted comments on this change.
Patch set 13:
Mostly looks good.
(5 comments)
File third_party/WebKit/Source/core/exported/WebDevToolsAgentImpl.cpp:
Patch Set #13, Line 386:
InspectorAgent::CallSessionInitCallbacks(session, &session_init_args);Nit: Instead of passing InspectorAgent::SessionInitArgs, I'd prefer simply passing the three parameters.
File third_party/WebKit/Source/core/exported/WebViewBase.cpp:
Why do you need to move line 34 - 49 to WebViewBase.cpp?
File third_party/WebKit/Source/core/inspector/InspectorBaseAgent.cpp:
Patch Set #13, Line 13:
WTF::Vector<InspectorAgent::SessionInitCallback>;WTF:: is not needed.
Patch Set #13, Line 26:
for (auto& cb : GetSessionInitCallbackVector()) {cb => callback
File third_party/WebKit/Source/web/WebViewImpl.h:
What is this change for?
Would you help me understand when we need to add NON_EXPORTED_BASE?
To view, visit change 544726. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 15:Commit-Queue +1
Nicholas Verne (Gerrit)
(6 comments)
Patch Set #13, Line 386:
InspectorAgent::CallSessionInitCallbacks(session, &session_init_args);Nit: Instead of passing InspectorAgent::SessionInitArgs, I'd prefer simply
Ack. There are 4 parameters (.page too). But we intend to delete this code ASAP, so it shouldn't matter! May I leave it as is?
File third_party/WebKit/Source/core/exported/WebViewBase.cpp:
Why do you need to move line 34 - 49 to WebViewBase.cpp?
WebDevToolsAgentImpl makes use of AllInstances. While I was making WebViewBase.cpp, it seemed sensible to move UseExternalPopupMenus, but not strictly necessary. I'll move them back to WebViewImpl.cpp if you like.
File third_party/WebKit/Source/core/inspector/InspectorBaseAgent.cpp:
Patch Set #13, Line 13:
SessionInitCallbackVector& GetSessionInitCallbackVector() {WTF:: is not needed.
Done
Patch Set #13, Line 26:
callback(session, args);cb => callback
Done
File third_party/WebKit/Source/modules/ModulesInitializer.cpp:
Patch Set #13, Line 183:
// FIXME remove this and restore to WebDevToolsAgentImpl once that
Please add a FIXME with the link to the bug that explains how this is going
What is this change for?
IIRC without this I got failures in the Windows builds. Now there's real code for WebViewBase statics in another lib.
To view, visit change 544726. To unsubscribe, visit settings.
Kentaro Hara (Gerrit)
Kentaro Hara posted comments on this change.
Patch set 15:
Please fix the format of the CL description :)
(2 comments)
Patch Set #13, Line 386:
InspectorAgent::CallSessionInitCallbacks(session, &session_init_args);
Ack. There are 4 parameters (.page too). But we intend to delete this code
It's not really common in Blink to use a struct to pass a couple of parameters to one function.
Even if the code is tentative, we should avoid not land half-baked code on ToT.
File third_party/WebKit/Source/core/exported/WebViewBase.cpp:
WebDevToolsAgentImpl makes use of AllInstances. While I was making WebViewB
Yeah, that sounds nicer.
To view, visit change 544726. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 16:Commit-Queue +1
Nicholas Verne (Gerrit)
Nicholas Verne uploaded patch set #19 to this change.
Moves WebDevToolsAgentImpl to core.
WebDevToolsAgentImpl uses many InspectorAgents that are implemented in modules/.
To enable these agents' use from core/ we now register a callback from
ModulesInitializer::Initialize that can append the modules/ InspectorAgents
to an InspectorSession.
Since WebDevToolsAgentImpl needs to use static methods of WebView and
WebViewBase directly, these need to be availble in core/ as well. We created
WebViewBase.cpp and moved the definitions there. Also, to keep Windows happy
WebViewBase is now a NON_EXPORTED_BASE of WebViewImpl, since there is now
code for WebViewBase static methods in different library and Windows otherwise
fails to link
warning C4275: non dll-interface class 'blink::WebViewBase' used as base for
dll-interface class 'blink::WebViewImpl'
Bug:712963
Change-Id: I9159327618118b2a2f652ed15a76271040b5bc30
---
M third_party/WebKit/Source/core/exported/BUILD.gn
R third_party/WebKit/Source/core/exported/WebDevToolsAgentImpl.cpp
R third_party/WebKit/Source/core/exported/WebDevToolsAgentImpl.h
A third_party/WebKit/Source/core/exported/WebViewBase.cpp
M third_party/WebKit/Source/core/exported/WebViewBase.h
M third_party/WebKit/Source/core/inspector/BUILD.gn
A third_party/WebKit/Source/core/inspector/InspectorBaseAgent.cpp
M third_party/WebKit/Source/core/inspector/InspectorBaseAgent.h
M third_party/WebKit/Source/modules/ModulesInitializer.cpp
M third_party/WebKit/Source/web/BUILD.gn
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
14 files changed, 147 insertions(+), 61 deletions(-)
To view, visit change 544726. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 19:
(1 comment)
IIRC without this I got failures in the Windows builds. Now there's real co
Without NON_EXPORTED_BASE (which just suppresses the warning), you get
../../third_party/WebKit/Source\web/WebViewImpl.h(101): warning C4275: non dll-interface class 'blink::WebViewBase' used as base for dll-interface class 'blink::WebViewImpl'i.e. if you export a class, you need to export its bases if there is actual code in the base class (e.g. static functions) coming from another dll (in this case, core/). Since AllInstances had to move to core/, it became necessary.
To view, visit change 544726. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Patch Set #13, Line 386:
InspectorAgent::CallSessionInitCallbacks(session, &session_init_args);
It's not really common in Blink to use a struct to pass a couple of paramet
Interesting. I always found long parameter lists more error prone, especially when you
can't name the params in the callback typedef.e.g. using Callback = void(int, int, int, bool, bool)
allows me to pass the wrong parameter in the wrong position without noticing. I've seen a number of long-standing, subtle bugs from this in other projects. I'm far less likely to make a mistake if the params are in a struct with sensible names. I can even set the members in any order and still know the code will be correct.
To view, visit change 544726. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 20:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot posted comments on this change.
Patch set 20:
CQ is trying da patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"No SessionInitArgs" https://chromium-review.googlesource.com/c/544726/20
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/544726/20
Bot data: {"action": "start", "triggered_at": "2017-07-15T07:27:11.0Z", "cq_cfg_revision": "63e34448474381620aa6fce8c68b7f2c4ced7b75", "revision": "ab57b7532e64626b0c60d75d7bad3954d9f96255"}
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/490700)
Bot data: {"action": "cancel", "triggered_at": "2017-07-15T07:27:11.0Z", "cq_cfg_revision": "63e34448474381620aa6fce8c68b7f2c4ced7b75", "revision": "ab57b7532e64626b0c60d75d7bad3954d9f96255"}
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 20:Commit-Queue +1
Kentaro Hara (Gerrit)
Kentaro Hara posted comments on this change.
Patch set 20:Code-Review +1
LGTM
(2 comments)
File third_party/WebKit/Source/core/inspector/InspectorBaseAgent.h:
Patch Set #20, Line 75:
Member<Page>);Use raw pointers (instead of Members) for on-stack pointers.
File third_party/WebKit/Source/modules/ModulesInitializer.cpp:
Patch Set #20, Line 183:
// FIXME remove this and restore to WebDevToolsAgentImpl once thatTODO(nverne):
To view, visit change 544726. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 21:
(2 comments)
Use raw pointers (instead of Members) for on-stack pointers.
Done
File third_party/WebKit/Source/modules/ModulesInitializer.cpp:
Patch Set #20, Line 183:
// TODO(nverne): remove this and restore to WebDevToolsAgentImpl once thatTODO(nverne):
Done
To view, visit change 544726. To unsubscribe, visit settings.
Nicholas Verne (Gerrit)
Nicholas Verne posted comments on this change.
Patch set 21:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot posted comments on this change.
Patch set 21:
CQ is trying da patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"one more Member" https://chromium-review.googlesource.com/c/544726/21
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/544726/21
Bot data: {"action": "start", "triggered_at": "2017-07-15T23:11:53.0Z", "cq_cfg_revision": "63e34448474381620aa6fce8c68b7f2c4ced7b75", "revision": "567502261014c2a1cd263f792d41af3d3e48e171"}
Commit Bot (Gerrit)
Commit Bot merged this change.
Moves WebDevToolsAgentImpl to core.
WebDevToolsAgentImpl uses many InspectorAgents that are implemented in modules/.
To enable these agents' use from core/ we now register a callback from
ModulesInitializer::Initialize that can append the modules/ InspectorAgents
to an InspectorSession.
Since WebDevToolsAgentImpl needs to use static methods of WebView and
WebViewBase directly, these need to be availble in core/ as well. We created
WebViewBase.cpp and moved the definitions there. Also, to keep Windows happy
WebViewBase is now a NON_EXPORTED_BASE of WebViewImpl, since there is now
code for WebViewBase static methods in different library and Windows otherwise
fails to link
warning C4275: non dll-interface class 'blink::WebViewBase' used as base for
dll-interface class 'blink::WebViewImpl'
Bug: 712963
Change-Id: I9159327618118b2a2f652ed15a76271040b5bc30
Reviewed-on: https://chromium-review.googlesource.com/544726
Commit-Queue: Nicholas Verne <nve...@chromium.org>
Reviewed-by: Kentaro Hara <har...@chromium.org>
Reviewed-by: Sasha Morrissey <sas...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487003}
---
M third_party/WebKit/Source/core/exported/BUILD.gn
R third_party/WebKit/Source/core/exported/WebDevToolsAgentImpl.cpp
R third_party/WebKit/Source/core/exported/WebDevToolsAgentImpl.h
A third_party/WebKit/Source/core/exported/WebViewBase.cpp
M third_party/WebKit/Source/core/exported/WebViewBase.h
M third_party/WebKit/Source/core/inspector/BUILD.gn
A third_party/WebKit/Source/core/inspector/InspectorBaseAgent.cpp
M third_party/WebKit/Source/core/inspector/InspectorBaseAgent.h
M third_party/WebKit/Source/modules/ModulesInitializer.cpp
M third_party/WebKit/Source/web/BUILD.gn
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
14 files changed, 143 insertions(+), 61 deletions(-)