Closed
Bug 1258464
Opened 8 years ago
Closed 8 years ago
[geckoview] Remove org.mozilla.gecko.GeckoView references to Fennec
Categories
(Core Graveyard :: Embedding: GRE Core, defect)
Core Graveyard
Embedding: GRE Core
Tracking
(firefox48 fixed)
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: nalexander, Assigned: nalexander)
References
Details
Attachments
(2 files)
After Bug 1258450, we have: nalexander@chocho ~/M/gecko> java -cp mobile/android/build/classycle/classycle-1.4.1.jar classycle.dependency.DependencyChecker -mergeInnerClasses -dependencies=@mobile/android/base/geckoview.ddf objdir-droid/gradle/build/mobile/android/app/intermediates/packaged/local/debug/classes.jar show onlyShortestPaths allResults Set [lib] has 133 classes. Set [app] has 715 classes. check [lib] independentOf [app] Unexpected dependencies found: org.mozilla.gecko.GeckoView -> org.mozilla.gecko.GeckoActivity -> org.mozilla.gecko.Tab -> org.mozilla.gecko.Tabs -> org.mozilla.gecko.R <snip> This ticket tracks getting rid of those compile-time dependencies. I see two parts: {R, GeckoActivity} and {Tab, Tabs}. The former comes from styling the GeckoView with a default URL and a parameter to skip initialization. (See https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/values/attrs.xml#143.) As b2gdroid disappears, nothing will consume these, so I suggest we remove them for now until we hash out a good way to return them. The latter is harder because Tab is used to provide actual GeckoView functionality. However, Fennec doesn't consume them, so nothing is currently consuming them. Again, I suggest we remove these until we have an approach not tied to Fennec's Tab/Tabs. These new approaches might include re-growing some subset of Fennec's Tabs! But we should think carefully about the model we want to support for GeckoView: is it WebView like, where each View corresponds to a tab (XUL <browser> element); or is it Fennec like, where each View corresponds to a web browser (XUL <deck> element)? I think snorp and jchen have opinions here.
Assignee | ||
Comment 1•8 years ago
|
||
These have no consumer and deserve to be properly reworked. Review commit: https://reviewboard.mozilla.org/r/41843/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/41843/
Attachment #8733622 -
Flags: review?(snorp)
Assignee | ||
Comment 2•8 years ago
|
||
This will be re-implemented without reference to Fennec's Tab/Tabs data structures. Review commit: https://reviewboard.mozilla.org/r/41845/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/41845/
Attachment #8733623 -
Flags: review?(snorp)
Comment on attachment 8733622 [details] MozReview Request: Bug 1258464 - Part 1: Remove vestigial GeckoView attrs and initialization. r?snorp https://reviewboard.mozilla.org/r/41843/#review39267
Attachment #8733622 -
Flags: review?(snorp) → review+
Attachment #8733622 -
Flags: review+
Comment on attachment 8733622 [details] MozReview Request: Bug 1258464 - Part 1: Remove vestigial GeckoView attrs and initialization. r?snorp https://reviewboard.mozilla.org/r/41843/#review39269 It would probably be good for jchen to look at this too ::: mobile/android/base/java/org/mozilla/gecko/GeckoView.java (Diff revision 1) > - if (!doInit) > - return; > - > - // If running outside of a GeckoActivity (eg, from a library project), > - // load the native code and disable content providers > - boolean isGeckoActivity = false; Wait, do we really not need this anymore for non-Fennec usage? Or are you saying we should stop caring about non-Fennec usage?
Comment on attachment 8733623 [details] MozReview Request: Bug 1258464 - Part 2: Remove GeckoView implementation built on Fennec's Tab/Tabs. r?snorp https://reviewboard.mozilla.org/r/41845/#review39273 Yessssssss
Attachment #8733623 -
Flags: review?(snorp) → review+
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #4) > Comment on attachment 8733622 [details] > MozReview Request: Bug 1258464 - Part 1: Remove vestigial GeckoView attrs > and initialization. r?snorp > > https://reviewboard.mozilla.org/r/41843/#review39269 > > It would probably be good for jchen to look at this too > > ::: mobile/android/base/java/org/mozilla/gecko/GeckoView.java > (Diff revision 1) > > - if (!doInit) > > - return; > > - > > - // If running outside of a GeckoActivity (eg, from a library project), > > - // load the native code and disable content providers > > - boolean isGeckoActivity = false; > > Wait, do we really not need this anymore for non-Fennec usage? Or are you > saying we should stop caring about non-Fennec usage? The latter. We should re-grow non-Fennec usage -- including possibly having resources with attributes -- at a later time. We have no consumers for this, and it's holding back splitting the monolith, so let's get rid of it. jchen: could you look at the Part 1 patch, just in case?
Flags: needinfo?(nchen)
Comment 7•8 years ago
|
||
Comment on attachment 8733622 [details] MozReview Request: Bug 1258464 - Part 1: Remove vestigial GeckoView attrs and initialization. r?snorp https://reviewboard.mozilla.org/r/41843/#review39565 I was working on a consumer at one point [1], but that project is far from finished. I'm okay with this change in the meantime. [1] https://github.com/darchons/fenneclite
Attachment #8733622 -
Flags: review+
Updated•8 years ago
|
Flags: needinfo?(nchen)
Assignee | ||
Comment 8•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/dcffecae84b741d837295ad77d0dd9e7ece391eb Bug 1258464 - Part 1: Remove vestigial GeckoView attrs and initialization. r=snorp https://hg.mozilla.org/integration/fx-team/rev/ab088eb19b46d9e8dc5551e39aa4a871a4c9ee23 Bug 1258464 - Part 2: Remove GeckoView implementation built on Fennec's Tab/Tabs. r=snorp
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → nalexander
Status: NEW → ASSIGNED
Backout: https://hg.mozilla.org/integration/fx-team/rev/791698f3f8cf https://hg.mozilla.org/integration/fx-team/rev/d202b268a228
Comment 10•8 years ago
|
||
sorry had to back this out since this or the other push caused https://treeherder.mozilla.org/logviewer.html#?job_id=8313401&repo=fx-team
Flags: needinfo?(nalexander)
Assignee | ||
Comment 11•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8787613fd050
Assignee | ||
Comment 12•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=397405a12bd9
Assignee | ||
Comment 13•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/d8cc2430bf0b6e6c10aa3c5e264af8ce3721ae4e Bug 1258464 - Part 1: Remove vestigial GeckoView attrs and initialization. r=snorp https://hg.mozilla.org/integration/fx-team/rev/4b8075ea02463c44f5d6c08584ac81c5dc0d62fd Bug 1258464 - Part 2: Remove GeckoView implementation built on Fennec's Tab/Tabs. r=snorp
Comment 15•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d8cc2430bf0b https://hg.mozilla.org/mozilla-central/rev/4b8075ea0246
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•