Closed Bug 1343182 Opened 7 years ago Closed 7 years ago

Add support for updatePlaces' ignoreErrors, ignoreResults, and aGroupNotifications to insertMany API surface

Categories

(Toolkit :: Places, defect, P2)

Product:
Toolkit
Component:
Places
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: Gijs, Assigned: ganesh2583, Mentored)

References

Details

(Keywords: good-first-bug, Whiteboard: [good first bug][lang=js])

Attachments

(1 file, 7 obsolete files)

fixed tests
5.65 KB, patch
mak
: review+
Details | Diff | Splinter Review 
In bug 1341097 I added various bits of support to mozIAsyncHistory's updatePlaces API so consumers can ask not to have callbacks posted (to the main thread) for individual rows/items. Those bits aren't currently exposed thoroughly to History.jsm's insertMany, and they should be.
What I care about is that insertMany callbacks become optional and it uses ignoreNNN properties when callbacks are not provided.
It should also probably group notifications by default.
Priority: -- → P2
The scope of the bug is to change History.jsm::insertMany so that both of its callbacks become optional and, if they are not provided, it should set ignoreErrors and ignoreResults properties in the updatePlaces callback object, accordingly.
It should also pass true as the third argument to updatePlaces, to group notifications.
Tests may need adjustements for notifications grouping, will for sure need to check optional callbacks work as expected.
Tests like in toolkit/components/places/tests/history/
Keywords: good-first-bug
Whiteboard: [good first bug][lang=js]
(In reply to Marco Bonardo [::mak] from comment #2)
> The scope of the bug is to change History.jsm::insertMany so that both of
> its callbacks become optional and, if they are not provided, it should set
> ignoreErrors and ignoreResults properties in the updatePlaces callback
> object, accordingly.
> It should also pass true as the third argument to updatePlaces, to group
> notifications.
> Tests may need adjustements for notifications grouping, will for sure need
> to check optional callbacks work as expected.
> Tests like in toolkit/components/places/tests/history/

Hi Marco,

I would like to work on this bug. I did a setup following the page https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds. Please let me know on how to get started on this. 

Regards,
Ganesh
Hi Ganesh, I'm happy you'd like to help. The comment you quoted from Mak does provide the details you need to get started.
If you have more specific questions, please express them and needinfo me, as MArk said the current information should be enough to start, you can use http://searchfox.org/ or http://dxr.mozilla.org/ to search the codebase.
Attached patch Initail Patch with fix. (obsolete) — — Splinter Review 
Hi,

I have added the initial patch with the fix. I basically verified if the call backs are not provided and then constructed a updatePlaces callback object according to comment #2 and called the updatePlaces function with that. Let me know if my understanding was correct and any changes are required to the patch.

Regards,
Ganesh
Attachment #8850071 - Flags: review?(markh)
Attachment #8850071 - Flags: review?(mak77)
Comment on attachment 8850071 [details] [diff] [review]
Initail Patch with fix.

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

We should at least add a test for onManyFrecenciesChanged, since there is already a test that we are fine with no callbacks.

In this inserter() task (http://searchfox.org/mozilla-central/rev/9af9b1c7946ebef6056d2ee4c368d496395cf1c8/toolkit/components/places/tests/history/test_insert.js#200) I'd suggest adding an helper function to the top of the test, something like:

function promiseManyFrecenciesChanged() {
  return new Promise((resolve, reject) => {
    let obs = new NavHistoryObserver();
    obs.onManyFrecenciesChanged = () => {
      PlacesUtils.history.removeObserver(obs);
      resolve();
    };
    obs.onFrecencyChanged = () => {
      PlacesUtils.history.removeObserver(obs);
      reject();
    };
    PlacesUtils.history.addObserver(obs, false);
  });
}

and then use it around the insertMany call here:
http://searchfox.org/mozilla-central/rev/9af9b1c7946ebef6056d2ee4c368d496395cf1c8/toolkit/components/places/tests/history/test_insert.js#229
let promiseManyFrecencies = promiseManyFrecenciesChanged();
result = yield PlacesUtils.history.insertMany(pageInfos);
yield promiseManyFrecencies;

::: toolkit/components/places/History.jsm
@@ +252,4 @@
>        let info = validatePageInfo(pageInfo);
>        infos.push(info);
>      }
> +	

trailing  spaces here
Also, the indentation is wrong, you are using TABs, we indent with whitespaces.
Please setup your editor for indentation as 2 whitespaces.

@@ +272,5 @@
> +			ignoreResults: false,
> +			aGroupNotifications: true,
> +		};
> +		this._asyncHistory.updatePlaces(validatedConvertedPageInfos, updatePlacesCallbackNoError);
> +	}

