Don't stomp on non-sync pending updates.

Don't stomp on non-sync pending updates.


BUG=61581, chromium-os:7037
TEST=ExtensionsServiceTest.UpdatePendingExternalCrxWinsOverSync

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=64788

[Erik does not do reviews, 2010-10-28 00:10:54]

LGTM

I'm OK with you landing this as-is in order to address the bug, but I'm
concerned that we may not have the overall big picture logic correct for
handling the precedence of conflicting install locations.  Could you please add
a TODO to come back to this issue?

http://codereview.chromium.org/4147007/diff/1/2
File chrome/browser/extensions/extensions_service.cc (right):

http://codereview.chromium.org/4147007/diff/1/2#newcode731
chrome/browser/extensions/extensions_service.cc:731: // If a non-sync update is
pending, a sync request should not
This looks like it only affects sync as a source.  Is it ever a good thing to
have the same extension pending from multiple locations?

http://codereview.chromium.org/4147007/diff/1/2#newcode745
chrome/browser/extensions/extensions_service.cc:745: if
(!it->second.is_from_sync && is_from_sync)
shouldn't you just move this if (is_from_sync) around the whole outer block
here?  no need to do the work otherwise.

http://codereview.chromium.org/4147007/diff/1/3
File chrome/browser/extensions/extensions_service_unittest.cc (right):

http://codereview.chromium.org/4147007/diff/1/3#newcode1650
chrome/browser/extensions/extensions_service_unittest.cc:1650: theme_crx,
GURL(), PendingExtensionInfo::THEME, false, false, false);
either comment these bools or make them const named variables for doc purposes.