Bug 26905

Summary: Don't use PlatformMessagePortChannel in any files
Product: WebKit Reporter: John Abd-El-Malek <jam>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: atwilson, fishd, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Proposed patch
fishd: review-
updated changelog fishd: review+

John Abd-El-Malek
Reported 2009-07-01 14:26:35 PDT
This will allow Chromium port's version to be implemented outside the WebKit repository. The reason to do this is so that our version can derive from classes that are in the Chromium repository.
Attachments
Proposed patch (4.60 KB, patch)
2009-07-01 15:14 PDT, John Abd-El-Malek
fishd: review-
updated changelog (4.58 KB, patch)
2009-07-01 17:56 PDT, John Abd-El-Malek
fishd: review+
John Abd-El-Malek
Comment 1 2009-07-01 15:14:27 PDT
Created attachment 32146 [details] Proposed patch
Eric Seidel (no email)
Comment 2 2009-07-01 16:30:52 PDT
Seems kind strange. That will make it so that it's never possible to build a version of WebKit from the WebKit repository. That seems directly in contrast with historical goals...
Darin Fisher (:fishd, Google)
Comment 3 2009-07-01 16:53:16 PDT
Comment on attachment 32146 [details] Proposed patch > Index: WebCore/ChangeLog ... > + Reviewed by NOBODY (OOPS!). > + > + Small refactoring of MessagePortChannel so that a port's PlatformMessagePortChannel doesn't > + have to reside in the WebKit repository. I think it would be more accurate to say "so that PlatformMessagePortChannel may be defined at the WebKit layer" Otherwise, R=me
Sam Weinig
Comment 4 2009-07-01 17:15:31 PDT
Wouldn't that be a layering violation? Or are you considering adding a client interface for that?
John Abd-El-Malek
Comment 5 2009-07-01 17:56:15 PDT
Created attachment 32166 [details] updated changelog
John Abd-El-Malek
Comment 6 2009-07-01 17:59:21 PDT
Eric: I'm not sure I understand your concern. If you're referring to someone just going to webkit.org and fetching/building, that'll work as before. I just moved some functions around. What I did is equivalent to how ResourceHandleInternal is declared in ResourceHandle.h but it's up to the port to define the class. Sam: not sure what you mean?
Sam Weinig
Comment 7 2009-07-01 20:52:00 PDT
> Sam: not sure what you mean? WebCore classes should not depend on WebKit with the exception of client interfaces. If that does happen, that is considered a layering violation.
Darin Fisher (:fishd, Google)
Comment 8 2009-07-01 21:43:13 PDT
> WebCore classes should not depend on WebKit with the exception of client > interfaces. If that does happen, that is considered a layering violation. The Chromium port works differently to help avoid an extraneous number of interfaces (vtables) between WebKit and WebCore. Instead, what we do is specify a set of 'undefined' functions (see ChromiumBridge) and sometimes 'undefined' classes (ResourceHandle is a good example) that are left to the WebKit layer to define. In the WebKit layer, those undefined WebCore symbols and classes are implemented in terms of WebKit API interfaces (vtables). Note: This WebKit API I speak of is the Chromium WebKit API, which is still brewing in the Chromium repository (soon to be upstreamed to svn.webkit.org; see my post to webkit-dev if you are interested). Since only the Chromium port has need to route such things up through WebKit, this helps keep WebCore simple, while keeping the set of dependencies reasonably well scoped. If you think about it, this is actually very similar to the WebKitSystemInterface approach that PLATFORM(MAC) uses. That encapsulates the set of additional dependencies required by the PLATFORM(MAC) port of WebCore. In Chromium's case, we preferred to bridge up to the embedder instead of having the embedder also provide support below WebCore.
Darin Fisher (:fishd, Google)
Comment 9 2009-07-01 23:27:32 PDT
Sam Weinig
Comment 10 2009-07-01 23:45:07 PDT
(In reply to comment #8) > > WebCore classes should not depend on WebKit with the exception of client > > interfaces. If that does happen, that is considered a layering violation. > > The Chromium port works differently to help avoid an extraneous number of > interfaces (vtables) between WebKit and WebCore. Instead, what we do is > specify a set of 'undefined' functions (see ChromiumBridge) and sometimes > 'undefined' classes (ResourceHandle is a good example) that are left to the > WebKit layer to define. In the WebKit layer, those undefined WebCore symbols > and classes are implemented in terms of WebKit API interfaces (vtables). Note: > This WebKit API I speak of is the Chromium WebKit API, which is still brewing > in the Chromium repository (soon to be upstreamed to svn.webkit.org; see my > post to webkit-dev if you are interested). Since only the Chromium port has > need to route such things up through WebKit, this helps keep WebCore simple, > while keeping the set of dependencies reasonably well scoped. > > If you think about it, this is actually very similar to the > WebKitSystemInterface approach that PLATFORM(MAC) uses. That encapsulates the > set of additional dependencies required by the PLATFORM(MAC) port of WebCore. > In Chromium's case, we preferred to bridge up to the embedder instead of having > the embedder also provide support below WebCore. > While this bug is probably the wrong venue to have this discussion, I do want to state that I don't think ports making different architectural decisions is good idea for the health of the project. It is something I seriously object to. In previous discussions with members of the Chromium team it has been indicated that the bridge was a temporary measure and the existing forms of abstraction would be adopted and extended where necessary (glue on the bottom in WebCore/platform/ and glue on top in the form of client interfaces).
Note You need to log in before you can comment on or make changes to this bug.