Bug 1649529: Deprecate GeckoSession parcelability; r=geckoview-reviewers,agi, a=jcristau
authorAaron Klotz <aklotz@mozilla.com>
Thu, 02 Jul 2020 15:10:56 +0000
changeset 601910 8314c4d75b906e4be84484b91c7366da22acedc7
parent 601909 8b651aee50c34fa63284d91f92212d4fda7a2889
child 601911 f5e5c772293f3d5c923cc5003958690c577d691d
push id13350
push userjcristau@mozilla.com
push dateFri, 03 Jul 2020 15:09:58 +0000
treeherdermozilla-beta@6e11fb81cbfb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgeckoview-reviewers, agi, jcristau
bugs1649529
milestone79.0
Bug 1649529: Deprecate GeckoSession parcelability; r=geckoview-reviewers,agi, a=jcristau 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)
@@ -406,16 +417,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
@@ -25,16 +25,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
@@ -731,9 +733,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