|
|
Created:
7 years, 1 month ago by michaelpg Modified:
7 years ago CC:
chromium-reviews, benjhayden+dwatch_chromium.org, asanka, dcheng, tfarina, xiyuan Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionFix dragging supported files to tab strip.
Use the inferred MIME type of download items, rather than the server value,
when deciding if the tab strip can allow a drop. Get the MIME type
asynchronously so we don't crash (IO must be allowed in the call to
GetMimeTypeFromFile).
Also check the profile's plugins when determining whether a MIME type is
supported.
Regardless of whether a file is supported, the dragging operation should
still be allowed so that spring-loaded tabs work with file drags.
BUG=274288
R=asanka@chromium.org, sky@chromium.org
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=238253
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=238531
Patch Set 1 #
Total comments: 12
Patch Set 2 : Moved logic to BrowserTabStripController #
Total comments: 6
Patch Set 3 : #
Total comments: 14
Patch Set 4 : Fixes #
Total comments: 7
Patch Set 5 : #
Total comments: 3
Patch Set 6 : Add missing header #Messages
Total messages: 28 (0 generated)
Asanka - as we discussed, getting the MIME type asynchronously is tricky. This solution assumes a download item dragged into the tabstrip is supported until the tabstrip determines otherwise; it kicks off a task to determine the MIME type as soon as the drag enters the tabstrip. So it's possible to drag and drop an unsupported file (which will then get re-downloaded, meaning as far as I can tell it's copied locally) if you drag and drop it extremely quickly.
https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... File chrome/browser/ui/views/tabs/tab_strip.cc (right): https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... chrome/browser/ui/views/tabs/tab_strip.cc:1473: if (!drag_file_supported_ || !event.data().GetURLAndTitle(&url, &title) || We shouldn't need to check url/title twice. https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... File chrome/browser/ui/views/tabs/tab_strip.h (right): https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... chrome/browser/ui/views/tabs/tab_strip.h:585: // The URL from the latest DropTargetEvent. All of this state, including the weak factory, should move to DropInfo. You may need to change the life of DropInfo, but that's ok. https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... File chrome/browser/ui/views/tabs/tab_strip_controller.h (right): https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... chrome/browser/ui/views/tabs/tab_strip_controller.h:113: virtual Profile* GetProfile() = 0; I think you can get the profile from the browsercontext for a WebContents.
https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... File chrome/browser/ui/views/tabs/tab_strip.cc (right): https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... chrome/browser/ui/views/tabs/tab_strip.cc:1427: if (event.data().GetURLAndTitle(&url, &title) && url.is_valid()) { The logic below here only applies to file:// URLs. For other schemes, we'll have to assume that the URL is supported since the MIME type won't be known until the request has been issued.
https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... File chrome/browser/ui/views/tabs/tab_strip.cc (right): https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... chrome/browser/ui/views/tabs/tab_strip.cc:1427: if (event.data().GetURLAndTitle(&url, &title) && url.is_valid()) { On 2013/11/15 19:15:22, asanka wrote: > The logic below here only applies to file:// URLs. For other schemes, we'll have > to assume that the URL is supported since the MIME type won't be known until the > request has been issued. None-the-less there is no point doing the same test as is done @ 1473.
https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... File chrome/browser/ui/views/tabs/tab_strip.cc (right): https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... chrome/browser/ui/views/tabs/tab_strip.cc:1427: if (event.data().GetURLAndTitle(&url, &title) && url.is_valid()) { On 2013/11/15 20:46:37, sky wrote: > On 2013/11/15 19:15:22, asanka wrote: > > The logic below here only applies to file:// URLs. For other schemes, we'll > have > > to assume that the URL is supported since the MIME type won't be known until > the > > request has been issued. > > None-the-less there is no point doing the same test as is done @ 1473. Yup.
https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... File chrome/browser/ui/views/tabs/tab_strip.cc (right): https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... chrome/browser/ui/views/tabs/tab_strip.cc:1427: if (event.data().GetURLAndTitle(&url, &title) && url.is_valid()) { On 2013/11/15 20:46:37, sky wrote: > On 2013/11/15 19:15:22, asanka wrote: > > The logic below here only applies to file:// URLs. For other schemes, we'll > have > > to assume that the URL is supported since the MIME type won't be known until > the > > request has been issued. > > None-the-less there is no point doing the same test as is done @ 1473. The check here esnures we don't request unnecessary IO. asanka@ is right that I should add the condition url.SchemaIsFile() to this if statement. drop_data_supported_ defaults to true, so for non-file-schema URLs, the check at 1473 will still be necessary.... unless I set drag_file_supported to false in an else statement here, regardless of the schema. Which makes sense now that I think about it. https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... File chrome/browser/ui/views/tabs/tab_strip_controller.h (right): https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... chrome/browser/ui/views/tabs/tab_strip_controller.h:113: virtual Profile* GetProfile() = 0; On 2013/11/15 00:48:54, sky wrote: > I think you can get the profile from the browsercontext for a WebContents. Could you please clarify this for me? The TabStrip takes a TabStripController which is just an abstract class -- where can I get the WebContents from?
https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... File chrome/browser/ui/views/tabs/tab_strip_controller.h (right): https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... chrome/browser/ui/views/tabs/tab_strip_controller.h:113: virtual Profile* GetProfile() = 0; On 2013/11/15 21:08:01, Michael Giuffrida wrote: > On 2013/11/15 00:48:54, sky wrote: > > I think you can get the profile from the browsercontext for a WebContents. > > Could you please clarify this for me? The TabStrip takes a TabStripController > which is just an abstract class -- where can I get the WebContents from? You are right. In which case I would prefer to keep this class clean of the Profile and move as much of this logic into BrowserTabStripController. It can take the request and asynchronously call back to the TabStrip with the results.
https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... File chrome/browser/ui/views/tabs/tab_strip.cc (right): https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... chrome/browser/ui/views/tabs/tab_strip.cc:1473: if (!drag_file_supported_ || !event.data().GetURLAndTitle(&url, &title) || On 2013/11/15 00:48:54, sky wrote: > We shouldn't need to check url/title twice. Done, as per above. https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... File chrome/browser/ui/views/tabs/tab_strip.h (right): https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... chrome/browser/ui/views/tabs/tab_strip.h:585: // The URL from the latest DropTargetEvent. On 2013/11/15 00:48:54, sky wrote: > All of this state, including the weak factory, should move to DropInfo. You may > need to change the life of DropInfo, but that's ok. I've moved the URL and supported boolean into DropInfo. I moved the WeakFactoryPtr to browser_tab_strip_controller, since that's where the MIME type will call back so we can check for profile plugins, if that makes sense. https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... File chrome/browser/ui/views/tabs/tab_strip_controller.h (right): https://codereview.chromium.org/68133020/diff/1/chrome/browser/ui/views/tabs/... chrome/browser/ui/views/tabs/tab_strip_controller.h:113: virtual Profile* GetProfile() = 0; On 2013/11/15 22:07:18, sky wrote: > On 2013/11/15 21:08:01, Michael Giuffrida wrote: > > On 2013/11/15 00:48:54, sky wrote: > > > I think you can get the profile from the browsercontext for a WebContents. > > > > Could you please clarify this for me? The TabStrip takes a TabStripController > > which is just an abstract class -- where can I get the WebContents from? > > You are right. In which case I would prefer to keep this class clean of the > Profile and move as much of this logic into BrowserTabStripController. It can > take the request and asynchronously call back to the TabStrip with the results. Done.
Bump... if anybody's still around, could you take a look? Thanks.
/download/ lgtm https://codereview.chromium.org/68133020/diff/270001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc (right): https://codereview.chromium.org/68133020/diff/270001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc:89: // extension. Must be called from an thread that allows IO. Nit: s/from an/on a/ https://codereview.chromium.org/68133020/diff/270001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc:592: content::PluginService::GetInstance()->GetPluginInfo( Is it safe to make this call on the UI thread? https://codereview.chromium.org/68133020/diff/270001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/tab_strip_controller.h (right): https://codereview.chromium.org/68133020/diff/270001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip_controller.h:24: class Profile; Nit: unneeded?
addressed nits https://codereview.chromium.org/68133020/diff/270001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc (right): https://codereview.chromium.org/68133020/diff/270001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc:89: // extension. Must be called from an thread that allows IO. On 2013/11/25 22:35:05, asanka wrote: > Nit: s/from an/on a/ Done. https://codereview.chromium.org/68133020/diff/270001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc:592: content::PluginService::GetInstance()->GetPluginInfo( On 2013/11/25 22:35:05, asanka wrote: > Is it safe to make this call on the UI thread? Yes. PluginService::GetPluginInfoArray says "This can be called from any thread." The implementation calls PluginList::GetPluginInfoArray, which could load the plugin list (that's where the disk I/O would be), but is guaranteed not to given the arguments passed into it. https://codereview.chromium.org/68133020/diff/270001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/tab_strip_controller.h (right): https://codereview.chromium.org/68133020/diff/270001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip_controller.h:24: class Profile; On 2013/11/25 22:35:05, asanka wrote: > Nit: unneeded? right, thanks!
https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc (right): https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc:91: base::FilePath full_path; DCHECK not on UI thread. https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc:424: void BrowserTabStripController::CheckFileSupported(GURL url) { const GURL& https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/tab_strip.cc (right): https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip.cc:994: if (drop_info_->url == url) How do you know drop_info_ is non-null? https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip.cc:995: drop_info_->file_supported = supported; If supported is false should we destroy drop_info_? https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip.cc:1420: drop_info_->file_supported = false; Can't we set drop_info_ to NULL here? https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip.cc:2460: file_supported(true) { Is initializing to true really the right thing? What if the drag completes before we figure out the type? https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/tab_strip_controller.h (right): https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip_controller.h:117: // invoke TabStrip::FileSupported to report the result. Document that FileSupported() is invoked asynchronously.
Made some fixes, notably to un-break text dragging. https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc (right): https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc:91: base::FilePath full_path; On 2013/11/26 00:12:29, sky wrote: > DCHECK not on UI thread. Done. https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc:424: void BrowserTabStripController::CheckFileSupported(GURL url) { On 2013/11/26 00:12:29, sky wrote: > const GURL& Done. https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/tab_strip.cc (right): https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip.cc:994: if (drop_info_->url == url) On 2013/11/26 00:12:29, sky wrote: > How do you know drop_info_ is non-null? It definitely could be null, thanks. https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip.cc:995: drop_info_->file_supported = supported; On 2013/11/26 00:12:29, sky wrote: > If supported is false should we destroy drop_info_? No, it would just get re-created in SetDropIndex. We also want to persist drop_info_ because we want to support spring-loaded tabs, i.e., you can hold an unsupported file over a tab like imgur.com to switch to that tab and then drop it onto that page. https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip.cc:1420: drop_info_->file_supported = false; On 2013/11/26 00:12:29, sky wrote: > Can't we set drop_info_ to NULL here? No, we need drop_info in case text is being dragged (unless of course the text being dragged starts with "file:"). Also because we could be dragging text, we shouldn't even set file_supported = false here. https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip.cc:2460: file_supported(true) { On 2013/11/26 00:12:29, sky wrote: > Is initializing to true really the right thing? What if the drag completes > before we figure out the type? I would rather we try to open it even if it means creating a copy of the file to "redownload" instead of doing nothing or waiting on the IO which in this hypothetical would be slower than the user. https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/tab_strip_controller.h (right): https://codereview.chromium.org/68133020/diff/340001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip_controller.h:117: // invoke TabStrip::FileSupported to report the result. On 2013/11/26 00:12:29, sky wrote: > Document that FileSupported() is invoked asynchronously. Done. https://codereview.chromium.org/68133020/diff/360001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/tab_strip.cc (right): https://codereview.chromium.org/68133020/diff/360001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip.cc:1453: !event.data().GetURLAndTitle(&url, &title) || !url.is_valid()) Apparently we *do* need this check, because the BrowserRootView will do some fancy autocompletion if we're dragging text and it only does that on the drop.
Drive by https://codereview.chromium.org/68133020/diff/360001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc (right): https://codereview.chromium.org/68133020/diff/360001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc:582: GURL url, Pass by reference.
https://codereview.chromium.org/68133020/diff/360001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/tab_strip.cc (right): https://codereview.chromium.org/68133020/diff/360001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip.cc:1453: !event.data().GetURLAndTitle(&url, &title) || !url.is_valid()) On 2013/11/26 04:50:13, Michael Giuffrida wrote: > Apparently we *do* need this check, because the BrowserRootView will do some > fancy autocompletion if we're dragging text and it only does that on the drop. Can't we just check if drop_info_->url is not empty? https://codereview.chromium.org/68133020/diff/360001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/tab_strip_controller.h (right): https://codereview.chromium.org/68133020/diff/360001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip_controller.h:116: // Called asynchronously to determine if the file type of the URL is This isn't called asynchronously. The result is asynchronous, but not the call.
If there are still questions about the drop info, feel free to stop by or IM me. https://codereview.chromium.org/68133020/diff/360001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc (right): https://codereview.chromium.org/68133020/diff/360001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc:582: GURL url, On 2013/11/28 09:07:39, dcheng wrote: > Pass by reference. Done. https://codereview.chromium.org/68133020/diff/360001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/tab_strip.cc (right): https://codereview.chromium.org/68133020/diff/360001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip.cc:1453: !event.data().GetURLAndTitle(&url, &title) || !url.is_valid()) On 2013/12/02 16:22:50, sky wrote: > On 2013/11/26 04:50:13, Michael Giuffrida wrote: > > Apparently we *do* need this check, because the BrowserRootView will do some > > fancy autocompletion if we're dragging text and it only does that on the drop. > > Can't we just check if drop_info_->url is not empty? The URL could be non-empty but unsupported. For instance, if you're dragging a downloaded .exe, we discover that it's unsupported so we don't allow the drop here. We need to keep the URL around, so we can still hover over a tab to bring it to the foreground and drop in a form field or something. In that case we'll need that URL and don't care that it's unsupported. https://codereview.chromium.org/68133020/diff/360001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/tab_strip_controller.h (right): https://codereview.chromium.org/68133020/diff/360001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/tab_strip_controller.h:116: // Called asynchronously to determine if the file type of the URL is On 2013/12/02 16:22:50, sky wrote: > This isn't called asynchronously. The result is asynchronous, but not the call. Removed asynchronous claim from comment.
LGTM
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/michaelpg@chromium.org/68133020/400001
Message was sent while issue was closed.
Change committed as 238253
Message was sent while issue was closed.
This failed to compile in the tree and was reverted, but works locally and in the trybots. If I try to compile this file locally using the command line at http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%2... (the key flag seems to be -DNDEBUG) then I can reproduce the compile error: browser_tab_strip_controller.cc: In member function 'virtual void BrowserTabStripController::CheckFileSupported(const GURL&)': browser_tab_strip_controller.cc:432:22: error: no matching function for call to 'PostTaskAndReplyWithResult(base::SequencedWorkerPool*, tracked_objects::Location, base::Callback<std::basic_string<char>()>, base::Callback<void(const std::basic_string<char>&)>)' browser_tab_strip_controller.cc:432:22: note: candidate is: ../../base/task_runner_util.h:55:6: note: template<class TaskReturnType, class ReplyArgType> bool base::PostTaskAndReplyWithResult(base::TaskRunner*, const tracked_objects::Location&, const base::Callback<TaskReturnType()>&, const base::Callback<void(ReplyArgType)>&) I have to explicitly cast from SequencedWorkerPool* to TaskRunner* to compile without errors. But what I'm doing is replicated in committed code: https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/... Does anything jump out at anyone? https://codereview.chromium.org/68133020/diff/400001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc (right): https://codereview.chromium.org/68133020/diff/400001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc:427: content::BrowserThread::GetBlockingPool(), This is causing compile errors in the tree.
Message was sent while issue was closed.
https://codereview.chromium.org/68133020/diff/400001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc (right): https://codereview.chromium.org/68133020/diff/400001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc:427: content::BrowserThread::GetBlockingPool(), On 2013/12/03 01:58:36, Michael Giuffrida wrote: > This is causing compile errors in the tree. Wild guess: you need to include the header for SequencedWorkerPool so that the compiler knows it can be implicitly cast to a TaskRunner.
Re-trying commit. https://codereview.chromium.org/68133020/diff/400001/chrome/browser/ui/views/... File chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc (right): https://codereview.chromium.org/68133020/diff/400001/chrome/browser/ui/views/... chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc:427: content::BrowserThread::GetBlockingPool(), On 2013/12/03 02:15:20, dcheng wrote: > On 2013/12/03 01:58:36, Michael Giuffrida wrote: > > This is causing compile errors in the tree. > > Wild guess: you need to include the header for SequencedWorkerPool so that the > compiler knows it can be implicitly cast to a TaskRunner. Doh, that's exactly it. I wonder why it didn't complain locally.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/michaelpg@chromium.org/68133020/420001
Retried try job too often on mac_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/michaelpg@chromium.org/68133020/420001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/michaelpg@chromium.org/68133020/420001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/michaelpg@chromium.org/68133020/420001
Message was sent while issue was closed.
Change committed as 238531 |