Reflect scanning status in command UI. Only allow import once scanning is complete.

Reflect scanning status in command UI. Only allow import once scanning is complete.

This is a sizeable change that reworks the flow of control.
Introduce an ImportController in the foreground that manages scanning.
Move cloud-import command logic out of file_manager_commands into ImportController.
Update MediaImportHandler to accept a ScanResult instead of an array of entries.
Make MediaScanner observable, so that commands can be updated when a scan finishes.

NOTE: I due to slow and frequently broken (not by me) ChromeOS builds, I have not yet sucessfully tested this on ChromeOS. Still, I wanted to get the change mailed. All tests are passing. I'll remedy the situation with ChromeOS ASAP.

BUG=420680
TEST=browser_test: FileManagerJsTest.*

Committed: https://crrev.com/66e1ad07571c8d2d1a64adab5e03680d07fac03f
Cr-Commit-Position: refs/heads/master@{#309229}

[Steve McKay, 2014-12-18 00:02:36]

Refactoring input into ImporterCommand for easier testing.

[Steve McKay, 2014-12-18 00:40:10]

Barebones (and probably doesn't work yet) test for import controller.

[Steve McKay, 2014-12-18 00:41:41]

smckay@chromium.org changed reviewers:
+ mtomasz@chromium.org

[Steve McKay, 2014-12-18 00:41:41]

Note that this is not yet thoroughly tested. Test is still in progress. But
since you're in TOK and I'm in LA, earlier feedback is better :)

[mtomasz, 2014-12-18 01:39:05]

https://codereview.chromium.org/792233009/diff/40001/chrome/app/chromeos_stri...
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/792233009/diff/40001/chrome/app/chromeos_stri...
chrome/app/chromeos_strings.grdp:449: Insufficient space on local device!
Local device may sound cryptic to typical user. How about "your device" / "the
device"? WDYT?

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/background/js/media_import_handler_unittest.js
(right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/background/js/media_import_handler_unittest.js:80:
var scan = new TestScanResult(media);
nit: scan -> result / scanResult?

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/background/js/media_scanner.js (right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/background/js/media_scanner.js:256: return
!this.settled_;
DefaultScanResult should be a sink for results. It doesn't scan, hence
isScanning is confusing to me here. How about moving the member isScanning to
MediaScanner? As that's the class which performs scanning. WDYT?

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/background/js/media_scanner.js:275:
importer.DefaultScanResult.prototype.whenFinished = function() {
Is it possible to move whenFinished to MediaScanner? Currently the method sounds
like "When scan results are finished", which is not really logical. We don't
finish results, but scan.

Alternatively we could tweak the naming only. Instead finishing, we could commit
(like in transactional db). Then:
"When scan results are finished" would turn into "When scan results are
committed". It makes more sense, as we may commit results once scan is
completed.

Similarly, instead of rejecting scan, we should reject results.

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/common/js/importer_common.js (right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/common/js/importer_common.js:133: * @extends
{VolumeManagerCommon.VolumeInfoProvider}
nit: Remove this extend?

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/foreground/js/file_manager_commands.js
(right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/file_manager_commands.js:1044:
fileManager.importController.execute(input);
This works, but why not simply passing RuntimeCommandInput(fileManager) to
constructor of ImportController? We wouldn't need to create a new delegate
object for each invocation.

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/foreground/js/import_controller.js (right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/import_controller.js:9: * Class that
orchestrates background activity and UI changes on
To be honest, I'm getting lost in the architecture. It's quite a lot of classes.
Could you draft a small class diagram of the cloud import and quickly describe
responsibilities of each of them?

[Steve McKay, 2014-12-18 03:34:40]

Respond to review comments.

[Steve McKay, 2014-12-18 03:34:53]

More to come tomorrow. Cheers!

https://codereview.chromium.org/792233009/diff/40001/chrome/app/chromeos_stri...
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/792233009/diff/40001/chrome/app/chromeos_stri...
chrome/app/chromeos_strings.grdp:449: Insufficient space on local device!
On 2014/12/18 01:39:04, mtomasz wrote:
> Local device may sound cryptic to typical user. How about "your device" / "the
> device"? WDYT?

Done. Though I doubt any of these strings will last their first encounter with
PM-Ben :)

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/background/js/media_import_handler_unittest.js
(right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/background/js/media_import_handler_unittest.js:80:
var scan = new TestScanResult(media);
On 2014/12/18 01:39:04, mtomasz wrote:
> nit: scan -> result / scanResult?

Done.

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/background/js/media_scanner.js (right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/background/js/media_scanner.js:256: return
!this.settled_;
On 2014/12/18 01:39:04, mtomasz wrote:
> DefaultScanResult should be a sink for results. It doesn't scan, hence
> isScanning is confusing to me here. How about moving the member isScanning to
> MediaScanner? As that's the class which performs scanning. WDYT?

I can rename "isFinished" if that would feel more intuitive.

I'm treating MediaScanner as more of a factory and ScanResult as a live object.
It could easily have been named just "importer.Scan", but I think ScanResult is
more in line with how I want folks to think of it...which is as a sort of
see-through Promise.

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/background/js/media_scanner.js:275:
importer.DefaultScanResult.prototype.whenFinished = function() {
On 2014/12/18 01:39:04, mtomasz wrote:
> Is it possible to move whenFinished to MediaScanner? Currently the method
sounds
> like "When scan results are finished", which is not really logical. We don't
> finish results, but scan.
> 
> Alternatively we could tweak the naming only. Instead finishing, we could
commit
> (like in transactional db). Then:
> "When scan results are finished" would turn into "When scan results are
> committed". It makes more sense, as we may commit results once scan is
> completed.
> 
> Similarly, instead of rejecting scan, we should reject results.

"Committed" doesn't seem right to me, but I can swap out use of "isScanning"
with "isFinished" or "isFinal" which should be more consistent with how you're
looking at this. Though honestly I prefer "finished" to "final".

Clients that are interested in *any* scan results being finished can observe
MediaScanner. Clients that want to know about a particular result, should either
ask "are you finished" or join the Promise (whenFinished).

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/common/js/importer_common.js (right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/common/js/importer_common.js:133: * @extends
{VolumeManagerCommon.VolumeInfoProvider}
On 2014/12/18 01:39:04, mtomasz wrote:
> nit: Remove this extend?

It does extend. In CommandInput we proxy the call to
fileManager.volumeManager.getVolumeInfo. This hides the concrete dependency on
both file manager and volume manager in a way that is easy to stub out in a
test.

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/foreground/js/file_manager_commands.js
(right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/file_manager_commands.js:1044:
fileManager.importController.execute(input);
On 2014/12/18 01:39:04, mtomasz wrote:
> This works, but why not simply passing RuntimeCommandInput(fileManager) to
> constructor of ImportController? We wouldn't need to create a new delegate
> object for each invocation.

Okay. Lemme do a little refactoring. I'll ping you back when that is done.

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/foreground/js/import_controller.js (right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/import_controller.js:9: * Class that
orchestrates background activity and UI changes on
On 2014/12/18 01:39:04, mtomasz wrote:
> To be honest, I'm getting lost in the architecture. It's quite a lot of
classes.
> Could you draft a small class diagram of the cloud import and quickly describe
> responsibilities of each of them?

Sure. But can I do it concurrently? I'd hate to block submission of this CL.
Bascially we're feeling our way through the class hierarchy as we implement
requirements. This is pretty much necessary as the environment, where we're
basically naive, places various constraints...that we lear as we go.

But for now: ImportController is the main class responsible for orchestrating
import activities as they touch the UI. It kicks off scans, handles command
updates, request button updates, should object to the user when there is
insufficient space, caches scan results, and when instructed by the
user...initiates scan.

media_scanner.js...scans media...providing a view on what's happening.
media_import_handler...is the import pipeline...taking a set of results (as
specified by a ScanResult) and does stuff like copying them to local disk (and
will do content dedupe).

Later we're going to need to add stuff like monitoring of drive client for sync
events...then update history when sync events happen. And more...but I can't
think of it right now.

[mtomasz, 2014-12-18 08:59:06]

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/background/js/media_scanner.js (right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/background/js/media_scanner.js:256: return
!this.settled_;
On 2014/12/18 03:34:52, Steve McKay wrote:
> On 2014/12/18 01:39:04, mtomasz wrote:
> > DefaultScanResult should be a sink for results. It doesn't scan, hence
> > isScanning is confusing to me here. How about moving the member isScanning
to
> > MediaScanner? As that's the class which performs scanning. WDYT?
> 
> I can rename "isFinished" if that would feel more intuitive.
> 
> I'm treating MediaScanner as more of a factory and ScanResult as a live
object.
> It could easily have been named just "importer.Scan", but I think ScanResult
is
> more in line with how I want folks to think of it...which is as a sort of
> see-through Promise.

Got it. I think it makes sense to keep MediaResults a live object if clients use
partial results. If not, then we could just return Promise<ScanResult>, rather
than ScanResult directly.

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/background/js/media_scanner.js:275:
importer.DefaultScanResult.prototype.whenFinished = function() {
On 2014/12/18 03:34:52, Steve McKay wrote:
> On 2014/12/18 01:39:04, mtomasz wrote:
> > Is it possible to move whenFinished to MediaScanner? Currently the method
> sounds
> > like "When scan results are finished", which is not really logical. We don't
> > finish results, but scan.
> > 
> > Alternatively we could tweak the naming only. Instead finishing, we could
> commit
> > (like in transactional db). Then:
> > "When scan results are finished" would turn into "When scan results are
> > committed". It makes more sense, as we may commit results once scan is
> > completed.
> > 
> > Similarly, instead of rejecting scan, we should reject results.
> 
> "Committed" doesn't seem right to me, but I can swap out use of "isScanning"
> with "isFinished" or "isFinal" which should be more consistent with how you're
> looking at this. Though honestly I prefer "finished" to "final".
> 
> Clients that are interested in *any* scan results being finished can observe
> MediaScanner. Clients that want to know about a particular result, should
either
> ask "are you finished" or join the Promise (whenFinished).

I like isFinal, as it means that results are final, so they won't be modified
anymore. Saying that results are finished, sounds strange to me.

As for partial results, why clients would observe MediaScanner? ScanResult
supports partial results, so shouldn't we talk to ScanResult directly to get
partial results? Am I missing anything here?

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/common/js/importer_common.js (right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/common/js/importer_common.js:133: * @extends
{VolumeManagerCommon.VolumeInfoProvider}
On 2014/12/18 03:34:52, Steve McKay wrote:
> On 2014/12/18 01:39:04, mtomasz wrote:
> > nit: Remove this extend?
> 
> It does extend. In CommandInput we proxy the call to
> fileManager.volumeManager.getVolumeInfo. This hides the concrete dependency on
> both file manager and volume manager in a way that is easy to stub out in a
> test.

Got it. Thanks.

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/foreground/js/import_controller.js (right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/import_controller.js:9: * Class that
orchestrates background activity and UI changes on
On 2014/12/18 03:34:52, Steve McKay wrote:
> On 2014/12/18 01:39:04, mtomasz wrote:
> > To be honest, I'm getting lost in the architecture. It's quite a lot of
> classes.
> > Could you draft a small class diagram of the cloud import and quickly
describe
> > responsibilities of each of them?
> 
> Sure. But can I do it concurrently? I'd hate to block submission of this CL.
> Bascially we're feeling our way through the class hierarchy as we implement
> requirements. This is pretty much necessary as the environment, where we're
> basically naive, places various constraints...that we lear as we go.
> 
> But for now: ImportController is the main class responsible for orchestrating
> import activities as they touch the UI. It kicks off scans, handles command
> updates, request button updates, should object to the user when there is
> insufficient space, caches scan results, and when instructed by the
> user...initiates scan.
> 
> media_scanner.js...scans media...providing a view on what's happening.
> media_import_handler...is the import pipeline...taking a set of results (as
> specified by a ScanResult) and does stuff like copying them to local disk (and
> will do content dedupe).
> 
> Later we're going to need to add stuff like monitoring of drive client for
sync
> events...then update history when sync events happen. And more...but I can't
> think of it right now.

Thanks for explanation. Sure, let's parallelize. I have to run from the office.
I'll take a look again early morning in TOK.

[Steve McKay, 2014-12-18 22:48:21]

Make tests pass and stuff generally work as expected! Don't load controller
unless feature is enabled...making the lastKnownImportEnabled obsolete.

[Steve McKay, 2014-12-18 23:04:11]

Okay. I think I've addressed all of your comments and my promises.

One sort of big change in this patch set is: Don't load controller unless
feature is enabled && nuke the lastKnownImportEnabled business...that we added
for the command stuff in the first place. I think this leaves us with a nicer
arrangement entirely.

Also, I added coverage for ImportController which improves the situation where
we had no coverage of the cloud-import command in file_manager_commonds. NOTE:
I'm *not* striving for comprehensive coverage in the tests at this point, just
adequate coverage. As we nail down behaviors in the code, we can add coverage.

Lastly, since this is your Friday, and I'll be gone before you get back (and
gone for two weeks), please if possible LGTM. You know I'm good for promises I
make :)

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/background/js/media_scanner.js (right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/background/js/media_scanner.js:256: return
!this.settled_;
On 2014/12/18 08:59:06, mtomasz wrote:
> On 2014/12/18 03:34:52, Steve McKay wrote:
> > On 2014/12/18 01:39:04, mtomasz wrote:
> > > DefaultScanResult should be a sink for results. It doesn't scan, hence
> > > isScanning is confusing to me here. How about moving the member isScanning
> to
> > > MediaScanner? As that's the class which performs scanning. WDYT?
> > 
> > I can rename "isFinished" if that would feel more intuitive.
> > 
> > I'm treating MediaScanner as more of a factory and ScanResult as a live
> object.
> > It could easily have been named just "importer.Scan", but I think ScanResult
> is
> > more in line with how I want folks to think of it...which is as a sort of
> > see-through Promise.
> 
> Got it. I think it makes sense to keep MediaResults a live object if clients
use
> partial results. If not, then we could just return Promise<ScanResult>, rather
> than ScanResult directly.

Indeed we'll want partial results...we'll want to show the user how many files
have been found and their size...while scanning is happening.

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/background/js/media_scanner.js:275:
importer.DefaultScanResult.prototype.whenFinished = function() {
On 2014/12/18 08:59:06, mtomasz wrote:
> On 2014/12/18 03:34:52, Steve McKay wrote:
> > On 2014/12/18 01:39:04, mtomasz wrote:
> > > Is it possible to move whenFinished to MediaScanner? Currently the method
> > sounds
> > > like "When scan results are finished", which is not really logical. We
don't
> > > finish results, but scan.
> > > 
> > > Alternatively we could tweak the naming only. Instead finishing, we could
> > commit
> > > (like in transactional db). Then:
> > > "When scan results are finished" would turn into "When scan results are
> > > committed". It makes more sense, as we may commit results once scan is
> > > completed.
> > > 
> > > Similarly, instead of rejecting scan, we should reject results.
> > 
> > "Committed" doesn't seem right to me, but I can swap out use of "isScanning"
> > with "isFinished" or "isFinal" which should be more consistent with how
you're
> > looking at this. Though honestly I prefer "finished" to "final".
> > 
> > Clients that are interested in *any* scan results being finished can observe
> > MediaScanner. Clients that want to know about a particular result, should
> either
> > ask "are you finished" or join the Promise (whenFinished).
> 
> I like isFinal, as it means that results are final, so they won't be modified
> anymore. Saying that results are finished, sounds strange to me.
> 
> As for partial results, why clients would observe MediaScanner? ScanResult
> supports partial results, so shouldn't we talk to ScanResult directly to get
> partial results? Am I missing anything here?

Renamed isScanning > isFinal.

As to the why...
There are different reasons to know about different aspects of scanning.

1) When commands are updated we'll want to get to current state of the scan or
initiate one if the right circumstances are met. That's all in ImportController.
Having this information at hand, even when scan is incomplete will let us say
something like this (*super paraphrased*):

"Scanning...found 22 files so far. Requires XXMB of space on your device."

We *could* decide to stop the scan as soon as a device has insufficient space.

2) We need to update commands when *any* scan finishes. That'll cause the
command system to poll us...at which point we'll find that the scan is finished
and can show the user something like this:

"Import 2,200 photos to Google Drive. Wow, 2,200!?!?"

Given the disconnect between scan's being finished > kicking command system for
an update, then it polling us. I feel like the observer pattern on MediaScanner
is simple and clean. There is no other purpose ImportController would need to
listen on the whenFinal Promise...since only the imported would need to get the
*final* results as they are needed for importing.

Basically, we can have a single listener on MediaScanner, or individual
listeners on each scan. I prefer the single as it is less management.

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
File ui/file_manager/file_manager/foreground/js/import_controller.js (right):

https://codereview.chromium.org/792233009/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/import_controller.js:9: * Class that
orchestrates background activity and UI changes on
On 2014/12/18 08:59:06, mtomasz wrote:
> On 2014/12/18 03:34:52, Steve McKay wrote:
> > On 2014/12/18 01:39:04, mtomasz wrote:
> > > To be honest, I'm getting lost in the architecture. It's quite a lot of
> > classes.
> > > Could you draft a small class diagram of the cloud import and quickly
> describe
> > > responsibilities of each of them?
> > 
> > Sure. But can I do it concurrently? I'd hate to block submission of this CL.
> > Bascially we're feeling our way through the class hierarchy as we implement
> > requirements. This is pretty much necessary as the environment, where we're
> > basically naive, places various constraints...that we lear as we go.
> > 
> > But for now: ImportController is the main class responsible for
orchestrating
> > import activities as they touch the UI. It kicks off scans, handles command
> > updates, request button updates, should object to the user when there is
> > insufficient space, caches scan results, and when instructed by the
> > user...initiates scan.
> > 
> > media_scanner.js...scans media...providing a view on what's happening.
> > media_import_handler...is the import pipeline...taking a set of results (as
> > specified by a ScanResult) and does stuff like copying them to local disk
(and
> > will do content dedupe).
> > 
> > Later we're going to need to add stuff like monitoring of drive client for
> sync
> > events...then update history when sync events happen. And more...but I can't
> > think of it right now.
> 
> Thanks for explanation. Sure, let's parallelize. I have to run from the
office.
> I'll take a look again early morning in TOK.

Acknowledged.

[Steve McKay, 2014-12-18 23:40:13]

Update controller to return a response with instructions to be interpreted in
command, rather than manipulating command event directly.

[Steve McKay, 2014-12-18 23:48:11]

Patchset #6 (id:100001) has been deleted

[Steve McKay, 2014-12-18 23:48:43]

Update controller to return a response with instructions to be interpreted in
command, rather than manipulating command event directly.

[Steve McKay, 2014-12-19 00:01:41]

Fix a small command visibility bug.

[mtomasz, 2014-12-19 05:10:09]

lgtm

https://codereview.chromium.org/792233009/diff/140001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/import_controller_unittest.js
(right):

https://codereview.chromium.org/792233009/diff/140001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/import_controller_unittest.js:201: 
nit: Remove \n?

[Steve McKay, 2014-12-19 05:15:09]

Respond to review comments.

[Steve McKay, 2014-12-19 05:15:49]

DOne. Thanks! Submitting.

https://codereview.chromium.org/792233009/diff/140001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/import_controller_unittest.js
(right):

https://codereview.chromium.org/792233009/diff/140001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/import_controller_unittest.js:201: 
On 2014/12/19 05:10:09, mtomasz wrote:
> nit: Remove \n?

Done.

[Steve McKay, 2014-12-19 05:16:05]

The CQ bit was checked by smckay@chromium.org

[commit-bot: I haz the power, 2014-12-19 05:16:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/792233009/160001

[commit-bot: I haz the power, 2014-12-19 05:20:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-19 05:20:30]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_x64_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Steve McKay, 2014-12-19 06:49:41]

Merge w/ master...naughty, naughty master.

[Steve McKay, 2014-12-19 18:08:20]

Fix FM.jstests.cc && Finalize scan results in
MediaImportHandlerTest...unbreaking the test.

[Steve McKay, 2014-12-19 18:08:38]

The CQ bit was checked by smckay@chromium.org

[commit-bot: I haz the power, 2014-12-19 18:08:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/792233009/200001

[commit-bot: I haz the power, 2014-12-19 19:33:00]

Committed patchset #10 (id:200001)

[commit-bot: I haz the power, 2014-12-19 19:33:57]

Patchset 10 (id:??) landed as
https://crrev.com/66e1ad07571c8d2d1a64adab5e03680d07fac03f
Cr-Commit-Position: refs/heads/master@{#309229}