you should not invoke updatePlaces here, the internal insertMany function that is invoked just below already does that.
You should do some of this inside that internal function, in particular there you can directly pass true as groupNotification of the already existing updatePlaces call.
And still there you should set the ignoreNNN property on the object that is being already passed to updatePlaces
Attachment #8850071 - Flags: review?(markh)
Attachment #8850071 - Flags: review?(mak77)
Attachment #8850071 - Flags: review-
I'm working on the review comments. Will update soon. I have a query, can you please let me know what is meant by ignoreNNN property. I could not find a reference of it in the code base.
By ignoreNNN I meant both ignoreResults or ignoreErrors, sorry I didn't figure it was confusing, maybe I should have said "ignore*"!
I'm planing to update the updatePlaces call in the https://dxr.mozilla.org/mozilla-central/source/toolkit/components/places/History.jsm#1026 as below. I added ignore* calls to the call back object and passing the 3rd argument for groupNotification as true by default. Please let me know your thoughts on this.

PlacesUtils.asyncHistory.updatePlaces(infos, {
    ignoreErrors: true,
    ignoreResults: true,
    handleCompletion: () => {
        let pageInfo = mergeUpdateInfoIntoPageInfo(result);
        onResultData.push(pageInfo);
        if (onResultData.length) {
            resolve();
        } else {
            reject({
                message: "No items were added to history."
            })
        }
    }
}, true);
(In reply to ganesh2583 from comment #10)
> I'm planing to update the updatePlaces call in the
> https://dxr.mozilla.org/mozilla-central/source/toolkit/components/places/
> History.jsm#1026 as below. I added ignore* calls to the call back object and
> passing the 3rd argument for groupNotification as true by default. Please
> let me know your thoughts on this.
> 
> PlacesUtils.asyncHistory.updatePlaces(infos, {
>     ignoreErrors: true,
>     ignoreResults: true,

Ok, but those should be set to true only if the respective callbacks are not provided.
Hi Marco,

I have implemented the code review comments. Added ignoreNNN properties conditionally. I was not sure if I have to remove implementation of handleError and handleResult. If I have to remove them, should we get rid of notifyOnResult(onResultData, onResult); notifyOnResult(onErrorData, onError); as well inside the handleCompletion fucntion. Please let me know. Added the test you mentioned as will.

Regards,
Ganesh
Attachment #8851246 - Flags: review?(mak77)
Comment on attachment 8851246 [details] [diff] [review]
Implemented the code review comments and attached the patch

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

when attaching a new patch, you can set the previous one as obsolete. You can do that later too by editing the details of the previous patch.

The commit message should outline what the patch does in general, ready for landing, not specifics about things like "second attempt" or "fixed code reviews", that's not what the commit message is about.

In general the patch looks ok, just some details to fix, and a proper commit message. You can also already add r=mak at the end of the commit message, so it's ready for check-in.
thank you for your contribution.

::: toolkit/components/places/History.jsm
@@ +1032,5 @@
>          let pageInfo = mergeUpdateInfoIntoPageInfo(result);
>          onResultData.push(pageInfo);
>        },
> +      ignoreErrors: (if(!onError) ? true : undefined),
> +      ignoreResults: (if(!onResult) ? true : undefined),

you can just set them to !!onError and !!onResult, so they will just be true or false.

::: toolkit/components/places/tests/history/test_insert.js
@@ +210,5 @@
> +            reject();
> +          };
> +          PlacesUtils.history.addObserver(obs, false);
> +        });
> +    }

