Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
6 years, 6 months ago by oystein (OOO til 10th of July) Modified:
6 years, 4 months ago Reviewers:
abarth-chromium, esprehn, dcheng, eseidel CC:
abarth-chromium, blink-reviews, dglazkov+blink, jamesr, shatch, site-isolation-dev_chromium.org Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Visibility:
Public. |
DescriptionNavigation transitions: Added notifyTransitionsShown and setupTransitionsView to WebLocalFrame
The former is called on the outgoing page, and will hide any transition elements and enable any exit transition animations.
The latter is called on the transition layer, and will replace the document (about:blank) with the serialized markup, enforce sandboxing, and turn off script execution.
R=eseidel,esprehn
BUG=370696
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179431
Patch Set 1 : #
Total comments: 23
Patch Set 2 : Review fixes #
Total comments: 14
Patch Set 3 : #Patch Set 4 : #
Total comments: 7
Patch Set 5 : Review fixes #
Total comments: 4
Patch Set 6 : Review fix #
Created: 6 years, 4 months ago
Messages
Total messages: 22 (0 generated)
Two more pieces of the puzzle; Blink endpoints which are called on the outgoing document and on the transition document. I suspect there's better ways of both hiding the transition elements on the outgoing page and replacing the current document on the transition layer than we're currently doing; ptal and let me know :).
https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... File Source/web/WebLocalFrameImpl.cpp (right): https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1300: RefPtr<HTMLCollection> metaElements = frame()->document()->getElementsByTagName(HTMLNames::metaTag.localName()); We don't use node lists of querySelector inside Source/ like this. For example your code below makes us walk the entire document twice looking for <meta> elements. You need to keep track the meta elements for transitions in a Vector in document using ::insertedInto and attributeChanged notifications in HTMLMetaElement. https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1303: unsigned length = metaElements->length(); Touching length() walks the entire document, then you walk it again below. https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1311: String(metaElement->content().impl()).split(';', tokens); metaElement->content().string().split(...) https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1324: toElement(node)->setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone); Modifying parts of the page like this doesn't seem right. https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1332: frame()->document()->styleEngine()->enableExitTransitionStylesheets(); You can't do this, closeURL() shuts the FrameLoader down and dispatches unload events. I don't think you can then make us do recalcStyle and such later on the same document. https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1346: String tempMarkup = String("<!DOCTYPE html><meta name=\"viewport\" content=\"width=device-width, user-scalable=0\">") + String(markup); You should talk to abarth@ about this.
You should talk to abarth@ about how this integrates, I don't think the FrameLoader logic is right.
https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... File Source/web/WebLocalFrameImpl.cpp (right): https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1298: void WebLocalFrameImpl::notifyTransitionShown(int elementsToHide) This is way too much logic for WebLocalFrameImpl. Please move the logic that interacts with core objects into core. https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1341: frame()->document()->enforceSandboxFlags(SandboxAll); Can you assert that document()->securityOrigin()->isUnique() here? I'm not sure you're successfully gotten a unique security origin. https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1342: frame()->page()->settings().setScriptEnabled(false); This line is redundant with enforceSandboxFlags(SandboxAll); https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1346: String tempMarkup = String("<!DOCTYPE html><meta name=\"viewport\" content=\"width=device-width, user-scalable=0\">") + String(markup); This should be included in |markup|. I'm not sure where |markup| is generated, but whoever generates it should generate the complete string, including the doctype and the viewport. https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1348: } All this logic should be in core also. https://codereview.chromium.org/316053007/diff/20001/public/web/WebFrame.h File public/web/WebFrame.h (right): https://codereview.chromium.org/316053007/diff/20001/public/web/WebFrame.h#ne... public/web/WebFrame.h:475: virtual void setupTransitionView(const WebString& markup) = 0; I'm not sure this is the right way to load this content into the frame. Rather than replacing the content of the document (always dicey), you should trigger a navigation of the frame to a new document with replacement data consisting of your markup. Specifically, I think you want to call something like WebFrame::loadHTMLString. Perhaps a function like WebFrame::loadTransitionHTML that calls WebFrame::loadData under the hood, like WebFrame::loadHTMLString calls WebFrame::loadData? Notice that WebFrame::loadData loads the data via the FrameLoader so that the rest of the system works correctly.
https://codereview.chromium.org/316053007/diff/20001/public/web/WebFrame.h File public/web/WebFrame.h (right): https://codereview.chromium.org/316053007/diff/20001/public/web/WebFrame.h#ne... public/web/WebFrame.h:475: virtual void setupTransitionView(const WebString& markup) = 0; I don't think you'd ever want to call this on a remote frame? As such, I think it'd be more appropriate to only add this API to WebLocalFrame.h.
Second version up; ptal :) https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... File Source/web/WebLocalFrameImpl.cpp (right): https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1300: RefPtr<HTMLCollection> metaElements = frame()->document()->getElementsByTagName(HTMLNames::metaTag.localName()); On 2014/06/13 08:55:20, esprehn wrote: > We don't use node lists of querySelector inside Source/ like this. For example > your code below makes us walk the entire document twice looking for <meta> > elements. You need to keep track the meta elements for transitions in a Vector > in document using ::insertedInto and attributeChanged notifications in > HTMLMetaElement. Somewhat improved now. Is it worth using ::insertedInto and attributeChanged to keep track a runtime list of the meta elements though, when we at most do this twice per document (the ones which have navigation transitions defined? https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1303: unsigned length = metaElements->length(); On 2014/06/13 08:55:20, esprehn wrote: > Touching length() walks the entire document, then you walk it again below. Done. https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1311: String(metaElement->content().impl()).split(';', tokens); On 2014/06/13 08:55:20, esprehn wrote: > metaElement->content().string().split(...) Done. https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1324: toElement(node)->setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone); On 2014/06/13 08:55:19, esprehn wrote: > Modifying parts of the page like this doesn't seem right. Is there a better way of hiding these elements? https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1332: frame()->document()->styleEngine()->enableExitTransitionStylesheets(); On 2014/06/13 08:55:19, esprehn wrote: > You can't do this, closeURL() shuts the FrameLoader down and dispatches unload > events. I don't think you can then make us do recalcStyle and such later on the > same document. I believe dispatching the unload events first was actually the idea here, so that we wouldn't disrupt the normal page flow when we hide the transition elements and apply any exit animations (from any transition-exiting stylesheets). That still works, so I assume recalcStyle still works :). https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1341: frame()->document()->enforceSandboxFlags(SandboxAll); On 2014/06/13 17:29:01, abarth wrote: > Can you assert that document()->securityOrigin()->isUnique() here? I'm not sure > you're successfully gotten a unique security origin. Done, though I'm not quite sure what I'm doing here. The page URL is basically just about:blank, should we be using some other URL here with the domain we're navigating to? https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1342: frame()->page()->settings().setScriptEnabled(false); On 2014/06/13 17:29:01, abarth wrote: > This line is redundant with enforceSandboxFlags(SandboxAll); Done. https://codereview.chromium.org/316053007/diff/20001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1346: String tempMarkup = String("<!DOCTYPE html><meta name=\"viewport\" content=\"width=device-width, user-scalable=0\">") + String(markup); On 2014/06/13 17:29:01, abarth wrote: > This should be included in |markup|. I'm not sure where |markup| is generated, > but whoever generates it should generate the complete string, including the > doctype and the viewport. Done. https://codereview.chromium.org/316053007/diff/20001/public/web/WebFrame.h File public/web/WebFrame.h (right): https://codereview.chromium.org/316053007/diff/20001/public/web/WebFrame.h#ne... public/web/WebFrame.h:475: virtual void setupTransitionView(const WebString& markup) = 0; On 2014/06/18 18:12:17, dcheng wrote: > I don't think you'd ever want to call this on a remote frame? As such, I think > it'd be more appropriate to only add this API to WebLocalFrame.h. Done. https://codereview.chromium.org/316053007/diff/20001/public/web/WebFrame.h#ne... public/web/WebFrame.h:475: virtual void setupTransitionView(const WebString& markup) = 0; On 2014/06/13 17:29:01, abarth wrote: > I'm not sure this is the right way to load this content into the frame. > > Rather than replacing the content of the document (always dicey), you should > trigger a navigation of the frame to a new document with replacement data > consisting of your markup. Specifically, I think you want to call something > like WebFrame::loadHTMLString. Perhaps a function like > WebFrame::loadTransitionHTML that calls WebFrame::loadData under the hood, like > WebFrame::loadHTMLString calls WebFrame::loadData? Notice that > WebFrame::loadData loads the data via the FrameLoader so that the rest of the > system works correctly. Done! Right now I just call loadData() from setupTransitionView() though, should it be moved to some intermediate function?
You get a ping! And you get a ping! Everyone gets a ping!
This CL has no tests. https://codereview.chromium.org/316053007/diff/60001/Source/core/dom/Document... File Source/core/dom/Document.cpp (right): https://codereview.chromium.org/316053007/diff/60001/Source/core/dom/Document... Source/core/dom/Document.cpp:5736: continue; What if the meta tag has changed since the caller selected |elementsToHide|. Wouldn't it make more sense to read the meta element once and then operate on the contents instead of passing around a numerical reference to a moving target. https://codereview.chromium.org/316053007/diff/60001/Source/core/dom/Document... Source/core/dom/Document.cpp:5738: for (unsigned nodeIdx = 0; nodeIdx < nodeListLength; ++nodeIdx) { nodeIndex https://codereview.chromium.org/316053007/diff/60001/Source/core/dom/Document... Source/core/dom/Document.cpp:5740: toElement(node)->setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone); This all looks like something that could be better implemented using blink-in-js... https://codereview.chromium.org/316053007/diff/60001/Source/core/dom/Document.h File Source/core/dom/Document.h (right): https://codereview.chromium.org/316053007/diff/60001/Source/core/dom/Document... Source/core/dom/Document.h:429: void hideTransitionElements(unsigned elementsToHide); I don't understand what it means to represent a set of elements to hide as an unsigned. https://codereview.chromium.org/316053007/diff/60001/Source/web/WebLocalFrame... File Source/web/WebLocalFrameImpl.cpp (right): https://codereview.chromium.org/316053007/diff/60001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1336: void WebLocalFrameImpl::notifyTransitionShown(int elementsToHide) Perhaps this function should be called "beginExitTransition" ? https://codereview.chromium.org/316053007/diff/60001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1341: frame()->document()->hideTransitionElements((unsigned)elementsToHide); Why randomly cast from signed to unsigned? Will anyone ever call this function with a negative number? https://codereview.chromium.org/316053007/diff/60001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1355: } These all seem like separate steps that the embedder should perform. https://codereview.chromium.org/316053007/diff/60001/public/web/WebLocalFrame.h File public/web/WebLocalFrame.h (right): https://codereview.chromium.org/316053007/diff/60001/public/web/WebLocalFrame... public/web/WebLocalFrame.h:35: virtual void notifyTransitionShown(int elementsToHide) = 0; didShowTransition ? Is the WebLocalFrame being notified of something or being instructed to notify someone else? If so, who? Why is elementsToHide represented as an int? https://codereview.chromium.org/316053007/diff/60001/public/web/WebLocalFrame... public/web/WebLocalFrame.h:36: virtual void setupTransitionView(const WebData& markup) = 0; What does it meant to setup a transition view? Is this a request to navigate the WebLocalFrame? Maybe we should switch the frame into transition mode and then kick off the navigation? This API doesn't make much sense.
Apologies for taking so long to get back to this one; had a few other things pending. I'll add two layout tests if this approach looks OK. ptal :) https://codereview.chromium.org/316053007/diff/60001/Source/core/dom/Document... File Source/core/dom/Document.cpp (right): https://codereview.chromium.org/316053007/diff/60001/Source/core/dom/Document... Source/core/dom/Document.cpp:5736: continue; On 2014/06/21 05:38:36, abarth wrote: > What if the meta tag has changed since the caller selected |elementsToHide|. > Wouldn't it make more sense to read the meta element once and then operate on > the contents instead of passing around a numerical reference to a moving target. I've changed this to just pass around the CSS selector string used instead of a numerical index, should be more robust. The main issue is that Blink doesn't know which set of transition elements should be hidden; Chrome is essentially choosing which set to use based on something CSP-like https://codereview.chromium.org/316053007/diff/60001/Source/core/dom/Document... Source/core/dom/Document.cpp:5738: for (unsigned nodeIdx = 0; nodeIdx < nodeListLength; ++nodeIdx) { On 2014/06/21 05:38:36, abarth wrote: > nodeIndex Done. https://codereview.chromium.org/316053007/diff/60001/Source/web/WebLocalFrame... File Source/web/WebLocalFrameImpl.cpp (right): https://codereview.chromium.org/316053007/diff/60001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1336: void WebLocalFrameImpl::notifyTransitionShown(int elementsToHide) On 2014/06/21 05:38:36, abarth wrote: > Perhaps this function should be called "beginExitTransition" ? Done. https://codereview.chromium.org/316053007/diff/60001/Source/web/WebLocalFrame... Source/web/WebLocalFrameImpl.cpp:1355: } On 2014/06/21 05:38:36, abarth wrote: > These all seem like separate steps that the embedder should perform. I moved the setIsTransitionDocument to a separate API function now, and renamed this one navigateToSandboxedMarkup. https://codereview.chromium.org/316053007/diff/60001/public/web/WebLocalFrame.h File public/web/WebLocalFrame.h (right): https://codereview.chromium.org/316053007/diff/60001/public/web/WebLocalFrame... public/web/WebLocalFrame.h:36: virtual void setupTransitionView(const WebData& markup) = 0; On 2014/06/21 05:38:36, abarth wrote: > What does it meant to setup a transition view? Is this a request to navigate > the WebLocalFrame? Maybe we should switch the frame into transition mode and > then kick off the navigation? This API doesn't make much sense. Done.
Out of curiosity, what happens today if you try to use navigation transitions during a navigation where a process swap is forced?
On 2014/07/22 21:24:58, dcheng (OOO) wrote: > Out of curiosity, what happens today if you try to use navigation transitions > during a navigation where a process swap is forced? The Blink side of that works fine currently, but there's some issues we're working on Chrome-side still. See in https://codereview.chromium.org/358973005/ (patch set #3 has cross-process code ripped out, but it's still in #2) and the code in the CrossSiteResourceHandler. Essentially we store some state in a TransitionRequestManager when a navigation starts, which the CrossSiteResourceHandler then picks up when we get a response to the request, which is then forwarded to the (possibly new) renderer.
abarth: ping :). Does this look more sensible now, and does it make sense for this to be tested through new unit tests in web/tests?
https://codereview.chromium.org/316053007/diff/140001/Source/core/dom/Documen... File Source/core/dom/Document.cpp (right): https://codereview.chromium.org/316053007/diff/140001/Source/core/dom/Documen... Source/core/dom/Document.cpp:5740: RefPtr<StaticNodeList> nodeList = querySelectorAll(AtomicString(cssSelector), exceptionState); If we're going to use cssSelector as an AtomicString, we should declare that as the type of the argument. https://codereview.chromium.org/316053007/diff/140001/Source/web/WebLocalFram... File Source/web/WebLocalFrameImpl.cpp (right): https://codereview.chromium.org/316053007/diff/140001/Source/web/WebLocalFram... Source/web/WebLocalFrameImpl.cpp:1230: frame()->loader().closeURL(); Will this cause us to dispatch the unload event twice? It looks like FrameLoader::detachFromParent also calls closeURL, and closeURL will call dispatchUnloadEvents... https://codereview.chromium.org/316053007/diff/140001/Source/web/WebLocalFram... Source/web/WebLocalFrameImpl.cpp:1231: frame()->document()->styleEngine()->enableExitTransitionStylesheets(); It seems like the document is in a strange state at this point. We've already fired the unload event, but the document's scripts can continue to execute. Normally the unload even is the last thing that happens to a page before it gets ripped out of the view... How does this work with onbeforeunload? Is that event fired before or after the unload event? Can you add tests to cover all of these cases? https://codereview.chromium.org/316053007/diff/140001/Source/web/WebLocalFram... Source/web/WebLocalFrameImpl.cpp:1237: frame()->document()->setIsTransitionDocument(); Given that this function operates exclusively on the document, it should be an API on WebDocument.
New version up, with unit tests added. PTAL :) https://codereview.chromium.org/316053007/diff/140001/Source/core/dom/Documen... File Source/core/dom/Document.cpp (right): https://codereview.chromium.org/316053007/diff/140001/Source/core/dom/Documen... Source/core/dom/Document.cpp:5740: RefPtr<StaticNodeList> nodeList = querySelectorAll(AtomicString(cssSelector), exceptionState); On 2014/07/29 17:07:15, abarth wrote: > If we're going to use cssSelector as an AtomicString, we should declare that as > the type of the argument. Done. https://codereview.chromium.org/316053007/diff/140001/Source/web/WebLocalFram... File Source/web/WebLocalFrameImpl.cpp (right): https://codereview.chromium.org/316053007/diff/140001/Source/web/WebLocalFram... Source/web/WebLocalFrameImpl.cpp:1230: frame()->loader().closeURL(); On 2014/07/29 17:07:15, abarth wrote: > Will this cause us to dispatch the unload event twice? It looks like > FrameLoader::detachFromParent also calls closeURL, and closeURL will call > dispatchUnloadEvents... After discussing this with Simon, it turns out this call was something left over from an older iteration of this and was actually no longer needed. Removed this now, which should simplify this quite a lot and no longer introduce any inconsistency with the unload handlers. https://codereview.chromium.org/316053007/diff/140001/Source/web/WebLocalFram... Source/web/WebLocalFrameImpl.cpp:1237: frame()->document()->setIsTransitionDocument(); On 2014/07/29 17:07:15, abarth wrote: > Given that this function operates exclusively on the document, it should be an > API on WebDocument. Done.
abarth: ping :) On 2014/07/30 22:11:51, Oystein wrote: > New version up, with unit tests added. PTAL :) > > https://codereview.chromium.org/316053007/diff/140001/Source/core/dom/Documen... > File Source/core/dom/Document.cpp (right): > > https://codereview.chromium.org/316053007/diff/140001/Source/core/dom/Documen... > Source/core/dom/Document.cpp:5740: RefPtr<StaticNodeList> nodeList = > querySelectorAll(AtomicString(cssSelector), exceptionState); > On 2014/07/29 17:07:15, abarth wrote: > > If we're going to use cssSelector as an AtomicString, we should declare that > as > > the type of the argument. > > Done. > > https://codereview.chromium.org/316053007/diff/140001/Source/web/WebLocalFram... > File Source/web/WebLocalFrameImpl.cpp (right): > > https://codereview.chromium.org/316053007/diff/140001/Source/web/WebLocalFram... > Source/web/WebLocalFrameImpl.cpp:1230: frame()->loader().closeURL(); > On 2014/07/29 17:07:15, abarth wrote: > > Will this cause us to dispatch the unload event twice? It looks like > > FrameLoader::detachFromParent also calls closeURL, and closeURL will call > > dispatchUnloadEvents... > > After discussing this with Simon, it turns out this call was something left over > from an older iteration of this and was actually no longer needed. Removed this > now, which should simplify this quite a lot and no longer introduce any > inconsistency with the unload handlers. > > https://codereview.chromium.org/316053007/diff/140001/Source/web/WebLocalFram... > Source/web/WebLocalFrameImpl.cpp:1237: > frame()->document()->setIsTransitionDocument(); > On 2014/07/29 17:07:15, abarth wrote: > > Given that this function operates exclusively on the document, it should be an > > API on WebDocument. > > Done.
LGTM! https://codereview.chromium.org/316053007/diff/180001/Source/web/WebDocument.cpp File Source/web/WebDocument.cpp (right): https://codereview.chromium.org/316053007/diff/180001/Source/web/WebDocument.... Source/web/WebDocument.cpp:276: Document* document = unwrap<Document>(); RefPtr<Document> I don't think it matters, but there's no reason to use a raw ptr here. https://codereview.chromium.org/316053007/diff/180001/Source/web/tests/WebDoc... File Source/web/tests/WebDocumentTest.cpp (right): https://codereview.chromium.org/316053007/diff/180001/Source/web/tests/WebDoc... Source/web/tests/WebDocumentTest.cpp:22: using namespace blink; You shouldn't need this.
LGTM!
Great, thanks! :) https://codereview.chromium.org/316053007/diff/180001/Source/web/WebDocument.cpp File Source/web/WebDocument.cpp (right): https://codereview.chromium.org/316053007/diff/180001/Source/web/WebDocument.... Source/web/WebDocument.cpp:276: Document* document = unwrap<Document>(); On 2014/08/01 22:27:32, abarth wrote: > RefPtr<Document> > > I don't think it matters, but there's no reason to use a raw ptr here. Done. https://codereview.chromium.org/316053007/diff/180001/Source/web/tests/WebDoc... File Source/web/tests/WebDocumentTest.cpp (right): https://codereview.chromium.org/316053007/diff/180001/Source/web/tests/WebDoc... Source/web/tests/WebDocumentTest.cpp:22: using namespace blink; On 2014/08/01 22:27:32, abarth wrote: > You shouldn't need this. Looks like it's necessary in the web/tests/* files, for whatever reason; most of them have this. Some of them twice in a row due to the recent mass rename, but I assume we'll do a batch job for that at some point :).
The CQ bit was checked by oysteine@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/oysteine@chromium.org/316053007/200001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
Message was sent while issue was closed.
Change committed as 179431 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
