952465 - [Session Restore] Replace Components.utils.reportError with Console.jsm

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[David Teller [:Yoric] - still alive but not very active]

Session Restore uses Components.utils.reportError (often abbreviated as Cu.reportError) to report errors. We should rather use the more complete Console.jsm.

The code that needs to be patched lives in:
http://dxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/src/

Comment 1

[Peiyong Lin[:lpy](UTC+8)]

Attachment: bug952465.patch

replace reportError with console.error

Comment 2

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8350876 [details] [diff] [review]
bug952465.patch

Review of attachment 8350876 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks.
r=me with the changes below, if it passes tests

::: browser/components/sessionstore/src/SessionFile.jsm
@@ +35,5 @@
>  Cu.import("resource://gre/modules/osfile/_PromiseWorker.jsm", this);
>  Cu.import("resource://gre/modules/Promise.jsm");
>  Cu.import("resource://gre/modules/AsyncShutdown.jsm");
>  
> +let console = Cu.import("resource://gre/modules/devtools/Console.jsm", {}).console;

Let's make it lazy:
 XPCOMUtils.defineLazyModuleGetter(this, "console", "resource://...");

This will ensure that we load the Console module only when we actually need it, which will make startup a little faster. Also, could you move this a bit lower, close to the other calls to |XPCOMUtils.defineLazyModuleGetter|?

Same thing for the other files.

@@ +103,5 @@
>    captureErrors: function captureErrors(promise) {
>      return promise.then(
>        null,
>        function onError(reason) {
> +        console.error("Uncaught asynchronous error: " + reason + " at\n" + reason.stack);

Ideally, this should be
 console.error("Uncaught asynchronous error", reason, "at", reason.stack);

However, we'll completely remove TaskUtils one of these days, so that's not very important.

@@ +181,5 @@
>          this._recordTelemetry(msg.telemetry);
>        } catch (ex) {
>          TelemetryStopwatch.cancel("FX_SESSION_RESTORE_WRITE_FILE_LONGEST_OP_MS", refObj);
> +        console.error("Could not write session state file " + this.path
> +                      + ": " + ex);

This will be more precise if you write
  console.error("Could not write session state file", this.path, ex);

Comment 3

[Peiyong Lin[:lpy](UTC+8)]

Attachment: bug952465-Final.patch

Thank you Yoric :)

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/c0c89e5d0cf1

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c0c89e5d0cf1