I reverted this, now I'm going to recommend that we fork off of e518eacbd for fixing this. That being said, things are going to get a lot uglier post 2.9, so we should probably take a 2.9pre branch and fix it here with the idea that the plugins have to use this. Does that make sense?
On Thu, Jun 6, 2013 at 11:01 AM, Joe Bowser <bows...@gmail.com> wrote: > I created a new bug, but yeah, it's the Media AutoTests in MobileSpec. > Debugging shows that they're being fed a URI that is missing a / > > https://issues.apache.org/jira/browse/CB-3628 > > On Thu, Jun 6, 2013 at 10:57 AM, Braden Shepherdson <bra...@chromium.org> > wrote: >> I'm prepared to have it reverted like it was on the 2.8.x branch. I'll >> revert the revert in a branch and try to fix them. I'm working with the app >> harness currently and it depends on the DataResource work, but I don't >> think it depends crucially on it. Even if it is vital, I could still fix it >> up in a branch and use that for our app harness work. >> >> It's not hard to revert it and put it back later, so I think we should go >> with that. I'll push it again when it's actually fixed. I agree with you >> that the unit tests are a good idea, as well. >> >> Joe, can you give me some test cases where it's failing? Is there a bug? I >> don't have a clear sense of the cases where it crashes or does the wrong >> thing. >> >> Braden >> >> >> On Thu, Jun 6, 2013 at 1:45 PM, Joe Bowser <bows...@gmail.com> wrote: >> >>> I'm OK with it staying in there if: >>> >>> 1. There's unit tests in the test directory to make sure it works. This >>> should be unit tested >>> 2. The unit tests in that directory pass >>> 3. All the existing MobileSpec tests pass. >>> >>> Now, I think that this is going to be tricky, which is why I want it out. >>> However, I don't know what the final decision on the plugins going in on >>> 2.9.0 or 3.0.0 is, so I don't know whether we should just suck it up and >>> fix it or rip it out yet. This code seems like it'd work fine for files, >>> but not for remote URIs, since that's where we're having the problems. >>> >>> Is that cool? >>> >>> On Jun 6, 2013 8:00 AM, "Braden Shepherdson" <bra...@chromium.org> wrote: >>> >>> > Its intention is to provide a single place for URL handling by plugins. >>> > Then plugins can capture new schemes, and handle URLs that have been >>> > modified by other plugins into URLs they know how to handle. It unifies >>> > that various bits of code in different places that knew how to handle >>> > gallery URLs and so on. >>> > >>> > If we want to revert it on master and only push it when we're sure it's >>> > working, I can take on the task of rehabilitating it eventually, or it >>> can >>> > wait until Andrew is back. It isn't vital to the app harness, though it >>> > prevents some things from working like app harness-hosted apps accessing >>> > gallery URLs. >>> > >>> > Braden >>> > >>> > >>> > On Wed, Jun 5, 2013 at 11:37 PM, Joe Bowser <bows...@gmail.com> wrote: >>> > >>> > > The reverts were only on the 2.8.0 branch, not on master. It's >>> > > currently totally broken right now. >>> > > >>> > > On Wed, Jun 5, 2013 at 8:09 PM, Michal Mocny <mmo...@chromium.org> >>> > wrote: >>> > > > 100 yard summary: our intern Shravan from last term was adding this >>> as >>> > > part >>> > > > of his app-harness work. This specific change landed a too hastily >>> as >>> > > > there were some issues in corner cases (perhaps over-eagerness due to >>> > > time >>> > > > pressure as he approach term end), but all actual uses of >>> DataResource >>> > > > should have been reverted before 2.8 branch (right?), and so just >>> idle >>> > > code >>> > > > remains in the codebase. The plan is to fix the remaining issues >>> > before >>> > > > re-adding its usage.. but Andrew was working on that, hence the >>> delay. >>> > > > >>> > > > The specifics details of why it has been added / what its used for, I >>> > > will >>> > > > defer to some others (Max/Braden?) who would know the answer. >>> > > > >>> > > > As far as I am aware, leaving it in isn't harmful, but perhaps >>> leaving >>> > it >>> > > > in unfixed in isn't helpful either. Lets see what Max/Braden say. >>> > > > >>> > > > >>> > > > On Wed, Jun 5, 2013 at 4:54 PM, Joe Bowser <bows...@gmail.com> >>> wrote: >>> > > > >>> > > >> Hey >>> > > >> >>> > > >> Why is DataResouce still in master? I don't want this code to go >>> into >>> > > >> 2.9.0 or 3.0.0, since I have no idea what this is trying to >>> > > >> accomplish. I'm going to start ripping it out of master tomorrow if >>> > > >> someone doesn't tell me why it should still be here. >>> > > >> >>> > > >> Seriously, WTF? >>> > > >> >>> > > >>> > >>>
