Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
110723007:
Add the viewer toolbar to the PDF extension (Closed)
|
Created:
7 years ago by raymes Modified:
6 years, 10 months ago CC:
chromium-reviews, chromium-apps-reviews_chromium.org, arv+watch_chromium.org, extensions-reviews_chromium.org, chrome-apps-syd-reviews_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionAdd the viewer toolbar to the PDF extension.
BUG=303491
R=arv@chromium.org, boliu@chromium.org, ganetsky@chromium.org
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=247578
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=249257
Patch Set 1 #
Total comments: 20
Patch Set 2 : #
Total comments: 11
Patch Set 3 : #Patch Set 4 : . #
Total comments: 7
Patch Set 5 : #Patch Set 6 : #
Total comments: 6
Patch Set 7 : #Patch Set 8 : #Patch Set 9 : #
Total comments: 2
Patch Set 10 : #Patch Set 11 : #Patch Set 12 : #
Messages
Total messages: 34 (0 generated)
https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... File chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html (right): https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html:14: if (this.src) { You might want a srcChanged watcher https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html:17: if (mql.matches) dpi = mql.matches ? 'hi' : 'low'; https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html:26: this.className += ' latchable'; You probably want a latchableChanged watcher instead of doing this. I also prefer this.classList.add('latchable') and this.classList.remove('latchable') https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... File chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html (right): https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:4: <div id="hover" on-mouseover="{{fadeIn}}" on-mousemove="{{fadeIn}}" on-mouseout="{{fadeOut}}"> You can probably kill the div, and move these handlers to be on polymer-element itself https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:16: fadeIn: function(event, detail, sender) { Are the arguments necessary? https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:21: this.fadingIn = false; Do you intend for fadingIn to be public or private? https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:22: this.updateStyle(); I think it would be be just have a fadingIn published attribute on the custom element, and write a fadingInChanged listener that calls updateStyle https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:28: this.style.opacity = 1; You can probably just set this all up with data binding directly in CSS. Something like <style> :host { opacity: {{ fadingIn ? 1 : 0 }} -webkit-transition: {{ fadingIn ? 'opacity 0.4s ease-in-out' : '-webkit-transition opacity 0.4s ease-in-out 3s' }} } </style> https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:37: setTimeout(function() { me.updateStyle(); }, 400); Delete var me = this Change to setTimeout(this.updateStyle.bind(this), 400); https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... File chrome/browser/resources/pdf/index.html (right): https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/index.html:275: </html> Is this file the output of a call to vulcanize? If so, can you not check it in, and make that part of the build process?
Thanks for the really helpful comments! https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... File chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html (right): https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html:14: if (this.src) { On 2013/12/17 20:57:32, ganetsky1 wrote: > You might want a srcChanged watcher Done. https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html:17: if (mql.matches) On 2013/12/17 20:57:32, ganetsky1 wrote: > dpi = mql.matches ? 'hi' : 'low'; Done. https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html:26: this.className += ' latchable'; On 2013/12/17 20:57:32, ganetsky1 wrote: > You probably want a latchableChanged watcher instead of doing this. I also > prefer this.classList.add('latchable') and this.classList.remove('latchable') Done. https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... File chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html (right): https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:4: <div id="hover" on-mouseover="{{fadeIn}}" on-mousemove="{{fadeIn}}" on-mouseout="{{fadeOut}}"> On 2013/12/17 20:57:32, ganetsky1 wrote: > You can probably kill the div, and move these handlers to be on polymer-element > itself Done. https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:16: fadeIn: function(event, detail, sender) { On 2013/12/17 20:57:32, ganetsky1 wrote: > Are the arguments necessary? Done. https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:21: this.fadingIn = false; On 2013/12/17 20:57:32, ganetsky1 wrote: > Do you intend for fadingIn to be public or private? It seems easier to have it public based on your hints below and it doesn't seem problematic :) https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:22: this.updateStyle(); On 2013/12/17 20:57:32, ganetsky1 wrote: > I think it would be be just have a fadingIn published attribute on the custom > element, and write a fadingInChanged listener that calls updateStyle Done. https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:28: this.style.opacity = 1; Cool - I didn't know you could do that! I've gone to the trouble of allowing the fade in to finish before starting the fade out. This fixes a bug in the current viewer toolbar where if you quickly mouse over and then mouse out the toolbar stays transparent for a few seconds before fading out. This wouldn't be easy to do with the inline css case. https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:37: setTimeout(function() { me.updateStyle(); }, 400); On 2013/12/17 20:57:32, ganetsky1 wrote: > Delete var me = this > Change to setTimeout(this.updateStyle.bind(this), 400); Done. https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... File chrome/browser/resources/pdf/index.html (right): https://codereview.chromium.org/110723007/diff/1/chrome/browser/resources/pdf... chrome/browser/resources/pdf/index.html:275: </html> Yep. That's what I would like to do. It's not going to be straightforward - started a mail thread which I cc'd you on. For the moment I will probably check in this file to move forward and hopefully we will get this in as part of the build process.
https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... File chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html (right): https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html:13: srcChanged: function() { fyi you can use oldValue, newValue as arguments here https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html:21: } You probably want an else branch which unsets this.$.icon.style.backgroundImage. In other words, respond to someone clearing the src attribute, don't ignore it as you are doing now. https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html:23: latchableChanged: function() { fyi, you can use oldValue, newValue as arguments here https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... File chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html (right): https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:14: this.fadingInChanged(); Not sure this is necessary, considering you set fadingIn to false in initialization https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:22: fadingInChanged: function() { fyi, you can use oldValue, newValue as arguments here https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:34: setTimeout(this.fadingInChanged.bind(this), 400); Do you really want to call the change watcher again (it is called whenever something changes), or do you want to call something more specific after timeout.
https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... File chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html (right): https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html:21: } On 2013/12/18 20:05:05, ganetsky1 wrote: > You probably want an else branch which unsets this.$.icon.style.backgroundImage. > In other words, respond to someone clearing the src attribute, don't ignore it > as you are doing now. Done. https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... File chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html (right): https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:14: this.fadingInChanged(); On 2013/12/18 20:05:05, ganetsky1 wrote: > Not sure this is necessary, considering you set fadingIn to false in > initialization When I tested this, it is needed. It seems like it's not needed if the element is defined with <viewer-toolbar fadingIn='...'> but if that is not the case, fadingInChanged will never be called. https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:34: setTimeout(this.fadingInChanged.bind(this), 400); Yep I want to call the change watcher again. Basically it's saying "if we're fading in, wait until we've fully faded in and then check again whether we should fade out". I could pull everything out into a separate function which fadingInChanged would call, but it doesn't seem necessary.
https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... File chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html (right): https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:34: setTimeout(this.fadingInChanged.bind(this), 400); On 2013/12/18 23:28:54, raymes wrote: > Yep I want to call the change watcher again. Basically it's saying "if we're > fading in, wait until we've fully faded in and then check again whether we > should fade out". I could pull everything out into a separate function which > fadingInChanged would call, but it doesn't seem necessary. I think your timing is off. I think what you want to do is 1) Immediately return from the changed watcher if in midst of a fade in 2) Always schedule a 400ms setTimeout right when you start the fadeIn transition, and only then
https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... File chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html (right): https://codereview.chromium.org/110723007/diff/20001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:34: setTimeout(this.fadingInChanged.bind(this), 400); Hmm, not sure how the timing is off? This code says that if fadingIn == false, start fading out UNLESS we haven't finished fading in, in which case we should schedule to fade out after we're sure we would have finished fading in (which is after 400ms). When the function is called back in to after 400ms we re-evaluate whether we should start fading out and we may get delayed again, which is important because the state could have changed in that time.
The timer starts counting 400ms from the attribute change event, not from the start of the transition. It is correct in the sense that it waits long enough, but it potentially waits too long. Also, I am not entirely sure your code never interrupts ongoing transitions, but I don't know CSS well enough to say. On Dec 19, 2013 8:47 PM, <raymes@chromium.org> wrote: > > https://codereview.chromium.org/110723007/diff/20001/ > chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html > File > chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html > (right): > > https://codereview.chromium.org/110723007/diff/20001/ > chrome/browser/resources/pdf/elements/viewer-toolbar/ > viewer-toolbar.html#newcode34 > chrome/browser/resources/pdf/elements/viewer-toolbar/ > viewer-toolbar.html:34: > setTimeout(this.fadingInChanged.bind(this), 400); > Hmm, not sure how the timing is off? > > This code says that if fadingIn == false, start fading out UNLESS we > haven't finished fading in, in which case we should schedule to fade out > after we're sure we would have finished fading in (which is after > 400ms). > > When the function is called back in to after 400ms we re-evaluate > whether we should start fading out and we may get delayed again, which > is important because the state could have changed in that time. > > https://codereview.chromium.org/110723007/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
ptal - I changed the timing to use setTimeout and clearTimeout instead of relying on the 3s transition timer. The timing should hopefully by right and clear now =)
https://codereview.chromium.org/110723007/diff/70001/chrome/browser/resources... File chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html (right): https://codereview.chromium.org/110723007/diff/70001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html:18: } You may want to consider moving this out to ready: function() {...} https://codereview.chromium.org/110723007/diff/70001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html:29: this.classList.remove('latchable') Semicolon? Also, maybe you want to use { } around each branch. I don't know the style guidelines here. https://codereview.chromium.org/110723007/diff/70001/chrome/browser/resources... File chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html (right): https://codereview.chromium.org/110723007/diff/70001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:36: } There are numerous things I don't understand here: 1) You need to set timerIsRunning to false when either the timer expires, or the timer is cancelled. But you don't seem to set timerIsRunning to false when the timer expires. 2) I don't see the initiation of a fade transition, just a an opacity change for 3 seconds from now. 3) Why not collapse the timerIsRunning variable and the timerId variable into one? Just set timerId to undefined whenever the timer is not running.
https://codereview.chromium.org/110723007/diff/70001/chrome/browser/resources... File chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html (right): https://codereview.chromium.org/110723007/diff/70001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html:18: } On 2014/01/03 18:14:39, ganetsky1 wrote: > You may want to consider moving this out to ready: function() {...} Done. https://codereview.chromium.org/110723007/diff/70001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.html:29: this.classList.remove('latchable') Added the semicolons. The braces aren't required for single-line if's IIUC https://codereview.chromium.org/110723007/diff/70001/chrome/browser/resources... File chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html (right): https://codereview.chromium.org/110723007/diff/70001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:36: } 1) Done, thanks. 2) The transition has been moved to the CSS. 3) Done.
Drive by... I was curious how you were using Polymer in Chrome. https://codereview.chromium.org/110723007/diff/230001/chrome/browser/resource... File chrome/browser/resources/pdf/elements/viewer-button/viewer-button.css (right): https://codereview.chromium.org/110723007/diff/230001/chrome/browser/resource... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.css:15: background-image: -webkit-linear-gradient(rgb(60, 80, 119), rgb(15, 24, 41)); No need for the webkit prefix. Try not to use prefix unless you have to. https://codereview.chromium.org/110723007/diff/230001/chrome/browser/resource... File chrome/browser/resources/pdf/index.html (right): https://codereview.chromium.org/110723007/diff/230001/chrome/browser/resource... chrome/browser/resources/pdf/index.html:258: <viewer-button src="button_fit_page.png" latchable="true"></viewer-button> Use boolean attributes for latchable? https://codereview.chromium.org/110723007/diff/230001/chrome/browser/resource... chrome/browser/resources/pdf/index.html:270: <script src="pdf.js" language="javascript" type="text/javascript"></script> <script src="pdf.js"></script> everything else is just cruft
https://codereview.chromium.org/110723007/diff/230001/chrome/browser/resource... File chrome/browser/resources/pdf/elements/viewer-button/viewer-button.css (right): https://codereview.chromium.org/110723007/diff/230001/chrome/browser/resource... chrome/browser/resources/pdf/elements/viewer-button/viewer-button.css:15: background-image: -webkit-linear-gradient(rgb(60, 80, 119), rgb(15, 24, 41)); On 2014/01/06 15:42:43, arv wrote: > No need for the webkit prefix. > > Try not to use prefix unless you have to. Done. https://codereview.chromium.org/110723007/diff/230001/chrome/browser/resource... File chrome/browser/resources/pdf/index.html (right): https://codereview.chromium.org/110723007/diff/230001/chrome/browser/resource... chrome/browser/resources/pdf/index.html:258: <viewer-button src="button_fit_page.png" latchable="true"></viewer-button> On 2014/01/06 15:42:43, arv wrote: > Use boolean attributes for latchable? Done. https://codereview.chromium.org/110723007/diff/230001/chrome/browser/resource... chrome/browser/resources/pdf/index.html:270: <script src="pdf.js" language="javascript" type="text/javascript"></script> On 2014/01/06 15:42:43, arv wrote: > <script src="pdf.js"></script> > > everything else is just cruft Done.
Thanks for the comments arv!
On 2014/01/07 23:20:20, raymes wrote: > Thanks for the comments arv! ganetsky: ping :)
On 2014/01/07 23:20:20, raymes wrote: > Thanks for the comments arv! ganetsky: ping :)
https://codereview.chromium.org/110723007/diff/70001/chrome/browser/resources... File chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html (right): https://codereview.chromium.org/110723007/diff/70001/chrome/browser/resources... chrome/browser/resources/pdf/elements/viewer-toolbar/viewer-toolbar.html:36: } On 2014/01/06 00:22:45, raymes wrote: > 1) Done, thanks. > 2) The transition has been moved to the CSS. > 3) Done. Please correct me if I'm wrong somewhere: There still needs to be something that initiates the transition... Your CSS is watching for changes in opacity... NOT for changes in fadingIn Therefore, when fadingIn is changed from true to false, this code waits 3 seconds, and THEN sets opacity to 0, which I think would THEN initiate the transition. Also, this code is one-sided, it sets a timer when fading in goes from true to false, but not the other way around. Is that intentional and correct?
lgtm As per our discussion, I now understand how the transitions work
arv: could you give OWNERS approval for chrome/browser/resources/* ? Thanks!
LGTM with nits https://codereview.chromium.org/110723007/diff/530001/chrome/browser/resource... File chrome/browser/resources/pdf/index.in.html (right): https://codereview.chromium.org/110723007/diff/530001/chrome/browser/resource... chrome/browser/resources/pdf/index.in.html:6: <!DOCTYPE HTML> The doctype should be on the first line https://codereview.chromium.org/110723007/diff/530001/chrome/browser/resource... chrome/browser/resources/pdf/index.in.html:26: <body marginwidth="0" marginheight="0" > move to css body { margin: 0; }
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/raymes@chromium.org/110723007/570001
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/raymes@chromium.org/110723007/570001
Failed to apply patch for chrome/browser/resources/pdf/index.js:
While running patch -p1 --forward --force --no-backup-if-mismatch;
A chrome/browser/resources/pdf/index.js
Copied third_party/polymer/polymer/polymer.js ->
chrome/browser/resources/pdf/index.js
patching file chrome/browser/resources/pdf/index.js
Hunk #1 FAILED at 27.
1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/resources/pdf/index.js.rej
Patch: NR
third_party/polymer/polymer/polymer.js->chrome/browser/resources/pdf/index.js
Index: chrome/browser/resources/pdf/index.js
diff --git a/third_party/polymer/polymer/polymer.js
b/chrome/browser/resources/pdf/index.js
similarity index 63%
copy from third_party/polymer/polymer/polymer.js
copy to chrome/browser/resources/pdf/index.js
index
89da01d0bc464e16f00d974b70f3f7e7c4bc472a..e05a104786b4765501a8bf7e683144404335d52c
100644
--- a/third_party/polymer/polymer/polymer.js
+++ b/chrome/browser/resources/pdf/index.js
@@ -27,4 +27,425 @@
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// @version: 0.1.1
Polymer={},"function"==typeof window.Polymer&&(Polymer={}),function(a){function
b(a,b){return a&&b&&Object.getOwnPropertyNames(b).forEach(function(c){var
d=Object.getOwnPropertyDescriptor(b,c);d&&(Object.defineProperty(a,c,d),"function"==typeof
d.value&&(d.value.nom=c))}),a}a.extend=b}(Polymer),function(a){function
b(a,b,d){return a?a.stop():a=new c(this),a.go(b,d),a}var
c=function(a){this.context=a};c.prototype={go:function(a,b){this.callback=a,this.handle=setTimeout(this.complete.bind(this),b)},stop:function(){this.handle&&(clearTimeout(this.handle),this.handle=null)},complete:function(){this.handle&&(this.stop(),this.callback.call(this.context))}},a.job=b}(Polymer),function(){var
a={};HTMLElement.register=function(b,c){a[b]=c},HTMLElement.getPrototypeForTag=function(b){var
c=b?a[b]:HTMLElement.prototype;return
c||Object.getPrototypeOf(document.createElement(b))};var
b=Event.prototype.stopPropagation;Event.prototype.stopPropagation=function(){this.cancelBubble=!0,b.apply(this,arguments)},HTMLImports.importer.preloadSelectors+=",
polymer-element link[rel=stylesheet]"}(Polymer),function(a){function b(a){var
c=b.caller,g=c.nom,h=c._super;if(h||(g||(g=c.nom=e.call(this,c)),g||console.warn("called
super() on a method not installed declaratively (has no .nom
property)"),h=d(c,g,f(this))),h){var i=h[g];return
i._super||d(i,g,h),i.apply(this,a||[])}}function
c(a,b,c){for(;a;){if(a[b]!==c&&a[b])return a;a=f(a)}}function d(a,b,d){return
a._super=c(d,b,a),a._super&&(a._super[b].nom=b),a._super}function e(a){for(var
b=this.__proto__;b&&b!==HTMLElement.prototype;){for(var
c,d=Object.getOwnPropertyNames(b),e=0,f=d.length;f>e&&(c=d[e]);e++){var
g=Object.getOwnPropertyDescriptor(b,c);if("function"==typeof
g.value&&g.value===a)return c}b=b.__proto__}}function f(a){return
a.__proto__}a.super=b}(Polymer),function(a){function b(a,b){var d=typeof
b;return b instanceof Date&&(d="date"),c[d](a,b)}var
c={string:function(a){return a},date:function(a){return new
Date(Date.parse(a)||Date.now())},"boolean":function(a){return""===a?!0:"false"===a?!1:!!a},number:function(a){var
b=parseFloat(a);return
String(b)===a?b:a},object:function(a,b){if(null===b)return a;try{return
JSON.parse(a.replace(/'/g,'"'))}catch(c){return
a}},"function":function(a,b){return
b}};a.deserializeValue=b}(Polymer),function(a){var
b={};b.declaration={},b.instance={},a.api=b}(Polymer),function(a){var
b={async:function(a,b,c){Platform.flush(),b=b&&b.length?b:[b];var
d=function(){(this[a]||a).apply(this,b)}.bind(this);return
c?setTimeout(d,c):requestAnimationFrame(d)},fire:function(a,b,c,d){var
e=c||this;return e.dispatchEvent(new CustomEvent(a,{bubbles:void
0!==d?d:!0,detail:b})),b},asyncFire:function(){this.async("fire",arguments)},classFollows:function(a,b,c){b&&b.classList.remove(c),a&&a.classList.add(c)}};b.asyncMethod=b.async,a.api.instance.utils=b}(Polymer),function(a){function
b(a){for(;a.parentNode;)a=a.parentNode;return a.host}var
c=window.logFlags||{},d="on-",e={EVENT_PREFIX:d,hasEventPrefix:function(a){return
a&&"o"===a[0]&&"n"===a[1]&&"-"===a[2]},removeEventPrefix:function(a){return
a.slice(f)},addHostListeners:function(){var
a=this.eventDelegates;c.events&&Object.keys(a).length>0&&console.log("[%s]
addHostListeners:",this.localName,a),this.addNodeListeners(this,a,this.hostEventListener)},addNodeListeners:function(a,b,c){var
d;for(var e in
b)d||(d=c.bind(this)),this.addNodeListener(a,e,d)},addNodeListener:function(a,b,c){a.addEventListener(b,c)},hostEventListener:function(a){if(!a.cancelBubble){c.events&&console.group("[%s]:
hostEventListener(%s)",this.localName,a.type);var
b=this.findEventDelegate(a);b&&(c.events&&console.log("[%s] found host handler
name
[%s]",this.localName,b),this.dispatchMethod(this,b,[a,a.detail,this])),c.events&&console.groupEnd()}},findEventDelegate:function(a){return
this.eventDelegates[a.type]},dispatchMethod:function(a,b,d){if(a){c.events&&console.group("[%s]
dispatch [%s]",a.localName,b);var e="function"==typeof
b?b:a[b];e&&e[d?"apply":"call"](a,d),c.events&&console.groupEnd(),Platform.flush()}},prepareBinding:function(a,d){return
e.hasEventPrefix(d)?function(f,g){c.events&&console.log('event: [%s].%s =>
[%s].%s()"',g.localName,f.localName,a);var h=function(c){var
d=b(g);if(d&&d.dispatchMethod){var
e=d,h=a;"@"==a[0]&&(e=f,h=Path.get(a.slice(1)).getValueFrom(f)),d.dispatchMet...
g.addEventListener(i,h,!1),{close:function(){c.events&&console.log('event.remove:
[%s].%s =>
[%s].%s()"',g.localName,d,f.localName,a),g.removeEventListener(i,h,!1)}}}:void
0}},f=d.length;a.api.instance.events=e}(Polymer),function(a){var
b={copyInstanceAttributes:function(){var a=this._instanceAttributes;for(var b in
a)this.hasAttribute(b)||this.setAttribute(b,a[b])},takeAttributes:function(){if(this._publishLC)for(var
a,b=0,c=this.attributes,d=c.length;(a=c[b])&&d>b;b++)this.attributeToProperty(a.name,a.value)},attributeToProperty:function(b,c){var
b=this.propertyForAttribute(b);if(b){if(c&&c.search(a.bindPattern)>=0)return;var
d=this[b],c=this.deserializeValue(c,d);c!==d&&(this[b]=c)}},propertyForAttribute:function(a){var
b=this._publishLC&&this._publishLC[a];return
b},deserializeValue:function(b,c){return
a.deserializeValue(b,c)},serializeValue:function(a,b){return"boolean"===b?a?"":void
0:"object"!==b&&"function"!==b&&void 0!==a?a:void
0},reflectPropertyToAttribute:function(a){var b=typeof
this[a],c=this.serializeValue(this[a],b);void
0!==c?this.setAttribute(a,c):"boolean"===b&&this.removeAttribute(a)}};a.api.instance.attributes=b}(Polymer),function(a){function
b(a){return new CompoundPathObserver(a.notifyPropertyChanges,a)}function
c(a,b,c,e){d.bind&&console.log(f,c.localName||"object",e,a.localName,b);var
g=Path.get(e),h=g.getValueFrom(c);return(null===h||void
0===h)&&g.setValueFrom(c,a[b]),PathObserver.defineProperty(a,b,c,e)}var
d=window.logFlags||{},e={observeProperties:function(){var
a=this._observeNames,c=this._publishNames;if(a&&a.length||c&&c.length){for(var
d,e=this._propertyObserver=b(this),f=0,g=a.length;g>f&&(d=a[f]);f++){e.addPath(this,d);var
h=Object.getOwnPropertyDescriptor(this.__proto__,d);h&&h.value&&this.observeArrayValue(d,h.value,null)}for(var
d,f=0,g=c.length;g>f&&(d=c[f]);f++)this.observe&&void
0!==this.observe[d]||e.addPath(this,d);e.start()}},notifyPropertyChanges:function(a,b,c,d){for(var
e,f,g={},h=0,i=c.length;i>h;h++)c[h]&&(e=d[2*h+1],void
0!==this.publish[e]&&this.reflectPropertyToAttribute(e),f=this.observe[e],f&&(this.observeArrayValue(e,a[h],b[h]),g[f]||(g[f]=!0,this.invokeMethod(f,[b[h],a[h],arguments]))))},observeArrayValue:function(a,b,c){var
e=this.observe[a];if(e&&(Array.isArray(c)&&(d.observe&&console.log("[%s]
observeArrayValue: unregister observer
[%s]",this.localName,a),this.unregisterObserver(a+"__array")),Array.isArray(b))){d.observe&&console.log("[%s]
observeArrayValue: register observer [%s]",this.localName,a,b);var f=this,g=new
ArrayObserver(b,function(a,b){f.invokeMethod(e,[b])});this.registerObserver(a+"__array",g)}},bindProperty:function(a,b,d){return
c(this,a,b,d)},unbindAllProperties:function(){this._propertyObserver&&this._propertyObserver.close(),this.unregisterObservers()},unbindProperty:function(a){return
this.unregisterObserver(a)},invokeMethod:function(a,b){var
c=this[a]||a;"function"==typeof
c&&c.apply(this,b)},registerObserver:function(a,b){var
c=this._observers||(this._observers={});c[a]=b},unregisterObserver:function(a){var
b=this._observers;return b&&b[a]?(b[a].close(),b[a]=null,!0):void
0},unregisterObservers:function(){if(this._observers){for(var
a,b,c=Object.keys(this._observers),d=0,e=c.length;e>d&&(a=c[d]);d++)b=this._observers[a],b.close();this._observers={}}}},f="[%s]:
bindProperties: [%s] to
[%s].[%s]";a.api.instance.properties=e}(Polymer),function(a){function
b(a){d(a,c)}function c(a){a.unbindAll()}function d(a,b){if(a){b(a);for(var
c=a.firstChild;c;c=c.nextSibling)d(c,b)}}var
e=window.logFlags||0,f=a.api.instance.events,g=PolymerExpressions.prototype.prepareBinding;PolymerExpressions.prototype.prepareBinding=function(a,b,c){return
f.prepareBinding(a,b,c)||g.call(this,a,b,c)};var h=new
PolymerExpressions,i={syntax:h,instanceTemplate:function(a){return
a.bindingDelegate=this.syntax,a.createInstance(this)},bind:function(a,b,c){this._elementPrepared||this.prepareElement();var
d=this.propertyForAttribute(a);if(d){this.unbind(a);var
e=this.bindProperty(d,b,c);return
e.path=c,this.reflectPropertyToAttribute(d),this.bindings[a]=e}return
this.super(arguments)},asyncUnbindAll:function(){this._unbound||(e.unbind&&console.log("[%s]
asyncUnbindAll",this.localName),this._unbindAllJob=this.job(this._unbindAllJob,this.unbindAll,0))},unbindAll:function(){if(!this._unbound){this.unbindAllProperties(),this.super();for(var
a=this.shadowRoot;a;)b(a),a=a.olderShadowRoot;this._unbound=!0}},cancelUnbindAll:function(a){return
this._unbound?(e.unbind&&console.wa…
(message too large)
Message was sent while issue was closed.
Committed patchset #11 manually as r247578.
Message was sent while issue was closed.
A revert of this CL has been created in https://codereview.chromium.org/141553006/ by haitaol@chromium.org. The reason for reverting is: License check failure: http://build.chromium.org/p/chromium.linux/builders/Android%20Builder%20%28db....
+benm for OWNERS for android_webview/tools/third_party_files_whitelist.txt
+boliu@chromium.org for OWNERS ping for android_webview/tools/third_party_files_whitelist.txt
On 2014/01/31 01:05:52, raymes wrote: > mailto:+boliu@chromium.org for OWNERS > > ping for android_webview/tools/third_party_files_whitelist.txt lgtm Is a lot of polymer files going to be added to Chrome source tree? If so, long term solution is probably teach the tool about the polymer license.
The CQ bit was checked by raymes@chromium.org
On 2014/01/31 07:05:04, boliu wrote: > On 2014/01/31 01:05:52, raymes wrote: > > mailto:+boliu@chromium.org for OWNERS > > > > ping for android_webview/tools/third_party_files_whitelist.txt > > lgtm > > Is a lot of polymer files going to be added to Chrome source tree? If so, long > term solution is probably teach the tool about the polymer license. Hopefully we'll find a better solution to using polymer elements than having to check-in vulcanize output. This would avoid this issue altogether. Thanks!
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/raymes@chromium.org/110723007/710001
The CQ bit was unchecked by commit-bot@chromium.org
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...
Message was sent while issue was closed.
Committed patchset #12 manually as r249257. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