indentation is wrong inside the promise callback, every indentation step should just be 2 whitespaces, not 4.
Attachment #8851246 - Flags: review?(mak77) → feedback+
I have implemented your review comments. Set the previous 2 patches as Obsolete. Added more meaning full commit message and marked you for ready for check in. Please let me know if you still see any indentation issues. I have set the indentation to 2 white spaces. 

Please let me know if there are any other interesting bugs that I can work on. Would be happy to take up and learn.
Attachment #8850071 - Attachment is obsolete: true
Attachment #8851246 - Attachment is obsolete: true
Attachment #8852350 - Flags: review?(mak77)
Comment on attachment 8852350 [details] [diff] [review]
Bug 1343182 - Added updatePlaces's ignoreErrors, ignoreResults and aGroupNotifications to insertMany API in History.jsm. Added a test for onManyFrecenciesChanged.r=mak

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

You can eventually continue working on related stuff, that is bug 1282770. I see that you already commented there. And that bug basically depends on this one, since we need ignore properties support to be able to convert some of the consumers.

::: toolkit/components/places/History.jsm
@@ +1032,5 @@
>          let pageInfo = mergeUpdateInfoIntoPageInfo(result);
>          onResultData.push(pageInfo);
>        },
> +      ignoreErrors: !!onError,
> +      ignoreResults: !!onResult,

oops, my fault, should just be !onError and !onResult.
Indeed we should ignoreError if onError is NOT provided.
I'm sorry :\

did you try to run the tests with ./mach build faster && ./mach xpcshel-test toolkit/components/places/tests/ ? this should have failed them...
It's important to run the tests in the area you are modifying before attaching new patches.

@@ +1043,4 @@
>            reject({message: "No items were added to history."})
>          }
>        }
> +    },true);

missing space before true

