To Santos, Actually, Yangang and me is a peer of developers to do this work. So, I'd like to answer your questions.
Seems, the interfaces of "StorageMonitor" are invoked somewhere in code of Chrome, it's a module, is that right? But, the story of "Chromedriver" is different: (1) It's an independent program, communicating with Chrome through WebSocket (following some conventions of Devtools). (2) The "xwalkdriver" doesn't need to upgrade with "chromedriver", unless there is new "standard" defined by WebDriver and the implementation of "chromedriver" can be reused by "xwalkdriver" again. (3) It's an independent program, then why it exists in "../src/chrome" folder? I takes it as something like the test scripts. They are independent with Chrome in code, but logically coexistent, so they are arranged in "../src/chrome/test" folder. (4) Why not take existent code of "chromedriver" as a module, then extent it to implement "xwalkdriver"? Unfortunately, there are not obvious interfaces are exposed for that bulk of code, and we have to change something "from top to bottom" to implement out things also. And, in my personal opinion, it's not worth to summarize that bulk of code as a module, it's just a simple http server. It looks big just because there are many parallel "commands transaction" code. Actually, I can "copy" any http server code (e.g., Mongoose) to implement it, but why not? Part of existent code is working well, so I just modify and use it (of course with right license). Hope these resolve your doubts. Cheers, Peter Wang -----Original Message----- From: Santos, Thiago Sent: Friday, March 7, 2014 11:03 PM To: Alexis Menard; Han, YangangX Cc: Wang, Peter H; [email protected]; [email protected] Subject: Re: [Crosswalk-dev] Pull request for implementating xwalk WebDriver is submmited. On 07-03-2014 14:37, Alexis Menard wrote: > Hi, > > > On Fri, Mar 7, 2014 at 6:55 AM, Han, YangangX <[email protected] > <mailto:[email protected]>> wrote: > > Hi, all, > For the before commit is too huge to review. So after talked with > Halton & Yongsheng. > Peter and I have update the Crosswalk Web Driver > PR(_https://github.com/crosswalk-project/crosswalk/pull/1616_). > Please help to review it. If you have any question, Please comment it. > We split the patch into a series of small patches. Totally commit 8 > patches > 1 Code base of xwalk WebDriver, copied from > ../src/chrome/test/chromed > > > I haven't look the actual problem in details but hardcopy is a bad. My > quick look is that we're forking ~19000 LOC, this is simply a big > *no-go*. There is no maintenance plan on how to keep that updated with > upstream (for bug and security fixes) and I don't think we should > waste resources maintaining such a big directory. > > Experience showed that anything we fork as small as it is is a pain > (one recent tiny example gyp_xwalk which is a fork of gyp_chromium). > In some cases we do fork because it's in our interest and because > there is no other way (see for example Tizen support) but forking an > entire directory is no-go. > > This is not the approach we've been taking in Crosswalk up until now. > I myself enforced the rule (with some heated discussions granted) and > I believe it was for the good of the project. We can rebase in a > manageable way still. > > I don't think we should use the code in chrome/test/chromedriver as-is. > We do have a motivation on making it live outside the chrome/ > directory then let's make it a component and land it upstream in > components/ (or whatever is more suitable). Identify what are the > public interfaces, what pieces needs to stay in chrome/ and move the > rest in components/. I haven't study the feasibility of this but we > really should spend time on that because it's a far better approach. > We've been doing this with success on StorageMonitor and the NaCl > support. We did all the work upstream and waited it to arrive in a rebase to > start using it. > > I myself finished what Yael started on NaCl, we could have gone the > forked way as you guys are doing, it was simply unmanageable, Chromium > moves too fast. You're delaying the feature for few months but it's > better than a feature gets delayed than the entire project because we > have a bunch of forked code that we have to deal with at each rebase > (which will most likely break). > > Other fundamental mistake in that following approach is that you land > in the git history code depending on chrome/ layer (even in the > meantime) which is something we don't want in Crosswalk. > > Again I'm not opposed to the feature, thus I agree with the Intent to > Implement but I disagree with the way it's done. > If you have a look of how the StorageMonitor support is implemented right now on Crosswalk, it is a hybrid solution. Currently we are building the StorageMonitor files inside src/chrome as a component (check the patches on our Chrome tree), but I'm not moving them to src/components. It is just a buildsystem trick. Meanwhile, upstream, I did the actual move. I'm expecting an almost seamless transition next time we do a Chromium rebase and we can get rid of some custom patches on our tree. Maybe you can try to do something like that. Cheers, _______________________________________________ Crosswalk-dev mailing list [email protected] https://lists.crosswalk-project.org/mailman/listinfo/crosswalk-dev
