Closed
Bug 1204965
Opened 9 years ago
Closed 9 years ago
Merge larch back to mozilla-central
Categories
(Firefox OS Graveyard :: General, defect)
Firefox OS Graveyard
General
Tracking
(firefox44 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: kgrandon, Assigned: kgrandon)
References
Details
Attachments
(1 file, 7 obsolete files)
(deleted),
patch
|
kgrandon
:
review+
|
Details | Diff | Splinter Review |
Filing a tracking bug to get larch merged back to m-c. I'm not yet sure if we want to track this in individual bugs, but opening this for discussion/tracking.
Assignee | ||
Comment 1•9 years ago
|
||
I have a current diff of all changes here: https://github.com/KevinGrandon/gecko-projects/compare/gecko_dev_master...KevinGrandon:larch
I'm also uploading a patch here with the full changes.
Assignee | ||
Comment 2•9 years ago
|
||
Fabrice - any opinion here? Are we ready to do this, and should we do so by landing individual bugs to m-c?
Flags: needinfo?(fabrice)
Comment 3•9 years ago
|
||
I'm not too worried about the code merge itself, but we need to figure out:
- releng builds.
- can we run some/all of the browser.html tests? are there any?
Flags: needinfo?(fabrice)
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to [:fabrice] Fabrice Desré from comment #3)
> I'm not too worried about the code merge itself, but we need to figure out:
> - releng builds.
Seems like we should be able to do this since we're building multiple products on larch now. Just need to hookup the new mozconfigs, right?
> - can we run some/all of the browser.html tests? are there any?
There is one test in browser.html, and horizon will have tests as well. I would like to run the horizon tests on treeherder, and potentially browser.html tests as well.
Comment 5•9 years ago
|
||
(In reply to Kevin Grandon :kgrandon from comment #4)
> (In reply to [:fabrice] Fabrice Desré from comment #3)
> > I'm not too worried about the code merge itself, but we need to figure out:
> > - releng builds.
>
> Seems like we should be able to do this since we're building multiple
> products on larch now. Just need to hookup the new mozconfigs, right?
Sure, but the load generated by 2 new products on all integration branches + m-c will be higher than just being on larch.
> > - can we run some/all of the browser.html tests? are there any?
>
> There is one test in browser.html, and horizon will have tests as well. I
> would like to run the horizon tests on treeherder, and potentially
> browser.html tests as well.
Sounds good.
Assignee | ||
Comment 6•9 years ago
|
||
Comment on attachment 8661347 [details] [diff] [review]
Current full diff of larch to mozilla-central
Review of attachment 8661347 [details] [diff] [review]:
-----------------------------------------------------------------
First pass notes.
::: b2g/app/b2g.js
@@ -440,1 @@
> pref("dom.meta-viewport.enabled", true);
Needs to be wrapped by MOZ_MULET || MOZ_GRAPHENE.
@@ -1072,5 @@
> // Enable webapps add-ons
> pref("dom.apps.customization.enabled", true);
>
> -// Original caret implementation on collapsed selection.
> -pref("touchcaret.enabled", false);
Needs to be wrapped in MOZ_GRAPHENE.
@@ +1090,5 @@
> pref("dom.mapped_arraybuffer.enabled", true);
> #endif
>
> +// BroadcastChannel API
> +pref("dom.broadcastChannel.enabled", true);
Remove this.
::: b2g/branding/unofficial/configure.sh
@@ -3,4 @@
> # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>
> MOZ_APP_DISPLAYNAME=B2G
> -MOZ_UPDATER=
Revert this if it doesn't break.
::: b2g/installer/package-manifest.in
@@ -477,4 @@
> @RESPATH@/components/NetworkStatsManager.manifest
> @RESPATH@/components/NetworkStatsServiceProxy.js
> @RESPATH@/components/NetworkStatsServiceProxy.manifest
> -@RESPATH@/components/TetheringService.js
We should not change this.
::: dom/ipc/Blob.cpp
@@ +4447,5 @@
> MOZ_ASSERT(mOwnsBlobImpl);
>
> // In desktop e10s the file picker code sends this message.
> +
> +#if defined(MOZ_CHILD_PERMISSIONS) && !defined(MOZ_GRAPHENE)
Exclude this, and land in bug 1180288.
::: dom/workers/test/promise_worker.js
@@ +170,4 @@
> xhr.open("GET", "testXHR.txt", false);
> xhr.send(null);
>
> + todo(!handlerExecuted, "Sync XHR should not trigger microtask execution.");
This is from bug 1185187.
https://github.com/KevinGrandon/gecko-projects/commit/91b752fdf8331a2aa2999e407610f77fa25af60e
Let's back this out.
::: gfx/vr/OVR_CAPIShim.c
@@ +1,1 @@
> +/************************************************************************************
This file should not be here.
::: mobile/android/app/mobile.js
@@ +804,5 @@
> pref("dom.payment.provider.0.type", "mozilla/payments/pay/v1");
> pref("dom.payment.provider.0.requestMethod", "GET");
>
> +// Enable Cardboard VR on mobile, assuming VR at all is enabled
> +pref("dom.vr.cardboard.enabled", true);
Seems like VR. Don't make this change.
Assignee | ||
Comment 7•9 years ago
|
||
Here is a patch which fixes all of the found issues in the initial pass.
Attachment #8661347 -
Attachment is obsolete: true
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8665142 -
Attachment is obsolete: true
Comment hidden (obsolete) |
Assignee | ||
Comment 10•9 years ago
|
||
New patch, fixing ifdef statements to address emulator mochitest failures.
Attachment #8665182 -
Attachment is obsolete: true
Assignee | ||
Comment 12•9 years ago
|
||
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8665448 [details] [diff] [review]
Larch -> mozilla-central merge
Review of attachment 8665448 [details] [diff] [review]:
-----------------------------------------------------------------
Fabrice - please take a look if you can. I would like to review this and get it landed to start working on treeherder tests for browser.html/horizon.
I've tested a device build and a mulet build and everything seems fine to me. I think the try run looks ok as well, but please verify: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ad0583f26ad
Thanks!
Attachment #8665448 -
Flags: review?(fabrice)
Comment 14•9 years ago
|
||
Comment on attachment 8665448 [details] [diff] [review]
Larch -> mozilla-central merge
Review of attachment 8665448 [details] [diff] [review]:
-----------------------------------------------------------------
Looks fine to me, thanks!
Mike, we are moving to m-c the changes we did on larch for the browser.html & horizon runtime. You did look at them at the time, there should not be anything new in configure.in
Attachment #8665448 -
Flags: review?(mh+mozilla)
Attachment #8665448 -
Flags: review?(fabrice)
Attachment #8665448 -
Flags: review+
Comment 15•9 years ago
|
||
Comment on attachment 8665448 [details] [diff] [review]
Larch -> mozilla-central merge
Review of attachment 8665448 [details] [diff] [review]:
-----------------------------------------------------------------
Only skimmed over the build system changes. I really hate our mozconfigs.
::: b2g/app/macbuild/Contents/Info.plist.in
@@ +32,4 @@
> <true/>
> <key>NSPrincipalClass</key>
> <string>GeckoNSApplication</string>
> + <key>CFBundleURLTypes</key>
Indentation doesn't seem to match
::: b2g/graphene/config/horizon-mozconfigs/common
@@ +1,5 @@
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +# This file is included at the top of all b2g mozconfigs
That's obviously not true :)
::: configure.in
@@ +7521,5 @@
> dnl ========================================================
> +dnl = Horizon build options - set default preferences for
> +dnl the horizon project.
> +dnl ========================================================
> +if test -n "$MOZ_HORIZON"; then
What the hell is horizon? Same applies to graphene, btw. Code names are nice, but somewhere, there needs to be an explanation what it is for.
Attachment #8665448 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 16•9 years ago
|
||
Thanks for the review, will work on addressing your comments.
(In reply to Mike Hommey [:glandium] from comment #15)
> ::: configure.in
> @@ +7521,5 @@
> > dnl ========================================================
> > +dnl = Horizon build options - set default preferences for
> > +dnl the horizon project.
> > +dnl ========================================================
> > +if test -n "$MOZ_HORIZON"; then
>
> What the hell is horizon? Same applies to graphene, btw. Code names are
> nice, but somewhere, there needs to be an explanation what it is for.
It's a VR product. I'll add a comment which will provide some more context.
Assignee | ||
Comment 17•9 years ago
|
||
Updated comments, fixed whitespace, and rebased against m-c. Carrying reviews.
Running on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8fed7128cfaf
Attachment #8665448 -
Attachment is obsolete: true
Attachment #8667976 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → kevingrandon
Assignee | ||
Comment 18•9 years ago
|
||
Oops, the graphene specific binary files with that last patch were dropped. This is the same patch with the review fixes, but just with the additional blobs. Carrying the review.
Attachment #8668103 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8667976 -
Attachment is obsolete: true
Assignee | ||
Comment 19•9 years ago
|
||
Adding checkin-needed for the patch here: https://bug1204965.bmoattachments.org/attachment.cgi?id=8668103
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8fed7128cfaf
Thanks!
Keywords: checkin-needed
Comment 20•9 years ago
|
||
Hi,
this failed to apply:
renamed 1204965 -> 0001-Bug-1204965-Graphene-support.-Merge-larch-into-mozil.patch
applying 0001-Bug-1204965-Graphene-support.-Merge-larch-into-mozil.patch
patching file b2g/chrome/jar.mn
Hunk #1 FAILED at 11
1 out of 2 hunks FAILED -- saving rejects to file b2g/chrome/jar.mn.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and refresh 0001-Bug-1204965-Graphene-support.-Merge-larch-into-mozil.patch
Kevin, could you take a look, thanks!
Flags: needinfo?(kevingrandon)
Keywords: checkin-needed
Assignee | ||
Comment 21•9 years ago
|
||
Rebased patch, carrying review.
Attachment #8668103 -
Attachment is obsolete: true
Flags: needinfo?(kevingrandon)
Attachment #8668288 -
Flags: review+
Assignee | ||
Comment 22•9 years ago
|
||
This was rebased since the try run, but the rebase was quite simple and I don't think it should impact the try run.
Adding checkin-needed for the patch here: https://bug1204965.bmoattachments.org/attachment.cgi?id=8668103
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8fed7128cfaf
Keywords: checkin-needed
Assignee | ||
Comment 23•9 years ago
|
||
Hold on, noticed that webapps.js in devtools looks wrong, I need to fix that first.
Keywords: checkin-needed
Comment 24•9 years ago
|
||
Comment 25•9 years ago
|
||
Hi Kevin,
Why are touchcaret and selectioncarets enabled on graphine in [1]? Do you bump into some try fail before? Enable both of them should have no effect since accessiblecaret are enabled on b2g now.
[1] https://hg.mozilla.org/integration/mozilla-inbound/diff/de3b5f164ac5/b2g/app/b2g.js#l1.49
Flags: needinfo?(kevingrandon)
Comment 26•9 years ago
|
||
Carets should be disabled.
They are disabled here: https://hg.mozilla.org/integration/mozilla-inbound/file/bc20a0abec61/b2g/graphene/graphene.js#l30
Comment 27•9 years ago
|
||
Re comment 26:
I notice that graphene.js are included at the bottom b2g.js. If carets are not needed by graphine, I would like to remove the prefs. They are disabled by default in all.js. Filed bug 1210337.
Flags: needinfo?(kevingrandon)
Comment 28•9 years ago
|
||
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Is FxOS :: Runtime a better location for this bug? Core :: DOM seems random to me, but I wasn't sure.
Flags: needinfo?(kevingrandon)
Assignee | ||
Comment 30•9 years ago
|
||
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #29)
> Is FxOS :: Runtime a better location for this bug? Core :: DOM seems random
> to me, but I wasn't sure.
Yeah, Firefox OS : General seems better for now - until graphene has a product.
Component: DOM → General
Flags: needinfo?(kevingrandon)
Product: Core → Firefox OS
Target Milestone: mozilla44 → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•