Merge Webkit at r70949: Initial merge by git.

Change-Id: I77b8645c083b5d0da8dba73ed01d4014aab9848e

7 files changed, 89 insertions, 186 deletions

diff --git a/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py b/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py
index 94519a7..1cc8e2e 100644
--- a/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py
+++ b/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py
@@ -113,9 +113,6 @@ class Attachment(object):
     def commit_queue(self):
         return self._attachment_dictionary.get("commit-queue")
 
-    def in_rietveld(self):
-        return self._attachment_dictionary.get("in-rietveld")
-
     def url(self):
         # FIXME: This should just return
         # self._bugzilla().attachment_url_for_id(self.id()). scm_unittest.py
@@ -221,9 +218,6 @@ class Bug(object):
         # a valid committer.
         return filter(lambda patch: patch.committer(), patches)
 
-    def in_rietveld_queue_patches(self):
-        return [patch for patch in self.patches() if patch.in_rietveld() == None]
-
 
 # A container for all of the logic for making and parsing buzilla queries.
 class BugzillaQueries(object):
@@ -287,16 +281,6 @@ class BugzillaQueries(object):
         return sum([self._fetch_bug(bug_id).commit_queued_patches()
                     for bug_id in self.fetch_bug_ids_from_commit_queue()], [])
 
-    def fetch_first_patch_from_rietveld_queue(self):
-        # rietveld-queue processes all patches that don't have in-rietveld set.
-        query_url = "buglist.cgi?query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&field0-0-0=flagtypes.name&type0-0-0=notsubstring&value0-0-0=in-rietveld&field0-1-0=attachments.ispatch&type0-1-0=equals&value0-1-0=1&order=Last+Changed&field0-2-0=attachments.isobsolete&type0-2-0=equals&value0-2-0=0"
-        bugs = self._fetch_bug_ids_advanced_query(query_url)
-        if not len(bugs):
-            return None
-
-        patches = self._fetch_bug(bugs[0]).in_rietveld_queue_patches()
-        return patches[0] if len(patches) else None
-
     def _fetch_bug_ids_from_review_queue(self):
         review_queue_url = "buglist.cgi?query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=review?"
         return self._fetch_bug_ids_advanced_query(review_queue_url)
@@ -502,8 +486,6 @@ class Bugzilla(object):
         self._parse_attachment_flag(
                 element, 'review', attachment, 'reviewer_email')
         self._parse_attachment_flag(
-                element, 'in-rietveld', attachment, 'rietveld_uploader_email')
-        self._parse_attachment_flag(
                 element, 'commit-queue', attachment, 'committer_email')
         return attachment
 
@@ -591,11 +573,12 @@ class Bugzilla(object):
             self.authenticated = True
             return
 
+        credentials = Credentials(self.bug_server_host, git_prefix="bugzilla")
+
         attempts = 0
         while not self.authenticated:
             attempts += 1
