Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
9360005:
Handle termination/crashes of an extension hosted in the ExtensionDialog. (Closed)
|
Created:
8 years, 10 months ago by rkc Modified:
8 years, 10 months ago CC:
chromium-reviews, mihaip+watch_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionHandle termination/crashes of an extension hosted in the ExtensionDialog.
This is part of the work needed for the screensaver that we're planning on implementing for retail mode. If our hosted screensaver crashes, we need to be able to restart it, for which we're adding functionality to Extension dialog. Since the only current implementation of the ExtensionDialog is the Select File dialog on ChromeOS, I've modified the code to handle the case when the select file dialog crashes (which basically closes the extension window and reloads the extension for future use).
As part of the change, I've implemented the ability to reload component extensions, which was broken till now. On trying to reload a component extension on crash would throw up a meaningless error message; now the extension would be reloaded.
BUG=chromium-os:23365
TEST=Tested with crashing the Select File dialog several times to make sure it got reloaded every time.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=121561
Patch Set 1 #Patch Set 2 : . #
Total comments: 8
Patch Set 3 : Review changes. #
Total comments: 9
Patch Set 4 : . #
Total comments: 7
Patch Set 5 : nits + todo #
Total comments: 8
Patch Set 6 : . #
Messages
Total messages: 20 (0 generated)
Updated reviewers. Aaron, if you feel there is someone else on your team that should review the component extension reload parts instead, please do add them to the reviewer's list, thanks!
http://codereview.chromium.org/9360005/diff/1001/chrome/browser/extensions/co... File chrome/browser/extensions/component_loader.cc (right): http://codereview.chromium.org/9360005/diff/1001/chrome/browser/extensions/co... chrome/browser/extensions/component_loader.cc:129: for (RegisteredComponentExtensions::iterator it = indentation http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/exte... File chrome/browser/ui/views/extensions/extension_dialog_observer.h (right): http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/exte... chrome/browser/ui/views/extensions/extension_dialog_observer.h:21: virtual void ExtensionDialogTerminated(ExtensionDialog* popup) = 0; Maybe add a comment indicating when this function will be called? http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/sele... File chrome/browser/ui/views/select_file_dialog_extension.cc (right): http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/sele... chrome/browser/ui/views/select_file_dialog_extension.cc:136: // The extension would have been unloaded because of the termination, I'm not particularly knowledgeable about our extension mechanisms, but could we just leave the extension unloaded, then reload it the next time the user tries to open the file manager or a file picker? It feels odd to me to do this in a posted task.
On 2012/02/08 05:32:31, James Cook (Chromium) wrote: > http://codereview.chromium.org/9360005/diff/1001/chrome/browser/extensions/co... > File chrome/browser/extensions/component_loader.cc (right): > > http://codereview.chromium.org/9360005/diff/1001/chrome/browser/extensions/co... > chrome/browser/extensions/component_loader.cc:129: for > (RegisteredComponentExtensions::iterator it = > indentation > > http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/exte... > File chrome/browser/ui/views/extensions/extension_dialog_observer.h (right): > > http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/exte... > chrome/browser/ui/views/extensions/extension_dialog_observer.h:21: virtual void > ExtensionDialogTerminated(ExtensionDialog* popup) = 0; > Maybe add a comment indicating when this function will be called? > > http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/sele... > File chrome/browser/ui/views/select_file_dialog_extension.cc (right): > > http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/sele... > chrome/browser/ui/views/select_file_dialog_extension.cc:136: // The extension > would have been unloaded because of the termination, > I'm not particularly knowledgeable about our extension mechanisms, but could we > just leave the extension unloaded, then reload it the next time the user tries > to open the file manager or a file picker? I really like that idea, too. I don't think that we should have the concept of terminated extensions. We should just have loaded/unloaded. Whether a process is running for a loaded extension is orthagonal. If a process crashes, then the next time somebody runs a URL that corresponds to the extension, a process will get created naturally. In the special case of background pages, we can put up UI telling the user that the extension went down, but for tabs that is not necessary because we already have sadtab. I'll look at what this would take tonight.
Addressed the current review comments; I'll wait for Aaron's investigation before proceeding. http://codereview.chromium.org/9360005/diff/1001/chrome/browser/extensions/co... File chrome/browser/extensions/component_loader.cc (right): http://codereview.chromium.org/9360005/diff/1001/chrome/browser/extensions/co... chrome/browser/extensions/component_loader.cc:129: for (RegisteredComponentExtensions::iterator it = On 2012/02/08 05:32:31, James Cook (Chromium) wrote: > indentation Done. http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/exte... File chrome/browser/ui/views/extensions/extension_dialog_observer.h (right): http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/exte... chrome/browser/ui/views/extensions/extension_dialog_observer.h:21: virtual void ExtensionDialogTerminated(ExtensionDialog* popup) = 0; On 2012/02/08 05:32:31, James Cook (Chromium) wrote: > Maybe add a comment indicating when this function will be called? Done. http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/sele... File chrome/browser/ui/views/select_file_dialog_extension.cc (right): http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/sele... chrome/browser/ui/views/select_file_dialog_extension.cc:136: // The extension would have been unloaded because of the termination, At this time there is no method of knowing what 'state' an extension is in other than if it's enabled. Adding functionality to check if an extension is in a terminated state or not is a possibility but I'll defer a decision on this till Aaron is done investigating the possibility of not needing to reload crashed extensions at all. On 2012/02/08 05:32:31, James Cook (Chromium) wrote: > I'm not particularly knowledgeable about our extension mechanisms, but could we > just leave the extension unloaded, then reload it the next time the user tries > to open the file manager or a file picker? > > It feels odd to me to do this in a posted task.
Yeah, I think my idea about simplifying crashed extension handling is going to work: http://codereview.chromium.org/9370013/ I don't know if you'll even need this change anymore. - a On Wed, Feb 8, 2012 at 4:24 PM, <rkc@chromium.org> wrote: > Addressed the current review comments; I'll wait for Aaron's investigation > before proceeding. > > > > > http://codereview.chromium.org/9360005/diff/1001/chrome/browser/extensions/co... > File chrome/browser/extensions/component_loader.cc (right): > > http://codereview.chromium.org/9360005/diff/1001/chrome/browser/extensions/co... > chrome/browser/extensions/component_loader.cc:129: for > (RegisteredComponentExtensions::iterator it = > On 2012/02/08 05:32:31, James Cook (Chromium) wrote: >> >> indentation > > > Done. > > > http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/exte... > File chrome/browser/ui/views/extensions/extension_dialog_observer.h > (right): > > http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/exte... > chrome/browser/ui/views/extensions/extension_dialog_observer.h:21: > virtual void ExtensionDialogTerminated(ExtensionDialog* popup) = 0; > On 2012/02/08 05:32:31, James Cook (Chromium) wrote: >> >> Maybe add a comment indicating when this function will be called? > > > Done. > > > http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/sele... > File chrome/browser/ui/views/select_file_dialog_extension.cc (right): > > http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/sele... > chrome/browser/ui/views/select_file_dialog_extension.cc:136: // The > extension would have been unloaded because of the termination, > At this time there is no method of knowing what 'state' an extension is > in other than if it's enabled. > > Adding functionality to check if an extension is in a terminated state > or not is a possibility but I'll defer a decision on this till Aaron is > done investigating the possibility of not needing to reload crashed > extensions at all. > > > > On 2012/02/08 05:32:31, James Cook (Chromium) wrote: >> >> I'm not particularly knowledgeable about our extension mechanisms, but > > could we >> >> just leave the extension unloaded, then reload it the next time the > > user tries >> >> to open the file manager or a file picker? > > >> It feels odd to me to do this in a posted task. > > > http://codereview.chromium.org/9360005/
+yoz I found some issues with getting rid of terminated extensions. Yoyo, what do you think of this change?
http://codereview.chromium.org/9360005/diff/7001/chrome/browser/ui/views/sele... File chrome/browser/ui/views/select_file_dialog_extension.cc (right): http://codereview.chromium.org/9360005/diff/7001/chrome/browser/ui/views/sele... chrome/browser/ui/views/select_file_dialog_extension.cc:132: void SelectFileDialogExtension::ExtensionDialogTerminated( Why do we need this bit? From looking at the current code, it looks like the extension will get reloaded (assuming ExtensionService actually knows how to reload component extensions, which it does with your other changes) the first time anyone tries to navigate a view to a corresponding chrome-extension:// URL.
http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/sele... File chrome/browser/ui/views/select_file_dialog_extension.cc (right): http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/sele... chrome/browser/ui/views/select_file_dialog_extension.cc:136: // The extension would have been unloaded because of the termination, Likewise, I'm not sure why this posted task is necessary. On 2012/02/09 00:24:22, Rahul Chaturvedi wrote: > At this time there is no method of knowing what 'state' an extension is in other > than if it's enabled. > > Adding functionality to check if an extension is in a terminated state or not is > a possibility but I'll defer a decision on this till Aaron is done investigating > the possibility of not needing to reload crashed extensions at all. GetTerminatedExtension is supposed to do this (I'm not certain it works for component extensions). > > On 2012/02/08 05:32:31, James Cook (Chromium) wrote: > > I'm not particularly knowledgeable about our extension mechanisms, but could > we > > just leave the extension unloaded, then reload it the next time the user tries > > to open the file manager or a file picker? > > > > It feels odd to me to do this in a posted task. > http://codereview.chromium.org/9360005/diff/7001/chrome/browser/extensions/ex... File chrome/browser/extensions/extension_service.cc (right): http://codereview.chromium.org/9360005/diff/7001/chrome/browser/extensions/ex... chrome/browser/extensions/extension_service.cc:694: true, I don't think we should we be including disabled extensions here. They should be passed to EnableExtension instead. Anyhow, if this is only used to determine if the crashed extension is a component extension, you shouldn't need to change this; just see my comment below. http://codereview.chromium.org/9360005/diff/7001/chrome/browser/extensions/ex... chrome/browser/extensions/extension_service.cc:703: if (IsExtensionEnabled(extension_id)) { Likewise, this is probably incorrect -- we only want currently loaded extensions to go here. It should be a no-op for an extension with a crashed process. http://codereview.chromium.org/9360005/diff/7001/chrome/browser/extensions/ex... chrome/browser/extensions/extension_service.cc:730: if (current_extension->location() == Extension::COMPONENT) { You could use ComponentLoader::Exists(extension_id)
http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/sele... File chrome/browser/ui/views/select_file_dialog_extension.cc (right): http://codereview.chromium.org/9360005/diff/1001/chrome/browser/ui/views/sele... chrome/browser/ui/views/select_file_dialog_extension.cc:136: // The extension would have been unloaded because of the termination, On 2012/02/09 22:18:03, Yoyo Zhou wrote: > Likewise, I'm not sure why this posted task is necessary. > > On 2012/02/09 00:24:22, Rahul Chaturvedi wrote: > > At this time there is no method of knowing what 'state' an extension is in > other > > than if it's enabled. > > > > Adding functionality to check if an extension is in a terminated state or not > is > > a possibility but I'll defer a decision on this till Aaron is done > investigating > > the possibility of not needing to reload crashed extensions at all. > > GetTerminatedExtension is supposed to do this (I'm not certain it works for > component extensions). > > > > > On 2012/02/08 05:32:31, James Cook (Chromium) wrote: > > > I'm not particularly knowledgeable about our extension mechanisms, but could > > we > > > just leave the extension unloaded, then reload it the next time the user > tries > > > to open the file manager or a file picker? > > > > > > It feels odd to me to do this in a posted task. > > So we need to at least close the dialog window, which otherwise remains open with no content in it :) I tested this with removing the explicit reload code (the posted task) and the extension doesn't reload. With this code removed, the crashed extension notification does come up, clicking the "reload" option on which does reload the extension and allows the select file dialog to come up again, but not otherwise. http://codereview.chromium.org/9360005/diff/7001/chrome/browser/extensions/ex... File chrome/browser/extensions/extension_service.cc (right): http://codereview.chromium.org/9360005/diff/7001/chrome/browser/extensions/ex... chrome/browser/extensions/extension_service.cc:694: true, On 2012/02/09 22:18:03, Yoyo Zhou wrote: > I don't think we should we be including disabled extensions here. They should be > passed to EnableExtension instead. > > Anyhow, if this is only used to determine if the crashed extension is a > component extension, you shouldn't need to change this; just see my comment > below. Done. http://codereview.chromium.org/9360005/diff/7001/chrome/browser/extensions/ex... chrome/browser/extensions/extension_service.cc:703: if (IsExtensionEnabled(extension_id)) { On 2012/02/09 22:18:03, Yoyo Zhou wrote: > Likewise, this is probably incorrect -- we only want currently loaded extensions > to go here. It should be a no-op for an extension with a crashed process. Done. http://codereview.chromium.org/9360005/diff/7001/chrome/browser/extensions/ex... chrome/browser/extensions/extension_service.cc:730: if (current_extension->location() == Extension::COMPONENT) { On 2012/02/09 22:18:03, Yoyo Zhou wrote: > You could use ComponentLoader::Exists(extension_id) Done. http://codereview.chromium.org/9360005/diff/7001/chrome/browser/ui/views/sele... File chrome/browser/ui/views/select_file_dialog_extension.cc (right): http://codereview.chromium.org/9360005/diff/7001/chrome/browser/ui/views/sele... chrome/browser/ui/views/select_file_dialog_extension.cc:132: void SelectFileDialogExtension::ExtensionDialogTerminated( On 2012/02/09 20:23:23, Aaron Boodman wrote: > Why do we need this bit? From looking at the current code, it looks like the > extension will get reloaded (assuming ExtensionService actually knows how to > reload component extensions, which it does with your other changes) the first > time anyone tries to navigate a view to a corresponding chrome-extension:// URL. So we need to at least close the dialog window, which otherwise remains open with no content in it :) I tested this with removing the explicit reload code (the posted task) and the extension doesn't reload. With this code removed, the crashed extension notification does come up, clicking the "reload" option on which does reload the extension and allows the select file dialog to come up again, but not otherwise.
http://codereview.chromium.org/9360005/diff/7001/chrome/browser/ui/views/sele... File chrome/browser/ui/views/select_file_dialog_extension.cc (right): http://codereview.chromium.org/9360005/diff/7001/chrome/browser/ui/views/sele... chrome/browser/ui/views/select_file_dialog_extension.cc:132: void SelectFileDialogExtension::ExtensionDialogTerminated( On 2012/02/10 00:35:42, Rahul Chaturvedi wrote: > On 2012/02/09 20:23:23, Aaron Boodman wrote: > > Why do we need this bit? From looking at the current code, it looks like the > > extension will get reloaded (assuming ExtensionService actually knows how to > > reload component extensions, which it does with your other changes) the first > > time anyone tries to navigate a view to a corresponding chrome-extension:// > URL. > > So we need to at least close the dialog window, which otherwise remains open > with no content in it :) > > I tested this with removing the explicit reload code (the posted task) and the > extension doesn't reload. With this code removed, the crashed extension > notification does come up, clicking the "reload" option on which does reload the > extension and allows the select file dialog to come up again, but not otherwise. Does it work without using PostTask? This seems somewhat suspect. If we need to wait for the extension to get Unloaded, it seems like we should wait for that event (EXTENSION_UNLOADED) instead of EXTENSION_TERMINATED. http://codereview.chromium.org/9360005/diff/10002/chrome/browser/extensions/e... File chrome/browser/extensions/extension_service.cc (right): http://codereview.chromium.org/9360005/diff/10002/chrome/browser/extensions/e... chrome/browser/extensions/extension_service.cc:15: #include "base/json/json_value_serializer.h" This looks unused.
lgtm http://codereview.chromium.org/9360005/diff/10002/chrome/browser/extensions/e... File chrome/browser/extensions/extension_service.cc (right): http://codereview.chromium.org/9360005/diff/10002/chrome/browser/extensions/e... chrome/browser/extensions/extension_service.cc:15: #include "base/json/json_value_serializer.h" On 2012/02/10 00:45:28, Yoyo Zhou wrote: > This looks unused. don't forget to remove this
LGTM assuming style nits below fixed. http://codereview.chromium.org/9360005/diff/10002/chrome/browser/extensions/c... File chrome/browser/extensions/component_loader.cc (right): http://codereview.chromium.org/9360005/diff/10002/chrome/browser/extensions/c... chrome/browser/extensions/component_loader.cc:129: for (RegisteredComponentExtensions::iterator it = 2 space indentation http://codereview.chromium.org/9360005/diff/10002/chrome/browser/ui/views/sel... File chrome/browser/ui/views/select_file_dialog_extension.cc (right): http://codereview.chromium.org/9360005/diff/10002/chrome/browser/ui/views/sel... chrome/browser/ui/views/select_file_dialog_extension.cc:98: owner_browser_(0), Prefer NULL here since owner_browser_ is a pointer type. owner_window_ could be NULL as well.
http://codereview.chromium.org/9360005/diff/10002/chrome/browser/extensions/c... File chrome/browser/extensions/component_loader.cc (right): http://codereview.chromium.org/9360005/diff/10002/chrome/browser/extensions/c... chrome/browser/extensions/component_loader.cc:129: for (RegisteredComponentExtensions::iterator it = On 2012/02/10 20:43:41, James Cook (Chromium) wrote: > 2 space indentation Done. http://codereview.chromium.org/9360005/diff/10002/chrome/browser/extensions/e... File chrome/browser/extensions/extension_service.cc (right): http://codereview.chromium.org/9360005/diff/10002/chrome/browser/extensions/e... chrome/browser/extensions/extension_service.cc:15: #include "base/json/json_value_serializer.h" On 2012/02/10 20:35:16, Aaron Boodman wrote: > On 2012/02/10 00:45:28, Yoyo Zhou wrote: > > This looks unused. > > don't forget to remove this Done. http://codereview.chromium.org/9360005/diff/10002/chrome/browser/ui/views/sel... File chrome/browser/ui/views/select_file_dialog_extension.cc (right): http://codereview.chromium.org/9360005/diff/10002/chrome/browser/ui/views/sel... chrome/browser/ui/views/select_file_dialog_extension.cc:98: owner_browser_(0), On 2012/02/10 20:43:41, James Cook (Chromium) wrote: > Prefer NULL here since owner_browser_ is a pointer type. owner_window_ could be > NULL as well. Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rkc@chromium.org/9360005/14002
Presubmit check for 9360005-14002 failed and returned exit status 1. Running presubmit commit checks ... ** Presubmit ERRORS ** Missing LGTM from an OWNER for: chrome/browser/ui/views/extensions/extension_dialog.cc,chrome/browser/ui/views/select_file_dialog_extension.h,chrome/browser/ui/views/extensions/extension_dialog_observer.h,chrome/browser/ui/views/select_file_dialog_extension.cc Presubmit checks took 2.4s to calculate. Was the presubmit check useful? Please send feedback & hate mail to maruel@chromium.org!
Added pkasting for owner review.
LGTM http://codereview.chromium.org/9360005/diff/14002/chrome/browser/ui/views/ext... File chrome/browser/ui/views/extensions/extension_dialog_observer.h (right): http://codereview.chromium.org/9360005/diff/14002/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/extension_dialog_observer.h:22: // terminated. If not intentional, this would be typically happen Nit: How about just: // terminated, e.g. if the extension crashes. Also it's weird that a method called "ExtensionDialogTerminated" is supposedly called when something the dialog hosts, rather than the dialog itself, is terminated. Maybe this should be renamed to something more accurate. http://codereview.chromium.org/9360005/diff/14002/chrome/browser/ui/views/sel... File chrome/browser/ui/views/select_file_dialog_extension.cc (right): http://codereview.chromium.org/9360005/diff/14002/chrome/browser/ui/views/sel... chrome/browser/ui/views/select_file_dialog_extension.cc:134: LOG(ERROR) << "Select File Dialog Extension crashed."; Don't LOG unless you really need it. Logs are generally a very poor way of capturing problem data because we don't get log data back from users. http://codereview.chromium.org/9360005/diff/14002/chrome/browser/ui/views/sel... chrome/browser/ui/views/select_file_dialog_extension.cc:149: MessageLoop::current()->PostTask( Nit: Wrapping is kinda crazy. Just do: MessageLoop::current()->PostTask(FROM_HERE, base::Bind(&ExtensionService::ReloadExtension, base::Unretained(owner_browser_->profile()->GetExtensionService()), extension_id)); http://codereview.chromium.org/9360005/diff/14002/chrome/browser/ui/views/sel... File chrome/browser/ui/views/select_file_dialog_extension.h (right): http://codereview.chromium.org/9360005/diff/14002/chrome/browser/ui/views/sel... chrome/browser/ui/views/select_file_dialog_extension.h:88: Browser* owner_browser_; Nit: Add comments about this. In particular explain why this is guaranteed not to be deleted while we're still alive.
Addressed pkasting@'s review comments. http://codereview.chromium.org/9360005/diff/14002/chrome/browser/ui/views/ext... File chrome/browser/ui/views/extensions/extension_dialog_observer.h (right): http://codereview.chromium.org/9360005/diff/14002/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/extension_dialog_observer.h:22: // terminated. If not intentional, this would be typically happen On 2012/02/10 21:09:27, Peter Kasting wrote: > Nit: How about just: > > // terminated, e.g. if the extension crashes. > > Also it's weird that a method called "ExtensionDialogTerminated" is supposedly > called when something the dialog hosts, rather than the dialog itself, is > terminated. Maybe this should be renamed to something more accurate. Done. http://codereview.chromium.org/9360005/diff/14002/chrome/browser/ui/views/sel... File chrome/browser/ui/views/select_file_dialog_extension.cc (right): http://codereview.chromium.org/9360005/diff/14002/chrome/browser/ui/views/sel... chrome/browser/ui/views/select_file_dialog_extension.cc:134: LOG(ERROR) << "Select File Dialog Extension crashed."; On 2012/02/10 21:09:27, Peter Kasting wrote: > Don't LOG unless you really need it. Logs are generally a very poor way of > capturing problem data because we don't get log data back from users. Done. http://codereview.chromium.org/9360005/diff/14002/chrome/browser/ui/views/sel... chrome/browser/ui/views/select_file_dialog_extension.cc:149: MessageLoop::current()->PostTask( On 2012/02/10 21:09:27, Peter Kasting wrote: > Nit: Wrapping is kinda crazy. Just do: > > MessageLoop::current()->PostTask(FROM_HERE, > base::Bind(&ExtensionService::ReloadExtension, > base::Unretained(owner_browser_->profile()->GetExtensionService()), > extension_id)); Done. http://codereview.chromium.org/9360005/diff/14002/chrome/browser/ui/views/sel... File chrome/browser/ui/views/select_file_dialog_extension.h (right): http://codereview.chromium.org/9360005/diff/14002/chrome/browser/ui/views/sel... chrome/browser/ui/views/select_file_dialog_extension.h:88: Browser* owner_browser_; On 2012/02/10 21:09:27, Peter Kasting wrote: > Nit: Add comments about this. In particular explain why this is guaranteed not > to be deleted while we're still alive. Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rkc@chromium.org/9360005/18002
Change committed as 121561 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
