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)

defect
Not set
normal

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.
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+
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+
(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 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+
Flags: needinfo?(nchen)
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: nobody → nalexander
Status: NEW → ASSIGNED
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)
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
Relanded.  Thanks, Carsten!
Flags: needinfo?(nalexander)
https://hg.mozilla.org/mozilla-central/rev/d8cc2430bf0b
https://hg.mozilla.org/mozilla-central/rev/4b8075ea0246
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.