-            (username, password) = Credentials(
-                self.bug_server_host, git_prefix="bugzilla").read_credentials()
+            username, password = credentials.read_credentials()
 
             log("Logging in as %s..." % username)
             self.browser.open(self.bug_server_url +
@@ -766,8 +749,6 @@ class Bugzilla(object):
             return self.browser.find_control(type='select', nr=0)
         elif flag_name == "commit-queue":
             return self.browser.find_control(type='select', nr=1)
-        elif flag_name == "in-rietveld":
-            return self.browser.find_control(type='select', nr=2)
         raise Exception("Don't know how to find flag named \"%s\"" % flag_name)
 
     def clear_attachment_flags(self,
diff --git a/WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py b/WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py
index 3a454d6..df1fcf6 100644
--- a/WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py
@@ -83,11 +83,6 @@ class BugzillaTest(unittest.TestCase):
                 status="+"
                 setter="two@test.com"
            />
-          <flag name="in-rietveld"
-                id="17933"
-                status="+"
-                setter="three@test.com"
-           />
         </attachment>
 '''
     _expected_example_attachment_parsing = {
@@ -103,8 +98,6 @@ class BugzillaTest(unittest.TestCase):
         'reviewer_email' : 'one@test.com',
         'commit-queue' : '+',
         'committer_email' : 'two@test.com',
-        'in-rietveld': '+',
-        'rietveld_uploader_email': 'three@test.com',
         'attacher_email' : 'christian.plesner.hansen@gmail.com',
     }
 
diff --git a/WebKitTools/Scripts/webkitpy/common/net/credentials.py b/WebKitTools/Scripts/webkitpy/common/net/credentials.py
index 1c3e6c0..30480b3 100644
--- a/WebKitTools/Scripts/webkitpy/common/net/credentials.py
+++ b/WebKitTools/Scripts/webkitpy/common/net/credentials.py
@@ -48,6 +48,7 @@ except ImportError:
 
 
 class Credentials(object):
+    _environ_prefix = "webkit_bugzilla_"
 
     def __init__(self, host, git_prefix=None, executive=None, cwd=os.getcwd(),
                  keyring=keyring):
@@ -58,8 +59,17 @@ class Credentials(object):
         self._keyring = keyring
 
     def _credentials_from_git(self):
-        return [Git.read_git_config(self.git_prefix + "username"),
-                Git.read_git_config(self.git_prefix + "password")]
+        try:
+            if not Git.in_working_directory(self.cwd):
+                return (None, None)
+            return (Git.read_git_config(self.git_prefix + "username"),
+                    Git.read_git_config(self.git_prefix + "password"))
+        except OSError, e:
+            # Catch and ignore OSError exceptions such as "no such file
+            # or directory" (OSError errno 2), which imply that the Git
+            # command cannot be found/is not installed.
+            pass
+        return (None, None)
 
     def _keychain_value_with_label(self, label, source_text):
         match = re.search("%s\"(?P<value>.+)\"" % label,
@@ -110,21 +120,28 @@ class Credentials(object):
         else:
             return [None, None]
 
-    def read_credentials(self):
-        username = None
-        password = None
+    def _read_environ(self, key):
+        environ_key = self._environ_prefix + key
+        return os.environ.get(environ_key.upper())
 
-        try:
-            if Git.in_working_directory(self.cwd):
-                (username, password) = self._credentials_from_git()
-        except OSError, e:
-            # Catch and ignore OSError exceptions such as "no such file 
-            # or directory" (OSError errno 2), which imply that the Git
-            # command cannot be found/is not installed.
-            pass
+    def _credentials_from_environment(self):
+        return (self._read_environ("username"), self._read_environ("password"))
+
+    def _offer_to_store_credentials_in_keyring(self, username, password):
+        if not self._keyring:
+            return
+        if not User().confirm("Store password in system keyring?", User.DEFAULT_NO):
+            return
+        self._keyring.set_password(self.host, username, password)
 
+    def read_credentials(self):
+        username, password = self._credentials_from_environment()
+        # FIXME: We don't currently support pulling the username from one
+        # source and the password from a separate source.
+        if not username or not password:
+            username, password = self._credentials_from_git()
         if not username or not password:
-            (username, password) = self._credentials_from_keychain(username)
+            username, password = self._credentials_from_keychain(username)
 
         if username and not password and self._keyring:
             password = self._keyring.get_password(self.host, username)
@@ -132,13 +149,7 @@ class Credentials(object):
         if not username:
             username = User.prompt("%s login: " % self.host)
         if not password:
-            password = getpass.getpass("%s password for %s: " % (self.host,
-                                                                 username))
+            password = getpass.getpass("%s password for %s: " % (self.host, username))
+            self._offer_to_store_credentials_in_keyring(username, password)
 
-            if self._keyring:
-                store_password = User().confirm(
-                    "Store password in system keyring?", User.DEFAULT_NO)
-                if store_password:
-                    self._keyring.set_password(self.host, username, password)
-
-        return [username, password]
+        return (username, password)
diff --git a/WebKitTools/Scripts/webkitpy/common/net/credentials_unittest.py b/WebKitTools/Scripts/webkitpy/common/net/credentials_unittest.py
index d30291b..6f2d909 100644
--- a/WebKitTools/Scripts/webkitpy/common/net/credentials_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/common/net/credentials_unittest.py
@@ -26,6 +26,8 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+from __future__ import with_statement
+
 import os
 import tempfile
 import unittest
@@ -34,6 +36,21 @@ from webkitpy.common.system.executive import Executive
 from webkitpy.common.system.outputcapture import OutputCapture
 from webkitpy.thirdparty.mock import Mock
 
+
+# FIXME: Other unit tests probably want this class.
+class _TemporaryDirectory(object):
+    def __init__(self, **kwargs):
+        self._kwargs = kwargs
+        self._directory_path = None
+
+    def __enter__(self):
+        self._directory_path = tempfile.mkdtemp(**self._kwargs)
+        return self._directory_path
+
+    def __exit__(self, type, value, traceback):
+        os.rmdir(self._directory_path)
+
+
 class CredentialsTest(unittest.TestCase):
     example_security_output = """keychain: "/Users/test/Library/Keychains/login.keychain"
 class: "inet"
@@ -101,40 +118,59 @@ password: "SECRETSAUCE"
         self._assert_security_call()
         self._assert_security_call(username="foo")
 
+    def test_credentials_from_environment(self):
+        executive_mock = Mock()
+        credentials = Credentials("example.com", executive=executive_mock)
+
+        saved_environ = os.environ.copy()
+        os.environ['WEBKIT_BUGZILLA_USERNAME'] = "foo"
+        os.environ['WEBKIT_BUGZILLA_PASSWORD'] = "bar"
+        username, password = credentials._credentials_from_environment()
+        self.assertEquals(username, "foo")
+        self.assertEquals(password, "bar")
+        os.environ = saved_environ
+
     def test_read_credentials_without_git_repo(self):
+        # FIXME: This should share more code with test_keyring_without_git_repo
         class FakeCredentials(Credentials):
             def _is_mac_os_x(self):
                 return True
+
             def _credentials_from_keychain(self, username):
-                return ["test@webkit.org", "SECRETSAUCE"]
+                return ("test@webkit.org", "SECRETSAUCE")
+
+            def _credentials_from_environment(self):
+                return (None, None)
+
+        with _TemporaryDirectory(suffix="not_a_git_repo") as temp_dir_path:
+            credentials = FakeCredentials("bugs.webkit.org", cwd=temp_dir_path)
+            # FIXME: Using read_credentials here seems too broad as higher-priority
+            # credential source could be affected by the user's environment.
+            self.assertEqual(credentials.read_credentials(), ("test@webkit.org", "SECRETSAUCE"))
 
-        temp_dir_path = tempfile.mkdtemp(suffix="not_a_git_repo")
-        credentials = FakeCredentials("bugs.webkit.org", cwd=temp_dir_path)
-        self.assertEqual(credentials.read_credentials(), ["test@webkit.org", "SECRETSAUCE"])
-        os.rmdir(temp_dir_path)
 
     def test_keyring_without_git_repo(self):
+        # FIXME: This should share more code with test_read_credentials_without_git_repo
         class MockKeyring(object):
             def get_password(self, host, username):
                 return "NOMNOMNOM"
 
         class FakeCredentials(Credentials):
-            def __init__(self, cwd):
-                Credentials.__init__(self, "fake.hostname", cwd=cwd,
-                                     keyring=MockKeyring())
-
             def _is_mac_os_x(self):
                 return True
 
             def _credentials_from_keychain(self, username):
                 return ("test@webkit.org", None)
 
-        temp_dir_path = tempfile.mkdtemp(suffix="not_a_git_repo")
-        credentials = FakeCredentials(temp_dir_path)
-        try:
-            self.assertEqual(credentials.read_credentials(), ["test@webkit.org", "NOMNOMNOM"])
-        finally:
-            os.rmdir(temp_dir_path)
+            def _credentials_from_environment(self):
+                return (None, None)
+
+        with _TemporaryDirectory(suffix="not_a_git_repo") as temp_dir_path:
+            credentials = FakeCredentials("fake.hostname", cwd=temp_dir_path, keyring=MockKeyring())
+            # FIXME: Using read_credentials here seems too broad as higher-priority
+            # credential source could be affected by the user's environment.
+            self.assertEqual(credentials.read_credentials(), ("test@webkit.org", "NOMNOMNOM"))
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/WebKitTools/Scripts/webkitpy/common/net/rietveld.py b/WebKitTools/Scripts/webkitpy/common/net/rietveld.py
deleted file mode 100644
index b9a0821..0000000
--- a/WebKitTools/Scripts/webkitpy/common/net/rietveld.py
+++ /dev/null
@@ -1,79 +0,0 @@
-# Copyright (c) 2010 Google Inc. All rights reserved.
-#
-# Redistribution and use in source and binary forms, with or without
-# modification, are permitted provided that the following conditions are
-# met:
-#
-#     * Redistributions of source code must retain the above copyright
-# notice, this list of conditions and the following disclaimer.
-#     * Redistributions in binary form must reproduce the above
-# copyright notice, this list of conditions and the following disclaimer
-# in the documentation and/or other materials provided with the
-# distribution.
-#     * Neither the name of Google Inc. nor the names of its
-# contributors may be used to endorse or promote products derived from
-# this software without specific prior written permission.
-#
-# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-import logging
-import os
-import re
-import stat
-
-import webkitpy.common.config as config
-from webkitpy.common.system.deprecated_logging import log
-from webkitpy.common.system.executive import ScriptError
-import webkitpy.thirdparty.autoinstalled.rietveld.upload as upload
-
-
-class Rietveld(object):
-    def __init__(self, executive, dryrun=False):
-        self.dryrun = dryrun
-        self._executive = executive
-
-    def url_for_issue(self, codereview_issue):
-        if not codereview_issue:
-            return None
-        return "%s%s" % (config.codereview_server_url, codereview_issue)
-
-    def post(self, diff, patch_id, codereview_issue, message=None, cc=None):
-        if not message:
-            raise ScriptError("Rietveld requires a message.")
-
-        # Rietveld has a 100 character limit on message length.
-        if len(message) > 100:
-            message = message[:100]
-
-        args = [
-            # First argument is empty string to mimic sys.argv.
-            "",
-            "--assume_yes",
-            "--server=%s" % config.codereview_server_host,
-            "--message=%s" % message,
-            "--webkit_patch_id=%s" % patch_id,
-        ]
-        if codereview_issue:
-            args.append("--issue=%s" % codereview_issue)
-        if cc:
-            args.append("--cc=%s" % cc)
-
-        if self.dryrun:
-            log("Would have run %s" % args)
-            return
-
-        # Use RealMain instead of calling upload from the commandline so that
-        # we can pass in the diff ourselves. Otherwise, upload will just use
-        # git diff for git checkouts, which doesn't respect --git-commit.
-        issue, patchset = upload.RealMain(args, data=diff)
-        return issue
diff --git a/WebKitTools/Scripts/webkitpy/common/net/rietveld_unittest.py b/WebKitTools/Scripts/webkitpy/common/net/rietveld_unittest.py
deleted file mode 100644
index 9c5a29e..0000000
--- a/WebKitTools/Scripts/webkitpy/common/net/rietveld_unittest.py
+++ /dev/null
@@ -1,39 +0,0 @@
-# Copyright (c) 2010 Google Inc. All rights reserved.
-#
-# Redistribution and use in source and binary forms, with or without
-# modification, are permitted provided that the following conditions are
-# met:
-#
-#     * Redistributions of source code must retain the above copyright
-# notice, this list of conditions and the following disclaimer.
-#     * Redistributions in binary form must reproduce the above
-# copyright notice, this list of conditions and the following disclaimer
-# in the documentation and/or other materials provided with the
-# distribution.
-#     * Neither the name of Google Inc. nor the names of its
-# contributors may be used to endorse or promote products derived from
-# this software without specific prior written permission.
-#
-# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-import unittest
-
-from webkitpy.common.net.rietveld import Rietveld
-from webkitpy.thirdparty.mock import Mock
-
-
-class RietveldTest(unittest.TestCase):
-    def test_url_for_issue(self):
-        rietveld = Rietveld(Mock())
-        self.assertEqual(rietveld.url_for_issue(34223),
-                         "https://wkrietveld.appspot.com/34223")
diff --git a/WebKitTools/Scripts/webkitpy/common/net/statusserver.py b/WebKitTools/Scripts/webkitpy/common/net/statusserver.py
index 3d03dcd..64dd77b 100644
--- a/WebKitTools/Scripts/webkitpy/common/net/statusserver.py
+++ b/WebKitTools/Scripts/webkitpy/common/net/statusserver.py
@@ -127,7 +127,7 @@ class StatusServer:
         self._browser.submit()
 
     def release_work_item(self, queue_name, patch):
-        _log.debug("Releasing work item %s from %s" % (patch.id(), queue_name))
+        _log.info("Releasing work item %s from %s" % (patch.id(), queue_name))
         return NetworkTransaction(convert_404_to_None=True).run(lambda: self._post_release_work_item(queue_name, patch))
 
     def update_work_items(self, queue_name, work_items):