Enable forced extension updates on NaCl arch mismatch

Enable forced extension updates on NaCl arch mismatch

This makes extensions aware of the platforms for which
they have platform-specific resources installed, if any.

This also hooks up the extension update code with some
additional logic to place an extension in forced-update
mode if it has platform-specific resources which don't
match the current NaCl architecture.

BUG=409948
TEST=install an extension which uses NaCl (QuickOffice for example). Rename the _platform-specific/<your-nacl-arch> directory some something else and force an update (e.g. via chrome://extensions button). Observe that a new CRX is downloaded and installed.

Committed: https://crrev.com/43ae1fb32df63bfd42c13914feb6a71f111fccb3
Cr-Commit-Position: refs/heads/master@{#293018}

[Sorin Jianu, 2014-08-30 03:28:01]

sorin@chromium.org changed reviewers:
+ sorin@chromium.org

[Sorin Jianu, 2014-08-30 03:28:01]

Thank you Ken!

[Ken Rockot(use gerrit already), 2014-09-02 18:30:07]

Ok - Cleaned this up in accordance with the recent discussion about how to
handle this. We're including a new installsource for logging, but update will be
forced with a 0.0.0.0 version number.

Ready for review.

[Ken Rockot(use gerrit already), 2014-09-02 19:11:17]

rockot@chromium.org changed reviewers:
+ yoz@chromium.org

[Ken Rockot(use gerrit already), 2014-09-02 19:11:18]

Yawzzz, could you please take a look?

[Yoyo Zhou, 2014-09-02 19:30:04]

LGTM with nits

https://chromiumcodereview.appspot.com/516293007/diff/40001/chrome/browser/ex...
File chrome/browser/extensions/updater/extension_downloader.cc (right):

https://chromiumcodereview.appspot.com/516293007/diff/40001/chrome/browser/ex...
chrome/browser/extensions/updater/extension_downloader.cc:674: if
(!fetch_data.DidForceUpdate(id)) {
This could be at 669.

https://chromiumcodereview.appspot.com/516293007/diff/40001/chrome/browser/ex...
File chrome/browser/extensions/updater/manifest_fetch_data.cc (right):

https://chromiumcodereview.appspot.com/516293007/diff/40001/chrome/browser/ex...
chrome/browser/extensions/updater/manifest_fetch_data.cc:77: if (force_update) {
nit: no braces

[Ken Rockot(use gerrit already), 2014-09-02 20:04:30]

Thanks.

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/updater/extension_downloader.cc (right):

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
chrome/browser/extensions/updater/extension_downloader.cc:674: if
(!fetch_data.DidForceUpdate(id)) {
On 2014/09/02 19:30:04, Yoyo Zhou wrote:
> This could be at 669.

Doh! Done.

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/updater/manifest_fetch_data.cc (right):

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
chrome/browser/extensions/updater/manifest_fetch_data.cc:77: if (force_update) {
On 2014/09/02 19:30:04, Yoyo Zhou wrote:
> nit: no braces

Done.

[Ken Rockot(use gerrit already), 2014-09-02 20:04:36]

The CQ bit was checked by rockot@chromium.org

[Sorin Jianu, 2014-09-02 20:06:39]

Thank you Ken!

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/updater/extension_downloader.cc (right):

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
chrome/browser/extensions/updater/extension_downloader.cc:220: if
(extension.HasPlatformSpecificResources() &&
Do we need both conditions?

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/updater/extension_downloader.h (right):

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
chrome/browser/extensions/updater/extension_downloader.h:171: const std::string&
install_source_override);
Do we want to have both parameters?

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/updater/manifest_fetch_data.cc (right):

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
chrome/browser/extensions/updater/manifest_fetch_data.cc:83: std::string
installed_version = force_update ? "0.0.0.0" : version;
could be made const.

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/updater/manifest_fetch_data.h (right):

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
chrome/browser/extensions/updater/manifest_fetch_data.h:80: // Indiciates
whether or not a given extension ID was forced to update.
Indicates.

Or rephrase to "Returns true if a given ext...."

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
chrome/browser/extensions/updater/manifest_fetch_data.h:102: 
Comment? The other private members have comments.

[Ken Rockot(use gerrit already), 2014-09-02 20:07:23]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2014-09-02 20:19:21]

Thanks for the additional comments; replies inline

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/updater/extension_downloader.cc (right):

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
chrome/browser/extensions/updater/extension_downloader.cc:220: if
(extension.HasPlatformSpecificResources() &&
On 2014/09/02 20:06:39, Sorin Jianu wrote:
> Do we need both conditions?

Yes. Many extensions don't have any platform-specific resources.

For force-update to kick in, it is necessary that an extension uses
platform-specific resources AND doesn't have resources for the current platform.

I thought it would be better to expose these two interfaces separately on
Extension and combine them here, rather than define a single interface that
specifically addresses this condition.

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/updater/extension_downloader.h (right):

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
chrome/browser/extensions/updater/extension_downloader.h:171: const std::string&
install_source_override);
On 2014/09/02 20:06:39, Sorin Jianu wrote:
> Do we want to have both parameters?

Not really, but I didn't like the idea of conflating "force update" with "force
update because of a NaCl arch mismatch", or the idea of conflating "override the
install source" with "force update."

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/updater/manifest_fetch_data.cc (right):

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
chrome/browser/extensions/updater/manifest_fetch_data.cc:83: std::string
installed_version = force_update ? "0.0.0.0" : version;
On 2014/09/02 20:06:39, Sorin Jianu wrote:
> could be made const.

Done.

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/updater/manifest_fetch_data.h (right):

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
chrome/browser/extensions/updater/manifest_fetch_data.h:80: // Indiciates
whether or not a given extension ID was forced to update.
On 2014/09/02 20:06:39, Sorin Jianu wrote:
> Indicates.
> 
> Or rephrase to "Returns true if a given ext...."

Done.

https://codereview.chromium.org/516293007/diff/40001/chrome/browser/extension...
chrome/browser/extensions/updater/manifest_fetch_data.h:102: 
On 2014/09/02 20:06:39, Sorin Jianu wrote:
> Comment? The other private members have comments.

Done.

[Sorin Jianu, 2014-09-02 20:23:16]

lgtm

Thank you Ken!

[Sorin Jianu, 2014-09-02 20:23:17]

lgtm

Thank you Ken!

[Ken Rockot(use gerrit already), 2014-09-02 20:24:45]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2014-09-02 20:28:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rockot@chromium.org/516293007/80001

[Ken Rockot(use gerrit already), 2014-09-02 21:08:51]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2014-09-02 21:09:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rockot@chromium.org/516293007/100001

[commit-bot: I haz the power, 2014-09-02 22:08:31]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[commit-bot: I haz the power, 2014-09-02 23:15:28]

Committed patchset #5 (id:100001) as 4a92281fa5d331860d65a59ba45dc882a5c71df4

[Shu Chen, 2014-09-03 07:14:44]

shuchen@chromium.org changed reviewers:
+ shuchen@chromium.org

[Shu Chen, 2014-09-03 07:14:45]

This cl caused linux_chromeos crash on start:

shuchen@chenshu:~/chrome/src$ out/Release/chrome --user-data-dir=~/.config/cros
--login-manager
[25056:25056:0903/125913:ERROR:sandbox_linux.cc(305)] InitializeSandbox() called
with multiple threads in process gpu-process
[3,1957558528:04:59:13.776650] Native Client module will be loaded at base
address 0x00002cbf00000000
[25029:25029:0903/125914:ERROR:extension_pref_value_map.cc(75)] NOTREACHED() hit
in CanExtensionControlPref.
shuchen@chenshu:~/chrome/src$ out/Release/chrome --user-data-dir=~/.config/cros
--login-manager --enable-virtual-keyboard
--enable-experimental-input-view-features
[30420:30420:0903/140211:ERROR:sandbox_linux.cc(305)] InitializeSandbox() called
with multiple threads in process gpu-process
[30393:30393:0903/140211:FATAL:thread_restrictions.cc(38)] Function marked as
IO-only was called from a thread that disallows IO!  If this thread really
should be allowed to make IO calls, adjust the call to
base::ThreadRestrictions::SetIOAllowed() in this thread's startup.
Cannot upload crash dump: cannot exec /sbin/crash_reporter

Crash_reporter failed to process crash report
Aborted (core dumped)

[dzhioev (left Google), 2014-09-03 12:44:40]

A revert of this CL (patchset #5 id:100001) has been created in
https://codereview.chromium.org/532183003/ by dzhioev@chromium.org.

The reason for reverting is: Chrome OS build with DCHECKs enabled fails on
start. This CL breaks assumption, that GAIA extension can be loaded without
performing IO-operations.

Log and stack trace:
[16275:16275:0903/161435:WARNING:renderer_freezer.cc(55)] Cgroup freezer does
not exist or is not writable. Processes will not be frozen during suspend.
[16275:16275:0903/161435:WARNING:configuration_policy_pref_store.cc(30)] Policy
RemoteAccessClientFirewallTraversal: This policy has been deprecated.
[16275:16275:0903/161437:FATAL:thread_restrictions.cc(38)] Function marked as
IO-only was called from a thread that disallows IO!  If this thread really
should be allowed to make IO calls, adjust the call to
base::ThreadRestrictions::SetIOAllowed() in this thread's startup.
#0 0x7fe3122e555e base::debug::StackTrace::StackTrace()
#1 0x7fe31237c462 logging::LogMessage::~LogMessage()
#2 0x7fe3124824af base::ThreadRestrictions::AssertIOAllowed()
#3 0x7fe3123630a1 base::PathExists()
#4 0x7fe321cfd5f3 extensions::(anonymous
namespace)::CollectPlatformSpecificResourceArchs()
#5 0x7fe321cfad22 extensions::Extension::InitFromValue()
#6 0x7fe321cfa490 extensions::Extension::Create()
#7 0x7fe321cfa114 extensions::Extension::Create()
#8 0x7fe3233ebe1b extensions::ComponentLoader::Load()
#9 0x7fe3233ec3cf extensions::ComponentLoader::Add()
#10 0x7fe3233ec32a extensions::ComponentLoader::Add()
#11 0x7fe3233ec2b0 extensions::ComponentLoader::Add()
#12 0x7fe32437505f (anonymous namespace)::LoadGaiaAuthExtension()
#13 0x7fe324374d5e extensions::GaiaAuthExtensionLoader::LoadIfNeeded()
#14 0x7fe324237f7e ScopedGaiaAuthExtension::ScopedGaiaAuthExtension()
#15 0x7fe322f68bb2 chromeos::WebUILoginView::Init()
#16 0x7fe322f556ef chromeos::LoginDisplayHostImpl::InitLoginWindowAndView()
#17 0x7fe322f52640 chromeos::LoginDisplayHostImpl::LoadURL()
#18 0x7fe322f5226a chromeos::LoginDisplayHostImpl::StartWizard()
#19 0x7fe322f54acd chromeos::LoginDisplayHostImpl::StartPostponedWebUI()
#20 0x7fe322f541f8 chromeos::LoginDisplayHostImpl::Observe()
#21 0x7fe322f54bfd chromeos::LoginDisplayHostImpl::Observe()
#22 0x7fe31b222377 content::NotificationServiceImpl::Notify()
#23 0x7fe322d51a24 chromeos::(anonymous
namespace)::UserWallpaperDelegate::OnWallpaperAnimationFinished()
#24 0x7fe31188692f ash::RootWindowController::OnWallpaperAnimationFinished()
#25 0x7fe3117c633a ash::(anonymous
namespace)::ShowWallpaperAnimationObserver::OnImplicitAnimationsCompleted()
#26 0x7fe311f72d30 ui::ImplicitAnimationObserver::CheckCompleted()
#27 0x7fe311f72cd5 ui::ImplicitAnimationObserver::SetActive()
#28 0x7fe311f965d5
ui::ScopedLayerAnimationSettings::~ScopedLayerAnimationSettings()
#29 0x7fe3117c5ec7 ash::DesktopBackgroundWidgetController::StartAnimating()
#30 0x7fe3117bbb0c ash::DesktopBackgroundController::InstallDesktopController()
#31 0x7fe3117bbc03
ash::DesktopBackgroundController::InstallDesktopControllerForAllWindows()
#32 0x7fe3117bb3cc
ash::DesktopBackgroundController::SetDesktopBackgroundImageMode()
#33 0x7fe3117bb096 ash::DesktopBackgroundController::SetWallpaperImage()
#34 0x7fe322f9a6b9 chromeos::WallpaperManager::DoSetDefaultWallpaper()
#35 0x7fe322f99b73
chromeos::WallpaperManager::PendingWallpaper::ProcessRequest()
#36 0x7fe322fbac72 base::internal::RunnableAdapter<>::Run()
#37 0x7fe322fbabe9 base::internal::InvokeHelper<>::MakeItSo()
#38 0x7fe322fbaba5 base::internal::Invoker<>::Run()
#39 0x7fe3122cc9ce base::Callback<>::Run()
#40 0x7fe312488f86 base::Timer::RunScheduledTask()
#41 0x7fe3124890bc base::BaseTimerTaskInternal::Run()
#42 0x7fe312489382 base::internal::RunnableAdapter<>::Run()
#43 0x7fe3124892ec base::internal::InvokeHelper<>::MakeItSo()
#44 0x7fe312489295 base::internal::Invoker<>::Run()
#45 0x7fe3122cc9ce base::Callback<>::Run()
#46 0x7fe3122eb9b3 base::debug::TaskAnnotator::RunTask()
#47 0x7fe3123a1c57 base::MessageLoop::RunTask()
#48 0x7fe3123a1d9b base::MessageLoop::DeferOrRunPendingTask()
#49 0x7fe3123a228d base::MessageLoop::DoDelayedWork()
#50 0x7fe3122a1f25 base::MessagePumpGlib::Run()
#51 0x7fe3123a17f0 base::MessageLoop::RunHandler()
#52 0x7fe3124093b2 base::RunLoop::Run()
#53 0x7fe324217e4d ChromeBrowserMainParts::MainMessageLoopRun()
#54 0x7fe31ad2a8bf content::BrowserMainLoop::RunMainMessageLoopParts()
#55 0x7fe31ad344c7 content::BrowserMainRunnerImpl::Run()
#56 0x7fe31ad251b1 content::BrowserMain()
#57 0x7fe31abb997f content::RunNamedProcessTypeMain()
#58 0x7fe31abbcce8 content::ContentMainRunnerImpl::Run()
#59 0x7fe31abb8ee5 content::ContentMain()
#60 0x7fe3206d3505 ChromeMain
#61 0x7fe3206d34b2 main

Cannot upload crash dump: cannot exec /sbin/crash_reporter

Crash_reporter failed to process crash report
.

[commit-bot: I haz the power, 2014-09-10 03:21:44]

Patchset 5 (id:??) landed as
https://crrev.com/43ae1fb32df63bfd42c13914feb6a71f111fccb3
Cr-Commit-Position: refs/heads/master@{#293018}