Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
480243003:
Implement smoother autosizing of the extension options overlay (Closed)
DescriptionImplement smoother autosizing of the extension options overlay
- Use element.animate to resize the overlay for embedded extension
options. When an <extensionoptions> element is embedded in a WebUI
overlay, if its size changes the overlay will smoothly expand to
surround the element.
- Add an API to <extensionoptions> to defer autosizing until its
embedder is ready. This is used to 'pin' the element's size until
the overlay is done expanding.
- Move the <extensionoptions> off screen until the overlay is done
expanding.
- Prevent <extensionoptions> from shrinking after the initial sizing.
This makes the look of the embedded options page more consistent.
BUG=386842
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=291286
Committed: https://crrev.com/a9438c47225d119a953ebf195210e655cb88dbc5
Cr-Commit-Position: refs/heads/master@{#291583}
Patch Set 1 #
Total comments: 11
Patch Set 2 : Remove magic numbers #Patch Set 3 : Implement deferred autosizing #Patch Set 4 : Use overlay size instead of <extensionoptions> size for initial animation keyframe #Patch Set 5 : Fix syntax error #Patch Set 6 : Move <extensionoptions> off screen until overlay is ready #
Total comments: 19
Patch Set 7 : Address comments #
Total comments: 4
Patch Set 8 : Address comments and some overlay code cleanup #Patch Set 9 : Rebase #Patch Set 10 : Fix variable names in autosize test #Patch Set 11 : Fix merge conflict with CL 444813002 #Patch Set 12 : rebase #Messages
Total messages: 38 (0 generated)
PTAL
Neat :) but did you have any luck delaying showing the dialogue until the first resize event? https://codereview.chromium.org/480243003/diff/1/chrome/browser/resources/ext... File chrome/browser/resources/extensions/extension_options_overlay.js (right): https://codereview.chromium.org/480243003/diff/1/chrome/browser/resources/ext... chrome/browser/resources/extensions/extension_options_overlay.js:67: // maxheight is the overlay window's max height minus the header's height Nit: end comment in a period. If it fits on 80 characters (not sure whether it's 79 or 80 there). If not, I wouldn't add a period either. https://codereview.chromium.org/480243003/diff/1/chrome/browser/resources/ext... chrome/browser/resources/extensions/extension_options_overlay.js:69: parseInt($('extension-options-overlay').style.maxHeight) - 42; 42...? https://codereview.chromium.org/480243003/diff/1/chrome/browser/resources/ext... chrome/browser/resources/extensions/extension_options_overlay.js:70: extensionoptions.minwidth = 400; 400...? https://codereview.chromium.org/480243003/diff/1/chrome/browser/resources/ext... chrome/browser/resources/extensions/extension_options_overlay.js:73: var animationTime = 0.25 * Math.sqrt( Maybe you want to explain this formula :) https://codereview.chromium.org/480243003/diff/1/chrome/renderer/resources/ex... File chrome/renderer/resources/extensions/extension_options.js (right): https://codereview.chromium.org/480243003/diff/1/chrome/renderer/resources/ex... chrome/renderer/resources/extensions/extension_options.js:143: var animationTime = 0.25 * Math.sqrt( Why is there animation code inside the extension_options.js tag implementation?
https://codereview.chromium.org/480243003/diff/1/chrome/browser/resources/ext... File chrome/browser/resources/extensions/extension_options_overlay.js (right): https://codereview.chromium.org/480243003/diff/1/chrome/browser/resources/ext... chrome/browser/resources/extensions/extension_options_overlay.js:67: // maxheight is the overlay window's max height minus the header's height On 2014/08/19 17:02:48, kalman wrote: > Nit: end comment in a period. If it fits on 80 characters (not sure whether it's > 79 or 80 there). If not, I wouldn't add a period either. Done. https://codereview.chromium.org/480243003/diff/1/chrome/browser/resources/ext... chrome/browser/resources/extensions/extension_options_overlay.js:69: parseInt($('extension-options-overlay').style.maxHeight) - 42; On 2014/08/19 17:02:48, kalman wrote: > 42...? Fixed https://codereview.chromium.org/480243003/diff/1/chrome/browser/resources/ext... chrome/browser/resources/extensions/extension_options_overlay.js:70: extensionoptions.minwidth = 400; On 2014/08/19 17:02:48, kalman wrote: > 400...? Fixed https://codereview.chromium.org/480243003/diff/1/chrome/browser/resources/ext... chrome/browser/resources/extensions/extension_options_overlay.js:73: var animationTime = 0.25 * Math.sqrt( On 2014/08/19 17:02:48, kalman wrote: > Maybe you want to explain this formula :) Done. https://codereview.chromium.org/480243003/diff/1/chrome/renderer/resources/ex... File chrome/renderer/resources/extensions/extension_options.js (right): https://codereview.chromium.org/480243003/diff/1/chrome/renderer/resources/ex... chrome/renderer/resources/extensions/extension_options.js:143: var animationTime = 0.25 * Math.sqrt( On 2014/08/19 17:02:48, kalman wrote: > Why is there animation code inside the extension_options.js tag implementation? I need a way to delay <extensionoptions> from autosizing until the animation is done. I can calculate the animation delay here, or I could make it so that <extensionoptions> doesn't really 'auto' size, it would just fire the sizechanged event with its preferred size and have its container resize it at its leisure.
On 2014/08/19 17:02:48, kalman wrote: > Neat :) but did you have any luck delaying showing the dialogue until the first > resize event? > Still working on it, don't have anything functional yet.
https://codereview.chromium.org/480243003/diff/1/chrome/renderer/resources/ex... File chrome/renderer/resources/extensions/extension_options.js (right): https://codereview.chromium.org/480243003/diff/1/chrome/renderer/resources/ex... chrome/renderer/resources/extensions/extension_options.js:143: var animationTime = 0.25 * Math.sqrt( On 2014/08/19 18:20:13, ericzeng wrote: > On 2014/08/19 17:02:48, kalman wrote: > > Why is there animation code inside the extension_options.js tag > implementation? > > I need a way to delay <extensionoptions> from autosizing until the animation is > done. I can calculate the animation delay here, or I could make it so that > <extensionoptions> doesn't really 'auto' size, it would just fire the > sizechanged event with its preferred size and have its container resize it at > its leisure. Ah I see. It's still unfortunately specific to be doing this (and seems flaky). Could you provide an API for pinning resize or something? You pin, do the animation on the browse, then unpin?
On 2014/08/19 19:27:18, kalman wrote: > https://codereview.chromium.org/480243003/diff/1/chrome/renderer/resources/ex... > File chrome/renderer/resources/extensions/extension_options.js (right): > > https://codereview.chromium.org/480243003/diff/1/chrome/renderer/resources/ex... > chrome/renderer/resources/extensions/extension_options.js:143: var animationTime > = 0.25 * Math.sqrt( > On 2014/08/19 18:20:13, ericzeng wrote: > > On 2014/08/19 17:02:48, kalman wrote: > > > Why is there animation code inside the extension_options.js tag > > implementation? > > > > I need a way to delay <extensionoptions> from autosizing until the animation > is > > done. I can calculate the animation delay here, or I could make it so that > > <extensionoptions> doesn't really 'auto' size, it would just fire the > > sizechanged event with its preferred size and have its container resize it at > > its leisure. > > Ah I see. It's still unfortunately specific to be doing this (and seems flaky). > Could you provide an API for pinning resize or something? You pin, do the > animation on the browse, then unpin? I just implemented an API for deferring the autosize, although I'm not sure that this is even necessary. I originally did this because I didn't want the <extensionoptions> to expand over the borders of the overlay during the animation, but I think the CSS rule with the overflow fixes it.
I tried this out, and it's awesome. https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... File chrome/browser/resources/extensions/extension_options_overlay.css (right): https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.css:6: max-width: 100%; What do you need the max-width for? https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... File chrome/browser/resources/extensions/extension_options_overlay.js (right): https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:67: var headerStyle = You might want to add a meta-comment here, like: The extensionoptions content should be fitted to the title ... blah blah blah. https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:71: parseInt(headerStyle.paddingBottom); Can you get this from one of the other height properties, like offsetHeight or whatever (I can't remember the difference between them all). But adding together lineHeight+padding is a shame, and if you end up adding something like an imagine into the title I think it'd break. Oh also if the font is larger than the line height maybe? https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:80: extensionoptions.shouldDeferAutoSize(true); You mention in the comment that you don't actually need this; what happens if you leave it out? (It also seems like it'd be safer to do before you fiddle with the maxheight/minwidth, or am I missing the point?) https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:91: // constant for all sizes. Nit: could you say like "0.25 means the popup resizes at a rate of Xms per 100 pixels", or something meaningful. As an explanation, and so that people know what happens when you tweak it. https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:105: extensionoptions.resumeDeferredAutoSize(); Heh how did you figure out how to use animate... docs are very hard to find. The reason I ask: is is possible that onfinish won't be called? https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:112: $('extension-options-overlay-guest').style.left = '10000px'; This is to implement that off-screen resize? You should probably use a fixed position here, not absolute. And much as 10000px looks fine, I'd be more comfortable if you used the actual width (whatever the correct value is which keeps it out of the way with scrolling and zooming and whatnot). https://codereview.chromium.org/480243003/diff/100001/chrome/renderer/resourc... File chrome/renderer/resources/extensions/extension_options.js (right): https://codereview.chromium.org/480243003/diff/100001/chrome/renderer/resourc... chrome/renderer/resources/extensions/extension_options.js:271: ExtensionOptionsInternal.prototype.resumeDeferredAutoSize = function() { You could combine these into a method like setAutosize(true|false)? Which makes me think: is this the same thing as webview's autosize property? Just curious.
https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... File chrome/browser/resources/extensions/extension_options_overlay.js (right): https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:80: extensionoptions.shouldDeferAutoSize(true); On 2014/08/20 02:07:15, kalman wrote: > You mention in the comment that you don't actually need this; what happens if > you leave it out? > > (It also seems like it'd be safer to do before you fiddle with the > maxheight/minwidth, or am I missing the point?) Oh, I tried removing it and it turns out that it looks better if I leave it here. If I remove this feature, when you put the <extensionoptions> back into the frame, you momentarily see the guest view shifted down and to the right so that the top corner of the guest view positioned in the center of the screen, before it gets correctly repositioned. It doesn't really matter where this goes as long as it is before the element is attached. All this does is block the <extensionoptions> from resizing itself when sizechanged is fired. https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:105: extensionoptions.resumeDeferredAutoSize(); On 2014/08/20 02:07:15, kalman wrote: > Heh how did you figure out how to use animate... docs are very hard to find. > > The reason I ask: is is possible that onfinish won't be called? This was sufficient documentation for me to implement the feature: http://updates.html5rocks.com/2014/05/Web-Animations---element-animate-is-now... I don't know for sure if onfinish will be called, I just assumed that the docs are right. But really, can we prove anything true objectively? https://codereview.chromium.org/480243003/diff/100001/chrome/renderer/resourc... File chrome/renderer/resources/extensions/extension_options.js (right): https://codereview.chromium.org/480243003/diff/100001/chrome/renderer/resourc... chrome/renderer/resources/extensions/extension_options.js:271: ExtensionOptionsInternal.prototype.resumeDeferredAutoSize = function() { On 2014/08/20 02:07:15, kalman wrote: > You could combine these into a method like setAutosize(true|false)? > > Which makes me think: is this the same thing as webview's autosize property? > Just curious. shouldDeferAutoSize(true|false) does what you describe, resumeDeferredAutoSize just allows the most recent autosize dimensions be applied to the element. I should probably write a clearer comment and give it a better name. I think webview's autosize is just a switch for all autosizing. If this switch is flipped the autosize code is still being run, but I don't allow the DOM elements to change in size, I just get the preferred size from the browser plugin.
Also: is there anything to test here? https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... File chrome/browser/resources/extensions/extension_options_overlay.js (right): https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:105: extensionoptions.resumeDeferredAutoSize(); On 2014/08/20 02:23:30, ericzeng wrote: > On 2014/08/20 02:07:15, kalman wrote: > > Heh how did you figure out how to use animate... docs are very hard to find. > > > > The reason I ask: is is possible that onfinish won't be called? > > This was sufficient documentation for me to implement the feature: > http://updates.html5rocks.com/2014/05/Web-Animations---element-animate-is-now... > > I don't know for sure if onfinish will be called, I just assumed that the docs > are right. But really, can we prove anything true objectively? Specifically I was wondering about an onerror or oncancel or something, but it doesn't look like it: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... the only event is finish. (how's THAT for docs). https://codereview.chromium.org/480243003/diff/100001/chrome/renderer/resourc... File chrome/renderer/resources/extensions/extension_options.js (right): https://codereview.chromium.org/480243003/diff/100001/chrome/renderer/resourc... chrome/renderer/resources/extensions/extension_options.js:271: ExtensionOptionsInternal.prototype.resumeDeferredAutoSize = function() { On 2014/08/20 02:23:30, ericzeng wrote: > On 2014/08/20 02:07:15, kalman wrote: > > You could combine these into a method like setAutosize(true|false)? > > > > Which makes me think: is this the same thing as webview's autosize property? > > Just curious. > > shouldDeferAutoSize(true|false) does what you describe, resumeDeferredAutoSize > just allows the most recent autosize dimensions be applied to the element. I > should probably write a clearer comment and give it a better name. > > I think webview's autosize is just a switch for all autosizing. If this switch > is flipped the autosize code is still being run, but I don't allow the DOM > elements to change in size, I just get the preferred size from the browser > plugin. Ah. In that case, s/shouldDeferAutosize/setDeferAutosize/. and yes better comments would be nice.
This looks really interesting but I'd like more background to what problems you are trying to solve here (I only have a vague sense). Is it possible to chat with you in VC for 20 min or so?
On 2014/08/20 14:49:21, Fady Samuel wrote: > This looks really interesting but I'd like more background to what problems you > are trying to solve here (I only have a vague sense). Is it possible to chat > with you in VC for 20 min or so? Basically the options dialogue resizes very jarringly. This makes it so that the box doesn't even show up until the first resize, then further resizes have quick animations rather than jumping around (the latter not approved by the UI team :). I actually suggest that (if you have the chance) patch this in and give it a spin. It's pretty awesome. A release build makes the animations look a lot better, if you can do that.
https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... File chrome/browser/resources/extensions/extension_options_overlay.css (right): https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.css:6: max-width: 100%; On 2014/08/20 02:07:15, kalman wrote: > What do you need the max-width for? Don't really need it, but extension-error-overlay had it so I dunno https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... File chrome/browser/resources/extensions/extension_options_overlay.js (right): https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:67: var headerStyle = On 2014/08/20 02:07:15, kalman wrote: > You might want to add a meta-comment here, like: > > The extensionoptions content should be fitted to the title ... blah blah blah. Done. https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:91: // constant for all sizes. On 2014/08/20 02:07:15, kalman wrote: > Nit: could you say like "0.25 means the popup resizes at a rate of Xms per 100 > pixels", or something meaningful. As an explanation, and so that people know > what happens when you tweak it. Done. https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:105: extensionoptions.resumeDeferredAutoSize(); On 2014/08/20 02:29:34, kalman wrote: > On 2014/08/20 02:23:30, ericzeng wrote: > > On 2014/08/20 02:07:15, kalman wrote: > > > Heh how did you figure out how to use animate... docs are very hard to find. > > > > > > The reason I ask: is is possible that onfinish won't be called? > > > > This was sufficient documentation for me to implement the feature: > > > http://updates.html5rocks.com/2014/05/Web-Animations---element-animate-is-now... > > > > I don't know for sure if onfinish will be called, I just assumed that the docs > > are right. But really, can we prove anything true objectively? > > Specifically I was wondering about an onerror or oncancel or something, but it > doesn't look like it: > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... > > the only event is finish. > > (how's THAT for docs). Hm, I can't actually open that link, do you have to have some permission to view it? https://codereview.chromium.org/480243003/diff/100001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:112: $('extension-options-overlay-guest').style.left = '10000px'; On 2014/08/20 02:07:15, kalman wrote: > This is to implement that off-screen resize? > > You should probably use a fixed position here, not absolute. And much as 10000px > looks fine, I'd be more comfortable if you used the actual width (whatever the > correct value is which keeps it out of the way with scrolling and zooming and > whatnot). Done. https://codereview.chromium.org/480243003/diff/100001/chrome/renderer/resourc... File chrome/renderer/resources/extensions/extension_options.js (right): https://codereview.chromium.org/480243003/diff/100001/chrome/renderer/resourc... chrome/renderer/resources/extensions/extension_options.js:271: ExtensionOptionsInternal.prototype.resumeDeferredAutoSize = function() { On 2014/08/20 02:29:34, kalman wrote: > On 2014/08/20 02:23:30, ericzeng wrote: > > On 2014/08/20 02:07:15, kalman wrote: > > > You could combine these into a method like setAutosize(true|false)? > > > > > > Which makes me think: is this the same thing as webview's autosize property? > > > Just curious. > > > > shouldDeferAutoSize(true|false) does what you describe, resumeDeferredAutoSize > > just allows the most recent autosize dimensions be applied to the element. I > > should probably write a clearer comment and give it a better name. > > > > I think webview's autosize is just a switch for all autosizing. If this switch > > is flipped the autosize code is still being run, but I don't allow the DOM > > elements to change in size, I just get the preferred size from the browser > > plugin. > > Ah. > > In that case, s/shouldDeferAutosize/setDeferAutosize/. > > and yes better comments would be nice. Done.
On 2014/08/20 14:49:21, Fady Samuel wrote: > This looks really interesting but I'd like more background to what problems you > are trying to solve here (I only have a vague sense). Is it possible to chat > with you in VC for 20 min or so? I think I'm free all afternoon, so just put an event on my calendar.
This basically lgtm except for that calculation of the header height, except for that lineHeight stuff - and I defer to Fady for the tag changes. https://codereview.chromium.org/480243003/diff/120001/chrome/browser/resource... File chrome/browser/resources/extensions/extension_options_overlay.js (right): https://codereview.chromium.org/480243003/diff/120001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:74: var headerHeight = parseInt(headerStyle.lineHeight) + Remember to fix this; test by making the header style have a huge font size. Does $('extension-options-overlay-title').offsetHeight work? Or maybe that should be .scrollHeight. I can never remember. https://codereview.chromium.org/480243003/diff/120001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:84: Explain why you defer autosize.
> except for that calculation of the header height, except for > that lineHeight stuff Repeated myself a little there...
https://codereview.chromium.org/480243003/diff/120001/chrome/browser/resource... File chrome/browser/resources/extensions/extension_options_overlay.js (right): https://codereview.chromium.org/480243003/diff/120001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:74: var headerHeight = parseInt(headerStyle.lineHeight) + On 2014/08/20 20:08:57, kalman wrote: > Remember to fix this; test by making the header style have a huge font size. > Does $('extension-options-overlay-title').offsetHeight work? Or maybe that > should be .scrollHeight. I can never remember. offsetHeight works, thanks https://codereview.chromium.org/480243003/diff/120001/chrome/browser/resource... chrome/browser/resources/extensions/extension_options_overlay.js:84: On 2014/08/20 20:08:57, kalman wrote: > Explain why you defer autosize. Done, also moved it with the rest of the animation workaround code.
guest_view lgtm extension_options.js lgtm extension_options_events.js lgtm
The CQ bit was checked by ericzeng@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ericzeng@chromium.org/480243003/160001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_chromium_gn_compile_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...) android_dbg_tests_recipe on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...) linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_swarming on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_rel_swarming on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...) win_chromium_rel_swarming on tryserver.chromium.win (http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...) win_chromium_x64_rel_swarming on tryserver.chromium.win (http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_swarming on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by ericzeng@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ericzeng@chromium.org/480243003/180001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_chromium_gn_compile_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
Message was sent while issue was closed.
Committed patchset #10 (180001) as 291286
Message was sent while issue was closed.
On 2014/08/22 01:06:04, I haz the power (commit-bot) wrote: > Committed patchset #10 (180001) as 291286 This patch seems to have caused 'ExtensionOptionsApiTest.ExtensionCanEmbedOwnOptions' to fail in several bots: http://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%283%29/builds... http://build.chromium.org/p/chromium.mac/builders/Mac10.6%20Tests%20%282%29/b... http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/13122 http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%2...
Message was sent while issue was closed.
On 2014/08/22 04:04:17, sadrul wrote: > On 2014/08/22 01:06:04, I haz the power (commit-bot) wrote: > > Committed patchset #10 (180001) as 291286 > > This patch seems to have caused > 'ExtensionOptionsApiTest.ExtensionCanEmbedOwnOptions' to fail in several bots: > http://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%283%29/builds... > http://build.chromium.org/p/chromium.mac/builders/Mac10.6%20Tests%20%282%29/b... > http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/13122 > http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%2... There was problem when merging this CL and this CL: https://codereview.chromium.org/444813002/. This CL fixes the issue: https://codereview.chromium.org/496863004/
PTAL, hopefully I won't break everything this time.
still lgtm
The CQ bit was checked by kalman@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ericzeng@chromium.org/480243003/200001
The CQ bit was unchecked by ericzeng@chromium.org
The CQ bit was checked by ericzeng@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ericzeng@chromium.org/480243003/220001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_swarming on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Message was sent while issue was closed.
Committed patchset #12 (220001) as f85b5d1896989b38ec3fd2d392de34a80dd2c8ad
Message was sent while issue was closed.
Patchset 12 (id:??) landed as https://crrev.com/a9438c47225d119a953ebf195210e655cb88dbc5 Cr-Commit-Position: refs/heads/master@{#291583} |