::: toolkit/components/places/tests/history/test_insert.js
@@ +198,5 @@
>    });
>  
>    let inserter = Task.async(function*(name, filter, useCallbacks) {
> +    function promiseManyFrecenciesChanged(){
> +      return new Promise((resolve,reject) => {

missing space before reject

@@ +208,5 @@
> +        obs.onFrecencyChanged = () => {
> +          PlacesUtils.history.removeObserver(obs);
> +          reject();
> +        };
> +        PlacesUtils.history.addObserver(obs,false);

missing space before false (we always want space after comma).
You can use ./mach eslint toolkit/components/places to verify some syntax and coding style rules.
Attachment #8852350 - Flags: review?(mak77)
Assignee: nobody → ganesh2583
Status: NEW → ASSIGNED
I'm getting a lot of failures when I run ./mach build faster && ./mach xpcshell-test toolkit/components/places/tests/. Almost all tests failed with the reason "failed or timed out, will retry." Please let me know if I'm missing anything.
Maybe your build is broken, I'd suggest:
1. ./mach clobber
2. Check that in your .mozconfig you have ac_add_options --enable-artifact-builds
3. ./mach build (this will generate an artifact build in 10-15 minutes, depending on your internet speed, that is enough for testing javascript and style changes in Firefox)
4. try to run the build you obtained through ./mach run
5. at this point the previous commands should work fine
I see the below errors when I run ./mach build. I am on default branch and did a hg pull.

 1:07.13 c:\mozilla-build\mozmake\mozmake.EXE -f client.mk MOZ_PARALLEL_BUILD=4 -s
 1:11.55 Adding client.mk options from c:/mozilla-source/mozilla-central/mozconfig:
 1:11.55     MOZ_MAKE_FLAGS=-j4
 1:11.56     MOZ_OBJDIR=c:/mozilla-source/mozilla-central/objdir-frontend-debug-artifact
 1:11.56     OBJDIR=c:/mozilla-source/mozilla-central/objdir-frontend-debug-artifact
 1:11.56     FOUND_MOZCONFIG=c:/mozilla-source/mozilla-central/mozconfig
 1:15.75 Attempting to find a pushhead containing dd694af1f474fa313856bba8949ddcbc97acbd09 on mozilla-central.
 1:17.74 Attempting to find a pushhead containing dd694af1f474fa313856bba8949ddcbc97acbd09 on integration/mozilla-inbound.
 1:18.99 Attempting to find a pushhead containing dd694af1f474fa313856bba8949ddcbc97acbd09 on releases/mozilla-aurora.
 1:20.33 Error running mach:
 1:20.33
 1:20.33     ['--log-no-times', 'artifact', 'install']
 1:20.33
 1:20.34 The error occurred in code that was called by the mach command. This is either
 1:20.34 a bug in the called code itself or in the way that mach is calling it.
 1:20.34
 1:20.35 You should consider filing a bug for this issue.
 1:20.36
 1:20.36 If filing a bug, please include the full output of mach, including this error
 1:20.36 message.
 1:20.36
 1:20.36 The details of the failure are as follows:
 1:20.37
 1:20.37 Exception: Could not find any candidate pushheads in the last 50 revisions.
 1:20.37 Search started with dd694af1f474fa313856bba8949ddcbc97acbd09, which must be known to Mozilla automation.
 1:20.37
 1:20.38 see https://developer.mozilla.org/en-US/docs/Artifact_builds
 1:20.38
 1:20.38   File "c:/mozilla-source/mozilla-central\python/mozbuild/mozbuild/mach_commands.py", line 1530, in artifact_install
 1:20.39     return artifacts.install_from(source, self.distdir)
 1:20.40   File "c:/mozilla-source/mozilla-central\python/mozbuild\mozbuild\artifacts.py", line 1088, in install_from
 1:20.40     return self.install_from_recent(distdir)
 1:20.40   File "c:/mozilla-source/mozilla-central\python/mozbuild\mozbuild\artifacts.py", line 1049, in install_from_recent
 1:20.40     return self._install_from_hg_pushheads(hg_pushheads, distdir)
 1:20.40   File "c:/mozilla-source/mozilla-central\python/mozbuild\mozbuild\artifacts.py", line 1028, in _install_from_hg_pushheads
 1:20.41     for trees, hg_hash in hg_pushheads:
 1:20.41   File "c:/mozilla-source/mozilla-central\python/mozbuild\mozbuild\artifacts.py", line 946, in _find_pushheads
 1:20.41     rev=last_revs[0], num=NUM_PUSHHEADS_TO_QUERY_PER_PARENT))
 1:20.41 Makefile:234: recipe for target 'recurse_artifact' failed
 1:20.42 mozmake.EXE[4]: *** [recurse_artifact] Error 1
 1:20.42 c:/mozilla-source/mozilla-central/config/recurse.mk:32: recipe for target 'artifact' failed
 1:20.43 mozmake.EXE[3]: *** [artifact] Error 2
 1:20.43 c:/mozilla-source/mozilla-central/config/rules.mk:525: recipe for target 'default' failed
 1:20.43 mozmake.EXE[2]: *** [default] Error 2
 1:20.43 c:/mozilla-source/mozilla-central/client.mk:419: recipe for target 'realbuild' failed
 1:20.43 mozmake.EXE[1]: *** [realbuild] Error 2
 1:20.44 client.mk:170: recipe for target 'build' failed
 1:20.44 mozmake.EXE: *** [build] Error 2
 1:20.46 0 compiler warnings present.
2
did you run ./mach bootstrap (pick artifact desktop build there)?
did you pull a recent enough mozilla-central?
To update your repo:
(if your patch is in mercurial queue) hg qpop -a (to pop all the temporarily applied patches)
hg pull central
hg update central
hg qpush (until you reapply your patch on top of current central)
then try building again.
If instead your patch is committed already (not using mercurial queues) you can use
hg rebase -s your_changeset -d central
and use hg wip to check and find the various heads you may create

Otherwise, store your patch apart, and restart from scratch following instructions from here: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build
Until you have a build env that works, it may be hard to work on further issues.
I think there is an issue with the fix. I'm repeatedly getting failures with the fix and when I backed out the changes I made and ran the tests, they went through fine.

Upon further analysis, adding the below code at http://searchfox.org/mozilla-central/rev/9af9b1c7946ebef6056d2ee4c368d496395cf1c8/toolkit/components/places/tests/history/test_insert.js#229
let promiseManyFrecencies = promiseManyFrecenciesChanged();
result = yield PlacesUtils.history.insertMany(pageInfos);
yield promiseManyFrecencies;

is causing the failures. If I run the tests without the above code in test_insert.js, then there were no failures.

Can you please advice on how to proceed with this.
Flags: needinfo?(mak77)
I have ran the tests and I got the result as below:

Summary
=======

Ran 254 tests
Expected results: 253
Unexpected results: 0
Skipped: 1

OK

But still when I run the tests for second time I'm seeing failures, like below 

22:55.58 LOG: Thread-693 INFO xpcshell return code: None
22:55.58 LOG: Thread-693 ERROR toolkit/components/places/tests/favicons/test_favicons_conversions.js | Process still running after test!
rmtree() failed for "('c:\\mozilla-source\\mozilla-central\\objdir-frontend\\temp\\xpc-profile-pxmdb4',)". Reason: Access is denied (13). Retrying...
22:55.61 TEST_START: Thread-694 toolkit/components/places/tests/unit/test_frecency.js
22:56.37 TEST_END: Thread-694 FAIL, expected PASS
xpcshell return code: 0

Let me know if its anything familiar to you.
Attachment #8852350 - Attachment is obsolete: true
Attachment #8853644 - Flags: review?(mak77)
(In reply to ganesh2583 from comment #21)
> 22:55.58 LOG: Thread-693 ERROR
> toolkit/components/places/tests/favicons/test_favicons_conversions.js |
> Process still running after test!
> rmtree() failed for
> "('c:\\mozilla-source\\mozilla-central\\objdir-frontend\\temp\\xpc-profile-
> pxmdb4',)". Reason: Access is denied (13). Retrying...

This may be expected, especially on Windows where the file locks last more than usual.
It could also be caused by your antivirus, so you could maybe add the mozilla-source forder to its exclusions list.
Flags: needinfo?(mak77)
(In reply to Marco Bonardo [::mak] from comment #22)
 
> This may be expected, especially on Windows where the file locks last more
> than usual.
> It could also be caused by your antivirus, so you could maybe add the
> mozilla-source forder to its exclusions list.


Oh ok. Will look in that. Can you please let know if the patch is fine and can I get started in bug 1282770 ?
Comment on attachment 8853644 [details] [diff] [review]
Bug 1343182 - Added updatePlaces's ignoreErrors, ignoreResults and aGroupNotifications to insertMany API in History.jsm. Added a test for onManyFrecenciesChanged. r=mak

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

It looks good so far, let's see what Try thinks about it, then we can land if everything is fine.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1656159daecb1f4dd403336fab996ffa59a2507e
Attachment #8853644 - Flags: review?(mak77) → review+
Comment on attachment 8853644 [details] [diff] [review]
Bug 1343182 - Added updatePlaces's ignoreErrors, ignoreResults and aGroupNotifications to insertMany API in History.jsm. Added a test for onManyFrecenciesChanged. r=mak

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

::: toolkit/components/places/History.jsm
@@ +1046,1 @@
>            reject({message: "No items were added to history."})

Ah, there is a problem indeed shown by Try server, and maybe related to your local failures?
onResultData is filled by onResult, but if onResult is not provided, it won't be filled, and this will reject.
We need to change the code so that handleCompletion gets the updatedCount argument (see for example http://searchfox.org/mozilla-central/rev/990055a4902952e038cc350c9ffe1ac426d1c943/browser/components/migration/ChromeProfileMigrator.js#351)
and then change the code to do:
if (updatedCount > 0) {
  resolve();
} else {
  reject({message: "No items were added to history."})
}
Attachment #8853644 - Flags: review+
Hi Marco,

I made the change as below and ran the tests and 3 tests have failed. 

handleCompletion: (updatedCount) => {
        notifyOnResult(onResultData, onResult);
        notifyOnResult(onErrorData, onError);
        if (updatedCount > 0) {
          resolve();
        } else {
          reject({message: "No items were added to history."})
        }
}

Please find the summary as below:

Summary
=======

Ran 257 tests
Expected results: 253
Unexpected results: 3 (FAIL: 3)
Skipped: 1

Unexpected Results
==================

FAIL toolkit/components/places/tests/unit/test_000_frecency.js
FAIL toolkit/components/places/tests/unit/test_463863.js
FAIL toolkit/components/places/tests/unit/test_multi_queries.js
Tests were run in parallel. Try running with --sequential to make sure the failures were not caused by this.
(In reply to ganesh2583 from comment #26)
> Created attachment 8854050 [details]
> Attached the complete test run log after making the change
> 
> Hi Marco,
> 
> I made the change as below and ran the tests and 3 tests have failed. 
...
> FAIL toolkit/components/places/tests/unit/test_000_frecency.js
> FAIL toolkit/components/places/tests/unit/test_463863.js
> FAIL toolkit/components/places/tests/unit/test_multi_queries.js
> Tests were run in parallel. Try running with --sequential to make sure the
> failures were not caused by this.

Hi Ganesh,
  If you look through the log file you attached, you will see the output from each of those 3 tests, and you need to work out why each one is failing. Once you've worked out why, Mak or others can probably help you determine the correct course of action, but the first step is to determine the root cause of the failures.
you can also run a single test passing its path to ./mach test and often that helps checking the error message.

I suspect the problem is that updatedCount doesn't count embed visits. This is a bug in History.cpp
indeed here:
http://searchfox.org/mozilla-central/rev/381a7b8f8a78b481336cfa384cb0a5536e217e4a/toolkit/components/places/History.cpp#2984
we should keep a count of the number of embed visits we add, and then we should pass that number to InsertVisitedURIs::Start that will start counting from it, and pass it also to NotifyCompletion here
http://searchfox.org/mozilla-central/rev/381a7b8f8a78b481336cfa384cb0a5536e217e4a/toolkit/components/places/History.cpp#3022

otherwise updatedCount will be 0 and insertMany will throw thinking that no visit was successfully added.

Additionally StoreAndNotifyEmbedVisit should also check ignoreErrors and ignoreResults before notifying.

We can either file a separate bug to fix this and make this bug depend on that, you could try fixing it here. But note you won't be able to use an artifact build for this, since it doesn't support cpp. So you should remove the artifact option from .mozconfig and do a full build (./mach build may take from 20 minutes to 1 hour, then you can use ./mach build toolkit/components/places for incremental builds that are much faster), and I don't know if you know and want to work on cpp.
Let me know, we can do both.
I have gone through the changes you have mentioned. But I'm not so familiar with CPP programming. I would prefer having an new bug raised and dealt by an CPP programmer rather than me. Please let me know any bug related to JS or Java Programming so that I can contribute.
bug 1282770 is fine for you to work on, I will file a blocker to fix the cpp problem and likely try to find time to fix it myself.
Hi Marco,

I added the change you suggested in comment 25 and attached the patch. Will start working on bug 1282770 now.

Regards,
Ganesh
Attachment #8854050 - Attachment is obsolete: true
Just a small unbitrot of the patch on top of bug 1353783 so it's ready to land when it's the time.
Attachment #8853644 - Attachment is obsolete: true
Attachment #8854963 - Attachment is obsolete: true
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/38bbf2279a94
Added updatePlaces's ignoreErrors, ignoreResults and aGroupNotifications to insertMany API in History.jsm. r=mak
Keywords: checkin-needed
My fault, I ran a lot of tests, but not those 2 folders :(
There are a couple tests expecting single frecency notifications from insertMany... funny. I will look into those tomorrow, nothing to worry about.
The failing tests are:
browser/components/migration/tests/unit/test_automigration.js
browser/components/newtab/tests/xpcshell/test_PlacesProvider.js
Flags: needinfo?(mak77)
Attachment #8855291 - Attachment is obsolete: true
Flags: needinfo?(mak77)
builders are quite busy today, it may take a while, I'm setting checkin-needed in the meanwhile, please check the try build in comment 37 before pushing.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f92e59ad3033
Add updatePlaces' ignoreErrors, ignoreResults and aGroupNotifications to insertMany API in History.jsm. r=mak
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f92e59ad3033
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: