cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jesse <purplecabb...@gmail.com>
Subject Re: [Android] Third Party WebView Reference Implementation
Date Wed, 01 Oct 2014 22:04:40 GMT
Personally, I don't think Joe is way out of line here.  His wording may be
strong, and he is definitely passionate, but I would not characterize this
as slander. That said, I do work with Joe regularly, and I am used to his
tone.  I also think he tends to be vilified here, but this is all off
topic. Bottom line is, any change that impacts a (previously) public API
should probably be discussed/reviewed in a feature branch, especially in
Android land, as it the most turbulent.

Interfaces are generally a good way of defining the functionality that
*must* be exposed as a contract, and, as Sergey points out, we probably
need multiple interfaces. But I think an interface is the right choice.

Joe, is it really a requirement to have multiple cordova views co-exist at
runtime?  I would expect that the choice to use Mozilla-WebView or Chrome,
or cross-walk ... would be used for the entire app.
The entire plugin system is based on there only ever being one
cordova_plugins.js and one config.xml, so I am not sure how you would even
use multiple views in this case.



@purplecabbage
risingj.com

On Wed, Oct 1, 2014 at 2:19 PM, Michal Mocny <mmocny@chromium.org> wrote:

> On Wed, Oct 1, 2014 at 4:53 PM, Joe Bowser <bowserj@gmail.com> wrote:
>
> > On Wed, Oct 1, 2014 at 1:18 PM, Andrew Grieve <agrieve@chromium.org>
> > wrote:
> >
> > > On Wed, Oct 1, 2014 at 11:07 AM, Joe Bowser <bowserj@gmail.com> wrote:
> > >
> > > > On Wed, Oct 1, 2014 at 7:24 AM, Andrew Grieve <agrieve@chromium.org>
> > > > wrote:
> > > >
> > > > > Good idea (I think)!
> > > > >
> > > > > In my mind there's two main things wrong with our current
> > abstractions:
> > > > > 1. WebView plugins should extend a class instead of implement an
> > > > interface.
> > > > >   - Reason: You can't make *any* changes to an interface without
> > > breaking
> > > > > the compile for everyone that implements it
> > > > >
> > > >
> > > > Reason we use an interface: Java does not support multiple
> inheritance.
> > > > Both Ian and I have said this numerous times, and you wishing that
> this
> > > > wasn't the case isn't going to magically make this change.  This is
> the
> > > > only way this feature is even possible.  Also, this is an API that
> > we're
> > > > designing, and we shouldn't be messing with the interface.  We're
> stuck
> > > > with a lot of WebView-isms that may not even make sense on
> non-Chromium
> > > > webviews, but I think the surface has been decreased.
> > > >
> > >
> > > I don't think we need multiple inheritance. What would you want to
> > inherit
> > > from?
> > >
> >
> > AndroidWebView inherits form WebView, and the Mozilla WebView that I'm
> > working on inherits from GeckoView.  They both can be used as their own
> > views, and most of our tests still work.  By removing the interface and
> > saying that things aren't views anymore breaks the WebView feature that
> > we've worked on two releases ago that some people still use.  Sure, you
> > don't use it, and you have made it clear that you don't care about these
> > users, but we need to have some sort of consistency.  I'd rather have
> >
>
> I don't think that was made clear at all.  Andrew asked a valid question --
> is it impossible to have a civil tech discussion without being chewed out
> like this?  I won't comment on every other reply below, but generally
> countering disagreements with slander is just not a convincing argument.
>
>
> > people just rename a class than lose an entire feature because you don't
> > think it's important.
> >
> >
> > > I think it's unrealistic to think that we'll never want to add any
> > methods
> > > or parameters to the interface.
> > >
> > >
> > And this thinking is why Cordova is so terribly unstable, and why quality
> > has gone down since 3.0.  For all the talk about not changing APIs and
> not
> > screwing over our users, you seriously managed to do exactly that with
> your
> > refactors that you did on master without using a topic branch.  That's
> > exactly why we had to re-do the tag at 3.6.4, because Marcel and Martin
> ran
> > into problems with your refactor.  This is the line in the sand, this is
> an
> > API that we want third-party people to use, we can't keep making changes
> to
> > it in minor versions.
>
>
> >
> > >
> > >
> > > >
> > > > 2. We need to reduce the amount of copy & pasted code between the
> > > > > implementations.
> > > > >
> > > >
> > > > Given that this is literally the AndroidWebView that exists in the
> > 4.0.x
> > > > repo ripped out, this is copy and pasted code so that it actually
> > works.
> > > >
> > >
> > > To clarify: I think the distinction should be "what code is specific to
> > my
> > > webview" vs. "what code is specific to any webview within Cordova". And
> > > right now there's a good amount of Cordova-specific code in the
> > cross-walk
> > > and AndroidWebView code.
> > >
> > >
> > There's a lot of code that needs to be in a WebView for Cordova.  This is
> > because of the numerous use cases, such as multiple WebViews, where each
> > instance needs to keep track of its own set of plugins, for starters.
> > There are probably more use cases that need to be though of, but
> expecting
> > there to be no Cordova code is ridiculous.  Also, part of the reason
> we're
> > doing a Mozilla WebView is because it's so radically different from the
> > Chromium based WebViews we can use it to re-think what is needed.
> >
> > Honestly, if you think you can do a better job on the API and keep some
> > backwards compatibility with the current API, spin up a topic branch, but
> > the 4.0.x branch already contains code that was approved months ago, and
> > your opposition to it now is ridiculous.  It's like you're trying to
> derail
> > the work because of some unknown reason, which wouldn't be a new thing at
> > all.
> >
> >
> > >
> > > >
> > > >
> > > > >
> > > > > On Tue, Sep 30, 2014 at 3:58 PM, Sergey Grebnov (Akvelon) <
> > > > > v-segreb@microsoft.com> wrote:
> > > > >
> > > > > > +1
> > > > > > It would be also great if there is a way to extend/override
some
> > > > default
> > > > > > webview functionality as well (not only provide full webview
> > > > > implementation)
> > > > > >
> > > > > > For example, what will be recommended way to extend default
> > > > > > onReceivedHttpAuthRequest functionality if I want to
> automatically
> > > show
> > > > > > credentials dialog in case of 401 response?
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/cordova-android/blob/master/framework/src/org/apache/cordova/CordovaWebViewClient.java#L116
> > > > > >
> > > > > > Thx!
> > > > > > Sergey
> > > > > > -----Original Message-----
> > > > > > From: Joe Bowser [mailto:bowserj@gmail.com]
> > > > > > Sent: Tuesday, September 30, 2014 10:44 PM
> > > > > > To: dev
> > > > > > Subject: Re: [Android] Third Party WebView Reference
> Implementation
> > > > > >
> > > > > > On Tue, Sep 30, 2014 at 10:42 AM, Michal Mocny <
> > mmocny@chromium.org>
> > > > > > wrote:
> > > > > >
> > > > > > > To make sure I understand, this is implementing the system
> > webview
> > > as
> > > > > > > a pluggable-webview-via-plugin?
> > > > > > >
> > > > > > >
> > > > > > Yes.  The idea was to use this to figure out our API surface
and
> to
> > > see
> > > > > > what's necessary and what's not.  Also, we need a way to test
> this
> > > > > without
> > > > > > having to rely on third party bits from Intel or anyone else.
> > While
> > > > it's
> > > > > > the same implementation, the fact is that it's a slightly
> different
> > > > class
> > > > > > so we could in theory test this and hopefully automate it.
> > > > > >
> > > > > >
> > > > > > > This, is interesting.
> > > > > > >
> > > > > > > On Tue, Sep 30, 2014 at 12:47 PM, Joe Bowser <
> bowserj@gmail.com>
> > > > > wrote:
> > > > > > >
> > > > > > > > Hey
> > > > > > > >
> > > > > > > > So, as part of the work for Third Party Webviews,
I decided
> to
> > > > write
> > > > > > > > up a super quick reference implementation by just
copying the
> > > > > > > > AndroidWebView
> > > > > > > and
> > > > > > > > making it a plugin.  I haven't fully tested this yet,
but it
> > > should
> > > > > > > > work
> > > > > > > as
> > > > > > > > a third-party WebView.
> > > > > > > >
> > > > > > > > The tricky part that I found here is how much of Cordova
we
> > have
> > > to
> > > > > > > > keep exposed for Third-Party WebViews to work.
> > > > > > > >
> > > > > > > > Anyway, the work is on GitHub with Apache Licences.
 Once it
> > gets
> > > > > > > massaged
> > > > > > > > a bit more, it may need a home at Apache.
> > > > > > > >
> > > > > > > > https://github.com/infil00p/ThirdPartyWebViewRef
> > > > > > > >
> > > > > > > > Feel free to fork it and go through it and put feedback
on
> this
> > > > > thread.
> > > > > > > > It's the same code as AndroidWebView in the 4.0.x
branch, so
> > some
> > > > of
> > > > > > > > this may be using exposed APIs out of convenience.
> > > > > > > >
> > > > > > > > Joe
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message