Closed Bug 1442255 Opened 6 years ago Closed 6 years ago

Remove GeckoLoader.setLastIntent

Tracking

(firefox60 fixed)

Status:

RESOLVED FIXED

Milestone:

mozilla60

Tracking Flags:

Tracking

Status

firefox60

---

fixed

People

(Reporter: jchen, Assigned: jchen)

Details

Attachments

(7 files)

Bug 1442255 - 1. Replace setLastIntent with extras bundle; 6 years ago (inactive) Jim Chen [:jchen] [:darchons] 59 bytes, text/x-review-board-request	esawin : review+	Details
Bug 1442255 - 2. Replace GeckoThread fields with extras bundle; 6 years ago (inactive) Jim Chen [:jchen] [:darchons] 59 bytes, text/x-review-board-request	esawin : review+	Details
Bug 1442255 - 3. Use extras bundle to initialize Gecko in GeckoView; 6 years ago (inactive) Jim Chen [:jchen] [:darchons] 59 bytes, text/x-review-board-request	esawin : review+	Details
Bug 1442255 - 4. Use extras bundle to initialize Gecko in Fennec; 6 years ago (inactive) Jim Chen [:jchen] [:darchons] 59 bytes, text/x-review-board-request	esawin : review+	Details
Bug 1442255 - 5. Use extras bundle to initialize child processes; 6 years ago (inactive) Jim Chen [:jchen] [:darchons] 59 bytes, text/x-review-board-request	esawin : review+	Details
Bug 1442255 - 6. Adjust process count for GeckoView; 6 years ago (inactive) Jim Chen [:jchen] [:darchons] 59 bytes, text/x-review-board-request		Details
Bug 1442255 - 7. Switch the order of fd arguments; 6 years ago (inactive) Jim Chen [:jchen] [:darchons] 59 bytes, text/x-review-board-request		Details

(inactive) Jim Chen [:jchen] [:darchons]

Assignee

Description

•

6 years ago

Replace setLastIntent with an extras bundle for GeckoThread

Comment hidden (mozreview-request)

GeckoLoader.setLastIntent is not a very good API for setting environment
variables and is often forgotten. Replace it with an extras bundle that
is passed to setupGeckoEnvironment.

Review commit: https://reviewboard.mozilla.org/r/224302/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/224302/

Comment hidden (mozreview-request)

Introduce an extras bundle in GeckoThread, which we use to store things
such as extra main process arguments and child process FDs. Also use the
extras bundle to store environment variables to be passed to
GeckoLoader.

Review commit: https://reviewboard.mozilla.org/r/224304/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/224304/

Comment hidden (mozreview-request)

Use extras bundle to initialize Gecko in GeckoSession and
GeckoViewActivity.

Review commit: https://reviewboard.mozilla.org/r/224306/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/224306/

Comment hidden (mozreview-request)

Use extras bundle (e.g. from an Intent) to initialize Gecko in GeckoApp
and GeckoService.

Review commit: https://reviewboard.mozilla.org/r/224308/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/224308/

Comment hidden (mozreview-request)

Pass the extras bundle from the main process to the child process
through IChildProcess.start, instead of through the Intent, which never
worked because intent extras are not passed to Service.onBind.

Review commit: https://reviewboard.mozilla.org/r/224310/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/224310/

Eugen Sawin [:esawin]

Comment 6

•

6 years ago

mozreview-review

Comment on attachment 8955144 [details]
Bug 1442255 - 1. Replace setLastIntent with extras bundle;

https://reviewboard.mozilla.org/r/224302/#review230474

Attachment #8955144 - Flags: review?(esawin) → review+

Eugen Sawin [:esawin]

Comment 7

•

6 years ago

mozreview-review

Comment on attachment 8955145 [details]
Bug 1442255 - 2. Replace GeckoThread fields with extras bundle;

https://reviewboard.mozilla.org/r/224304/#review230480

Attachment #8955145 - Flags: review?(esawin) → review+

Eugen Sawin [:esawin]

Comment 8

•

6 years ago

mozreview-review

Comment on attachment 8955146 [details]
Bug 1442255 - 3. Use extras bundle to initialize Gecko in GeckoView;

