Closed Bug 1285041 Opened 4 years ago Closed 3 years ago

Investigate Chrome profile migration importing history when chrome is running

Categories

(Firefox :: Migration, defect, P3)

50 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 51
Tracking Status
firefox50 --- affected
firefox51 --- fixed

People

(Reporter: Gijs, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

After some feedback from our automigration project I looked into this a bit.

Apparently SQLite supports a 'read only' flag for opening db connections. Unfortunately, as far as I can tell, it doesn't actually help... I have a wip patch to add support to our sqlite.jsm wrapper, but the SELECT call on the url table still fails, presumably because Chrome keeps an EXCLUSIVE (https://www.sqlite.org/lockingv3.html ) lock on the DB. I haven't found a way of obtaining the data in this case, though maybe we can just copy the file and then read it, or something? Marco, do you know what our options are here? I think it's OK to not get everything, as opposed to getting absolutely nothing.
Flags: needinfo?(mak77)
Attached file readonly-sqlite.txt
The if'ing out of the cache_size assignment is a bit interesting - with this, at least opening the connection succeeds - but running any actual selects seems to trigger the locking returning SQLITE_LOCKED (IIRC the cache_size assignment returned SQLITE_BUSY but I could be wrong).
(In reply to :Gijs Kruitbosch from comment #0)
> Apparently SQLite supports a 'read only' flag for opening db connections.
> Unfortunately, as far as I can tell, it doesn't actually help... I have a
> wip patch to add support to our sqlite.jsm wrapper, but the SELECT call on
> the url table still fails, presumably because Chrome keeps an EXCLUSIVE
> (https://www.sqlite.org/lockingv3.html ) lock on the DB.

Yep, I think that's the case, exclusive locking means that only one process can access the db at a time, if Chrome has an open connection with exclusive locking, only the Chrome process can read/write to the db.

> I haven't found a
> way of obtaining the data in this case, though maybe we can just copy the
> file and then read it, or something?

I never tried, it may work, it depends if the locking is at the filesystem level, so it could also work on some filesystems but not on others.

I couldn't find enough information on the docs to tell, I'll just forward your question to Richard, who can surely help us with the available options, if any.
Flags: needinfo?(mak77) → needinfo?(drh)
Marco is correct:  Even in read-only mode, SQLite honors locks.  And so if Chrome has the database open in exclusive locking mode, it is inaccessible to other processes that following the SQLite locking protocol.

Your workaround is to not follow the SQLite locking protocol.  On unix specify the alternative "unix-none" VFS.  Just put "unix-none" as the fourth argument to sqlite3_open_v2().  Or specify the "vfs=unix-none" query parameter on a URI filename.  Then the exclusive lock held by Chrome will be ignored and Firefox will be able to read the database.

Two problems:  Firstly, without any locking, it is possible for Chrome to be writing the database at the same time as Firefox is reading.  If Chrome changes content out from under Firefox, that can cause Firefox to get and incorrect result or an SQLITE_CORRUPT error.  The incorrect result might occur, for example, if Firefox is using an index and Chrome has updated the index but not the main table.  The SQLITE_CORRUPT error is harmless, in the sense that the database is not really corrupt.  Subsequent attempts to read will likely work without errors.  Or, in the worst case, you might need to close and reopen the database connection to clear the rror.  Firefox needs to be prepared to deal with that situation.  Chrome writing to the database at the same time Firefox is reading should *not* cause a crash - we do a lot of testing for that case.

The second problem is that there is no equivalent for the unix-none VFS on the windows side.  But if this is something you need, we can have it for you in the next release of SQLite (3.14.0).
Dolske, what kind of priority does this have and/or do we know of other ways to deal with this issue on Windows or in general? Do you think it's worth it for sqlite to implement something as suggested in comment #3?

Feels like the most important is getting this working on Windows, but the first issue in comment #3 (incorrect results and/or "sqlite corrupt") is also problematic... Perhaps it'd make sense for us to experiment on non-Windows first? Equally, not sure what the current pipeline delay is for the fix to go into sqlite and for the new sqlite version to then make it into Firefox... Marco probably knows more about that?
Flags: needinfo?(mak77)
Flags: needinfo?(dolske)
Blocks: 1271800
(In reply to :Gijs Kruitbosch from comment #4)
> Equally, not sure what the current pipeline delay is for
> the fix to go into sqlite and for the new sqlite version to then make it
> into Firefox... Marco probably knows more about that?

We usually just wait for the new Sqlite version, then it's easy for us to upgrade since we have a zero-local-changes policy. The release date of 3.14.0 is up to them , we usually don't rush their releases unless there's serious problems (and this is not). Though if you really need the change, better letting them know earlier than later.

Note this will also need some Sqlite.jsm/Storage changes to be able to pass an ignoreLockingMode argument first to Sqlite.jsm and then through it to openAsyncDatabase. I'd not change the other legacy APIs, we only need this special case exposed through Sqlite.jsm.

Also, when that option is set we should also force the connection to be read-only.
Flags: needinfo?(mak77)
Just an FYI: The "Prerelease Snapshot" of SQLite 3.14.0 on the Download page (https://www.sqlite.org/download.html) contains a new "win32-none" VFS that works like unix-none by omitting all locks.  You can use the sqlite3.c and sqlite3.h files from this snapshot for testing prior to the official release, if you like.
(In reply to D. Richard Hipp from comment #6)
> Just an FYI: The "Prerelease Snapshot" of SQLite 3.14.0 on the Download page
> (https://www.sqlite.org/download.html) contains a new "win32-none" VFS that
> works like unix-none by omitting all locks.  You can use the sqlite3.c and
> sqlite3.h files from this snapshot for testing prior to the official
> release, if you like.

Thanks! I'll start by testing on a Unix-type system.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(drh)
(In reply to :Gijs Kruitbosch from comment #8)
> Created attachment 8772028 [details]
> Bug 1285041 - ignore locking when trying to read chrome DB file, f?mak
> 
> Review commit: https://reviewboard.mozilla.org/r/64964/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/64964/

So these are the basics... I tested on OS X and this seems to work. Things to consider:

- should we remove the 'close chrome' message on non-windows? I guess so?
- do we need to do something specific about the errors that might crop up per comment #3? Is there a way to automated-test that?
- at what point can we integrate this for windows and should we separate that to a different bug (commented out in the current patch to avoid issues when landing this).
Flags: needinfo?(dolske) → needinfo?(mak77)
(In reply to :Gijs Kruitbosch from comment #9)
> - should we remove the 'close chrome' message on non-windows? I guess so?

Does it hurt? This automated mode still has downsides and possible errors. Closing an app is an easy task and I think we should keep asking the user to do that when we can. This is not a silver bullet.

> - do we need to do something specific about the errors that might crop up
> per comment #3? Is there a way to automated-test that?

I don't think it's trivial to test it, sounds like there is risk to have intermittent pass/fail. You'd need a connection writing while another one reads, and you can't predict whether you'll get an exception, a wrong value, or it will just work.

> - at what point can we integrate this for windows and should we separate
> that to a different bug (commented out in the current patch to avoid issues
> when landing this).

As soon as we have the new version, it seems to be scheduled for July.
Flags: needinfo?(mak77)
Comment on attachment 8772028 [details]
Bug 1285041 - ignore locking when trying to read chrome DB file,

https://reviewboard.mozilla.org/r/64964/#review62330

::: browser/components/migration/ChromeProfileMigrator.js:328
(Diff revision 1)
> +        if (AppConstants.platform != "win") {
> +          dbOptions.ignoreLockingMode = true;
> +        }
> +        let db = yield Sqlite.openConnection(dbOptions);
>  
>          let rows = yield db.execute(`SELECT url, title, last_visit_time, typed_count

based on comment 3, this may throw and you'd get no result, since this tries to get all the rows at once.
This code may become quite complex if we want to handle all the possible cases.
As Richard said, every row fetch could return an exception, thus you should use the onrow callback, rather than the yield result... but at that point you have to restart the query and merge results... or you should just restart from scratch.
And he also said in some error cases you may have to close and reopen the connection.
So maybe you should fctor out this read and retry it a few times on a timer?

::: storage/mozStorageConnection.h:74
(Diff revision 1)
>     *        - |mozIStorageAsyncConnection|;
>     *        If |false|, the result also implements synchronous interface:
>     *        - |mozIStorageConnection|.
> +   * @param aIgnoreLockingMode
> +   *        If |true|, ignore locks in force on the file. Only usable with
> +   *        read-only connections. Defaults to false.

It may be worth to sum up the pitfalls of this mode, the fact if another connection is writing, any query could fail or return wrong data, thus this mode should not be commonly used.

::: storage/mozStorageConnection.cpp:492
(Diff revision 1)
> +, mIgnoreLockingMode(aIgnoreLockingMode)
>  , mStorageService(aService)
>  , mAsyncOnly(aAsyncOnly)
>  {
> +  if (mIgnoreLockingMode) {
> +    MOZ_RELEASE_ASSERT(mFlags & SQLITE_OPEN_READONLY,

why not just a 
MOZ_ASSERT(!mIgnoreLockingMode || mFlags...

you are already checking in the API entrance point and does't look like there's a way out of that. A relese assert seems excessive, when a debug one would be enough.

::: storage/mozStorageConnection.cpp:584
(Diff revision 1)
>  
>  nsresult
>  Connection::initialize()
>  {
>    NS_ASSERTION (!mDBConn, "Initialize called on already opened database!");
> +  MOZ_RELEASE_ASSERT(!mIgnoreLockingMode,

ditto

::: storage/mozStorageConnection.cpp:619
(Diff revision 1)
>  
>    nsAutoString path;
>    nsresult rv = aDatabaseFile->GetPath(path);
>    NS_ENSURE_SUCCESS(rv, rv);
>  
> +  nsAutoCString fileSystem;

let's name this baseVFS or similar, filesystem seems confusing

::: storage/mozStorageConnection.cpp:733
(Diff revision 1)
>    // or corrupt.  So this is executed regardless it being actually needed.
>    // The cache_size is calculated from the actual page_size, to save memory.
> +  int srv = SQLITE_OK;
> +  if (mFlags & SQLITE_OPEN_READWRITE) {
> -  nsAutoCString cacheSizeQuery(MOZ_STORAGE_UNIQUIFY_QUERY_STR
> +    nsAutoCString cacheSizeQuery(MOZ_STORAGE_UNIQUIFY_QUERY_STR
> -                               "PRAGMA cache_size = ");
> +                                 "PRAGMA cache_size = ");

Is this still needed with the ignored locking mode?
The reason I'm asking is that this is the first "real query" on the connection, that allows the open() call to return an error if the database is corrupt/unreadable.
By removing this, a read only connection will discover the error only when running the first query, rather than on open, and it may not be instrumented to handle that.

::: storage/mozStorageService.cpp:778
(Diff revision 1)
> +                                     &ignoreLockingMode);
> +    if (NS_FAILED(rv) && rv != NS_ERROR_NOT_AVAILABLE) {
> +      return NS_ERROR_INVALID_ARG;
> +    }
> +  }
> +  int flags = readOnly ? SQLITE_OPEN_READONLY : SQLITE_OPEN_READWRITE;

we can enforce readonly and locking mode here at the API level
Attachment #8772028 - Flags: review?(mak77)
Depends on: SQLite3.14.1
Hi everyone. Any update here?
(In reply to Chris More [:cmore] from comment #12)
> Hi everyone. Any update here?

I need to update the patch based on feedback. But even when I've done that, on Windows it would depend on the changes in bug 1293367, and uplifting both that and my hypothetical changes here to 49 at this stage of the beta cycle seems pretty far-fetched from a "stabilize on beta" perspective, with the release candidate going out at the beginning of next week, and the fact that those changes would be in code that gets run for all our sqlite connections, including all of places, cookies, etc. IOW, I think this is "wontfix" for the funnelcake we're going to run on 49. Sorry.
(In reply to :Gijs Kruitbosch from comment #13)
> (In reply to Chris More [:cmore] from comment #12)
> > Hi everyone. Any update here?
> 
> I need to update the patch based on feedback. But even when I've done that,
> on Windows it would depend on the changes in bug 1293367, and uplifting both
> that and my hypothetical changes here to 49 at this stage of the beta cycle
> seems pretty far-fetched from a "stabilize on beta" perspective, with the
> release candidate going out at the beginning of next week, and the fact that
> those changes would be in code that gets run for all our sqlite connections,
> including all of places, cookies, etc. IOW, I think this is "wontfix" for
> the funnelcake we're going to run on 49. Sorry.

I am not suggesting uplifting these into Fx 49. I am just curious in general since the Chrome population is so big and the possibility of the db being locked is relatively high. Just wanted to know where we were in general. Thanks for the update.
Comment on attachment 8772028 [details]
Bug 1285041 - ignore locking when trying to read chrome DB file,

https://reviewboard.mozilla.org/r/64964/#review62330

> we can enforce readonly and locking mode here at the API level

I think I've done this, but let me know if you think I should remove it from SQLite.jsm as well and/or if you meant something other than what I've done...
Comment on attachment 8772028 [details]
Bug 1285041 - ignore locking when trying to read chrome DB file,

https://reviewboard.mozilla.org/r/64964/#review76286

::: browser/components/migration/ChromeProfileMigrator.js:316
(Diff revision 2)
>    if (!historyFile.exists())
>      return null;
>  
> +  function getRows(dbOptions) {
> +    const RETRYLIMIT = 10;
> +    const RETRYINTERVAL = 50;

so in the end we will try for about 500ms. sounds a little bit short, maybe the interval could be bumped up to 100ms?

::: browser/components/migration/ChromeProfileMigrator.js:340
(Diff revision 2)
> +            if (didOpen) {
> +              yield db.close();
> +            }
> +          } catch (ex) {}
> +        }
> +        if (!rows) {

what if we can open and read, but the table is empty? maybe we should retry only if there's an exception?

::: storage/mozStorageConnection.cpp:622
(Diff revision 2)
>  
> +  nsAutoCString baseVFS;
> +  const char* rawVFS = nullptr;
> +  if (mIgnoreLockingMode) {
> +#ifdef XP_WIN
> +    baseVFS.AssignLiteral("win32-none");

you could probably define with #ifdef a:
static const char* sIgnoreLockingVFS = "nnn-none" and simplify this as

, mIgnoreLockingMode ? sIgnoreLockingVFS : nullptr);

::: storage/mozStorageService.cpp:751
(Diff revision 2)
>      return NS_ERROR_NOT_SAME_THREAD;
>    }
>    NS_ENSURE_ARG(aDatabaseStore);
>    NS_ENSURE_ARG(aCallback);
>  
>    nsCOMPtr<nsIFile> storageFile;

nit: storageFile definition could now be moved later in code, just before dbStore.

::: storage/mozStorageService.cpp:755
(Diff revision 2)
>  
>    nsCOMPtr<nsIFile> storageFile;
> -  int flags = SQLITE_OPEN_READWRITE;
> +  nsresult rv;
> +  bool readOnly = false;
> +  bool ignoreLockingMode = false;
> +  if (aOptions) {

Not mandatory, but I'd like to centralize aOptions management here at this point, instead of having multiple if (aOptions spread around), that means here we'd also set bool shared and int32_t growthIncrement and just use them later where appropriate.
It should make the code a bit easier to follow.

::: storage/mozStorageService.cpp:759
(Diff revision 2)
> +  bool ignoreLockingMode = false;
> +  if (aOptions) {
> +    rv = aOptions->GetPropertyAsBool(NS_LITERAL_STRING("readOnly"), &readOnly);
> +    if (NS_FAILED(rv) && rv != NS_ERROR_NOT_AVAILABLE) {
> +      return NS_ERROR_INVALID_ARG;
> +    }

at that point this whole

if (NS_FAILED &&& rv != ...
could be assigned to a temporary
#define FAIL_IF_SET_BUT_INVALID(rv) that could be reused for all the options

making the code even more readable.

::: storage/mozStorageService.cpp:785
(Diff revision 2)
>      }
>  
>      rv = storageFile->Clone(getter_AddRefs(storageFile));
>      MOZ_ASSERT(NS_SUCCEEDED(rv));
>  
>      // Ensure that SQLITE_OPEN_CREATE is passed in for compatibility reasons.

move the comment inside the if?

::: toolkit/modules/Sqlite.jsm:878
(Diff revision 2)
> + *
> + *   ignoreLockingMode -- (bool) Whether to ignore locks on the database held
> + *       by other connections. If used, implies readOnly. Defaults to false.
> + *       USE WITH EXTREME CAUTION. This mode WILL produce incorrect results or
> + *       return corruption errors if other connections write to the DB at the
> + *       same time. Because we force SQLITE_OPEN_READONLY, no writes are

I think the latter part (from "Because we force..." on) makes this less scary than it should be, this is really a special option for very very specific cases.
I'd remove the whole last phrase, since the first part says enough and it's scary enough alone.
Attachment #8772028 - Flags: review?(mak77)
Comment on attachment 8772028 [details]
Bug 1285041 - ignore locking when trying to read chrome DB file,

https://reviewboard.mozilla.org/r/64964/#review76286

> what if we can open and read, but the table is empty? maybe we should retry only if there's an exception?

rows would be an empty array, and so this check would fail. I've changed this to use the exception anyway to make it more obvious.

> Not mandatory, but I'd like to centralize aOptions management here at this point, instead of having multiple if (aOptions spread around), that means here we'd also set bool shared and int32_t growthIncrement and just use them later where appropriate.
> It should make the code a bit easier to follow.

Done, but the growthIncrement is only set if `storageFile` is non-nullptr, so I had to fidget about with it. Let me know if you want me to move it back down again instead, or if I misunderstood what you meant.
Comment on attachment 8772028 [details]
Bug 1285041 - ignore locking when trying to read chrome DB file,

https://reviewboard.mozilla.org/r/64964/#review77538

::: browser/components/migration/ChromeProfileMigrator.js:334
(Diff revision 3)
> +          didOpen = true;
> +          rows = yield db.execute(`SELECT url, title, last_visit_time, typed_count
> +                                   FROM urls WHERE hidden = 0`);
> +        } catch (ex) {
> +          exceptionSeen = ex;
> +          Cu.reportError(ex);

nit: maybe we should just report it once to the console, either when exceptionSeen is undefined (first time), or when retryCount == 1 (last time)?

::: browser/components/migration/ChromeProfileMigrator.js:364
(Diff revision 3)
> -        });
> +        };
>  
> -        let rows = yield db.execute(`SELECT url, title, last_visit_time, typed_count
> -                                     FROM urls WHERE hidden = 0`);
> -        yield db.close();
> +        let rows = yield getRows(dbOptions);
> +        if (!rows) {
> +          throw new Error("Couldn't get rows from the Chrome history database.");
> +        }

nit: off-hand it sounds like it would be less error prone to let getRows throw by itself, so it will alway return an array or throw, rather than return an array or return null.

::: storage/mozStorageService.cpp:818
(Diff revision 3)
>      // Just fall through with nullptr storageFile, this will cause the storage
>      // connection to use a memory DB.
>    }
>  
> -  int32_t growthIncrement = -1;
> -  if (aOptions && storageFile) {
> +  if (!storageFile) {
> +    growthIncrement = -1;

let's rather
if (!storageFile && growthIncrement >= 0) {
  return NS_ERROR_INVALID_ARG;
}

if someone sets growthIncrement on a non-file connection, it's a clear mistake we should notify to the consumer, I think.

::: toolkit/modules/Sqlite.jsm:877
(Diff revision 3)
> + *       set. If used, writing to the database will fail. Defaults to false.
> + *
> + *   ignoreLockingMode -- (bool) Whether to ignore locks on the database held
> + *       by other connections. If used, implies readOnly. Defaults to false.
> + *       USE WITH EXTREME CAUTION. This mode WILL produce incorrect results or
> + *       return corruption errors if other connections write to the DB at the

...return "false positive" corruption errors...
Attachment #8772028 - Flags: review?(mak77)
Comment on attachment 8772028 [details]
Bug 1285041 - ignore locking when trying to read chrome DB file,

https://reviewboard.mozilla.org/r/64964/#review77540
Attachment #8772028 - Flags: review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ef7939c0332e
ignore locking when trying to read chrome DB file, r=mak
Two questions:

1) I assume we've tested this and it does result in being able to import from Chrome while it's running? (Possibly dumb question, but I didn't see anything about this in the bug so far... :)

2) The ignoreLockingMode code comment and comment 3 talk about incorrect results and corruption... What's the practical upshot of this for migration? It sorta sounds like these are technicalities and we just need to be careful? Or does this mean it's possible the user could see corrupted/garbage bookmarks in Firefox?
(In reply to Justin Dolske [:Dolske] from comment #24)
> 
> It sorta sounds like these are technicalities and we just need to be
> careful? Or does this mean it's possible the user could see
> corrupted/garbage bookmarks in Firefox?

This does not exactly answer your question, since I do not know about the FF code, only about SQLite. My remarks are just background information. 

From the point of view of SQLite, when you access a database while ignoring locks, either the SQLite APIs will return errors, or the content of individual rows will be intact.  Foreign key and uniqueness constraints might not be right, and the rows you get back might not be exactly what the WHERE clause specified, but the rows you do get should be internally self-consistent.

The one exception to the previous paragraph is when a single row contains so much content that it spans multiple pages.  Probably the databases you are reading never have such large rows.  (You can check some samples by running the sqlite3_analyzer.exe utility available on the SQLite website against the sample databases and grepping for "Entries that use overflow" lines.)  And even if there are some rows that span multiple pages, the chance of you getting an internally inconsistent row back that SQLite does not detect and throw and error over are pretty slim.

Finally, as long as Chrome is not updating the database at the same instant you are reading it, all of your data will be completely correct and consistent.
(In reply to Justin Dolske [:Dolske] from comment #24)
> Two questions:
> 
> 1) I assume we've tested this and it does result in being able to import
> from Chrome while it's running? (Possibly dumb question, but I didn't see
> anything about this in the bug so far... :)

Yes.

> 2) The ignoreLockingMode code comment and comment 3 talk about incorrect
> results and corruption... What's the practical upshot of this for migration?
> It sorta sounds like these are technicalities and we just need to be
> careful? Or does this mean it's possible the user could see
> corrupted/garbage bookmarks in Firefox?

Note that this affects history, not bookmarks, which are in a JSON file. I am more OK with missing bits of old/new history but getting most of it, than I would be with bookmarks.

I would assume that what will happen if we get incorrect results is that either (a) we get old results that Chrome might have just removed and/or (b) we don't get new results that Chrome might have just added. I've seen (b) happen in practice, though I don't know to what degree that's just delay in Chrome actually writing to the db, either. I mean, that or "corruption" errors if things really go pearshapes, in which case we will abort the fetch from the DB, close our connection, and reopen it to try again after a small delay.

So based on that and comment #25, I think we'll be fine. If uniqueness constraints are violated the worst I'd expect it to result in is some history entries being duplicated, which again, I don't think will really materially affect the result of the import (though we could theoretically detect such cases).


Ni->me to figure out test failures.
Flags: needinfo?(gijskruitbosch+bugs)
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6267fc17f8c0
ignore locking when trying to read chrome DB file, r=mak
Relanded:

08:45	Gijs	mak: just to be 100% sure, it's not possible to have an in-memory sqlite db with a non- "-1" growthIncrement, right?
08:46	Gijs	mak: the "let's make this actually fail" behaviour case is failing me a unit test :P
08:46	mak	Gijs: it wouldn't make any sense since growth increment is for disk files...
08:46	Gijs	mak: right... so I'm fixing the test, unless you want me to get rid of the error code again if the test is expecting it to just be silently ignored?
08:47	mak	if you don't tell me which test, it's going to be hard to guess :D
08:47	Gijs	searchfox.org/mozilla-central/sour…test_storage_connection.js#357-359
08:47	Gijs	mak: ^ yeah, sorry, was looking for a link
08:47	Gijs	mak: so currently, I have a patch that removes the growthIncrement there, and then adds a case to the list of "this should throw" checks below for pretty much what those lines are currently doing
08:48	Gijs	pastebin.mozilla.org/8910558
08:48	mak	SGTM, let me just double check sqlite documentation, to be sure.
08:50	mak	ok, it0s undocumented
08:50	mak	off-hand I think it's ok to change the test, I don't see why one would want to set a growth increment on a memory db
08:50	mak	yes, rs=me
08:52	Gijs	mak: perfect, thanks!
Flags: needinfo?(gijskruitbosch+bugs)
https://hg.mozilla.org/mozilla-central/rev/6267fc17f8c0
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Blocks: 1309613
You need to log in before you can comment on or make changes to this bug.