Bug 1649529: Deprecate GeckoSession parcelability; r=geckoview-reviewers,agi
authorAaron Klotz <aklotz@mozilla.com>
Thu, 02 Jul 2020 15:10:56 +0000
changeset 538444 ff4059e08ee0a70c5e5a1c4a97d1c8acd624c681
parent 538443 2b560ad42c0da7d4ce3bd08cc895ace4911357bb
child 538445 76b698ec608a19eada9adb2826de59217dc21757
push id37563
push usercbrindusan@mozilla.com
push dateThu, 02 Jul 2020 21:49:48 +0000
treeherdermozilla-central@f0ac79e1ed53 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgeckoview-reviewers, agi
bugs1649529
milestone80.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1649529: Deprecate GeckoSession parcelability; r=geckoview-reviewers,agi Note: These comments reference GeckoView 82 instead of GeckoView 83 because I plan to uplift this to GeckoView 79. Ditto for the changelog entry. Tests that use the parcelable functionality are flagged to ignore deprecation warnings. Differential Revision: https://phabricator.services.mozilla.com/D81768
mobile/android/geckoview/api.txt
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/BaseSessionTest.kt
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/SessionLifecycleTest.kt
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md
--- a/mobile/android/geckoview/api.txt
+++ b/mobile/android/geckoview/api.txt
@@ -685,17 +685,17 @@ package org.mozilla.geckoview {
     method @AnyThread public void loadUri(@NonNull String, @Nullable GeckoSession, int, @Nullable Map<String,String>);
     method @AnyThread public void loadUri(@NonNull Uri);
     method @AnyThread public void loadUri(@NonNull Uri, @Nullable Map<String,String>);
     method @AnyThread public void loadUri(@NonNull Uri, int);
     method @AnyThread public void loadUri(@NonNull Uri, @Nullable Uri, int);
     method @AnyThread public void loadUri(@NonNull Uri, @Nullable Uri, int, @Nullable Map<String,String>);
     method @UiThread public void open(@NonNull GeckoRuntime);
     method @AnyThread public void purgeHistory();
-    method @AnyThread public void readFromParcel(@NonNull Parcel);
+    method @Deprecated @AnyThread public void readFromParcel(@NonNull Parcel);
     method @UiThread public void releaseDisplay(@NonNull GeckoDisplay);
     method @AnyThread public void reload();
     method @AnyThread public void reload(int);
     method @AnyThread public void restoreState(@NonNull GeckoSession.SessionState);
     method @AnyThread public void setActive(boolean);
     method @UiThread public void setAutofillDelegate(@Nullable Autofill.Delegate);
     method @AnyThread public void setContentBlockingDelegate(@Nullable ContentBlocking.Delegate);
     method @UiThread public void setContentDelegate(@Nullable GeckoSession.ContentDelegate);
@@ -705,17 +705,17 @@ package org.mozilla.geckoview {
     method @UiThread public void setNavigationDelegate(@Nullable GeckoSession.NavigationDelegate);
     method @UiThread public void setPermissionDelegate(@Nullable GeckoSession.PermissionDelegate);
     method @UiThread public void setProgressDelegate(@Nullable GeckoSession.ProgressDelegate);
     method @AnyThread public void setPromptDelegate(@Nullable GeckoSession.PromptDelegate);
     method @UiThread public void setScrollDelegate(@Nullable GeckoSession.ScrollDelegate);
     method @UiThread public void setSelectionActionDelegate(@Nullable GeckoSession.SelectionActionDelegate);
     method @AnyThread public void stop();
     method @UiThread protected void setShouldPinOnScreen(boolean);
-    field public static final Parcelable.Creator<GeckoSession> CREATOR;
+    field @Deprecated public static final Parcelable.Creator<GeckoSession> CREATOR;
     field public static final int FINDER_DISPLAY_DIM_PAGE = 2;
     field public static final int FINDER_DISPLAY_DRAW_LINK_OUTLINE = 4;
     field public static final int FINDER_DISPLAY_HIGHLIGHT_ALL = 1;
     field public static final int FINDER_FIND_BACKWARDS = 1;
     field public static final int FINDER_FIND_LINKS_ONLY = 8;
     field public static final int FINDER_FIND_MATCH_CASE = 2;
     field public static final int FINDER_FIND_WHOLE_WORD = 4;
     field public static final int LOAD_FLAGS_ALLOW_POPUPS = 8;
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/BaseSessionTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/BaseSessionTest.kt
@@ -102,16 +102,18 @@ open class BaseSessionTest(noErrorCollec
     fun createTestUrl(path: String) = GeckoSessionTestRule.TEST_ENDPOINT + path
 
     fun GeckoSession.loadTestPath(path: String) =
             this.loadUri(createTestUrl(path))
 
     inline fun GeckoSession.toParcel(lambda: (Parcel) -> Unit) {
         val parcel = Parcel.obtain()
         try {
+            // Bug 1650108: Remove this
+            @Suppress("DEPRECATION")
             this.writeToParcel(parcel, 0)
 
             val pos = parcel.dataPosition()
             parcel.setDataPosition(0)
 
             lambda(parcel)
 
             assertThat("Read parcel matches written parcel",
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/SessionLifecycleTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/SessionLifecycleTest.kt
@@ -74,16 +74,17 @@ class SessionLifecycleTest : BaseSession
     }
 
     @Test(expected = IllegalStateException::class)
     fun open_throwOnAlreadyOpen() {
         // Throw exception if retrying to open again; otherwise we would leak the old open window.
         sessionRule.session.open()
     }
 
+    @Suppress("DEPRECATION")
     @Test fun readFromParcel() {
         val session = sessionRule.createOpenSession()
 
         session.toParcel { parcel ->
             val newSession = sessionRule.createFromParcel(parcel)
 
             assertThat("New session has same settings",
                        newSession.settings, equalTo(session.settings))
@@ -92,67 +93,72 @@ class SessionLifecycleTest : BaseSession
             newSession.close()
             assertThat("New session can be closed", newSession.isOpen, equalTo(false))
         }
 
         sessionRule.session.reload()
         sessionRule.session.waitForPageStop()
     }
 
+    @Suppress("DEPRECATION")
     @Ignore //Disable test for frequent failures Bug 1532186
     @Test(expected = IllegalStateException::class)
     fun readFromParcel_throwOnAlreadyOpen() {
         //disable readFromParcel_throwOnAlreadyOpen for frequent failures Bug 1532186
         assumeThat(sessionRule.env.isDebugBuild, equalTo(true))
         // Throw exception if retrying to open again; otherwise we would leak the old open window.
         sessionRule.session.toParcel { parcel ->
             sessionRule.createOpenSession().readFromParcel(parcel)
         }
     }
 
+    @Suppress("DEPRECATION")
     @Test fun readFromParcel_canLoadPageAfterRead() {
         var newSession: GeckoSession? = null
 
         sessionRule.session.toParcel { parcel ->
             newSession = sessionRule.createFromParcel(parcel)
         }
 
         newSession!!.reload()
         newSession!!.waitForPageStop()
     }
 
+    @Suppress("DEPRECATION")
     @Test fun readFromParcel_closedSession() {
         val session = sessionRule.createClosedSession()
 
         session.toParcel { parcel ->
             val newSession = sessionRule.createFromParcel(parcel)
             assertThat("New session should not be open",
                        newSession.isOpen, equalTo(false))
         }
 
         sessionRule.session.reload()
         sessionRule.session.waitForPageStop()
     }
 
+    @Suppress("DEPRECATION")
     @Test fun readFromParcel_closedSessionAfterParceling() {
         val session = sessionRule.createOpenSession()
 
         session.toParcel { parcel ->
             assertThat("Session is still open", session.isOpen, equalTo(true))
             session.close()
 
             val newSession = sessionRule.createFromParcel(parcel)
             assertThat("New session should not be open",
                        newSession.isOpen, equalTo(false))
         }
 
         sessionRule.session.reload()
         sessionRule.session.waitForPageStop()
     }
 
+    @Suppress("DEPRECATION")
     @Test fun readFromParcel_closedSessionAfterReadParcel() {
         // disable test on opt for frequently failing Bug 1519591
         assumeThat(sessionRule.env.isDebugBuild, equalTo(true))
         val session = sessionRule.createOpenSession()
 
         session.toParcel { parcel ->
             assertThat("Session is still open", session.isOpen, equalTo(true))
             val newSession = sessionRule.createFromParcel(parcel)
@@ -161,42 +167,45 @@ class SessionLifecycleTest : BaseSession
             assertThat("Old session should be closed",
                     session.isOpen, equalTo(false))
         }
 
         sessionRule.session.reload()
         sessionRule.session.waitForPageStop()
     }
 
+    @Suppress("DEPRECATION")
     @Test fun readFromParcel_closeOpenAndLoad() {
         var newSession: GeckoSession? = null
 
         sessionRule.session.toParcel { parcel ->
             newSession = sessionRule.createFromParcel(parcel)
         }
 
         newSession!!.close()
         newSession!!.open()
 
         newSession!!.reload()
         newSession!!.waitForPageStop()
     }
 
+    @Suppress("DEPRECATION")
     @Test fun readFromParcel_allowCallsBeforeUnparceling() {
         val newSession = sessionRule.createClosedSession()
 
         newSession.loadTestPath(HELLO_HTML_PATH)
         newSession.reload()
 
         sessionRule.session.toParcel { parcel ->
             newSession.readFromParcel(parcel)
         }
         newSession.waitForPageStops(2)
     }
 
+    @Suppress("DEPRECATION")
     @Test fun readFromParcel_chained() {
         var session1: GeckoSession? = null
         var session2: GeckoSession? = null
         var session3: GeckoSession? = null
 
         sessionRule.session.toParcel { parcel ->
             session1 = sessionRule.createFromParcel(parcel)
         }
@@ -206,16 +215,17 @@ class SessionLifecycleTest : BaseSession
         session2!!.toParcel { parcel ->
             session3 = sessionRule.createFromParcel(parcel)
         }
 
         session3!!.reload()
         session3!!.waitForPageStop()
     }
 
+    @Suppress("DEPRECATION")
     @NullDelegate(GeckoSession.NavigationDelegate::class)
     @ClosedSessionAtStart
     @Test fun readFromParcel_moduleUpdated() {
         val session = sessionRule.createOpenSession()
 
         session.loadTestPath(HELLO_HTML_PATH)
         session.waitForPageStop()
 
@@ -239,16 +249,17 @@ class SessionLifecycleTest : BaseSession
 
         sessionRule.session.reload()
         sessionRule.session.waitForPageStop()
 
         assertThat("New session should receive navigation notifications",
                    onLocationCount, equalTo(1))
     }
 
+    @Suppress("DEPRECATION")
     @Test fun readFromParcel_focusedInput() {
         // When an input is focused, make sure SessionTextInput is still active after transferring.
         mainSession.loadTestPath(INPUTS_PATH)
         mainSession.waitForPageStop()
 
         mainSession.evaluateJS("document.querySelector('#input').focus()")
         mainSession.waitUntilCalled(object : Callbacks.TextInputDelegate {
             @AssertCalled(count = 1)
@@ -410,16 +421,17 @@ class SessionLifecycleTest : BaseSession
         // We should keep the session open when restoring the same open session.
         val view = testRestoreInstanceState(mainSession, mainSession)
         assertThat("View session is unchanged", view.session, equalTo(mainSession))
         assertThat("View session is open", view.session!!.isOpen, equalTo(true))
         view.session!!.reload()
         sessionRule.waitForPageStop()
     }
 
+    @Suppress("DEPRECATION")
     @Ignore // Bug 1533934 - disabled createFromParcel on pgo for frequent failures
     @Test fun createFromParcel() {
         val session = sessionRule.createOpenSession()
 
         session.toParcel { parcel ->
             val newSession = sessionRule.wrapSession(
                     GeckoSession.CREATOR.createFromParcel(parcel))
 
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
@@ -1284,44 +1284,64 @@ public class GeckoSession implements Par
         }
     }
 
     /* package */ void transferFrom(final GeckoSession session) {
         transferFrom(session.mWindow, session.mSettings, session.mId);
         session.mWindow = null;
     }
 
+    /**
+     * @deprecated Use {@link ProgressDelegate#onSessionStateChange(GeckoSession, GeckoSession.SessionState)} and
+     * {@link #restoreState} instead. This method will be removed in GeckoView 82.
+     */
+    @Deprecated // Bug 1650108
     @Override // Parcelable
     @AnyThread
     public int describeContents() {
         return 0;
     }
 
+    /**
+     * @deprecated Use {@link ProgressDelegate#onSessionStateChange(GeckoSession, GeckoSession.SessionState)} and
+     * {@link #restoreState} instead. This method will be removed in GeckoView 82.
+     */
+    @Deprecated // Bug 1650108
     @Override // Parcelable
     @AnyThread
     public void writeToParcel(final Parcel out, final int flags) {
         out.writeStrongInterface(mWindow);
         out.writeParcelable(mSettings, flags);
         out.writeString(mId);
     }
 
     // AIDL code may call readFromParcel even though it's not part of Parcelable.
+    /**
+     * @deprecated Use {@link ProgressDelegate#onSessionStateChange(GeckoSession, GeckoSession.SessionState)} and
+     * {@link #restoreState} instead. This method will be removed in GeckoView 82.
+     */
+    @Deprecated // Bug 1650108
     @AnyThread
     @SuppressWarnings("checkstyle:javadocmethod")
     public void readFromParcel(final @NonNull Parcel source) {
         final IBinder binder = source.readStrongBinder();
         final IInterface ifce = (binder != null) ?
                 binder.queryLocalInterface(Window.class.getName()) : null;
         final Window window = (ifce instanceof Window) ? (Window) ifce : null;
         final GeckoSessionSettings settings =
                 source.readParcelable(getClass().getClassLoader());
         final String id = source.readString();
         transferFrom(window, settings, id);
     }
 
+    /**
+     * @deprecated Use {@link ProgressDelegate#onSessionStateChange(GeckoSession, GeckoSession.SessionState)} and
+     * {@link #restoreState} instead. This field will be removed in GeckoView 82.
+     */
+    @Deprecated // Bug 1650108
     public static final Creator<GeckoSession> CREATOR = new Creator<GeckoSession>() {
         @Override
         @AnyThread
         public GeckoSession createFromParcel(final Parcel in) {
             final GeckoSession session = new GeckoSession();
             session.readFromParcel(in);
             return session;
         }
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md
@@ -29,16 +29,18 @@ exclude: true
   Use Glean to handle Gecko telemetry.
   ([bug 1644447]({{bugzilla}}1644447))
 - Added [`ensureBuiltIn`][79.3] that ensures that a built-in extension is
   installed without re-installing.
   ([bug 1635564]({{bugzilla}}1635564))
 - Added [`ProfilerController`][79.4], accessible via [`GeckoRuntime.getProfilerController`][79.5]
 to allow adding gecko profiler markers.
 ([bug 1624993]({{bugzilla}}1624993))
+- ⚠️ Deprecated `Parcelable` support in `GeckoSession` with the intention of removing
+  in GeckoView v82. ([bug 1649529]({{bugzilla}}1649529))
 
 [79.1]: {{javadoc_uri}}/WebExtension.TabDelegate.html#onOpenOptionsPage-org.mozilla.geckoview.WebExtension-
 [79.2]: {{javadoc_uri}}/WebNotification.html#source
 [79.3]: {{javadoc_uri}}/WebExtensionController.html#ensureBuiltIn-java.lang.String-java.lang.String-
 [79.4]: {{javadoc_uri}}/ProfilerController.html
 [79.5]: {{javadoc_uri}}/GeckoRuntime.html#getProfilerController--
 
 ## v78
@@ -735,9 +737,9 @@ to allow adding gecko profiler markers.
 [65.19]: {{javadoc_uri}}/GeckoSession.NavigationDelegate.LoadRequest.html#isRedirect
 [65.20]: {{javadoc_uri}}/GeckoSession.html#LOAD_FLAGS_BYPASS_CLASSIFIER
 [65.21]: {{javadoc_uri}}/GeckoSession.ContentDelegate.ContextElement.html
 [65.22]: {{javadoc_uri}}/GeckoSession.ContentDelegate.html#onContextMenu-org.mozilla.geckoview.GeckoSession-int-int-org.mozilla.geckoview.GeckoSession.ContentDelegate.ContextElement-
 [65.23]: {{javadoc_uri}}/GeckoSession.FinderResult.html
 [65.24]: {{javadoc_uri}}/CrashReporter.html#sendCrashReport-android.content.Context-android.os.Bundle-java.lang.String-
 [65.25]: {{javadoc_uri}}/GeckoResult.html
 
-[api-version]: a53322bd27a9a8968a3fbbc7e44ebefdd9a49ef8
+[api-version]: 3bf9118a008a2a172964a13ba3f79ff59fe1f77e