https://reviewboard.mozilla.org/r/224306/#review230484

Attachment #8955146 - Flags: review?(esawin) → review+

Eugen Sawin [:esawin]

Comment 9

•

6 years ago

mozreview-review

Comment on attachment 8955147 [details]
Bug 1442255 - 4. Use extras bundle to initialize Gecko in Fennec;

https://reviewboard.mozilla.org/r/224308/#review230490

::: mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java:105
(Diff revision 1)
>       */
>      public static String getSessionUUID() {
>          return sSessionUUID;
>      }
>  
> -    public static String addDefaultGeckoArgs(String args) {
> +    public static String[] getDefaultGeckoArgs() {

@Nullable

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/mozglue/SafeIntent.java:44
(Diff revision 1)
>              Log.w(LOGTAG, "Couldn't determine if intent had an extra.", e);
>              return false;
>          }
>      }
>  
> +    public Bundle getExtras() {

@Nullable

Attachment #8955147 - Flags: review?(esawin) → review+

Eugen Sawin [:esawin]

Comment 10

•

6 years ago

mozreview-review

Comment on attachment 8955148 [details]
Bug 1442255 - 5. Use extras bundle to initialize child processes;

https://reviewboard.mozilla.org/r/224310/#review230494

Attachment #8955148 - Flags: review?(esawin) → review+

Comment hidden (mozreview-request)

Set process count to 1 because we only support one child process right
now.

Review commit: https://reviewboard.mozilla.org/r/225192/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/225192/

Comment hidden (mozreview-request)

Switch the order of the IPC FD argument and the crash FD argument in
e10s calls, because the IPC FD is the primary FD, and the crash FD
should be grouped with the crash annotation FD.

Review commit: https://reviewboard.mozilla.org/r/225194/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/225194/

Comment hidden (mozreview-request)

Comment on attachment 8955144 [details]
Bug 1442255 - 1. Replace setLastIntent with extras bundle;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/224302/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8955145 [details]
Bug 1442255 - 2. Replace GeckoThread fields with extras bundle;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/224304/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8955146 [details]
Bug 1442255 - 3. Use extras bundle to initialize Gecko in GeckoView;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/224306/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8955147 [details]
Bug 1442255 - 4. Use extras bundle to initialize Gecko in Fennec;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/224308/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8955148 [details]
Bug 1442255 - 5. Use extras bundle to initialize child processes;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/224310/diff/1-2/

(inactive) Jim Chen [:jchen] [:darchons]

Assignee

Comment 18

•

6 years ago

mozreview-review

Comment on attachment 8956328 [details]
Bug 1442255 - 6. Adjust process count for GeckoView;

https://reviewboard.mozilla.org/r/225192/#review231162

Attachment #8956328 - Flags: review+

(inactive) Jim Chen [:jchen] [:darchons]

Assignee

Comment 19

•

6 years ago

mozreview-review

Comment on attachment 8956329 [details]
Bug 1442255 - 7. Switch the order of fd arguments;

https://reviewboard.mozilla.org/r/225194/#review231164

Attachment #8956329 - Flags: review+

Pulsebot

Comment 20

•

6 years ago

Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d7a43e59a1b4
1. Replace setLastIntent with extras bundle; r=esawin
https://hg.mozilla.org/integration/autoland/rev/08f906f3a0c8
2. Replace GeckoThread fields with extras bundle; r=esawin
https://hg.mozilla.org/integration/autoland/rev/c3305648ad30
3. Use extras bundle to initialize Gecko in GeckoView; r=esawin
https://hg.mozilla.org/integration/autoland/rev/5d0f72867f53
4. Use extras bundle to initialize Gecko in Fennec; r=esawin
https://hg.mozilla.org/integration/autoland/rev/62d1c57c37b4
5. Use extras bundle to initialize child processes; r=esawin
https://hg.mozilla.org/integration/autoland/rev/6a794e14fceb
6. Adjust process count for GeckoView; r=jchen
https://hg.mozilla.org/integration/autoland/rev/208c4c42f641
7. Switch the order of fd arguments; r=jchen

Raul Gurzau (:RaulG)

Comment 21

•

6 years ago

Backed out 7 changesets (bug 1442255) for failing c1 tests on mochitest/test_browserElement_inproc_PurgeHistory.html

Backout link: https://hg.mozilla.org/integration/autoland/rev/b1dc3a24a077e384e3c439453cd800636e2cf173

Push where failures started: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=208c4c42f641e189ac930ef0831b38d502c7374c

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=166221463&repo=autoland&lineNumber=2141

[task 2018-03-06T14:17:33.000Z] 14:17:33     INFO -  153 INFO TEST-START | dom/browser-element/mochitest/test_browserElement_inproc_PurgeHistory.html
[task 2018-03-06T14:22:56.709Z] 14:22:56     INFO -  154 INFO TEST-UNEXPECTED-FAIL | dom/browser-element/mochitest/test_browserElement_inproc_PurgeHistory.html | Test timed out.
[task 2018-03-06T14:22:56.710Z] 14:22:56     INFO -  reportError@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:121:7
[task 2018-03-06T14:22:56.710Z] 14:22:56     INFO -  TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:142:7

Flags: needinfo?(nchen)

(inactive) Jim Chen [:jchen] [:darchons]

Assignee

Updated

•

6 years ago

Flags: needinfo?(nchen)

Comment hidden (mozreview-request)

Comment on attachment 8955144 [details]
Bug 1442255 - 1. Replace setLastIntent with extras bundle;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/224302/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8955145 [details]
Bug 1442255 - 2. Replace GeckoThread fields with extras bundle;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/224304/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8955146 [details]
Bug 1442255 - 3. Use extras bundle to initialize Gecko in GeckoView;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/224306/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8955147 [details]
Bug 1442255 - 4. Use extras bundle to initialize Gecko in Fennec;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/224308/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8955148 [details]
Bug 1442255 - 5. Use extras bundle to initialize child processes;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/224310/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8956328 [details]
Bug 1442255 - 6. Adjust process count for GeckoView;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/225192/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8956329 [details]
Bug 1442255 - 7. Switch the order of fd arguments;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/225194/diff/1-2/

Pulsebot

Comment 29

•

6 years ago

Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/47c1e6667b85
1. Replace setLastIntent with extras bundle; r=esawin
https://hg.mozilla.org/integration/autoland/rev/ab953c63958b
2. Replace GeckoThread fields with extras bundle; r=esawin
https://hg.mozilla.org/integration/autoland/rev/10aa0b36b674
3. Use extras bundle to initialize Gecko in GeckoView; r=esawin
https://hg.mozilla.org/integration/autoland/rev/f5947781c990
4. Use extras bundle to initialize Gecko in Fennec; r=esawin
https://hg.mozilla.org/integration/autoland/rev/be83fd0b04b6
5. Use extras bundle to initialize child processes; r=esawin
https://hg.mozilla.org/integration/autoland/rev/ec84d9eb4351
6. Adjust process count for GeckoView; r=jchen
https://hg.mozilla.org/integration/autoland/rev/140e1bb89e8b
7. Switch the order of fd arguments; r=jchen

Bogdan Tara[:bogdan_tara | bogdant]

Comment 30

•

6 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/47c1e6667b85
https://hg.mozilla.org/mozilla-central/rev/ab953c63958b
https://hg.mozilla.org/mozilla-central/rev/10aa0b36b674
https://hg.mozilla.org/mozilla-central/rev/f5947781c990
https://hg.mozilla.org/mozilla-central/rev/be83fd0b04b6
https://hg.mozilla.org/mozilla-central/rev/ec84d9eb4351
https://hg.mozilla.org/mozilla-central/rev/140e1bb89e8b

Status: ASSIGNED → RESOLVED

Closed: 6 years ago

status-firefox60: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → Firefox 60

BMO Automation

Updated

•

5 years ago

Product: Firefox for Android → GeckoView

Dylan Hardison [:dylan] (he/him)

Updated

•

5 years ago

Target Milestone: Firefox 60 → mozilla60

You need to log in before you can comment on or make changes to this bug.